linux-kernel - Re: [PATCH 1/2] mmc: sdhci-of-dwcmshc: Add command queue support for rockchip SOCs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <8eec7ae4-c02f-4099-b5ec-065c8ea06f90@intel.com>
Date: Thu, 23 Oct 2025 13:13:07 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: Sebastian Reichel <sebastian.reichel@...labora.com>
CC: Ulf Hansson <ulf.hansson@...aro.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
	Heiko Stuebner <heiko@...ech.de>, <linux-mmc@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
	<linux-rockchip@...ts.infradead.org>, <kernel@...labora.com>, Yifeng Zhao
	<yifeng.zhao@...k-chips.com>
Subject: Re: [PATCH 1/2] mmc: sdhci-of-dwcmshc: Add command queue support for
 rockchip SOCs

On 16/10/2025 20:09, Sebastian Reichel wrote:
> Hi,
> 
> I will fix the typo in the commit message in PATCHv2.
> 
> On Thu, Oct 16, 2025 at 10:42:29AM +0300, Adrian Hunter wrote:
>>> +static void rk35xx_sdhci_cqe_enable(struct mmc_host *mmc)
>>> +{
>>> +	struct sdhci_host *host = mmc_priv(mmc);
>>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> +	struct dwcmshc_priv *dwc_priv = sdhci_pltfm_priv(pltfm_host);
>>> +	u32 reg;
>>> +
>>> +	reg = sdhci_readl(host, dwc_priv->vendor_specific_area2 + CQHCI_CFG);
>>> +	reg |= CQHCI_ENABLE;
>>> +	sdhci_writel(host, reg, dwc_priv->vendor_specific_area2 + CQHCI_CFG);
>>> +
>>> +	reg = sdhci_readl(host, SDHCI_PRESENT_STATE);
>>> +	while (reg & SDHCI_DATA_AVAILABLE) {
>>> +		sdhci_readl(host, SDHCI_BUFFER);
>>> +		reg = sdhci_readl(host, SDHCI_PRESENT_STATE);
>>> +	}
>>> +
>>> +	sdhci_writew(host, DWCMSHC_SDHCI_CQE_TRNS_MODE, SDHCI_TRANSFER_MODE);
>>> +
>>> +	sdhci_cqe_enable(mmc);
>>> +
>>> +	sdhci_writew(host, DWCMSHC_SDHCI_CQE_TRNS_MODE, SDHCI_TRANSFER_MODE);
>>
>> Transfer mode was set already 2 lines up
> 
> Indeed. I was not sure if this is an intentional quirk from Yifeng
> Zhao and thus kept this dual write.
> 
>>> +}
>>> +
>>> +static void rk35xx_sdhci_cqe_disabled(struct mmc_host *mmc, bool recovery)
>>
>> As mentioned elsewhere "disabled" -> "disable"
> 
> Ack, will fix in PATCHv2.
> 
>>> +{
>>> +	struct sdhci_host *host = mmc_priv(mmc);
>>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> +	struct dwcmshc_priv *dwc_priv = sdhci_pltfm_priv(pltfm_host);
>>> +	unsigned long flags;
>>> +	u32 ctrl;
>>> +
>>> +	mmc->cqe_ops->cqe_wait_for_idle(mmc);
>>
>> Is this necessary?  If so, it seems more like something that should be done by
>> cqhci itself.
> 
> The RK3588 TRM says that the software needs to verify that the eMMC
> controller is in idle state without any ongoing commands or data
> transfers before disabling the CQ_EN bit (CQHCI_ENABLE in the kernel).

Leave out cqe_wait_for_idle() but make use of ->pre_enable() and
->post_disable(), refer for example msdc_cqe_pre_enable() /
msdc_cqe_post_disable() or sdhci_tegra_cqe_pre_enable() /
sdhci_tegra_cqe_post_disable()