[RFC] mmc: meson-gx-mmc: add delay during poweroff

[RFC] mmc: meson-gx-mmc: add delay during poweroff

[Da Xue]

Regulators controlling the SD card power need some settling time for SD
cards to fully reset from UHS modes. The regulator framework seems to
ignore falling times set in the device tree causing a few boards with the
same hardware implementation to hang on reboot because the SD card still
had some voltage and did not reset properly to be initialized again.

Add a delay sufficiently long for the voltage to drop so that the SD card
can reset properly. Otherwise the reboot will hang at missing SD card
especially with Samsung cards.

Signed-off-by: Da Xue <da@libre.computer>
---
 drivers/mmc/host/meson-gx-mmc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 694bb443d5f3..a39906079d29 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -605,6 +605,7 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	case MMC_POWER_OFF:
 		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
 		mmc_regulator_disable_vqmmc(mmc);
+		msleep(50);
 
 		break;
 
-- 
2.39.5

Re: [RFC] mmc: meson-gx-mmc: add delay during poweroff

[Anand Moon]

Hi Da,

On Sat, 28 Jun 2025 at 09:15, Da Xue <da@libre.computer> wrote:
>
> Regulators controlling the SD card power need some settling time for SD
> cards to fully reset from UHS modes. The regulator framework seems to
> ignore falling times set in the device tree causing a few boards with the
> same hardware implementation to hang on reboot because the SD card still
> had some voltage and did not reset properly to be initialized again.
>
> Add a delay sufficiently long for the voltage to drop so that the SD card
> can reset properly. Otherwise the reboot will hang at missing SD card
> especially with Samsung cards.
>
Although the driver defines reset identifiers such as
RESET_SD_EMMC_A, RESET_SD_EMMC_B, and RESET_SD_EMMC_C,
It does not implement proper reset controller functionality,
specifically lacking support
for reset_control_assert() and reset_control_deassert() operations.

Thanks
-Anand

Re: [RFC] mmc: meson-gx-mmc: add delay during poweroff

[Martin Blumenstingl]

Hi Anand,

On Tue, Jul 1, 2025 at 12:00 PM Anand Moon <linux.amoon@gmail.com> wrote:
>
> Hi Da,
>
> On Sat, 28 Jun 2025 at 09:15, Da Xue <da@libre.computer> wrote:
> >
> > Regulators controlling the SD card power need some settling time for SD
> > cards to fully reset from UHS modes. The regulator framework seems to
> > ignore falling times set in the device tree causing a few boards with the
> > same hardware implementation to hang on reboot because the SD card still
> > had some voltage and did not reset properly to be initialized again.
> >
> > Add a delay sufficiently long for the voltage to drop so that the SD card
> > can reset properly. Otherwise the reboot will hang at missing SD card
> > especially with Samsung cards.
> >
> Although the driver defines reset identifiers such as
> RESET_SD_EMMC_A, RESET_SD_EMMC_B, and RESET_SD_EMMC_C,
> It does not implement proper reset controller functionality,
> specifically lacking support
> for reset_control_assert() and reset_control_deassert() operations.
I think there's a misunderstanding:
The meson-gx-mmc driver calls device_reset_optional() during .probe
which will internally call reset_control_reset().
So I don't see a problem here.

The patch seems more about power sequencing, where either the SD card
or regulator used to power the SD card requires a certain amount of
time (delay) when switching from ON -> OFF -> ON (my understanding is:
without this delay the card sees ON -> ON which fails to update some
state internally).

To me it's not clear if this is a property of the SD spec or rather
the regulator.
Ulf, Jerome - any ideas / inputs from you?


Best regards,
Martin

Re: [RFC] mmc: meson-gx-mmc: add delay during poweroff

[Jerome Brunet]

