linux-kernel - Re: [PATCH 2/3] arm64: dts: rockchip: Enable sdmmc2 on rock-3b and set it up for SDIO devices

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: <CA+Gksr+WvS-S+jeYYG=Bo9cemvnJmjsmU4aj9YnD3t8-HY7wbw@mail.gmail.com>
Date: Tue, 12 Nov 2024 15:35:44 +0100
From: Tamás Szűcs <tszucs@...ux.com>
To: Dragan Simic <dsimic@...jaro.org>
Cc: Jonas Karlman <jonas@...boo.se>, Tamás Szűcs <tszucs@...ux.com>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Heiko Stuebner <heiko@...ech.de>, FUKAUMI Naoki <naoki@...xa.com>, Chukun Pan <amadeus@....edu.cn>, 
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] arm64: dts: rockchip: Enable sdmmc2 on rock-3b and
 set it up for SDIO devices

Hi Jonas, Dragan,

I think it was totally fine to disable sdmmc2 at first, especially if
it couldn’t be tested or wasn’t needed right away. From what I’ve
seen, this board works great even at higher clock speeds than what
rk356x-base.dtsi suggests. I don’t have access to the RK3568 errata,
and there don’t seem to be any limits mentioned in the TRM either.
Overall, this board is doing just fine as it is.

Regarding device tree overlays, they would be ideal for implementing
secondary functions, such as PCIe endpoint mode for users with
specific requirements. However, the primary functions for PCIe on the
M2E will be root complex mode, along with SDIO host, etc. In my view,
the hardware is well-designed and interconnected. Users have a
reasonable expectation that these primary functions should work
seamlessly without additional configuration, right out of the box.

Dragan, what did you mean by SDIO related power timing requirements?

Kind regards,
Tamas



Tamás Szűcs
tszucs@...ux.com

On Tue, Nov 12, 2024 at 5:41 AM Dragan Simic <dsimic@...jaro.org> wrote:
>
> Hello Jonas and Tamas,
>
> On 2024-11-11 20:06, Jonas Karlman wrote:
> > On 2024-11-11 19:17, Tamás Szűcs wrote:
> >> Enable SDIO on Radxa ROCK 3 Model B M.2 Key E. Add all supported UHS-I
> >> rates and
> >> enable 200 MHz maximum clock. Also, allow host wakeup via SDIO IRQ.
> >>
> >> Signed-off-by: Tamás Szűcs <tszucs@...ux.com>
> >> ---
> >>  arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts | 8 +++++++-
> >>  1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts
> >> b/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts
> >> index 242af5337cdf..b7527ba418f7 100644
> >> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts
> >> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts
> >> @@ -688,14 +688,20 @@ &sdmmc2 {
> >>      cap-sd-highspeed;
> >>      cap-sdio-irq;
> >>      keep-power-in-suspend;
> >> +    max-frequency = <200000000>;
> >>      mmc-pwrseq = <&sdio_pwrseq>;
> >>      non-removable;
> >>      pinctrl-names = "default";
> >>      pinctrl-0 = <&sdmmc2m0_bus4 &sdmmc2m0_clk &sdmmc2m0_cmd>;
> >> +    sd-uhs-sdr12;
> >> +    sd-uhs-sdr25;
> >> +    sd-uhs-sdr50;
> >
> > I thought that lower speeds was implied by uhs-sdr104?
>
> Last time I went through the MMC drivers, they were implied.  IIRC,
> such backward mode compatibility is actually a requirement made by
> the MMC specification.
>
> >>      sd-uhs-sdr104;
> >> +    sd-uhs-ddr50;
> >>      vmmc-supply = <&vcc3v3_sys2>;
> >>      vqmmc-supply = <&vcc_1v8>;
> >> -    status = "disabled";
> >> +    wakeup-source;
> >> +    status = "okay";
> >
> > This should probably be enabled using an dt-overlay, there is no
> > SDIO device embedded on the board and the reason I left it disabled
> > in original board DT submission.
>
> Just went through the ROCK 3B schematic, version 1.51, and I think
> there should be no need for a separate overlay, because sdmmc2 goes
> to the M.2 slot on the board, which any user can plug an M.2 module
> into, and the SDIO interface is kind-of self-discoverable.
>
> Of course, all that unless there are some horribly looking :) error
> messages emitted to the kernel log when nothing is actually found,
> in which case the SDIO/MMC driers should be fixed first.  Also, I'm
> not sure what do we do with the possible SDIO-related power timing
> requirements?