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

From: Robin Murphy
Date: Fri Oct 11 2019 - 09:00:34 EST


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.

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.