[PATCH] regulator: fix: Move code to enable gpio regulator to pre_probe from ofdata_to_platdata

[PATCH] regulator: fix: Move code to enable gpio regulator to pre_probe from ofdata_to_platdata

[Lukasz Majewski]

The commit e8e9715df2d4 ("regulator: fixed: Modify enable-active-high behavior")
fixed the regulator driver behavior when 'enable-active-high' is defined.
Unfortunately, this patch used dm_regulator_platdata()'s "boot_on" member
to set GPIOD_IS_OUT_ACTIVE flag and enable the regulator.

The issue here is that regulator_common_ofdata_to_platdata() is called
_before_ regulator_pre_probe() function in which the 'regulator-boot-on'
property is asserted.

As a result the GPIOD_IS_OUT_ACTIVE flag is not set and gpio_request_by_name()
called in the former function is not enabling the regulator.
This is problematic for e.g. i.MX ethernet driver, which then tries to
perform initialization without power (and fails).

The solution here is to explicitly enable regulator in regulator_pre_probe()
callback only when 'regulator-boot-on' property is present in device tree.
The GPIOD_IS_OUT_ACTIVE flag is not set at all, but relevant gpio is
requested.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---

 drivers/power/regulator/regulator-uclass.c | 3 +++
 drivers/power/regulator/regulator_common.c | 5 -----
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
index 90961de95c..c9d26344d7 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -464,6 +464,9 @@ static int regulator_pre_probe(struct udevice *dev)
 	    (uc_pdata->min_uA == uc_pdata->max_uA))
 		uc_pdata->flags |= REGULATOR_FLAG_AUTOSET_UA;
 
+	if (uc_pdata->boot_on)
+		regulator_set_enable(dev, uc_pdata->boot_on);
+
 	return 0;
 }
 
