[PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settings

[Soeren Moch]

On 12.10.19 06:09, Shawn Lin wrote:
> On 2019/10/11 22:16, Soeren Moch wrote:
>> On 11.10.19 15:00, Robin Murphy wrote:
>>> On 11/10/2019 12:40, Soeren Moch wrote:
>>>> On 11.10.19 10:22, Jonas Karlman wrote:
>>>>> On 2019-10-04 19:24, SÃren Moch wrote:
>>>>>> On 04.10.19 17:33, Shawn Lin wrote:
>>>>>>> On 2019/10/4 22:20, Robin Murphy wrote:
>>>>>>>> On 04/10/2019 04:39, Soeren Moch wrote:
>>>>>>>>> On 04.10.19 04:13, Shawn Lin wrote:
>>>>>>>>>> On 2019/10/4 8:53, Soeren Moch wrote:
>>>>>>>>>>> On 04.10.19 02:01, Robin Murphy wrote:
>>>>>>>>>>>> On 2019-10-03 10:50 pm, Soeren Moch wrote:
>>>>>>>>>>>>> According to the RockPro64 schematic [1] the rk3399 sdmmc
>>>>>>>>>>>>> controller is
>>>>>>>>>>>>> connected to a microSD (TF card) slot, which cannot be
>>>>>>>>>>>>> switched to
>>>>>>>>>>>>> 1.8V.
>>>>>>>>>>>> Really? AFAICS the SDMMC0 wiring looks pretty much identical
>>>>>>>>>>>> to the
>>>>>>>>>>>> NanoPC-T4 schematic (it's the same reference design, after
>>>>>>>>>>>> all),
>>>>>>>>>>>> and I
>>>>>>>>>>>> know that board can happily drive a UHS-I microSD card with
>>>>>>>>>>>> 1.8v
>>>>>>>>>>>> I/Os,
>>>>>>>>>>>> because mine's doing so right now.
>>>>>>>>>>>>
>>>>>>>>>>>> Robin.
>>>>>>>>>>> OK, the RockPro64 does not allow a card reset (power cycle)
>>>>>>>>>>> since
>>>>>>>>>>> VCC3V0_SD is directly connected to VCC3V3_SYS (via R89555), the
>>>>>>>>>>> SDMMC0_PWH_H signal is not connected. So the card fails to
>>>>>>>>>>> identify
>>>>>>>>>>> itself after suspend or reboot when switched to 1.8V operation.
>>>>>>>> Ah, thanks for clarifying - I did overlook the subtlety that
>>>>>>>> U12 and
>>>>>>>> friends have "NC" as alternative part numbers, even though they
>>>>>>>> aren't actually marked as DNP. So it's still not so much
>>>>>>>> "cannot be
>>>>>>>> switched" as "switching can lead to other problems".
>>>>>>>>
>>>>>>>>>> I believe we addressed this issue long time ago, please check:
>>>>>>>>>>
>>>>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a11fc47f175c8d87018e89cb58e2d36c66534cb
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> Thanks for the pointer.
>>>>>>>>> In this case I guess I should use following patch instead:
>>>>>>>>>
>>>>>>>>> --- rk3399-rockpro64.dts.bak ÂÂ 2019-10-03 22:14:00.067745799
>>>>>>>>> +0200
>>>>>>>>> +++ rk3399-rockpro64.dtsÂÂÂ 2019-10-04 00:02:50.047892366 +0200
>>>>>>>>> @@ -619,6 +619,8 @@
>>>>>>>>> ÂÂÂÂÂÂÂ max-frequency = <150000000>;
>>>>>>>>> ÂÂÂÂÂÂÂ pinctrl-names = "default";
>>>>>>>>> ÂÂÂÂÂÂÂ pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_bus4>;
>>>>>>>>> +ÂÂÂ sd-uhs-sdr104;
>>>>>>>>> +ÂÂÂ vqmmc-supply = <&vcc_sdio>;
>>>>>>>>> ÂÂÂÂÂÂÂ status = "okay";
>>>>>>>>> ÂÂÂÂ};
>>>>>>>>> When I do so, the sd card is detected as SDR104, but a reboot
>>>>>>>>> hangs:
>>>>>>>>>
>>>>>>>>> Boot1: 2018-06-26, version: 1.14
>>>>>>>>> CPUId = 0x0
>>>>>>>>> ChipType = 0x10, 286
>>>>>>>>> Spi_ChipId = c84018
>>>>>>>>> no find rkpartition
>>>>>>>>> SpiBootInit:ffffffff
>>>>>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>>>>>> emmc reinit
>>>>>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>>>>>> emmc reinit
>>>>>>>>> mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
>>>>>>>>> mmc: ERROR: Card did not respond to voltage select!
>>>>>>>>> SdmmcInit=2 1
>>>>>>>>> mmc0:cmd5,32
>>>>>>>>> mmc0:cmd7,32
>>>>>>>>> mmc0:cmd5,32
>>>>>>>>> mmc0:cmd7,32
>>>>>>>>> mmc0:cmd5,32
>>>>>>>>> mmc0:cmd7,32
>>>>>>>>> SdmmcInit=0 1
>>>>>>>>>
>>>>>>>>> So I guess I should use a different miniloader for this reboot to
>>>>>>>>> work!?
>>>>>>>>> Or what else could be wrong here?
>>>>>>>> Hmm, I guess this is "the Tinkerboard problem" again - the patch
>>>>>>>> above would be OK if we could get as far as the kernel, but can't
>>>>>>>> help if the
>>>>>>> I didn't realize that SD was used as boot medium for RockPro64,
>>>>>>> but I
>>>>>>> did patch the vendor tree to solve the issue for Tinkerboard, see
>>>>>>> https://github.com/rockchip-linux/kernel/commit/a4ccde21f5a9f04f996fb02479cb9f16d3dc8dc0
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> My initial plan was to patching upstream kernel by adding
>>>>>>> ->shutdown,but
>>>>>>> never finish it.
>>>>>>>
>>>>>>>> offending card is itself the boot medium. There was a proposal
>>>>>>>> here:
>>>>>>>>
>>>>>>>> https://patchwork.kernel.org/patch/10817217/
>>>>>>> This RFC also looks good to me, but seems it needs volunteers
>>>>>>> to push it again.
>>>>>> Oh, I think this is a totally wrong way.
>>>>>>
>>>>>> While this might work for some cards, setting the controller's i/o
>>>>>> voltage to 3.3V while leaving the card at 1.8V configuration is
>>>>>> totally
>>>>>> against the specification, can lead to all kinds of strange
>>>>>> behaviour
>>>>>> and even cause hardware damage. It also would actively defend the
>>>>>> purpose of the above mentioned patch (6a11fc4) where the kernel
>>>>>> guesses
>>>>>> the i/o voltage from the card configuration and switches the
>>>>>> controller
>>>>>> accordingly. We would end up with a 1.8V card and controller
>>>>>> configuration and a regulator voltage of 3.3V. This would only work
>>>>>> with
>>>>>> good luck. Even if the kernel driver would switch the regulator
>>>>>> back to
>>>>>> 1.8V in this case, the voltage mismatch remains in the bootloader
>>>>>> when
>>>>>> this card contains the boot image.
>>>>>>
>>>>>> The only sane way I see to handle this is implementing the same
>>>>>> workaround (mode guessing) also in the bootloader (rockchip
>>>>>> miniloader
>>>>>> and u-boot SPL since both bootloader chains are supported for this
>>>>>> board).
>>>>>>
>>>>>> Or maybe I miss something?
>>>>> Thanks for your input, I have made a new series [1] with a similar
>>>>> approach but is limited to dw_mmc-rockchip
>>>>> and only changes the regulator at power_off after the regulator has
>>>>> been disabled (the vqmmc regulator in affected devices is always-on).
>>>> Thanks for your work on this. Unfortunately I still think that this is
>>>> the wrong approach.
>>>> I see several problems in your patch series:
>>>> - You introduced GPIO0_PA1 as regulator enable for RockPro64.
>>>> Unfortunately Pine64 decided to disable this regulator on Board
>>>> Version
>>>> 2.1 (real product version), see above. I have no idea why they did
>>>> this.
>>>> - You changed the i/o voltage from 3.0V to 3.3V. This is not
>>>> allowed on
>>>> RK3399 I/O bank F.
>>>>
>>>> Changing the i/o voltage to 3.0V/3.3V while the sd card is configured
>>>> for 1.8V is against the specification and dangerous. While
>>>> experimenting
>>>> with different images (ayufan, armbian) for my newly bought
>>>> RockPro64 I
>>>> killed a SD card (32GB Samsung UHS-I). I cannot reconstruct the exact
>>>> circumstances, but I'm pretty sure this happened due to the voltage
>>>> mismatch. Of course I'm not keen to experiment further with this and
>>>> kill more sd cards. This is not just an theoretical issue.
>>>>> To my knowledge the problem is not with the rockchip miniloader or
>>>>> u-boot SPL, it is the initial boot rom that tries to load
>>>>> the miniloader/SPL from a SD-card, so nothing that can be updated.
>>>> What I observed on my RockPro64:
>>>> The ROM bootloader always was able to load the next stage, maybe
>>>> due to
>>>> the low initial clock of 400kHz? Also the ROM bootloader cannot change
>>>> voltage regulator settings. So if the i/o voltage still is at 1.8V and
>>>> matching the sd card setting, there is no problem for the ROM
>>>> bootloader.
>>>
>>> Hmm, that makes me wonder if the problem might be not so much that the
>>> level of SDMMC0_VDD itself stays at 1.8V, but that at some point after
>>> the bootrom the GRF_IO_VSEL bit gets reset so the controller just
>>> stops being able to read anything as logic-high.
>> Would be great if someone from Rockchip could give some insights whether
>> and when during reboot GRF_IO_VSEL is reset (ATF before reboot, some SoC
>> soft-reset, ROM bootloader, rkminiloader, something different), Shawn?
>
> ROM code and miniloader never touch this GRF_IO_VSEL for sdmmc, and only
> kernel did since now it support UHS mode. After reboot, the value should
> depends on the (value-in-kernel && does-reboot-level-hold-this-reg?)
>
> Different SoCs+PMICs excute differents reboot level policy.
OK, thanks. RockPro64 uses RK3399 and RK808D.
Is there such a soft-reset for GRF_IO_VSEL in RK3399?