On Wed 02 Jul 2025 at 18:27, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Hi Anand,
>
> On Tue, Jul 1, 2025 at 12:00 PM Anand Moon <linux.amoon@gmail.com> wrote:
>>
>> Hi Da,
>>
>> On Sat, 28 Jun 2025 at 09:15, Da Xue <da@libre.computer> wrote:
>> >
>> > Regulators controlling the SD card power need some settling time for SD
>> > cards to fully reset from UHS modes. The regulator framework seems to
>> > ignore falling times set in the device tree causing a few boards with the
>> > same hardware implementation to hang on reboot because the SD card still
>> > had some voltage and did not reset properly to be initialized again.
>> >
>> > Add a delay sufficiently long for the voltage to drop so that the SD card
>> > can reset properly. Otherwise the reboot will hang at missing SD card
>> > especially with Samsung cards.
>> >
>> Although the driver defines reset identifiers such as
>> RESET_SD_EMMC_A, RESET_SD_EMMC_B, and RESET_SD_EMMC_C,
>> It does not implement proper reset controller functionality,
>> specifically lacking support
>> for reset_control_assert() and reset_control_deassert() operations.
> I think there's a misunderstanding:
> The meson-gx-mmc driver calls device_reset_optional() during .probe
> which will internally call reset_control_reset().
> So I don't see a problem here.
>
> The patch seems more about power sequencing, where either the SD card
> or regulator used to power the SD card requires a certain amount of
> time (delay) when switching from ON -> OFF -> ON (my understanding is:
> without this delay the card sees ON -> ON which fails to update some
> state internally).
>
> To me it's not clear if this is a property of the SD spec or rather
> the regulator.
> Ulf, Jerome - any ideas / inputs from you?

If, as the description suggest, the regulator framework somehow ignore
the timing set in DT, maybe this is what needs to be checked ?

TBH I would suspect the delays before the regulator framework itself.

Those assert/de-assert delays tend to be just copied from boards to
boards. Maybe some boards need different delays. If those are too short
for the actual HW, an ON -> OFF -> ON could result in a NOP.

>
>
> Best regards,
> Martin

-- 
Jerome

Re: [RFC] mmc: meson-gx-mmc: add delay during poweroff

[Da Xue]

On Wed, Jul 2, 2025 at 1:07 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
...
> If, as the description suggest, the regulator framework somehow ignore
> the timing set in DT, maybe this is what needs to be checked ?

The regulator framework only cares about timing for regulator on.
Regulator off just turns off the regulator and returns without delay.
The code makes incorrect assumptions. Then the kernel resets the board
without having enough time.
I can patch the regulator framework with the same code for regulator
on but that seems very hazardous given how many things might already
depend on the original behavior of returning immediately.

>
> TBH I would suspect the delays before the regulator framework itself.
>
> Those assert/de-assert delays tend to be just copied from boards to
> boards. Maybe some boards need different delays. If those are too short
> for the actual HW, an ON -> OFF -> ON could result in a NOP.

50ms should be sufficient for all boards as many boards don't even
have this functionality. < 30ms is sufficient most of the time.

>
...
>
> --
> Jerome

Re: [RFC] mmc: meson-gx-mmc: add delay during poweroff

[Martin Blumenstingl]

On Wed, Jul 2, 2025 at 7:22 PM Da Xue <da@libre.computer> wrote:
>
> On Wed, Jul 2, 2025 at 1:07 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
> ...
> > If, as the description suggest, the regulator framework somehow ignore
> > the timing set in DT, maybe this is what needs to be checked ?
>
> The regulator framework only cares about timing for regulator on.
> Regulator off just turns off the regulator and returns without delay.
There's an exception to this: gpio-regulators without an enable-gpios
property. My understanding is that regulator_disable() is a no-op in
that case (meson_mmc_set_ios() even has a comment above the
switch/case statement), see [0].

> The code makes incorrect assumptions. Then the kernel resets the board
> without having enough time.
Can you please name the board you're testing? I'm worried that I'll be
looking at one .dts but you're looking at another one.


Best regards,
Martin


[0] https://elixir.bootlin.com/linux/v6.15/source/drivers/regulator/core.c#L2980

Re: [RFC] mmc: meson-gx-mmc: add delay during poweroff

[Da Xue]

On Wed, Jul 2, 2025 at 2:40 PM Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> On Wed, Jul 2, 2025 at 7:22 PM Da Xue <da@libre.computer> wrote:
> >
> > On Wed, Jul 2, 2025 at 1:07 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
> > ...
> > > If, as the description suggest, the regulator framework somehow ignore
> > > the timing set in DT, maybe this is what needs to be checked ?
> >
> > The regulator framework only cares about timing for regulator on.
> > Regulator off just turns off the regulator and returns without delay.
> There's an exception to this: gpio-regulators without an enable-gpios
> property. My understanding is that regulator_disable() is a no-op in
> that case (meson_mmc_set_ios() even has a comment above the
> switch/case statement), see [0].
>
> > The code makes incorrect assumptions. Then the kernel resets the board
> > without having enough time.
> Can you please name the board you're testing? I'm worried that I'll be
> looking at one .dts but you're looking at another one.

https://github.com/libre-computer-project/libretech-linux/blob/master/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi#L481

