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

Yao Zi
2025-03-02T16:18:39Z

On Sun, Mar 02, 2025 at 12:56:42PM +0100, Jonas Karlman wrote:
> 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:

Ack. I got your point but there's a typo (s/mmc0/mmc1) in my reply.

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

The original patch keeps the order of nodes in the SoC devicetree
(sorted by MMIO address), but alphabetical order seems more common. Will
fix in v2, thanks.

> >>> + bus-width = <4>;
> >>> + cap-mmc-highspeed;
> >>> + cap-sd-highspeed;
> >>> + disable-wp;
> >>> + rockchip,default-sample-phase = <90>;
> >>> + sd-uhs-sdr104;

Thanks,
Yao Zi