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] [day] [month] [year] [list]
Message-ID: <CACT4zj8LxG_UeL22ERaP4XVwopdSjXz7mH95TyxXJ==WKZWHLw@mail.gmail.com>
Date: Tue, 2 Sep 2025 14:32:13 +0800
From: Ben Chuang <benchuanggli@...il.com>
To: Adrian Hunter <adrian.hunter@...el.com>
Cc: ulf.hansson@...aro.org, victor.shih@...esyslogic.com.tw, 
	ben.chuang@...esyslogic.com.tw, HL.Liu@...esyslogic.com.tw, 
	SeanHY.Chen@...esyslogic.com.tw, linux-mmc@...r.kernel.org, 
	linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH 1/2] mmc: sdhci-uhs2: Fix calling incorrect
 sdhci_set_clock() function

On Tue, Sep 2, 2025 at 12:50 AM Adrian Hunter <adrian.hunter@...el.com> wrote:
>
> On 01/09/2025 15:07, Adrian Hunter wrote:
> > On 01/09/2025 12:40, Ben Chuang wrote:
> >> From: Ben Chuang <ben.chuang@...esyslogic.com.tw>
> >>
> >> Fix calling incorrect sdhci_set_clock() in __sdhci_uhs2_set_ios() when the
> >> vendor defines its own sdhci_set_clock().
> >>
> >> Fixes: 10c8298a052b ("mmc: sdhci-uhs2: add set_ios()")
> >> Cc: stable@...r.kernel.org # v6.13+
> >> Signed-off-by: Ben Chuang <ben.chuang@...esyslogic.com.tw>
> >> ---
> >>  drivers/mmc/host/sdhci-uhs2.c | 5 ++++-
> >>  1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c
> >> index 0efeb9d0c376..704fdc946ac3 100644
> >> --- a/drivers/mmc/host/sdhci-uhs2.c
> >> +++ b/drivers/mmc/host/sdhci-uhs2.c
> >> @@ -295,7 +295,10 @@ static void __sdhci_uhs2_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> >>      else
> >>              sdhci_uhs2_set_power(host, ios->power_mode, ios->vdd);
> >>
> >> -    sdhci_set_clock(host, host->clock);
> >> +    if (host->ops->set_clock)
> >> +            host->ops->set_clock(host, host->clock);
> >> +    else
> >> +            sdhci_set_clock(host, host->clock);
> >
> > host->ops->set_clock is not optional.  So this should just be:
> >
> >       host->ops->set_clock(host, host->clock);
> >

I will update it. Thank you.

>
> Although it seems we are setting the clock in 2 places:
>
>         sdhci_uhs2_set_ios()
>                 sdhci_set_ios_common()
>                         host->ops->set_clock(host, ios->clock)
>               __sdhci_uhs2_set_ios
>                         sdhci_set_clock(host, host->clock)
>
> Do we really need both?
>

We only need one sdhci_set_clock() in __sdhci_uhs2_set_ios() for the
UHS-II card interface detection sequence.
Refer to Section 3.13.2, "Card Interface Detection Sequence" of the SD
Host Controller Standard Spec. Ver. 7.00,
First set the VDD1 power on and VDD2 power on, then enable the SD clock supply.

Do I need to add a separate patch or add it in the same patch like this?

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 3a17821efa5c..bd498b1bebce 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2369,7 +2369,8 @@ void sdhci_set_ios_common(struct mmc_host *mmc,
struct mmc_ios *ios)
                sdhci_enable_preset_value(host, false);

        if (!ios->clock || ios->clock != host->clock) {
-               host->ops->set_clock(host, ios->clock);
+               if (!mmc_card_uhs2(host->mmc))
+                       host->ops->set_clock(host, ios->clock);
                host->clock = ios->clock;

                if (host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK &&



Best regards,
Ben Chuang

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