Re: [PATCH 8/8] arm64: dts: rockchip: Enable SD-card interface on Radxa E20C

Jonas Karlman
2025-03-02T11:57:03Z

Hi Yao Zi,

On 2025-03-01 16:15, Yao Zi wrote:
> On Sat, Mar 01, 2025 at 02:01:05PM +0100, Jonas Karlman wrote:
>> Hi,
>>
>> On 2025-03-01 11:48, Yao Zi wrote:
>>> SD-card is available on Radxa E20C board.
>>>
>>> Signed-off-by: Yao Zi <ziyao@disroot.org>
>>> ---
>>> arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts | 14 ++++++++++++++
>>> 1 file changed, 14 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
>>> index d2cdb63d4a9d..473065aa4228 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts
>>> @@ -12,6 +12,10 @@ / {
>>> model = "Radxa E20C";
>>> compatible = "radxa,e20c", "rockchip,rk3528";
>>>
>>> + aliases {
>>> + mmc0 = &sdmmc;
>>
>> Suggest using mmc1 for sd-card because the e20c typically have onboard
>> emmc, compared to removable sd-card.
>
> My board doesn't have an eMMC: it's optional as well, but all variants
> of Radxa E20C come with an SD-card interface. The vendor devicetree sets
> sdmmc as mmc0 as well[1].

This is strange as Radxa typically want to align with mmc0=emmc and
mmc1=sd-card, as seen in [3] and [4].

Align with other Radxa products.
- mmc0 is eMMC
- mmc1 is microSD

Also mainline U-Boot for Rockchip SoCs typically always treat mmc0 as
emmc and mmc1 as sd-card, and for most SoCs it will even override the
board aliases to have some predictability across boards.

>
> I won't insist on it and am willing to take the change if you still
> consider mmc0 is better.

Yes, my position is that we should use following:

mmc0 = &sdhci;
mmc1 = &sdmmc;

I will send out a short sdhci series based on top of v2 of this series.
Driver changes was not needed to get basic sdhci working on RK3528 and
is only required to get HS400 modes working.

[3] https://lore.kernel.org/r/20240620224435.2752-1-naoki@radxa.com
[4] https://lore.kernel.org/r/20240619050047.1217-2-naoki@radxa.com

>
>>> + };
>>> +
>>> chosen {
>>> stdout-path = "serial0:1500000n8";
>>> };
>>> @@ -20,3 +24,13 @@ chosen {
>>> &uart0 {
>>> status = "okay";
>>> };
>>> +
>>> +&sdmmc {

This node should be placed above &uart0 to be in alphabetical order.

>>> + bus-width = <4>;
>>> + cap-mmc-highspeed;
>>> + cap-sd-highspeed;
>>> + disable-wp;
>>> + rockchip,default-sample-phase = <90>;
>>> + sd-uhs-sdr104;
>>
>> Are you sure uhs-sdr104 works as is should?
>
> In fact yes, tuning succeeds at 148.5MHz and results in 66MB/s reading
> speed.
>
>> Vendor kernel use a different "v2" tuning
>
> This isn't a problem. IMHO V2 tuning is more like a quick path, which
> tries inheritting the phase from firmware and then re-tunes roughly.
> Fine tunning is still a fallback here in case of failure, see the commit
> message in the downstream kernel[2]. And testing proves it's okay for
> RK3528 to issue fine-tuning always.

Thanks for this information, I did not inspect exactly what the v2
tuning meant, only observed that vendor kernel (incorrectly) used a DT
prop to indicate when v2 tuning should be used.

>
>> and this is also missing the vccio_sd vqmmc-supply to switch between
>> 3v3 and 1v8.
>
> But this is a problem, thanks for catching it! Somehow my card managed
> to run at 148.5MHz with 3v3 voltage level, but it's definitely a
> compatiblity issue. I'm surprised that the driver doesn't complain when
> switching to SDR modes without a regulator configured.
>
>> You could add following regulator for sdmmc:
>>
>> vccio_sd: regulator-vccio-sd {
>> compatible = "regulator-gpio";
>> gpios = <&gpio4 RK_PB6 GPIO_ACTIVE_HIGH>;
>> pinctrl-names = "default";
>> pinctrl-0 = <&sdmmc_vol_ctrl_h>;
>> regulator-name = "vccio_sd";
>> regulator-min-microvolt = <1800000>;
>> regulator-max-microvolt = <3300000>;
>> states = <1800000 0x0>, <3300000 0x1>;

This should also have something like:

vin-supply = <&vcc5v0_sys>;

>> };
>>
>> and following pinctrl:
>>
>> sdmmc {
>> sdmmc_vol_ctrl_h: sdmmc-vol-ctrl-h {
>> rockchip,pins = <4 RK_PB6 RK_FUNC_GPIO &pcfg_pull_none>;
>> };
>> };
>>
>> add then the power supplies to the sdmmc node:
>>
>> vmmc-supply = <&vcc_3v3>;
>> vqmmc-supply = <&vccio_sd>;
>>
>> That matches the schematics for e20c, and works when testing non-uhs modes.
>
> Thanks for the hints. Will rebase on your pinctrl series and get
> regulators and pinctrl settings applied in the next version.

Thanks :-)

Regards,
Jonas

>
>> Regards,
>> Jonas
>>
>>> + status = "okay";
>>> +};
>>
>
> Cheers,
> Yao Zi
>
> [1]: https://github.com/radxa/kernel/blob/2b0c8de7dc4c68947cda206dcc2e457e9677e426/arch/arm64/boot/dts/rockchip/rk3528-radxa-e20c.dts#L22-L26
> [2]: https://github.com/rockchip-linux/kernel/commit/795e052cc8610aa59a64b104f975cc4a45493d5d