diff --git a/drivers/power/regulator/regulator_common.c b/drivers/power/regulator/regulator_common.c
index 939efb2c0d..33b73b7c2f 100644
--- a/drivers/power/regulator/regulator_common.c
+++ b/drivers/power/regulator/regulator_common.c
@@ -12,16 +12,11 @@ int regulator_common_ofdata_to_platdata(struct udevice *dev,
 	struct regulator_common_platdata *dev_pdata, const char *enable_gpio_name)
 {
 	struct gpio_desc *gpio;
-	struct dm_regulator_uclass_platdata *uc_pdata;
 	int flags = GPIOD_IS_OUT;
 	int ret;
 
-	uc_pdata = dev_get_uclass_platdata(dev);
-
 	if (!dev_read_bool(dev, "enable-active-high"))
 		flags |= GPIOD_ACTIVE_LOW;
-	if (uc_pdata->boot_on)
-		flags |= GPIOD_IS_OUT_ACTIVE;
 
 	/* Get optional enable GPIO desc */
 	gpio = &dev_pdata->gpio;
-- 
2.20.1

[PATCH] regulator: fix: Move code to enable gpio regulator to pre_probe from ofdata_to_platdata

[Patrice CHOTARD]

Hi Lukasz

On 1/25/20 9:00 AM, Lukasz Majewski wrote:
> The commit e8e9715df2d4 ("regulator: fixed: Modify enable-active-high behavior")
> fixed the regulator driver behavior when 'enable-active-high' is defined.
> Unfortunately, this patch used dm_regulator_platdata()'s "boot_on" member
> to set GPIOD_IS_OUT_ACTIVE flag and enable the regulator.
>
> The issue here is that regulator_common_ofdata_to_platdata() is called
> _before_ regulator_pre_probe() function in which the 'regulator-boot-on'
> property is asserted.
>
> As a result the GPIOD_IS_OUT_ACTIVE flag is not set and gpio_request_by_name()
> called in the former function is not enabling the regulator.
> This is problematic for e.g. i.MX ethernet driver, which then tries to
> perform initialization without power (and fails).
>
> The solution here is to explicitly enable regulator in regulator_pre_probe()
> callback only when 'regulator-boot-on' property is present in device tree.
> The GPIOD_IS_OUT_ACTIVE flag is not set at all, but relevant gpio is
> requested.
>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> ---
>
>  drivers/power/regulator/regulator-uclass.c | 3 +++
>  drivers/power/regulator/regulator_common.c | 5 -----
>  2 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
> index 90961de95c..c9d26344d7 100644
> --- a/drivers/power/regulator/regulator-uclass.c
> +++ b/drivers/power/regulator/regulator-uclass.c
> @@ -464,6 +464,9 @@ static int regulator_pre_probe(struct udevice *dev)
>  	    (uc_pdata->min_uA == uc_pdata->max_uA))
>  		uc_pdata->flags |= REGULATOR_FLAG_AUTOSET_UA;
>  
> +	if (uc_pdata->boot_on)
> +		regulator_set_enable(dev, uc_pdata->boot_on);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/power/regulator/regulator_common.c b/drivers/power/regulator/regulator_common.c
> index 939efb2c0d..33b73b7c2f 100644
> --- a/drivers/power/regulator/regulator_common.c
> +++ b/drivers/power/regulator/regulator_common.c
> @@ -12,16 +12,11 @@ int regulator_common_ofdata_to_platdata(struct udevice *dev,
>  	struct regulator_common_platdata *dev_pdata, const char *enable_gpio_name)
>  {
>  	struct gpio_desc *gpio;
> -	struct dm_regulator_uclass_platdata *uc_pdata;
>  	int flags = GPIOD_IS_OUT;
>  	int ret;
>  
> -	uc_pdata = dev_get_uclass_platdata(dev);
> -
>  	if (!dev_read_bool(dev, "enable-active-high"))
>  		flags |= GPIOD_ACTIVE_LOW;
> -	if (uc_pdata->boot_on)
> -		flags |= GPIOD_IS_OUT_ACTIVE;
>  
>  	/* Get optional enable GPIO desc */
>  	gpio = &dev_pdata->gpio;

Reviewed-by: Patrice Chotard <patrice.chotard@st.com>

Tested-by: Patrice Chotard <patrice.chotard@st.com>

Tested on STM32MP1-ev1.

Thanks

Patrice

[PATCH] regulator: fix: Move code to enable gpio regulator to pre_probe from ofdata_to_platdata

[Lukasz Majewski]

Dear Tom

> The commit e8e9715df2d4 ("regulator: fixed: Modify enable-active-high
> behavior") fixed the regulator driver behavior when
> 'enable-active-high' is defined. Unfortunately, this patch used
> dm_regulator_platdata()'s "boot_on" member to set GPIOD_IS_OUT_ACTIVE
> flag and enable the regulator.
> 
> The issue here is that regulator_common_ofdata_to_platdata() is called
> _before_ regulator_pre_probe() function in which the
> 'regulator-boot-on' property is asserted.
> 
> As a result the GPIOD_IS_OUT_ACTIVE flag is not set and
> gpio_request_by_name() called in the former function is not enabling
> the regulator. This is problematic for e.g. i.MX ethernet driver,
> which then tries to perform initialization without power (and fails).
> 
> The solution here is to explicitly enable regulator in
> regulator_pre_probe() callback only when 'regulator-boot-on' property
> is present in device tree. The GPIOD_IS_OUT_ACTIVE flag is not set at
> all, but relevant gpio is requested.
> 
> Signed-off-by: Lukasz Majewski <lukma@denx.de>

Tom, could you pull this fix? (I would prefer to avoid pulling my own
fixes).

It has been already tested-by Patrice.

Thanks in advance.

> ---
> 
>  drivers/power/regulator/regulator-uclass.c | 3 +++
>  drivers/power/regulator/regulator_common.c | 5 -----
>  2 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/power/regulator/regulator-uclass.c
> b/drivers/power/regulator/regulator-uclass.c index
> 90961de95c..c9d26344d7 100644 ---
> a/drivers/power/regulator/regulator-uclass.c +++
> b/drivers/power/regulator/regulator-uclass.c @@ -464,6 +464,9 @@
> static int regulator_pre_probe(struct udevice *dev) (uc_pdata->min_uA
> == uc_pdata->max_uA)) uc_pdata->flags |= REGULATOR_FLAG_AUTOSET_UA;
>  
> +	if (uc_pdata->boot_on)
> +		regulator_set_enable(dev, uc_pdata->boot_on);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/power/regulator/regulator_common.c
> b/drivers/power/regulator/regulator_common.c index
> 939efb2c0d..33b73b7c2f 100644 ---
> a/drivers/power/regulator/regulator_common.c +++
> b/drivers/power/regulator/regulator_common.c @@ -12,16 +12,11 @@ int
> regulator_common_ofdata_to_platdata(struct udevice *dev, struct
> regulator_common_platdata *dev_pdata, const char *enable_gpio_name) {
>  	struct gpio_desc *gpio;
> -	struct dm_regulator_uclass_platdata *uc_pdata;
>  	int flags = GPIOD_IS_OUT;
>  	int ret;
>  
> -	uc_pdata = dev_get_uclass_platdata(dev);
> -
>  	if (!dev_read_bool(dev, "enable-active-high"))
>  		flags |= GPIOD_ACTIVE_LOW;
> -	if (uc_pdata->boot_on)
> -		flags |= GPIOD_IS_OUT_ACTIVE;
>  
>  	/* Get optional enable GPIO desc */
>  	gpio = &dev_pdata->gpio;




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200210/61406bc7/attachment.sig>

[PATCH] regulator: fix: Move code to enable gpio regulator to pre_probe from ofdata_to_platdata

[Tom Rini]

On Sat, Jan 25, 2020 at 09:00:58AM +0100, Lukasz Majewski wrote:

> The commit e8e9715df2d4 ("regulator: fixed: Modify enable-active-high behavior")
> fixed the regulator driver behavior when 'enable-active-high' is defined.
> Unfortunately, this patch used dm_regulator_platdata()'s "boot_on" member
> to set GPIOD_IS_OUT_ACTIVE flag and enable the regulator.
> 
> The issue here is that regulator_common_ofdata_to_platdata() is called
> _before_ regulator_pre_probe() function in which the 'regulator-boot-on'
> property is asserted.
> 
> As a result the GPIOD_IS_OUT_ACTIVE flag is not set and gpio_request_by_name()
> called in the former function is not enabling the regulator.
> This is problematic for e.g. i.MX ethernet driver, which then tries to
> perform initialization without power (and fails).
> 
> The solution here is to explicitly enable regulator in regulator_pre_probe()
> callback only when 'regulator-boot-on' property is present in device tree.
> The GPIOD_IS_OUT_ACTIVE flag is not set at all, but relevant gpio is
> requested.
> 
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> Reviewed-by: Patrice Chotard <patrice.chotard@st.com>
> Tested-by: Patrice Chotard <patrice.chotard@st.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200211/69408238/attachment.sig>