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

From: Florian Fainelli <f.fainelli@gmail.com>
To: Brian Norris <briannorris@chromium.org>,
	Florian Fainelli <f.fainelli@gmail.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Shawn Lin <shawn.lin@rock-chips.com>,
	linux-mmc@vger.kernel.org, Al Cooper <alcooperx@gmail.com>,
	Bjorn Andersson <andersson@kernel.org>,
	Sowjanya Komatineni <skomatineni@nvidia.com>,
	Broadcom internal kernel review list
	<bcm-kernel-feedback-list@broadcom.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Konrad Dybcio <konrad.dybcio@somainline.org>,
	NXP Linux Team <linux-imx@nxp.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Fabio Estevam <festevam@gmail.com>,
	Michal Simek <michal.simek@xilinx.com>,
	linux-kernel@vger.kernel.org, Shawn Guo <shawnguo@kernel.org>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	linux-arm-msm@vger.kernel.org, Haibo Chen <haibo.chen@nxp.com>,
	Andy Gross <agross@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	Faiz Abbas <faiz_abbas@ti.com>,
	Jonathan Hunter <jonathanh@nvidia.com>
Subject: Re: [PATCH v3 6/7] mmc: sdhci_am654: Fix SDHCI_RESET_ALL for CQHCI
Date: Tue, 25 Oct 2022 16:29:38 -0700	[thread overview]
Message-ID: <c677b825-012d-8f7f-2de9-16dd9363666c@gmail.com> (raw)
In-Reply-To: <Y1higmSUMLsxvXyq@google.com>

On 10/25/22 15: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.
>>
>> 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?
> 
> Now of course, you probably know behind the scenes that there are no
> other sdhci-brcmstb-relevant controllers that "support cqe", but AFAICT
> I have no way of knowing that a priori. The driver and bindings give
> (too much?) flexibility.

Yes that is fair enough, I will amend the binding document to make it 
clearer that 'supports-cqe' only applies in case of "brcm,bcm7216-sdhci" 
and not for other compatibles.
-- 
Florian


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel