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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z8nPjjbZvpRJ2R5B@pie.lan>
Date: Thu, 6 Mar 2025 16:43:54 +0000
From: Yao Zi <ziyao@...root.org>
To: Chukun Pan <amadeus@....edu.cn>
Cc: conor+dt@...nel.org, cristian.ciocaltea@...labora.com,
	detlev.casanova@...labora.com, devicetree@...r.kernel.org,
	heiko@...ech.de, jonas@...boo.se, krzk+dt@...nel.org,
	linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-mmc@...r.kernel.org,
	linux-rockchip@...ts.infradead.org
Subject: Re: [PATCH v2 7/8] arm64: dts: rockchip: Add SDMMC/SDIO controllers
 for RK3528

On Thu, Mar 06, 2025 at 10:00:09PM +0800, Chukun Pan wrote:
> Hi,
> 
> > +		sdio0: mmc@...10000 {
> > +			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?

> > +			resets = <&cru SRST_H_SDIO0>;
> > +			reset-names = "reset";
> > +			status = "disabled";
> > +		};
> > +
> > +		sdio1: mmc@...20000 {
> > +			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