mmc: meson-gx-mmc: add delay during poweroff
Subsystems (2)
RFC 1/1 mmc: meson-gx-mmc: add delay during poweroff
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.
--- drivers/mmc/host/meson-gx-mmc.c | 1 + 1 file changed, 1 insertion(+)
drivers/mmc/host/meson-gx-mmc.c +1 -0
@@ -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 ;
Signature · 1 line
2.39.5
Hi Da,
Quoted 11 lines · 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.
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
Martin Blumenstingl
reviewerHi Anand,
Quoted 15 lines · Anand Moon
Hi Da,
Quoted 11 lines · 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.
Quoted 5 lines
Although the driver defines reset identifiers such as
RESET_SD_EMMC_A,RESET_SD_EMMC_B, andRESET_SD_EMMC_C, It does not implement proper reset controller functionality, specifically lacking support
forreset_control_assert()andreset_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
Jerome Brunet
reviewerQuoted 34 lines · Martin Blumenstingl
Hi Anand,
Quoted 15 lines · Anand Moon
Hi Da,
Quoted 11 lines · 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.
Quoted 5 lines
Although the driver defines reset identifiers such as
RESET_SD_EMMC_A,RESET_SD_EMMC_B, andRESET_SD_EMMC_C, It does not implement proper reset controller functionality, specifically lacking support
forreset_control_assert()andreset_control_deassert()operations.I think there's a misunderstanding:
The meson-gx-mmc driver callsdevice_reset_optional()during .probe which will internally callreset_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).
Quoted 3 lines
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.
Quoted 4 lines
Best regards,
Martin
Signature · 1 line
Jerome
On Wed, Jul 2, 2025 at 1:07 PM Jerome Brunet <[email protected]> wrote:
...
Quoted 2 lines
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.
Quoted 6 lines
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.
Quoted 1 line
...
Quoted 3 lines
--
Jerome
Martin Blumenstingl
reviewerQuoted 8 lines · Da Xue
On Wed, Jul 2, 2025 at 1:07 PM Jerome Brunet <[email protected]> wrote:
...Quoted 2 lines
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].
Quoted 2 lines
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
Quoted 15 lines · Martin Blumenstingl
Quoted 8 lines · Da Xue
On Wed, Jul 2, 2025 at 1:07 PM Jerome Brunet <[email protected]> wrote:
...Quoted 2 lines
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].
Quoted 4 lines
Quoted 2 lines
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.
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.
Quoted 7 lines
Best regards,
Martin[0] https://elixir.bootlin.com/linux/v6.15/source/drivers/regulator/core.c#L2980
Martin Blumenstingl
reviewerQuoted 18 lines · Da Xue
Quoted 15 lines · Martin Blumenstingl
Quoted 8 lines · Da Xue
On Wed, Jul 2, 2025 at 1:07 PM Jerome Brunet <[email protected]> wrote:
...Quoted 2 lines
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].
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
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.
vcc_cardis a gpio regulator that gets toggledon->off->on.
Thanks, that clears things up as I was indeed looking at a gpio regulator while this is a fixed regulator!
Quoted 4 lines
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
Quoted 35 lines · Martin Blumenstingl
Quoted 18 lines · Da Xue
Quoted 15 lines · Martin Blumenstingl
Quoted 8 lines · Da Xue
On Wed, Jul 2, 2025 at 1:07 PM Jerome Brunet <[email protected]> wrote:
...Quoted 2 lines
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].Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
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.
vcc_cardis a gpio regulator that gets toggledon->off->on.Thanks, that clears things up as I was indeed looking at a gpio regulator while this is a fixed regulator!
Quoted 4 lines
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.
Quoted 8 lines
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.
Quoted 4 lines
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.
Quoted 47 lines · Da Xue
Quoted 35 lines · Martin Blumenstingl
Quoted 18 lines · Da Xue
Quoted 15 lines · Martin Blumenstingl
Quoted 8 lines · Da Xue
On Wed, Jul 2, 2025 at 1:07 PM Jerome Brunet <[email protected]> wrote:
...Quoted 2 lines
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].Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
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.
vcc_cardis a gpio regulator that gets toggledon->off->on.Thanks, that clears things up as I was indeed looking at a gpio regulator while this is a fixed regulator!
Quoted 4 lines
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.
Quoted 8 lines
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.
Quoted 6 lines
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.
Quoted 11 lines
Quoted 5 lines
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.
Kind regards
Uffe