vcc_card is a gpio regulator that gets toggled on->off->on.

I traced the regulator framework a few weeks ago and forgot the final
regulator disable function call, but that call basically returned
immediately while the regulator-enable function complement had delays
implemented.

>
>
> Best regards,
> Martin
>
>
> [0] https://elixir.bootlin.com/linux/v6.15/source/drivers/regulator/core.c#L2980

Re: [RFC] mmc: meson-gx-mmc: add delay during poweroff

[Martin Blumenstingl]

On Wed, Jul 2, 2025 at 9:07 PM Da Xue <da@libre.computer> wrote:
>
> On Wed, Jul 2, 2025 at 2:40 PM Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
> >
> > On Wed, Jul 2, 2025 at 7:22 PM Da Xue <da@libre.computer> wrote:
> > >
> > > On Wed, Jul 2, 2025 at 1:07 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
> > > ...
> > > > If, as the description suggest, the regulator framework somehow ignore
> > > > the timing set in DT, maybe this is what needs to be checked ?
> > >
> > > The regulator framework only cares about timing for regulator on.
> > > Regulator off just turns off the regulator and returns without delay.
> > There's an exception to this: gpio-regulators without an enable-gpios
> > property. My understanding is that regulator_disable() is a no-op in
> > that case (meson_mmc_set_ios() even has a comment above the
> > switch/case statement), see [0].
> >
> > > The code makes incorrect assumptions. Then the kernel resets the board
> > > without having enough time.
> > Can you please name the board you're testing? I'm worried that I'll be
> > looking at one .dts but you're looking at another one.
>
> https://github.com/libre-computer-project/libretech-linux/blob/master/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi#L481
>
> vcc_card is a gpio regulator that gets toggled on->off->on.
Thanks, that clears things up as I was indeed looking at a gpio
regulator while this is a fixed regulator!

