Re: [PATCH 3/3] arm64: dts: rockchip: fix RockPro64 sdmmc settingsãèææïéäçlinux-rockchip-bounces+shawn.lin=rock-chips.com@xxxxxxxxxxxxxxxxxxxäåã

[Jonas Karlman]

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).

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.

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.
>>>