linux-kernel - Re: [PATCH v2] mmc: sdhci-of-arasan: Ensure CD logic stabilization before power-up

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: <07748607-e13e-4cb6-a620-5d170c6cf208@amd.com>
Date: Tue, 29 Jul 2025 14:32:26 +0200
From: Michal Simek <michal.simek@....com>
To: Sai Krishna Potthuri <sai.krishna.potthuri@....com>,
 Adrian Hunter <adrian.hunter@...el.com>, Ulf Hansson <ulf.hansson@...aro.org>
Cc: linux-mmc@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, git@....com, saikrishna12468@...il.com
Subject: Re: [PATCH v2] mmc: sdhci-of-arasan: Ensure CD logic stabilization
 before power-up



On 7/29/25 08:45, Sai Krishna Potthuri wrote:
> During SD suspend/resume without a full card rescan (when using
> non-removable SD cards for rootfs), the SD card initialization may fail
> after resume. This occurs because, after a host controller reset, the
> card detect logic may take time to stabilize due to debounce logic.
> Without waiting for stabilization, the host may attempt powering up the
> card prematurely, leading to command timeouts during resume flow.
> Add sdhci_arasan_set_power_and_bus_voltage() to wait for the card detect
> stable bit before power up the card. Since the stabilization time
> is not fixed, a maximum timeout of one second is used to ensure
> sufficient wait time for the card detect signal to stabilize.
> 
> Signed-off-by: Sai Krishna Potthuri <sai.krishna.potthuri@....com>
> ---
> Changes in v2:
> - Use read_poll_timeout() instead of readl_poll_timeout().
> - Enable the CD stable check using platform specific quirk.
> - Define the quirk for Xilinx/AMD ZynqMP, Versal and Versal NET platforms.
> 
>   drivers/mmc/host/sdhci-of-arasan.c | 32 ++++++++++++++++++++++++++++--
>   1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index 42878474e56e..86da766fa480 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -99,6 +99,9 @@
>   #define HIWORD_UPDATE(val, mask, shift) \
>   		((val) << (shift) | (mask) << ((shift) + 16))
>   
> +#define CD_STABLE_TIMEOUT_US		1000000
> +#define CD_STABLE_MAX_SLEEP_US		10
> +
>   /**
>    * struct sdhci_arasan_soc_ctl_field - Field used in sdhci_arasan_soc_ctl_map
>    *
> @@ -206,6 +209,8 @@ struct sdhci_arasan_data {
>    * 19MHz instead
>    */
>   #define SDHCI_ARASAN_QUIRK_CLOCK_25_BROKEN BIT(2)
> +/* Enable CD stable check before power-up */
> +#define SDHCI_ARASAN_QUIRK_ENSURE_CD_STABLE	BIT(3)
>   };
>   
>   struct sdhci_arasan_of_data {
> @@ -514,6 +519,24 @@ static int sdhci_arasan_voltage_switch(struct mmc_host *mmc,
>   	return -EINVAL;
>   }
>   
> +static void sdhci_arasan_set_power_and_bus_voltage(struct sdhci_host *host, unsigned char mode,
> +						   unsigned short vdd)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
> +	u32 reg;
> +
> +	/*
> +	 * Ensure that the card detect logic has stabilized before powering up, this is
> +	 * necessary after a host controller reset.
> +	 */
> +	if (mode == MMC_POWER_UP && sdhci_arasan->quirks & SDHCI_ARASAN_QUIRK_ENSURE_CD_STABLE)
> +		read_poll_timeout(sdhci_readl, reg, reg & SDHCI_CD_STABLE, CD_STABLE_MAX_SLEEP_US,
> +				  CD_STABLE_TIMEOUT_US, false, host, SDHCI_PRESENT_STATE);
> +
> +	sdhci_set_power_and_bus_voltage(host, mode, vdd);
> +}
> +
>   static const struct sdhci_ops sdhci_arasan_ops = {
>   	.set_clock = sdhci_arasan_set_clock,
>   	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
> @@ -521,7 +544,7 @@ static const struct sdhci_ops sdhci_arasan_ops = {
>   	.set_bus_width = sdhci_set_bus_width,
>   	.reset = sdhci_arasan_reset,
>   	.set_uhs_signaling = sdhci_set_uhs_signaling,
> -	.set_power = sdhci_set_power_and_bus_voltage,
> +	.set_power = sdhci_arasan_set_power_and_bus_voltage,
>   	.hw_reset = sdhci_arasan_hw_reset,
>   };
>   
> @@ -570,7 +593,7 @@ static const struct sdhci_ops sdhci_arasan_cqe_ops = {
>   	.set_bus_width = sdhci_set_bus_width,
>   	.reset = sdhci_arasan_reset,
>   	.set_uhs_signaling = sdhci_set_uhs_signaling,
> -	.set_power = sdhci_set_power_and_bus_voltage,
> +	.set_power = sdhci_arasan_set_power_and_bus_voltage,
>   	.irq = sdhci_arasan_cqhci_irq,
>   };
>   
> @@ -1937,6 +1960,11 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
>   	if (of_device_is_compatible(np, "rockchip,rk3399-sdhci-5.1"))
>   		sdhci_arasan_update_clockmultiplier(host, 0x0);
>   
> +	if (of_device_is_compatible(np, "xlnx,zynqmp-8.9a") ||
> +	    of_device_is_compatible(np, "xlnx,versal-8.9a") ||
> +	    of_device_is_compatible(np, "xlnx,versal-net-emmc"))
> +		sdhci_arasan->quirks |= SDHCI_ARASAN_QUIRK_ENSURE_CD_STABLE;
> +

don't you want to introduce new quirks field in sdhci_arasan_of_data and just 
assigned them to sdhci_arasan->quirks in probe instead of calling 
of_device_is_compatible(). You already know which device you are running at when 
you are in probe. And it should also speedup boot.

sdhci_arasan->quirks = data->quirks;


Thanks,
Michal