RE: [PATCH] regulator: s5m8767: Convert to GPIO descriptors

[Peng Fan]

> Subject: Re: [PATCH] regulator: s5m8767: Convert to GPIO descriptors
> 
> On 24/03/2025 05:21, Peng Fan wrote:
> > On Tue, Mar 18, 2025 at 02:48:05PM +0100, Krzysztof Kozlowski
> wrote:
> >> On 18/03/2025 13:38, Peng Fan wrote:
> >>>> Also the commit message doesn't tell anything about the existing
> >>>> DTS files.
> >>>> Do we have this device described in any in the kernel? Do we
> have
> >>>> any googled examples? Why I'm asking because often the issue is
> the
> >>>> incorrect setting of the polarity, which needs to be carefully
> >>>> checked, esp. for the voltage regulators case.
> >>>
> >>>
> >>> Under arch/arm/boot/dts/samsung/, a few dtsi files have the
> property
> >>> with results from output of `grep "s5m8767"
> >>> ./arch/arm/boot/dts/samsung/ -rn | grep gpios`
> >>>
> >>> Exynos5250-spring.dts uses GPIO_ACTIVE_LOW.
> >>> Others use GPIO_ACTIVE_HIGH.
> >>>
> >> These are old devices and not many people are actually providing
> >> tests, so you need to preserve existing ABI. IOW, if previously GPIO
> >> flags were ignored, meaning "1" is ACTIVE_HIGH, then you must
> preserve this behavior.
> >
> > Per google,
> > Manual Reset function is for Hardware reset in the Active mode.
> 
> Why are you mentioning the reset functions? Which properties are
> these?

Oops, I got messed with the ASoC patches I did.


> 
> 
> > If MR1B and MR2B is kept low during the VLDO3 is active state, the
> > system makes all internal presetting registers as default in the
> > active mode (automatic power on sequence). If this hardware reset
> > function is not required, connect MRB pin to high.
> >
> > So the reset is ACTIVE LOW if my understanding is correct.
> >
> > To keep DTS unchanged, we need update polarity in gpiolib to force
> > GPIO_ACTIVE_LOW.
> 
> How are you going to achieve it if one DTS has LOW and other has
> HIGH?

With this gpiolib-of change, to fix polarity as HIGH.

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index eb667f8f1ead..aa5d112c6189 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -229,6 +229,10 @@ static void of_gpio_try_fixup_polarity(const struct device_node *np,
                 * treats it as "active low".
                 */
                { "ti,tsc2005",         "reset-gpios",  false },
+#endif
+#if IS_ENABLED(CONFIG_REGULATOR_S5M8767)
+               { "samsung,s5m8767-pmic", s5m8767,pmic-buck-dvs-gpios, false },
+               { "samsung,s5m8767-pmic", s5m8767,pmic-buck-ds-gpios, false },
 #endif
        };
        unsigned int i;

Thanks,
Peng.

> 
> >
> > please see whether this is ok for you.
> 
> I don't understand how this is related to the issue I raised.
> 
> Best regards,
> Krzysztof