Let's have another detailed look at the reboot process on my RockPro64
with SD UHS enabled:

[ÂÂ 84.675548] reboot: Restarting system
DDR Version 1.23 20190709
[...]
change freq to 856MHz 1,0
ch 0 ddrconfig = 0x101, ddrsize = 0x2020
ch 1 ddrconfig = 0x101, ddrsize = 0x2020
pmugrf_os_reg[2] = 0x3AA1FAA1, stride = 0xD
OUT
Boot1: 2018-06-26, version: 1.14
CPUId = 0x0
ChipType = 0x10, 287
Spi_ChipId = c84018
no find rkpartition
SpiBootInit:ffffffff
mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
mmc: ERROR: Card did not respond to voltage select!
emmc reinit
mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
mmc: ERROR: Card did not respond to voltage select!
emmc reinit
mmc: ERROR: SDHCI ERR:cmd:0x102,stat:0x18000
mmc: ERROR: Card did not respond to voltage select!
SdmmcInit=2 1
mmc0:cmd5,32
mmc0:cmd16,32
mmc0:cmd5,32
mmc0:cmd16,32
mmc0:cmd5,32ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ

mmc0:cmd16,32
SdmmcInit=0 1
powerOn 489837
Usb re Boot. 6489831
Usb re Boot. 12489837
Usb re Boot. 18489840
SoftReset
DDR Version 1.23 20190709
[boot loop]

