Re: [PATCH v3 6/7] mmc: sdhci_am654: Fix SDHCI_RESET_ALL for CQHCI

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/26/22 11:18, Brian Norris wrote:
Hi Adrian,

On Wed, Oct 26, 2022 at 08:36:48AM +0300, Adrian Hunter wrote:
On 26/10/22 01:26, Brian Norris wrote:
On Tue, Oct 25, 2022 at 02:53:46PM -0700, Florian Fainelli wrote:
On 10/25/22 14:45, Brian Norris wrote:
On Tue, Oct 25, 2022 at 04:10:44PM +0300, Adrian Hunter wrote:
On 24/10/22 20:55, Brian Norris wrote:
diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index 8f1023480e12..6a282c7a221e 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c

@@ -378,7 +379,7 @@ static void sdhci_am654_reset(struct sdhci_host *host, u8 mask)
   	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
   	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
-	sdhci_reset(host, mask);
+	sdhci_and_cqhci_reset(host, mask);
   	if (sdhci_am654->quirks & SDHCI_AM654_QUIRK_FORCE_CDTEST) {
   		ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);

What about sdhci_reset in sdhci_am654_ops ?

Oops, I think you caught a big fallacy in some of my patches: I assumed
there was a single reset() implementation in a given driver (an unwise
assumption, I realize). I see at least sdhci-brcmstb.c also has several
variant ops that call sdhci_reset(), and I should probably convert them
too.

I checked and found only sdhci_am654_ops

And...how about sdhci_j721e_8bit_ops in that same driver?

You got it right for sdhci-brcmstb.c because "supports-cqe" which gates the
enabling of CQE can only be found with the "brcm,bcm7216-sdhci" compatible
which implies using brcmstb_reset().

I don't see any in-tree device trees for these chips (which is OK), and
that's not what the Documentation/ says, and AFAICT nothing in the
driver is limiting other variants from specifying the "supports-cqe"
flag in their (out-of-tree) device tree. The closest thing I see is that
an *example* in brcm,sdhci-brcmstb.yaml shows "supports-cqe" only on
brcm,bcm7216-sdhci -- but an example is not a binding agreement. Am I
missing something?

It was mentioned in the patch from the Fixes tag.

OK, good note. If I don't patch the other seemingly-unaffected variants
in brcmstb, I'll at least update the commit message, since the code
doesn't tell me they're unaffected.

You can mention in the commit message that they are unaffected and quote me on that if you feel like this needs to be explicitly said.
--
Florian




[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux