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

[SÃren Moch]

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?

Soeren


>
>>
>> although I'm not sure what if any progress has been made since then.
>>
>> Robin.
>>
>>
>>
>
>