linux-kernel - Re: [PATCH 1/2] mmc: sdhci-uhs2: Fix calling incorrect sdhci_set

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