Hi Eric, On 00:40 Sun 02 Aug , Eric Chung wrote: > Fix pull-up/pull-down configuration to honor the argument parameter: > when argument is non-zero, enable the requested pull direction; > when argument is zero, clear all pull bits to disable.
No, This will be problem .. see my comment below > > Signed-off-by: Eric Chung <[email protected]> > > --- > v7: > - Simplify pull-up/pull-down configuration by using a single > clrsetbits_le32() call. > v6: > - Fix pull-up/pull-down configuration. > --- > drivers/pinctrl/spacemit/pinctrl-k1.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/pinctrl/spacemit/pinctrl-k1.c > b/drivers/pinctrl/spacemit/pinctrl-k1.c > index 6ed511879daf..4e65fad1e8c5 100644 > --- a/drivers/pinctrl/spacemit/pinctrl-k1.c > +++ b/drivers/pinctrl/spacemit/pinctrl-k1.c > @@ -409,7 +409,6 @@ static int spacemit_pinconf_set(struct udevice *dev, > unsigned int pin_selector, > struct spacemit_pinctrl_data *data; > struct spacemit_pinctrl_priv *priv = dev_get_priv(dev); > void __iomem *addr; > - u32 mask; > unsigned int io_type; > u8 ds; > bool found; > @@ -424,12 +423,12 @@ static int spacemit_pinconf_set(struct udevice *dev, > unsigned int pin_selector, > clrbits_le32(addr, PAD_PULLUP | PAD_PULLDOWN | PAD_PULL_EN); > break; > case PIN_CONFIG_BIAS_PULL_DOWN: > - mask = PAD_PULLDOWN | PAD_PULLUP | PAD_PULL_EN; > - clrsetbits_le32(addr, mask, PAD_PULLDOWN | PAD_PULL_EN); > + clrsetbits_le32(addr, PAD_PULLDOWN | PAD_PULLUP | PAD_PULL_EN, > + argument ? PAD_PULLDOWN | PAD_PULL_EN : 0); > break; .. > case PIN_CONFIG_BIAS_PULL_UP: > - mask = PAD_PULLDOWN | PAD_PULLUP | PAD_PULL_EN; > - clrsetbits_le32(addr, mask, PAD_PULLUP | PAD_PULL_EN); > + clrsetbits_le32(addr, PAD_PULLDOWN | PAD_PULLUP | PAD_PULL_EN, > + argument ? PAD_PULLUP | PAD_PULL_EN : 0); In kernel side, the argument for pull up settings has documentated as 0 - normal pull up 1 - strong pull up well, you can still use bias-disable property to disable bias settings if needed I'd suggest to have this aligned with kernel DT files, which would more easily for us to reuse upstream dts files > break; > case PIN_CONFIG_DRIVE_STRENGTH: > io_type = IO_TYPE_1V8; > > -- > 2.51.0 > -- Yixun Lan (dlan)
