diff mbox series

power: regulator: Fix power on/off delay issue

Message ID 20240923132409.28947-1-peng.fan@oss.nxp.com
State Changes Requested
Delegated to: Jaehoon Chung
Headers show
Series power: regulator: Fix power on/off delay issue | expand

Commit Message

Peng Fan (OSS) Sept. 23, 2024, 1:24 p.m. UTC
From: Ye Li <ye.li@nxp.com>

We meet SD initialization issue with some UHS-I SD cards on
iMX8MM/iMX93/iMX91 EVK. When sending operation condition to card,
the OCR does not return correct status. We find it is the issue
in MMC power cycle after below commit applied:
4fcba5d (regulator: implement basic reference counter)

When SD startup, the sequence of MMC power cycle is:
mmc_power_init(get vmmc_supply dev) -> mmc_power_off -> udelay(2000)
-> mmc_power_on

Before the commit, as a fixed regulator, the GPIO is set as:
  GPIO inactive (in mmc_power_init) ->
  GPIO inactive and delay off-on-delay-us (in mmc_power_off) ->
  udelay(2000) ->
  GPIO active (in mmc_power_on)

After the commit:
  GPIO inactive (in mmc_power_init) ->
  enable_count is 0, regulator_set_enable returns -EALREADY,
  GPIO is inactive but NO delay (in mmc_power_off) ->
  udelay(2000) ->
  GPIO active (in mmc_power_on)

Because the lost of off-on-delay-us, the SD card is not completely
power off. To fix the issue, add the delay after the GPIO setting in
regulator_common_of_to_plat which is called in device probing.
So in mmc_power_init, after GPIO is set to default inactive, the
off-on-delay-us is applied.

Fixes: 4fcba5d556b ("regulator: implement basic reference counter")
Signed-off-by: Ye Li <ye.li@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/power/regulator/regulator_common.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Jonas Karlman Sept. 23, 2024, 2:59 p.m. UTC | #1
Hi,

On 2024-09-23 15:24, Peng Fan (OSS) wrote:
> From: Ye Li <ye.li@nxp.com>
> 
> We meet SD initialization issue with some UHS-I SD cards on
> iMX8MM/iMX93/iMX91 EVK. When sending operation condition to card,
> the OCR does not return correct status. We find it is the issue
> in MMC power cycle after below commit applied:
> 4fcba5d (regulator: implement basic reference counter)
> 
> When SD startup, the sequence of MMC power cycle is:
> mmc_power_init(get vmmc_supply dev) -> mmc_power_off -> udelay(2000)
> -> mmc_power_on
> 
> Before the commit, as a fixed regulator, the GPIO is set as:
>   GPIO inactive (in mmc_power_init) ->
>   GPIO inactive and delay off-on-delay-us (in mmc_power_off) ->
>   udelay(2000) ->
>   GPIO active (in mmc_power_on)
> 
> After the commit:
>   GPIO inactive (in mmc_power_init) ->
>   enable_count is 0, regulator_set_enable returns -EALREADY,
>   GPIO is inactive but NO delay (in mmc_power_off) ->
>   udelay(2000) ->
>   GPIO active (in mmc_power_on)

If I understand correctly your regulator is enabled outside of the
reference counting, so the regulator common code never tries to disable
it.

This should probably be solved at the initialization of the reference
count, a gpio controlled regulator should possible check the gpio status
at probe.

Is your regulator flagged with regulator-boot-on flag?

If I remember correctly the reference count should be initialized to 1
with boot-on, if that is the case your regulator should have been
disabled and the delay would be applied.

> 
> Because the lost of off-on-delay-us, the SD card is not completely
> power off. To fix the issue, add the delay after the GPIO setting in
> regulator_common_of_to_plat which is called in device probing.
> So in mmc_power_init, after GPIO is set to default inactive, the
> off-on-delay-us is applied.
> 
> Fixes: 4fcba5d556b ("regulator: implement basic reference counter")
> Signed-off-by: Ye Li <ye.li@nxp.com>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/power/regulator/regulator_common.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/power/regulator/regulator_common.c b/drivers/power/regulator/regulator_common.c
> index e3565d32a01..2192e900697 100644
> --- a/drivers/power/regulator/regulator_common.c
> +++ b/drivers/power/regulator/regulator_common.c
> @@ -45,6 +45,12 @@ int regulator_common_of_to_plat(struct udevice *dev,
>  			dev_read_u32_default(dev, "u-boot,off-on-delay-us", 0);
>  	}
>  
> +	if ((flags & GPIOD_IS_OUT_ACTIVE) && plat->startup_delay_us)
> +		udelay(plat->startup_delay_us);
> +
> +	if (!(flags & GPIOD_IS_OUT_ACTIVE) && plat->off_on_delay_us)
> +		udelay(plat->off_on_delay_us);

I do not understand why adding the delay here would solve your issue if
your enable_count is 0 and the GPIO is already inactive why is the power
off delay needed before you power on for the first time? Is your
regulator somehow disabled at probe time or enabled before probe time?

Regards,
Jonas

> +
>  	return 0;
>  }
>
diff mbox series

Patch

diff --git a/drivers/power/regulator/regulator_common.c b/drivers/power/regulator/regulator_common.c
index e3565d32a01..2192e900697 100644
--- a/drivers/power/regulator/regulator_common.c
+++ b/drivers/power/regulator/regulator_common.c
@@ -45,6 +45,12 @@  int regulator_common_of_to_plat(struct udevice *dev,
 			dev_read_u32_default(dev, "u-boot,off-on-delay-us", 0);
 	}
 
+	if ((flags & GPIOD_IS_OUT_ACTIVE) && plat->startup_delay_us)
+		udelay(plat->startup_delay_us);
+
+	if (!(flags & GPIOD_IS_OUT_ACTIVE) && plat->off_on_delay_us)
+		udelay(plat->off_on_delay_us);
+
 	return 0;
 }