Messages in this thread | | | From | Bough Chen <> | Subject | RE: [PATCH 3/5] mms: sdhci-esdhc-imx: Fix SDHCI_RESET_ALL for CQHCI | Date | Tue, 18 Oct 2022 07:22:04 +0000 |
| |
> -----Original Message----- > From: Brian Norris <briannorris@chromium.org> > Sent: 2022年10月18日 11:57 > To: Ulf Hansson <ulf.hansson@linaro.org> > Cc: Shawn Lin <shawn.lin@rock-chips.com>; Adrian Hunter > <adrian.hunter@intel.com>; Shawn Guo <shawnguo@kernel.org>; Fabio > Estevam <festevam@gmail.com>; Faiz Abbas <faiz_abbas@ti.com>; > dl-linux-imx <linux-imx@nxp.com>; Bough Chen <haibo.chen@nxp.com>; Al > Cooper <alcooperx@gmail.com>; linux-mmc@vger.kernel.org; Pengutronix > Kernel Team <kernel@pengutronix.de>; linux-kernel@vger.kernel.org; Florian > Fainelli <f.fainelli@gmail.com>; Sascha Hauer <s.hauer@pengutronix.de>; > Thierry Reding <thierry.reding@gmail.com>; Michal Simek > <michal.simek@xilinx.com>; Jonathan Hunter <jonathanh@nvidia.com>; > Sowjanya Komatineni <skomatineni@nvidia.com>; > linux-arm-kernel@lists.infradead.org; Broadcom internal kernel review list > <bcm-kernel-feedback-list@broadcom.com>; Brian Norris > <briannorris@chromium.org> > Subject: [PATCH 3/5] mms: sdhci-esdhc-imx: Fix SDHCI_RESET_ALL for CQHCI > > [[ NOTE: this is completely untested by the author, but included solely > because, as noted in commit df57d73276b8 ("mmc: sdhci-pci: Fix > SDHCI_RESET_ALL for CQHCI for Intel GLK-based controllers"), "other > drivers using CQHCI might benefit from a similar change, if they > also have CQHCI reset by SDHCI_RESET_ALL." We've now seen the same > bug on at least MSM, Arasan, and Intel hardware. ]] > > SDHCI_RESET_ALL resets will reset the hardware CQE state, but we aren't > tracking that properly in software. When out of sync, we may trigger various > timeouts. > > It's not typical to perform resets while CQE is enabled, but this may occur in > some suspend or error recovery scenarios. > > Fixes: bb6e358169bf ("mmc: sdhci-esdhc-imx: add CMDQ support") > Signed-off-by: Brian Norris <briannorris@chromium.org> > --- > > drivers/mmc/host/sdhci-esdhc-imx.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c > b/drivers/mmc/host/sdhci-esdhc-imx.c > index 55981b0f0b10..222c83929e20 100644 > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > @@ -1288,6 +1288,13 @@ static void esdhc_set_uhs_signaling(struct > sdhci_host *host, unsigned timing) > > static void esdhc_reset(struct sdhci_host *host, u8 mask) { > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host); > + > + if ((host->mmc->caps2 & MMC_CAP2_CQE) && (mask & SDHCI_RESET_ALL) > && > + imx_data->socdata->flags & ESDHC_FLAG_CQHCI)
I think we can remove the condition " imx_data->socdata->flags & ESDHC_FLAG_CQHCI" here. According to code logic, host->mmc->caps2 & MMC_CAP2_CQE means it already contain imx_data->socdata->flags & ESDHC_FLAG_CQHCI
Best Regards Haibo Chen
> + cqhci_deactivate(host->mmc); > + > sdhci_reset(host, mask); > > sdhci_writel(host, host->ier, SDHCI_INT_ENABLE); > -- > 2.38.0.413.g74048e4d9e-goog
| |