Hi Eric, Thanks for your patch. However, I found a couple of issues while testing.
On Tue Jul 7, 2026 at 11:21 PM CST, Eric Chung wrote: > Dual-voltage GPIO banks default to 3.3V, but when externally supplied > with 1.8V the internal logic must be explicitly reconfigured to match. > > Add the ability to program IO power domain control registers through the > APBC block. These registers require unlocking the AIB Secure Access > Register (ASAR) before every read/write, since configuring a 1.8V domain > while 3.3V is externally supplied can cause back-powering and pin damage. > > Signed-off-by: Eric Chung <[email protected]> > > --- > v3: > - Add SYSCON dependency in Kconfig. > - Fix not sorted issue in the driver. > --- > drivers/pinctrl/spacemit/Kconfig | 2 +- > drivers/pinctrl/spacemit/pinctrl-k1.c | 83 > ++++++++++++++++++++++++++++++++++- > 2 files changed, 82 insertions(+), 3 deletions(-) > > diff --git a/drivers/pinctrl/spacemit/Kconfig > b/drivers/pinctrl/spacemit/Kconfig > index 6aab89e160c..ff754f5839c 100644 > --- a/drivers/pinctrl/spacemit/Kconfig > +++ b/drivers/pinctrl/spacemit/Kconfig > @@ -1,6 +1,6 @@ > config PINCTRL_SPACEMIT_K1 > bool "Spacemit K1 SoC pinctrl driver" > - depends on PINCTRL_GENERIC && DM > + depends on PINCTRL_GENERIC && DM && SYSCON > help > Supports pin multiplexing control on Spacemit K1 SoCs. > > diff --git a/drivers/pinctrl/spacemit/pinctrl-k1.c > b/drivers/pinctrl/spacemit/pinctrl-k1.c > index 3ebc397213b..95344f4ec72 100644 > --- a/drivers/pinctrl/spacemit/pinctrl-k1.c > +++ b/drivers/pinctrl/spacemit/pinctrl-k1.c [...] > + > static int spacemit_pinconf_set(struct udevice *dev, unsigned int > pin_selector, > unsigned int param, unsigned int argument) > { > @@ -456,6 +526,9 @@ static int spacemit_pinconf_set(struct udevice *dev, > unsigned int pin_selector, > dev_err(dev, "Invalid power source (%d)\n", argument); > return -EINVAL; > } > + if (found) > + spacemit_set_io_power_domain(dev, pin_selector, CONFIG_PINCONF is not enabled in spacemit_k1_defconfig. Therefore, with the default configuration, the generic pinctrl code skips pin configuration properties and spacemit_pinconf_set() is never called. Consequently, the IO power domain configuration added here is not exercised when building with spacemit_k1_defconfig. > + priv->io_pins[i].io_type); > break; > default: > return -EOPNOTSUPP; > @@ -485,6 +558,11 @@ static int spacemit_pinctrl_probe(struct udevice *dev) > dev_err(dev, "Fail to allocate memory\n"); > return -ENOMEM; > } > + priv->regmap = syscon_regmap_lookup_by_phandle(dev, "spacemit,apbc"); > + if (IS_ERR(priv->regmap)) { > + dev_warn(dev, "no syscon found, disable IO power domain > switching\n"); > + priv->regmap = NULL; > + } The APBC node with compatible "spacemit,k1-syscon-apbc" is bound by drivers/clk/spacemit/clk-k1.c as a UCLASS_CLK device. It is not registered as a UCLASS_SYSCON device, and the node does not have the "syscon" compatible required by the fallback path in syscon_regmap_lookup_by_phandle(). Therefore, this lookup fails and priv->regmap is set to NULL. I added a direct MMIO write to the UART registers here to confirm the failure path. The original dev_warn() is not visible because this probe happens before the normal console is initialized. To fix this, I suggest either making the clock driver register or expose the APBC regmap through the syscon infrastructure, or avoiding syscon here and obtaining the regmap directly using dev_read_phandle_with_args() followed by regmap_init_mem(). > > ret = clk_get_bulk(dev, &clks); > if (ret) { > @@ -512,6 +590,7 @@ static const struct spacemit_pinctrl_data k1_pinctrl_data > = { > .get_pins = k1_get_pins, > .get_functions = k1_get_functions, > .get_io_type = k1_get_io_type, > + .pin_to_io_pd_offset = spacemit_k1_pin_to_io_pd_offset, > }; > > static const struct udevice_id spacemit_pinctrl_ids[] = { -- Best regards, Junhui Liu
