On Fri, Jul 24, 2026 at 12:51 AM Junhui Liu <[email protected]> wrote:
>
> 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.
>
I've enabled CONFIG_PINCONF in the #5 patch of the patch set.
> > +
> > 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().
>
Thanks. Let me check this.
> >
> > 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
>
