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

[Jonas Karlman <jonas@kwiboo.se>]

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.

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