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.
>
>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.
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);
+
/* 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 <[email protected]>
>>> ---
>>> 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;
>>> }
>>>
>>
>
>
