linux-kernel - Re: [PATCH 1/3] arm64: dts: rockchip: Add supported UHS-I rates to sdmmc0 on rock-3b

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+GksrJzkxffSQbuseGFL0=2PDxV+TRevX0-NOkc6FNYLmNgYA@mail.gmail.com>
Date: Tue, 12 Nov 2024 15:36:02 +0100
From: Tamás Szűcs <tszucs@...ux.com>
To: Jonas Karlman <jonas@...boo.se>
Cc: 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>, Dragan Simic <dsimic@...jaro.org>, 
	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 1/3] arm64: dts: rockchip: Add supported UHS-I rates to
 sdmmc0 on rock-3b

Hi Jonas,

Thank you for pointing this out! I haven't noticed this before. I've
done some testing and I believe I am able to reproduce the issue you
described, although I cannot confirm the reason.
The only occasion I encounter any problems is when a UHS SD card or
SDIO device is connected to sdmmc0 during bootup. Sometimes the device
is recognized as HS only. Obviously no tuning value reported. Also,
sdmmc2 cuts out completely. I'm booting from eMMC and when the SD card
is removed in this state I lose my rootfs. Certainly, this needs more
attention but it seems to be unrelated to the changes here.

I need more time to check but are you sure this SD card during bootup
issue is gone with UHS-I disabled?

Also, in every other case, when you connect any device to sdmmc0 after
bootup, performance and stability is perfect.
Interestingly I also don't experience this behavior with an eMMC
device and / or an SDIO device connected to sdmmc2 during bootup. Only
sdmmc0 is problematic and only during bootup.

Any more thoughts on this are very welcome.

Kind regards,
Tamas



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

On Mon, Nov 11, 2024 at 8:00 PM Jonas Karlman <jonas@...boo.se> wrote:
>
> Hi Tamás,
>
> On 2024-11-11 19:17, Tamás Szűcs wrote:
> > Add all supported UHS-I rates to sdmmc0 and allow 200 MHz maximum clock to
> > benefit modern SD cards.
> >
> > Signed-off-by: Tamás Szűcs <tszucs@...ux.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts
> > index 3d0c1ccfaa79..242af5337cdf 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts
> > @@ -670,8 +670,14 @@ &sdmmc0 {
> >       bus-width = <4>;
> >       cap-sd-highspeed;
> >       disable-wp;
> > +     max-frequency = <200000000>;
> >       pinctrl-names = "default";
> >       pinctrl-0 = <&sdmmc0_bus4 &sdmmc0_clk &sdmmc0_cmd &sdmmc0_det>;
> > +     sd-uhs-sdr12;
> > +     sd-uhs-sdr25;
> > +     sd-uhs-sdr50;
> > +     sd-uhs-sdr104;
> > +     sd-uhs-ddr50;
>
> There is an issue with io-domain driver not always being probed before
> mmc driver, this typically result in io-domain being configured wrong,
> and mmc tuning happen before io-domain is correctly configured.
>
> You can usually observe this by looking at the tuning value during boot
> and comparing it to the tuning value after removing and re-insering a
> sd-card.
>
> Because of this uhs modes was left out from initial DT submission, some
> cards will work others wont, sd-uhs-sdr50 is known to be working with
> most cards even with the probe order issue.
>
> Also I thought that lower speeds where implied?
>
> Regards,
> Jonas
>
> >       vmmc-supply = <&vcc3v3_sd>;
> >       vqmmc-supply = <&vccio_sd>;
> >       status = "okay";
>