[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b2e94588-96e4-4afa-b767-e7f05a81dcfa@intel.com>
Date: Fri, 25 Jul 2025 11:14:59 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: "Potthuri, Sai Krishna" <sai.krishna.potthuri@....com>, "Simek, Michal"
<michal.simek@....com>, Ulf Hansson <ulf.hansson@...aro.org>
CC: "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "git (AMD-Xilinx)" <git@....com>,
"saikrishna12468@...il.com" <saikrishna12468@...il.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:
> [Public]
>
> Hi Adrian,
>
>> -----Original Message-----
>> From: Adrian Hunter <adrian.hunter@...el.com>
>> Sent: Thursday, July 24, 2025 9:50 PM
>> To: Potthuri, Sai Krishna <sai.krishna.potthuri@....com>; Simek, Michal
>> <michal.simek@....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 (AMD-Xilinx) <git@....com>;
>> saikrishna12468@...il.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:
>>> 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>
>>> ---
>>> 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
>>> *
>>> @@ -514,6 +517,23 @@ 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)
>>> +{
>>> + 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) {
>>> + readl_poll_timeout(host->ioaddr + SDHCI_PRESENT_STATE, reg,
>> reg & SDHCI_CD_STABLE,
>>> + CD_STABLE_MAX_SLEEP_US,
>> CD_STABLE_TIMEOUT_US);
>>> + }
>>
>> 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)
>
>>
>> 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.
> 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.
>
> Regards
> Sai Krishna
>
>>
>>> +
>>> + 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
>> @@
>>> 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 = {
>>> .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,
>>> };
>>>
>
Powered by blists - more mailing lists