Messages in this thread | | | Date | Wed, 19 Oct 2022 15:19:20 -0700 | From | Brian Norris <> | Subject | Re: [PATCH v2 2/7] mmc: sdhci-of-arasan: Fix SDHCI_RESET_ALL for CQHCI |
| |
On Wed, Oct 19, 2022 at 02:59:39PM -0700, Florian Fainelli wrote: > On 10/19/22 14:54, Brian Norris wrote: > > The same bug was already found and fixed for two other drivers, in v5.7 > > and v5.9: > > > > 5cf583f1fb9c mmc: sdhci-msm: Deactivate CQE during SDHC reset > > df57d73276b8 mmc: sdhci-pci: Fix SDHCI_RESET_ALL for CQHCI for Intel GLK-based controllers > > > > The latter is especially prescient, saying "other drivers using CQHCI > > might benefit from a similar change, if they also have CQHCI reset by > > SDHCI_RESET_ALL."
> > --- a/drivers/mmc/host/sdhci-of-arasan.c > > +++ b/drivers/mmc/host/sdhci-of-arasan.c > > @@ -366,6 +366,9 @@ static void sdhci_arasan_reset(struct sdhci_host *host, u8 mask) > > struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > > struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host); > > + if ((host->mmc->caps2 & MMC_CAP2_CQE) && (mask & SDHCI_RESET_ALL)) > > + cqhci_deactivate(host->mmc); > > + > > sdhci_reset(host, mask); > > Cannot this be absorbed by sdhci_reset() that all of these drivers appear to > be utilizing since you have access to the host and the mask to make that > decision?
It potentially could.
I don't know if this is a specified SDHCI behavior that really belongs in the common helper, or if this is just a commonly-shared behavior. Per the comments I quote above ("if they also have CQHCI reset by SDHCI_RESET_ALL"), I chose to leave that as an implementation-specific behavior.
I suppose it's not all that harmful to do this even if some SDHCI controller doesn't have the same behavior/quirk.
I guess I also don't know if any SDHCI controllers will support command queueing (MMC_CAP2_CQE) via somethings *besides* CQHCI. I see CQE support in sdhci-sprd.c without CQHCI, although that driver doesn't set MMC_CAP2_CQE.
Brian
| |