| Message ID | 20250301104835.36439-1-ziyao@disroot.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Support SD/SDIO controllers on RK3528 | expand |
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. > + }; > + > chosen { > stdout-path = "serial0:1500000n8"; > }; > @@ -20,3 +24,13 @@ chosen { > &uart0 { > status = "okay"; > }; > + > +&sdmmc { > + 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? Vendor kernel use a different "v2" tuning and this is also missing the vccio_sd vqmmc-supply to switch between 3v3 and 1v8. 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>; }; 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. Regards, Jonas > + status = "okay"; > +};
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]. I won't insist on it and am willing to take the change if you still consider mmc0 is better. > > + }; > > + > > chosen { > > stdout-path = "serial0:1500000n8"; > > }; > > @@ -20,3 +24,13 @@ chosen { > > &uart0 { > > status = "okay"; > > }; > > + > > +&sdmmc { > > + 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. > 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>; > }; > > 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. > 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
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
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
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; + }; + chosen { stdout-path = "serial0:1500000n8"; }; @@ -20,3 +24,13 @@ chosen { &uart0 { status = "okay"; }; + +&sdmmc { + bus-width = <4>; + cap-mmc-highspeed; + cap-sd-highspeed; + disable-wp; + rockchip,default-sample-phase = <90>; + sd-uhs-sdr104; + status = "okay"; +};
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(+)