From: Ben Chuang ben.chuang@genesyslogic.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@vger.kernel.org # v6.13+ Signed-off-by: Ben Chuang ben.chuang@genesyslogic.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); }
static int sdhci_uhs2_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
On 01/09/2025 12:40, Ben Chuang wrote:
From: Ben Chuang ben.chuang@genesyslogic.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@vger.kernel.org # v6.13+ Signed-off-by: Ben Chuang ben.chuang@genesyslogic.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);
} static int sdhci_uhs2_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
On 01/09/2025 15:07, Adrian Hunter wrote:
On 01/09/2025 12:40, Ben Chuang wrote:
From: Ben Chuang ben.chuang@genesyslogic.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@vger.kernel.org # v6.13+ Signed-off-by: Ben Chuang ben.chuang@genesyslogic.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);
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?
On Tue, Sep 2, 2025 at 12:50 AM Adrian Hunter adrian.hunter@intel.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@genesyslogic.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@vger.kernel.org # v6.13+ Signed-off-by: Ben Chuang ben.chuang@genesyslogic.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
On 02/09/2025 09:32, Ben Chuang wrote:
On Tue, Sep 2, 2025 at 12:50 AM Adrian Hunter adrian.hunter@intel.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@genesyslogic.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@vger.kernel.org # v6.13+ Signed-off-by: Ben Chuang ben.chuang@genesyslogic.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 &&
It can be a separate patch, but the whole of
if (!ios->clock || ios->clock != host->clock) { etc }
needs to move from sdhci_set_ios_common() into sdhci_set_ios() like further below. Note, once that is done, you need to add "host->clock = ios->clock;" to __sdhci_uhs2_set_ios() like: host->ops->set_clock(host, ios->clock); host->clock = ios->clock;
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 3a17821efa5c..ac7e11f37af7 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2367,23 +2367,6 @@ void sdhci_set_ios_common(struct mmc_host *mmc, struct mmc_ios *ios) (ios->power_mode == MMC_POWER_UP) && !(host->quirks2 & SDHCI_QUIRK2_PRESET_VALUE_BROKEN)) sdhci_enable_preset_value(host, false); - - if (!ios->clock || ios->clock != host->clock) { - host->ops->set_clock(host, ios->clock); - host->clock = ios->clock; - - if (host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK && - host->clock) { - host->timeout_clk = mmc->actual_clock ? - mmc->actual_clock / 1000 : - host->clock / 1000; - mmc->max_busy_timeout = - host->ops->get_max_timeout_count ? - host->ops->get_max_timeout_count(host) : - 1 << 27; - mmc->max_busy_timeout /= host->timeout_clk; - } - } } EXPORT_SYMBOL_GPL(sdhci_set_ios_common);
@@ -2410,6 +2393,23 @@ void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
sdhci_set_ios_common(mmc, ios);
+ if (!ios->clock || ios->clock != host->clock) { + host->ops->set_clock(host, ios->clock); + host->clock = ios->clock; + + if (host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK && + host->clock) { + host->timeout_clk = mmc->actual_clock ? + mmc->actual_clock / 1000 : + host->clock / 1000; + mmc->max_busy_timeout = + host->ops->get_max_timeout_count ? + host->ops->get_max_timeout_count(host) : + 1 << 27; + mmc->max_busy_timeout /= host->timeout_clk; + } + } + if (host->ops->set_power) host->ops->set_power(host, ios->power_mode, ios->vdd); else
On Wed, Sep 3, 2025 at 7:15 PM Adrian Hunter adrian.hunter@intel.com wrote:
On 02/09/2025 09:32, Ben Chuang wrote:
On Tue, Sep 2, 2025 at 12:50 AM Adrian Hunter adrian.hunter@intel.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@genesyslogic.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@vger.kernel.org # v6.13+ Signed-off-by: Ben Chuang ben.chuang@genesyslogic.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 &&It can be a separate patch, but the whole of
if (!ios->clock || ios->clock != host->clock) { etc }needs to move from sdhci_set_ios_common() into sdhci_set_ios() like further below. Note, once that is done, you need to add "host->clock = ios->clock;" to __sdhci_uhs2_set_ios() like: host->ops->set_clock(host, ios->clock); host->clock = ios->clock;
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 3a17821efa5c..ac7e11f37af7 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2367,23 +2367,6 @@ void sdhci_set_ios_common(struct mmc_host *mmc, struct mmc_ios *ios) (ios->power_mode == MMC_POWER_UP) && !(host->quirks2 & SDHCI_QUIRK2_PRESET_VALUE_BROKEN)) sdhci_enable_preset_value(host, false);
if (!ios->clock || ios->clock != host->clock) {host->ops->set_clock(host, ios->clock);host->clock = ios->clock;if (host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK &&host->clock) {host->timeout_clk = mmc->actual_clock ?mmc->actual_clock / 1000 :host->clock / 1000;mmc->max_busy_timeout =host->ops->get_max_timeout_count ?host->ops->get_max_timeout_count(host) :1 << 27;mmc->max_busy_timeout /= host->timeout_clk;}}} EXPORT_SYMBOL_GPL(sdhci_set_ios_common);
@@ -2410,6 +2393,23 @@ void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
sdhci_set_ios_common(mmc, ios);
if (!ios->clock || ios->clock != host->clock) {host->ops->set_clock(host, ios->clock);host->clock = ios->clock;if (host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK &&host->clock) {host->timeout_clk = mmc->actual_clock ?mmc->actual_clock / 1000 :host->clock / 1000;mmc->max_busy_timeout =host->ops->get_max_timeout_count ?host->ops->get_max_timeout_count(host) :1 << 27;mmc->max_busy_timeout /= host->timeout_clk;}}if (host->ops->set_power) host->ops->set_power(host, ios->power_mode, ios->vdd); else
I will add this as a separate patch and modify __sdhci_uhs2_set_ios().
Best regards, Ben Chuang
linux-stable-mirror@lists.linaro.org
-
Adrian Hunter -
Ben Chuang