> I traced the regulator framework a few weeks ago and forgot the final
> regulator disable function call, but that call basically returned
> immediately while the regulator-enable function complement had delays
> implemented.
Yep, for fixed regulators there's an "off-on-delay-us" device-tree
property (which translates to "off_on_delay" in the code).
Its implementation is smart enough to not waste time by adding delays
at runtime by implementing: on -> off + remember time -> wait
remaining time + on (meaning: if there was enough time between off and
the second on there's no additional wait) [0]. On system shutdown it
will not add any delay unfortunately (where Linux loses control over
time-keeping), meaning we can end up with too little waiting time.

Also my understanding is that it's not something that can be fixed in
u-boot or TF-A. This is because bootrom already has trouble reading
the next stage from an SD card (which is a valid boot media).


[0] https://elixir.bootlin.com/linux/v6.15/source/drivers/regulator/core.c#L2754

Re: [RFC] mmc: meson-gx-mmc: add delay during poweroff

[Da Xue]

On Wed, Jul 2, 2025 at 4:57 PM Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> On Wed, Jul 2, 2025 at 9:07 PM Da Xue <da@libre.computer> wrote:
> >
> > On Wed, Jul 2, 2025 at 2:40 PM Martin Blumenstingl
> > <martin.blumenstingl@googlemail.com> wrote:
> > >
> > > On Wed, Jul 2, 2025 at 7:22 PM Da Xue <da@libre.computer> wrote:
> > > >
> > > > On Wed, Jul 2, 2025 at 1:07 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
> > > > ...
> > > > > If, as the description suggest, the regulator framework somehow ignore
> > > > > the timing set in DT, maybe this is what needs to be checked ?
> > > >
> > > > The regulator framework only cares about timing for regulator on.
> > > > Regulator off just turns off the regulator and returns without delay.
> > > There's an exception to this: gpio-regulators without an enable-gpios
> > > property. My understanding is that regulator_disable() is a no-op in
> > > that case (meson_mmc_set_ios() even has a comment above the
> > > switch/case statement), see [0].
> > >
> > > > The code makes incorrect assumptions. Then the kernel resets the board
> > > > without having enough time.
> > > Can you please name the board you're testing? I'm worried that I'll be
> > > looking at one .dts but you're looking at another one.
> >
> > https://github.com/libre-computer-project/libretech-linux/blob/master/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi#L481
> >
> > vcc_card is a gpio regulator that gets toggled on->off->on.
> Thanks, that clears things up as I was indeed looking at a gpio
> regulator while this is a fixed regulator!
>
> > I traced the regulator framework a few weeks ago and forgot the final
> > regulator disable function call, but that call basically returned
> > immediately while the regulator-enable function complement had delays
> > implemented.
> Yep, for fixed regulators there's an "off-on-delay-us" device-tree
> property (which translates to "off_on_delay" in the code).
> Its implementation is smart enough to not waste time by adding delays
> at runtime by implementing: on -> off + remember time -> wait
> remaining time + on (meaning: if there was enough time between off and
> the second on there's no additional wait) [0]. On system shutdown it
> will not add any delay unfortunately (where Linux loses control over
> time-keeping), meaning we can end up with too little waiting time.

Yes, this is evident on quite a few Amlogic boards but occurred rarely
enough that it can be overlooked but never-the-less should be
addressed.

On our SM1 board, this occurs more often than not. With this patch, we
can reboot the loop indefinitely.

>
> Also my understanding is that it's not something that can be fixed in
> u-boot or TF-A. This is because bootrom already has trouble reading
> the next stage from an SD card (which is a valid boot media).

Correct, not fixable in TF-A or u-boot.

>
>
> [0] https://elixir.bootlin.com/linux/v6.15/source/drivers/regulator/core.c#L2754

Re: [RFC] mmc: meson-gx-mmc: add delay during poweroff

[Ulf Hansson]

On Wed, 2 Jul 2025 at 23:05, Da Xue <da@libre.computer> wrote:
>
> On Wed, Jul 2, 2025 at 4:57 PM Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
> >
> > On Wed, Jul 2, 2025 at 9:07 PM Da Xue <da@libre.computer> wrote:
> > >
> > > On Wed, Jul 2, 2025 at 2:40 PM Martin Blumenstingl
> > > <martin.blumenstingl@googlemail.com> wrote:
> > > >
> > > > On Wed, Jul 2, 2025 at 7:22 PM Da Xue <da@libre.computer> wrote:
> > > > >
> > > > > On Wed, Jul 2, 2025 at 1:07 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
> > > > > ...
> > > > > > If, as the description suggest, the regulator framework somehow ignore
> > > > > > the timing set in DT, maybe this is what needs to be checked ?
> > > > >
> > > > > The regulator framework only cares about timing for regulator on.
> > > > > Regulator off just turns off the regulator and returns without delay.
> > > > There's an exception to this: gpio-regulators without an enable-gpios
> > > > property. My understanding is that regulator_disable() is a no-op in
> > > > that case (meson_mmc_set_ios() even has a comment above the
> > > > switch/case statement), see [0].
> > > >
> > > > > The code makes incorrect assumptions. Then the kernel resets the board
> > > > > without having enough time.
> > > > Can you please name the board you're testing? I'm worried that I'll be
> > > > looking at one .dts but you're looking at another one.
> > >
> > > https://github.com/libre-computer-project/libretech-linux/blob/master/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi#L481
> > >
> > > vcc_card is a gpio regulator that gets toggled on->off->on.
> > Thanks, that clears things up as I was indeed looking at a gpio
> > regulator while this is a fixed regulator!
> >
> > > I traced the regulator framework a few weeks ago and forgot the final
> > > regulator disable function call, but that call basically returned
> > > immediately while the regulator-enable function complement had delays
> > > implemented.
> > Yep, for fixed regulators there's an "off-on-delay-us" device-tree
> > property (which translates to "off_on_delay" in the code).
> > Its implementation is smart enough to not waste time by adding delays
> > at runtime by implementing: on -> off + remember time -> wait
> > remaining time + on (meaning: if there was enough time between off and
> > the second on there's no additional wait) [0]. On system shutdown it
> > will not add any delay unfortunately (where Linux loses control over
> > time-keeping), meaning we can end up with too little waiting time.
>
> Yes, this is evident on quite a few Amlogic boards but occurred rarely
> enough that it can be overlooked but never-the-less should be
> addressed.
>
> On our SM1 board, this occurs more often than not. With this patch, we
> can reboot the loop indefinitely.

Even if this patch fixes the problem, it doesn't really seem like the
correct solution to me.

Would you mind trying to extend the regulator subsystem to deal with
this instead? Feel free to keep me in the loop if you post something
there.

>
> >
> > Also my understanding is that it's not something that can be fixed in
> > u-boot or TF-A. This is because bootrom already has trouble reading
> > the next stage from an SD card (which is a valid boot media).
>
> Correct, not fixable in TF-A or u-boot.
>
> >
> >
> > [0] https://elixir.bootlin.com/linux/v6.15/source/drivers/regulator/core.c#L2754

Kind regards
Uffe