| Message ID | 20230105144628.223420-1-marex@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: sdhci: Always apply sdhci-caps-mask and sdhci-caps to caps | expand |
On 5/01/23 16:46, Marek Vasut wrote: > 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. > > While at it, drop the always NULL caps and caps1 function parameters. Please re-base on top of: https://lore.kernel.org/linux-mmc/20230113110011.129835-1-adrian.hunter@intel.com/T/#t > > Signed-off-by: Marek Vasut <marex@denx.de> > --- > Cc: Adrian Hunter <adrian.hunter@intel.com> > Cc: Bough Chen <haibo.chen@nxp.com> > Cc: Ulf Hansson <ulf.hansson@linaro.org> > Cc: Zach Brown <zach.brown@ni.com> > To: linux-mmc@vger.kernel.org > --- > drivers/mmc/host/sdhci-cadence.c | 2 +- > drivers/mmc/host/sdhci.c | 23 +++++++---------------- > drivers/mmc/host/sdhci.h | 5 ++--- > 3 files changed, 10 insertions(+), 20 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c > index 6f2de54a59877..af1bd66402cb7 100644 > --- a/drivers/mmc/host/sdhci-cadence.c > +++ b/drivers/mmc/host/sdhci-cadence.c > @@ -390,7 +390,7 @@ static int sdhci_cdns_probe(struct platform_device *pdev) > host->mmc_host_ops.hs400_enhanced_strobe = > sdhci_cdns_hs400_enhanced_strobe; > sdhci_enable_v4_mode(host); > - __sdhci_read_caps(host, &version, NULL, NULL); > + __sdhci_read_caps(host, &version); > > sdhci_get_of_property(pdev); > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index f3af1bd0f7b95..0ed8c5b36ecb9 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); > > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > index 605eaee805f74..19695e542161e 100644 > --- a/drivers/mmc/host/sdhci.h > +++ b/drivers/mmc/host/sdhci.h > @@ -757,8 +757,7 @@ static inline void *sdhci_priv(struct sdhci_host *host) > return host->private; > } > > -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); > int sdhci_setup_host(struct sdhci_host *host); > void sdhci_cleanup_host(struct sdhci_host *host); > int __sdhci_add_host(struct sdhci_host *host); > @@ -767,7 +766,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead); > > static inline void sdhci_read_caps(struct sdhci_host *host) > { > - __sdhci_read_caps(host, NULL, NULL, NULL); > + __sdhci_read_caps(host, NULL); > } > > u16 sdhci_calc_clk(struct sdhci_host *host, unsigned int clock,
On 1/13/23 12:32, Adrian Hunter wrote: > On 5/01/23 16:46, Marek Vasut wrote: >> 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. >> >> While at it, drop the always NULL caps and caps1 function parameters. > > Please re-base on top of: > > https://lore.kernel.org/linux-mmc/20230113110011.129835-1-adrian.hunter@intel.com/T/#t Rebase on top of that series means I can just discard this entire patch, so done.
diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c index 6f2de54a59877..af1bd66402cb7 100644 --- a/drivers/mmc/host/sdhci-cadence.c +++ b/drivers/mmc/host/sdhci-cadence.c @@ -390,7 +390,7 @@ static int sdhci_cdns_probe(struct platform_device *pdev) host->mmc_host_ops.hs400_enhanced_strobe = sdhci_cdns_hs400_enhanced_strobe; sdhci_enable_v4_mode(host); - __sdhci_read_caps(host, &version, NULL, NULL); + __sdhci_read_caps(host, &version); sdhci_get_of_property(pdev); diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index f3af1bd0f7b95..0ed8c5b36ecb9 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); diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index 605eaee805f74..19695e542161e 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -757,8 +757,7 @@ static inline void *sdhci_priv(struct sdhci_host *host) return host->private; } -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); int sdhci_setup_host(struct sdhci_host *host); void sdhci_cleanup_host(struct sdhci_host *host); int __sdhci_add_host(struct sdhci_host *host); @@ -767,7 +766,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead); static inline void sdhci_read_caps(struct sdhci_host *host) { - __sdhci_read_caps(host, NULL, NULL, NULL); + __sdhci_read_caps(host, NULL); } u16 sdhci_calc_clk(struct sdhci_host *host, unsigned int clock,
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. While at it, drop the always NULL caps and caps1 function parameters. Signed-off-by: Marek Vasut <marex@denx.de> --- Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Bough Chen <haibo.chen@nxp.com> Cc: Ulf Hansson <ulf.hansson@linaro.org> Cc: Zach Brown <zach.brown@ni.com> To: linux-mmc@vger.kernel.org --- drivers/mmc/host/sdhci-cadence.c | 2 +- drivers/mmc/host/sdhci.c | 23 +++++++---------------- drivers/mmc/host/sdhci.h | 5 ++--- 3 files changed, 10 insertions(+), 20 deletions(-)