What we can see here:
The ROM bootloader is able to access the SD card after reboot, it loads
and starts the DDR config firmware without any problem. The DDR firmware
runs through and loads and starts the miniloader without any problem.
The miniloader then tries to find RKTRUST on SPI (empty on my board),
eMMC (not populated) and SD card and fails to access the SD card
properly, if this card is configured for UHS mode. (With SD card at HS
not UHS configuration the miniloader just loads and starts ATF, TEE and
u-boot without any problem.)

So in principle the UHS configured SD card can be accessed after reboot
without any problem. At least ROM bootloader and DDR firmware do so. The
miniloader seems to do something differently and therefore fails.
Shawn, any idea what's going on there?

Soeren


>
>> Or gets this VSEL changed only when switching the voltage regulator (via
>> io_domains,sdmmc-supply)?
>>
>> Soeren
>>>
>>> Robin.
>>>
>>>> So I think the normal reboot handling should be:
>>>> If the sd card can be switched off (preferred solution), do so and
>>>> reset
>>>> the controller i/o voltage to 3.0V/3.3V.
>>>> If the sd card can not be switched off, make sure to leave the
>>>> controller i/o voltage at the current setting. Make sure in later boot
>>>> stages to not change the controller i/o voltage to 3.0V/3.3V when the
>>>> card is configured for 1.8V. According to the patch mentioned above
>>>> this
>>>> behaviour already is implemented in linux, we also need this for the
>>>> bootloaders.
>>>>
>>>> Regards,
>>>> Soeren
>>>>>
>>>>> I have observed this issue on the following devices, and the patches
>>>>> at [1] makes it possible to reboot from SD-card after UHS has been
>>>>> enabled.
>>>>> - Asus Tinker Board (RK3288)
>>>>> - Rockchip Sapphire Board (RK3399)
>>>>> - Radxa Rock Pi 4 (RK3399)
>>>>> - Pine64 RockPro64 (RK3399)
>>>>> All of the above seem to use the RK808 regulator for sd io voltage.
>>>>>
>>>>> The ROC-RK3328-CC did not have this issue and seem to automatically
>>>>> reset to 3.3v.
>>>>>
>>>>> [1]
>>>>> https://github.com/Kwiboo/linux-rockchip/compare/next-20191011...next-20191011-mmc
>>>>>
>>>>>
>>>>> Regards,
>>>>> Jonas
>>>>>
>>>>>> Soeren
>>>>>>
>>>>>>
>>>>>>>> although I'm not sure what if any progress has been made since
>>>>>>>> then.
>>>>>>>>
>>>>>>>> Robin.
>>>>>>>>
>>>>
>>>>
>>
>>
>>
>>
>>
>
>