> -----Original Message-----
> From: Marek Vasut <marex@xxxxxxx>
> Sent: 2022年12月20日 9:53
> To: linux-mmc@xxxxxxxxxxxxxxx
> Cc: Marek Vasut <marex@xxxxxxx>; Adrian Hunter <adrian.hunter@xxxxxxxxx>;
> Ulf Hansson <ulf.hansson@xxxxxxxxxx>; Zach Brown <zach.brown@xxxxxx>
> Subject: [PATCH] [RFC] mmc: sdhci: Always apply sdhci-caps-mask and
> sdhci-caps to caps
>
> The original implementation in the commit referenced below only modifies
> caps in case no caps are passed to sdhci_read_caps() via parameter, this does
> not seem correct. Always modify the caps according to the properties from DT.
>
> 92e0c44b92e4 ("mmc: sdhci: Use sdhci-caps-mask and sdhci-caps to change
Need to add Fixes as below:
Fixes: 92e0c44b92e4 ("mmc: sdhci: Use sdhci-caps-mask and sdhci-caps to change the caps read during __sdhci_read_caps")
I did a grep under the /drivers/mmc/host, seems all callers use NULL for the parameter caps and caps1,
So maybe we could just simplify the code like this:
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index f3af1bd0f7b9..0ed8c5b36ecb 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -4090,8 +4090,7 @@ static int sdhci_set_dma_mask(struct sdhci_host *host)
return ret;
}
-void __sdhci_read_caps(struct sdhci_host *host, const u16 *ver,
- const u32 *caps, const u32 *caps1)
+void __sdhci_read_caps(struct sdhci_host *host, const u16 *ver)
{
u16 v;
u64 dt_caps_mask = 0;
@@ -4124,24 +4123,16 @@ void __sdhci_read_caps(struct sdhci_host *host, const u16 *ver,
if (host->quirks & SDHCI_QUIRK_MISSING_CAPS)
return;
- if (caps) {
- host->caps = *caps;
- } else {
- host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
- host->caps &= ~lower_32_bits(dt_caps_mask);
- host->caps |= lower_32_bits(dt_caps);
- }
+ host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
+ host->caps &= ~lower_32_bits(dt_caps_mask);
+ host->caps |= lower_32_bits(dt_caps);
if (host->version < SDHCI_SPEC_300)
return;
- if (caps1) {
- host->caps1 = *caps1;
- } else {
- host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
- host->caps1 &= ~upper_32_bits(dt_caps_mask);
- host->caps1 |= upper_32_bits(dt_caps);
- }
+ host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
+ host->caps1 &= ~upper_32_bits(dt_caps_mask);
+ host->caps1 |= upper_32_bits(dt_caps);
}
EXPORT_SYMBOL_GPL(__sdhci_read_caps);
Best Regards
Haibo Chen
> the caps read during __sdhci_read_caps")
> Signed-off-by: Marek Vasut <marex@xxxxxxx>
> ---
> Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> Cc: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> Cc: Zach Brown <zach.brown@xxxxxx>
> To: linux-mmc@xxxxxxxxxxxxxxx
> ---
> Note: I am sending it as an RFC, because it seems I might be missing
> something obvious.
> ---
> drivers/mmc/host/sdhci.c | 20 ++++++--------------
> 1 file changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index
> f3af1bd0f7b95..52719d3118ffd 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -4124,24 +4124,16 @@ void __sdhci_read_caps(struct sdhci_host *host,
> const u16 *ver,
> if (host->quirks & SDHCI_QUIRK_MISSING_CAPS)
> return;
>
> - if (caps) {
> - host->caps = *caps;
> - } else {
> - host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
> - host->caps &= ~lower_32_bits(dt_caps_mask);
> - host->caps |= lower_32_bits(dt_caps);
> - }
> + host->caps = caps ? *caps : sdhci_readl(host, SDHCI_CAPABILITIES);
> + host->caps &= ~lower_32_bits(dt_caps_mask);
> + host->caps |= lower_32_bits(dt_caps);
>
> if (host->version < SDHCI_SPEC_300)
> return;
>
> - if (caps1) {
> - host->caps1 = *caps1;
> - } else {
> - host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
> - host->caps1 &= ~upper_32_bits(dt_caps_mask);
> - host->caps1 |= upper_32_bits(dt_caps);
> - }
> + host->caps1 = caps1 ? *caps1 : sdhci_readl(host, SDHCI_CAPABILITIES_1);
> + host->caps1 &= ~upper_32_bits(dt_caps_mask);
> + host->caps1 |= upper_32_bits(dt_caps);
> }
> EXPORT_SYMBOL_GPL(__sdhci_read_caps);
>
> --
> 2.35.1
