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+Gksr+E6_cf46iKEufWLvQwAbsuC15nq6O6N6gP=x2-BVTnSA@mail.gmail.com>
Date: Wed, 13 Nov 2024 11:24:18 +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,

On Tue, Nov 12, 2024 at 11:37 PM Jonas Karlman <jonas@...boo.se> wrote:
>
> Hi Tamás,
>
> On 2024-11-12 15:36, Tamás Szűcs wrote:
> > 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?
>
> Yes, the issue is that the io voltage domain must be configured to match
> the io signal voltage used, and to use uhs the voltage changes from 3v3
> to 1v8. Causing a miss-match between io voltage domain config and the
> regulator voltage used during initial probe, unless io-domain driver
> happens to be fully loaded before mmc devices are probed.
>
> >
> > 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.
>
> Yes, as you have discovered, inserting the sd-card after system has
> booted and io-domain driver has been loaded, everything can work as
> expected with uhs speeds.
>
> Until this probe race condition has been solved booting with a sd-card
> inserted may or may not result in wrong tuning or other related issues.
>
> Because of this I advice not to enable uhs mode for sdmmc0 at this time.

All right, and thank you for the explanation. My hands are full at the
moment but let me think about this.

>
> Regards,
> Jonas
>
> >
> > 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";
> >>
>