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 for Android: free password hash cracker in your pocket

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

Message-ID: <303ad3910668e852d6670d1c79dc22e0@manjaro.org>
Date: Tue, 12 Nov 2024 16:15:23 +0100
From: Dragan Simic <dsimic@...jaro.org>
To: Tamás Szűcs <tszucs@...ux.com>
Cc: Jonas Karlman <jonas@...boo.se>, 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

Hello Tamas,

On 2024-11-12 15:35, Tamás Szűcs wrote:
> 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.

Sorry, I'm missing the point of mentioning some clock speeds?  Any
chances, please, to clarify that a bit?

> 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.

That's basically what I referred to in my earlier response, and in my
previous response regarding the UART.  Users would expect the Bluetooth
part to work as well, but the error messages I mentioned look nasty, so
perhaps something should be done about that first.

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

Whenever there's an SDIO module, there's usually some required timing
of the power rails.  Though, I don't know what's that like with the
non-standard M.2 SDIO modules that Radxa sells, which are intended to
be used on Radxa boards with "hybrid" M.2 slots.

Once again, please use inline replying. [*]

[*] https://en.wikipedia.org/wiki/Posting_style

> 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?