[AMD Official Use Only - AMD Internal Distribution Only]

Hi Adrian,

-----Original Message-----
From: Adrian Hunter <adrian.hunter@intel.com>
Sent: Friday, July 25, 2025 1:45 PM
To: Potthuri, Sai Krishna <sai.krishna.potthuri@amd.com>; Simek, Michal
[off-list ref]; Ulf Hansson [off-list ref]
Cc: linux-mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
kernel@vger.kernel.org; git (AMD-Xilinx) [off-list ref];
saikrishna12468@gmail.com
Subject: Re: [PATCH] mmc: sdhci-of-arasan: Ensure CD logic stabilization before
power-up

On 25/07/2025 08:49, Potthuri, Sai Krishna wrote:

quoted

[Public]

Hi Adrian,

quoted

-----Original Message-----
From: Adrian Hunter <adrian.hunter@intel.com>
Sent: Thursday, July 24, 2025 9:50 PM
To: Potthuri, Sai Krishna <sai.krishna.potthuri@amd.com>; Simek,
Michal [off-list ref]; Ulf Hansson [off-list ref]
Cc: linux-mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
linux- kernel@vger.kernel.org; git (AMD-Xilinx) [off-list ref];
saikrishna12468@gmail.com
Subject: Re: [PATCH] mmc: sdhci-of-arasan: Ensure CD logic
stabilization before power-up

On 21/07/2025 12:53, Sai Krishna Potthuri wrote:

quoted

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@amd.com>
---
 drivers/mmc/host/sdhci-of-arasan.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c

b/drivers/mmc/host/sdhci-of-arasan.c
index 42878474e56e..3ce55009ba4a 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

quoted

quoted

quoted

  *

@@ -514,6 +517,23 @@ static int sdhci_arasan_voltage_switch(struct

mmc_host *mmc,

quoted

    return -EINVAL;
 }

+static void sdhci_arasan_set_power_and_bus_voltage(struct
+sdhci_host *host,

unsigned char mode,

quoted

+                                              unsigned short vdd) {
+   u32 reg;
+
+   /*
+    * Ensure that the card detect logic has stabilized before
+ powering up, this

is

quoted

+    * necessary after a host controller reset.
+    */
+   if (mode == MMC_POWER_UP) {
+           readl_poll_timeout(host->ioaddr + SDHCI_PRESENT_STATE,
+ reg,

reg & SDHCI_CD_STABLE,

quoted

+                              CD_STABLE_MAX_SLEEP_US,

CD_STABLE_TIMEOUT_US);

quoted

+   }

Doesn't need {}

Will remove in v2.

Also probably better to access the register in a consistent manner i.e. use
read_poll_timeout(sdhci_readl,...,host, SDHCI_PRESENT_STATE)

Sure, I will update.

quoted

quoted

Will this work with all Arasan variants?

Yes, this is expected to work across all Arasan variants that comply
with the standard SDHCI register definitions. The SDHCI_CD_STABLE bit
is defined in both the standard SDHCI specification and Arasan's user guide.
On Xilinx/AMD Versal and ZynqMP platforms, the CD stable bit is
typically set within a few milliseconds. However, to be on the safer
side and ensure compatibility across all Arasan variants, a timeout of 1 second is

added.

A lower timeout would have less issue if there were devices that did not have
standard CD stable bit behaviour.

quoted

Please let me know if you prefer to increase the timeout or if this
logic should be enabled by a platform specific quirk.

If you are 100% confident it won't negatively affect other devices, then it is OK.
Otherwise it is better as a quirk.

For the devices that didn't have or broken CD stable behavior then they might see
increase in the initialization time that is the only impact I can see.
May be better I will enable the logic using platform specific quirk, so that other
devices will have zero impact and whoever needs it they will define the quirk.

Regards
Sai Krishna

quoted

Regards
Sai Krishna

quoted

quoted

+
+   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 +541,7

@@

quoted

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 +590,7 @@ static const struct sdhci_ops sdhci_arasan_cqe_ops

= {

quoted

quoted

quoted

    .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,
 };