On 2019-02-18 12:54, Ulf Hansson wrote: > On Sun, 17 Feb 2019 at 23:14, Jonas Karlman <jonas@xxxxxxxxx> wrote: >> Some boards have SD card connectors where the power rail cannot be switched >> off by the driver. If the card has not been power cycled, it may still be >> using 1.8V signaling after a warm re-boot. Bootroms expecting 3.3V signaling >> will fail to boot from a UHS card that continue to use 1.8V signaling. > Is the problem limited to a "warm manual re-boot" or does it exists > for an emergency reboot as well!? I think the issue may also exist for emergency reboot based on the Asus Tinker Board kernel commit at [1], not sure how this could be solved in an acceptable way. Personally I do not have sysrq enabled on my boards. [1] https://github.com/TinkerBoard/debian_kernel/commit/1e8970dbfc20166788e3428ec5203f7673f6087b > >> Set initial signal voltage in mmc_power_off() to allow re-boot to function. > I think this sounds like a reasonable way forward, to improve the situation. I may have exaggerated the issue in the above line, the board will reboot as long as UHS 1.8V signaling is not used, current upstream device trees is missing the sd-uhs-* flags and will thus reboot as they are capped at sd high speed and 3.3V signaling. > >> This fixes re-boot with UHS cards on Asus Tinker Board (Rockchip RK3288), >> same issue have been seen on some Rockchip RK3399 boards. >> >> I am sending this as a RFC because I have no insights into SD/MMC subsystem, >> this change fix a re-boot issue on my boards and does not break emmc/sdio. >> Is this an acceptable workaround? Any advice is appreciated. >> >> Signed-off-by: Jonas Karlman <jonas@xxxxxxxxx> >> --- >> drivers/mmc/core/core.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >> index 5bd58b95d318..69d7021916ae 100644 >> --- a/drivers/mmc/core/core.c >> +++ b/drivers/mmc/core/core.c >> @@ -1684,6 +1684,14 @@ void mmc_power_off(struct mmc_host *host) >> if (host->ios.power_mode == MMC_POWER_OFF) >> return; >> >> + mmc_set_initial_signal_voltage(host); > I would rather try to move this below mmc_set_initial_state() a few > lines below in mmc_power_off(). To me, it seems safer to do the > regular power off thingy first. > > Additionally, I would drop the added delay below, as there is already > a delay after calling mmc_set_initial_state() and I think/hope that > should be sufficient. Thanks for the suggestion, the reason I put the mmc_set_initial_signal_voltage() call before mmc_pwrseq_power_off() call was to do it in reverse order of mmc_power_on(), I will run some more tests on my boards to see if moving it below mmc_set_initial_state() also makes it possible to reboot with UHS cards using 1.8V signaling enabled. Regards, Jonas > >> + >> + /* >> + * This delay should be sufficient to allow the power supply >> + * to reach the minimum voltage. >> + */ >> + mmc_delay(host->ios.power_delay_ms); >> + >> mmc_pwrseq_power_off(host); >> >> host->ios.clock = 0; >> -- >> 2.17.1 >> > Kind regards > Uffe
