On 2019-02-25 17:19, Ulf Hansson wrote: > On Tue, 19 Feb 2019 at 11:12, Jonas Karlman <jonas@xxxxxxxxx> wrote: >> 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 >> > For eMMC we have pwrseq_emmc.c, where we register a restart handler to > cover these cases. Not very nice, but the best we could come up with. Thanks, I will see if I can adopt something similar in a followup patch. > >>>> 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. > Okay, I see. > >>>> 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. > Great, so I am expecting an update from you, thanks! I have done some more testing and concluded that mmc_set_initial_signal_voltage() must be called before host->ios.vdd = 0 statement or the Tinker Board would not reboot from sd-card when 1.8V signal voltage is used. The sleep was not needed on my device. I still want to do more tests on my other rockchip boards that have UHS support and is not enabled in device tree. Currently I test [1] where vccio_sd regulator-always-on/regulator-boot-on ensures the IO regulator is not powered off, sd-uhs-sdr* flags to enable UHS and mmc_set_initial_signal_voltage() to change IO regulator to 3.3V before reboot. I will follow up with more test results next week after I have tested on other rockchip rk3328/rk3399 devices. [1] https://github.com/Kwiboo/linux-rockchip/compare/v5.0-rc7...patch-rk-5.x-tinker-uhs Regards, Jonas > > [...] > > Kind regards > Uffe
