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

[Ye Li]

Hi Jonas, Peng,

On 5/15/2026 10:07 PM, Peng Fan wrote:
> On Mon, May 11, 2026 at 03:10:16PM +0200, Jonas Karlman wrote:
>> Hi again,
>>
>> On 5/11/2026 10:30 AM, Jonas Karlman wrote:
>>> Hi,
>>>
>>> On 5/11/2026 9:49 AM, Ye Li wrote:
>>>> SD initialization failure happens with some UHS-I SD cards on
>>>> iMX8MM/iMX93/iMX91 EVK after
>>>> commit 4fcba5d556b4 ("regulator: implement basic reference counter").
>>>> When sending operation condition to SD card, the OCR does not return
>>>> correct status. The root cause is regulator on/off delay is missed
>>>> in MMC power cycle with above commit, so SD card is not completely
>>>> power off.
>>>>
>>>> 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 above 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 immediately,
>>>>        so GPIO is inactive but No off-on-delay-us (in mmc_power_off) ->
>>
>> If the regulator is off at boot-on then this is working as intended.
>>
>> However, if probing the regulator turns it off and that requires a
>> off-on-delay-us delay, maybe you are missing a regulator-boot-on
>> property to indicate that the regulator is on at boot/reset. That should
>> also align the enable count with boot-on state of the regulator.

This regulator is on at boot-on. But there is no regulator-boot-on used 
in kernel upstream dts. This will require to add it specifically for u-boot.
>>
>> Looking at the kernel the off-on-delay-us is applied before a regulator
>> is enabled, not after it is disabled like in U-Boot. Maybe U-Boot should
>> do something similar, in simplest form always udelay off-on-delay-us
>> before the regulator is enabled.

Agree, moving off-on-delay-us before regulator is enabled should work. I 
will use it in v2.

> 
> Yeah. Something below should work(Not tested).
> 
> diff --git a/drivers/power/regulator/regulator_common.c b/drivers/power/regulator/regulator_common.c
> index 85af8d599ad..7bf25f6a176 100644
> --- a/drivers/power/regulator/regulator_common.c
> +++ b/drivers/power/regulator/regulator_common.c
> @@ -68,6 +68,9 @@ int regulator_common_set_enable(const struct udevice *dev,
>   	      dev->name, enable, plat->startup_delay_us,
>   	      dm_gpio_is_valid(&plat->gpio));
>   
> +	if (enable && plat->off_on_delay_us)
> +		udelay(plat->off_on_delay_us);
> +

Thanks for the patch. But I think off_on_delay_us should locate just 
before calling dm_gpio_set_value. Will send it in v2.

Best regards,
Ye Li

>   	/* Enable GPIO is optional */
>   	if (CONFIG_IS_ENABLED(DM_GPIO) && dm_gpio_is_valid(&plat->gpio)) {
>   		/* If previously enabled, increase count */
> @@ -97,9 +100,6 @@ int regulator_common_set_enable(const struct udevice *dev,
>   		if (enable && plat->startup_delay_us)
>   			udelay(plat->startup_delay_us);
>   
> -		if (!enable && plat->off_on_delay_us)
> -			udelay(plat->off_on_delay_us);
> -
>   		if (enable)
>   			plat->enable_count++;
>   		else
> 
> Regards
> Peng.
> 
>>
>>>>    udelay(2000) ->
>>>>    GPIO active (in mmc_power_on)
>>>>
>>>> Add the on/off delay and startup delay after GPIO request in
>>>> regulator_common_of_to_plat which is called in regulator device probing.
>>>> So in mmc_power_init, after GPIO is set to default inactive, the
>>>> off-on-delay-us is applied.
>>>>
>>>> Signed-off-by: Ye Li <ye.li 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 85af8d599ad..158284a3309 100644
>>>> --- a/drivers/power/regulator/regulator_common.c
>>>> +++ b/drivers/power/regulator/regulator_common.c
>>>> @@ -46,6 +46,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);
>>>
>>> regulator_common_of_to_plat() is for parsing the values, no udelay()
>>> should be applied here.
>>>
>>> regulator_common_set_enable() applies udelay() when the regulator is
>>> enabled.
>>
>> Or disabled.
>>
>>>
>>> Does your regulator .get_enable() ops not being called or does it not
>>> implement its own udelay() based on parsed values?
>>
>> I meant set_enable() here :-)
>>
>> Regards,
>> Jonas
>>
>>>
>>> Regards,
>>> Jonas
>>>
>>>> +
>>>>   	return 0;
>>>>   }
>>>>   
>>>
>>
>>