Re: [PATCH v2 7/8] arm64: dts: rockchip: Add SDMMC/SDIO controllers for RK3528

[Yao Zi <ziyao@disroot.org>]

On Fri, Mar 07, 2025 at 12:05:16AM +0100, Jonas Karlman wrote:
> On 2025-03-06 17:43, Yao Zi wrote:
> > On Thu, Mar 06, 2025 at 10:00:09PM +0800, Chukun Pan wrote:
> >> Hi,
> >>
> >>> +		sdio0: mmc@ffc10000 {
> >>> +			compatible = "rockchip,rk3528-dw-mshc",
> >>> +				     "rockchip,rk3288-dw-mshc";
> >>> +			reg = <0x0 0xffc10000 0x0 0x4000>;
> >>> +			clocks = <&cru HCLK_SDIO0>,
> >>> +				 <&cru CCLK_SRC_SDIO0>,
> >>> +				 <&cru SCLK_SDIO0_DRV>,
> >>> +				 <&cru SCLK_SDIO0_SAMPLE>;
> >>> +			clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
> >>> +			fifo-depth = <0x100>;
> >>> +			interrupts = <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH>;
> >>> +			max-frequency = <150000000>;
> >>> +			pinctrl-names = "default";
> >>> +			pinctrl-0 = <&sdio0_bus4>, <&sdio0_clk>, <&sdio0_cmd>,
> >>> +				    <&sdio0_det>, <&sdio0_pwren>;
> >>
> >> The sdio module is usually "non-removable", no need det,
> >> and pwren may be other gpio (use mmc-pwrseq). So it should
> >> be `pinctrl-0 = <&sdio0_bus4>, <&sdio0_clk>, <&sdio0_cmd>;`
> > 
> > This doesn't affect the fact that these two pins are assigned as
> > functional pins for SDIO0, as pointed out by the datasheet[1].
> > 
> > But with more digging, I found the reference design[2] of Rockchip
> > actually uses the two pins as normal GPIOs. This is more obvious in
> > downstream devicetree of an EVB[3]. Most of the existing boards (Radxa
> > 2A, ArmSOM Sige 1) follow the reference design.
> > 
> > For me, it's kind of surprising that the SDIO IP functions with two
> > functional pins assigned as different modes. I'm not sure whether we
> > should apply pin configuration for these two pins in the SoC devicetree.
> > Jonas, what do you think about it?
> 
> I think it make sense to match the pins used by reference boards, i.e.
> the pinconf most likely to be used by majority of boards that will use
> the sdio interface.

Thanks, will take it.

> Of my RK3528 boards, only ArmSoM Sige1 use sdio for onboard wifi and
> there I currently have following in my work-in-progress board DT [4]:
> 
>   pinctrl-names = "default";
>   pinctrl-0 = <&sdio0_bus4>, <&sdio0_clk>, <&sdio0_cmd>, <&clkm1_32k_out>;
> 
> The Radxa ROCK 2A/2F seem to use USB for wifi/bt.
> 
> [4] https://github.com/Kwiboo/linux-rockchip/blob/next-20250305-rk3528/arch/arm64/boot/dts/rockchip/rk3528-armsom-sige1.dts
> 
> Regards,
> Jonas
> 
> > 
> >>> +			resets = <&cru SRST_H_SDIO0>;
> >>> +			reset-names = "reset";
> >>> +			status = "disabled";
> >>> +		};
> >>> +
> >>> +		sdio1: mmc@ffc20000 {
> >>> +			compatible = "rockchip,rk3528-dw-mshc",
> >>> +				     "rockchip,rk3288-dw-mshc";
> >>> +			reg = <0x0 0xffc20000 0x0 0x4000>;
> >>> +			clocks = <&cru HCLK_SDIO1>,
> >>> +				 <&cru CCLK_SRC_SDIO1>,
> >>> +				 <&cru SCLK_SDIO1_DRV>,
> >>> +				 <&cru SCLK_SDIO1_SAMPLE>;
> >>> +			clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
> >>> +			fifo-depth = <0x100>;
> >>> +			interrupts = <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
> >>> +			max-frequency = <150000000>;
> >>> +			pinctrl-names = "default";
> >>> +			pinctrl-0 = <&sdio1_bus4>, <&sdio1_clk>, <&sdio1_cmd>,
> >>> +				    <&sdio1_det>, <&sdio1_pwren>;
> >>
> >> Same here.
> >>
> >>> +			resets = <&cru SRST_H_SDIO1>;
> >>> +			reset-names = "reset";
> >>> +			status = "disabled";
> >>> +		};
> >>
> >> Thanks,
> >> Chukun
> >>
> >> -- 
> >> 2.25.1
> >>
> > 
> > Best regards,
> > Yao Zi
> > 
> > [1]: https://github.com/DeciHD/rockchip_docs/blob/main/rk3528/Rockchip%C2%A0RK3528%C2%A0Datasheet%C2%A0V1.0-20230522.pdf
> > [2]: https://github.com/DeciHD/rockchip_docs/blob/main/rk3528/RK3528_BOX_REF_V10_20230525.pdf
> > [3]: https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3528-evb1-ddr4-v10.dtsi#L128
> 

Best regards,
Yao Zi