[PATCH] power: regulator: Fix power on/off delay issue

[Jonas Karlman]

Hi,

On 2024-09-23 15:24, Peng Fan (OSS) wrote:
> From: Ye Li <ye.li at 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 at nxp.com>
> Reviewed-by: Peng Fan <peng.fan at nxp.com>
> Signed-off-by: Peng Fan <peng.fan at 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;
>  }
>