* [PATCH] mmc: sdhci-of-arasan: Ensure CD logic stabilization before power-up
@ 2025-07-21 9:53 Sai Krishna Potthuri
2025-07-24 16:20 ` Adrian Hunter
0 siblings, 1 reply; 7+ messages in thread
From: Sai Krishna Potthuri @ 2025-07-21 9:53 UTC (permalink / raw)
To: Adrian Hunter, Michal Simek, Ulf Hansson
Cc: linux-mmc, linux-arm-kernel, linux-kernel, git, saikrishna12468,
Sai Krishna Potthuri
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
*
@@ -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);
+ }
+
+ 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,
};
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sdhci-of-arasan: Ensure CD logic stabilization before power-up
2025-07-21 9:53 [PATCH] mmc: sdhci-of-arasan: Ensure CD logic stabilization before power-up Sai Krishna Potthuri
@ 2025-07-24 16:20 ` Adrian Hunter
2025-07-25 5:49 ` Potthuri, Sai Krishna
0 siblings, 1 reply; 7+ messages in thread
From: Adrian Hunter @ 2025-07-24 16:20 UTC (permalink / raw)
To: Sai Krishna Potthuri, Michal Simek, Ulf Hansson
Cc: linux-mmc, linux-arm-kernel, linux-kernel, git, saikrishna12468
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@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
> *
> @@ -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 this work with all Arasan variants?
> +
> + 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,
> };
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] mmc: sdhci-of-arasan: Ensure CD logic stabilization before power-up
2025-07-24 16:20 ` Adrian Hunter
@ 2025-07-25 5:49 ` Potthuri, Sai Krishna
2025-07-25 8:14 ` Adrian Hunter
2025-08-01 13:03 ` Prasanna Kumar T S M
0 siblings, 2 replies; 7+ messages in thread
From: Potthuri, Sai Krishna @ 2025-07-25 5:49 UTC (permalink / raw)
To: Adrian Hunter, Simek, Michal, Ulf Hansson
Cc: linux-mmc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, git (AMD-Xilinx),
saikrishna12468@gmail.com
[Public]
Hi Adrian,
> -----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
> <michal.simek@amd.com>; Ulf Hansson <ulf.hansson@linaro.org>
> Cc: linux-mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; git (AMD-Xilinx) <git@amd.com>;
> 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:
> > 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
> > *
> > @@ -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.
>
> 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.
Please let me know if you prefer to increase the timeout or if this logic should be
enabled by a platform specific 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,
> > };
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sdhci-of-arasan: Ensure CD logic stabilization before power-up
2025-07-25 5:49 ` Potthuri, Sai Krishna
@ 2025-07-25 8:14 ` Adrian Hunter
2025-07-25 10:53 ` Potthuri, Sai Krishna
2025-08-01 13:03 ` Prasanna Kumar T S M
1 sibling, 1 reply; 7+ messages in thread
From: Adrian Hunter @ 2025-07-25 8:14 UTC (permalink / raw)
To: Potthuri, Sai Krishna, Simek, Michal, Ulf Hansson
Cc: linux-mmc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, git (AMD-Xilinx),
saikrishna12468@gmail.com
On 25/07/2025 08:49, Potthuri, Sai Krishna wrote:
> [Public]
>
> Hi Adrian,
>
>> -----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
>> <michal.simek@amd.com>; Ulf Hansson <ulf.hansson@linaro.org>
>> Cc: linux-mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>> kernel@vger.kernel.org; git (AMD-Xilinx) <git@amd.com>;
>> 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:
>>> 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
>>> *
>>> @@ -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,
>>> };
>>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] mmc: sdhci-of-arasan: Ensure CD logic stabilization before power-up
2025-07-25 8:14 ` Adrian Hunter
@ 2025-07-25 10:53 ` Potthuri, Sai Krishna
0 siblings, 0 replies; 7+ messages in thread
From: Potthuri, Sai Krishna @ 2025-07-25 10:53 UTC (permalink / raw)
To: Adrian Hunter, Simek, Michal, Ulf Hansson
Cc: linux-mmc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, git (AMD-Xilinx),
saikrishna12468@gmail.com
[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
> <michal.simek@amd.com>; Ulf Hansson <ulf.hansson@linaro.org>
> Cc: linux-mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; git (AMD-Xilinx) <git@amd.com>;
> 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:
> > [Public]
> >
> > Hi Adrian,
> >
> >> -----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 <michal.simek@amd.com>; Ulf Hansson <ulf.hansson@linaro.org>
> >> Cc: linux-mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >> linux- kernel@vger.kernel.org; git (AMD-Xilinx) <git@amd.com>;
> >> 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:
> >>> 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
> >>> *
> >>> @@ -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)
Sure, I will update.
>
> >
> >>
> >> 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.
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
>
> >
> > 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,
> >>> };
> >>>
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sdhci-of-arasan: Ensure CD logic stabilization before power-up
2025-07-25 5:49 ` Potthuri, Sai Krishna
2025-07-25 8:14 ` Adrian Hunter
@ 2025-08-01 13:03 ` Prasanna Kumar T S M
2025-08-04 7:11 ` Potthuri, Sai Krishna
1 sibling, 1 reply; 7+ messages in thread
From: Prasanna Kumar T S M @ 2025-08-01 13:03 UTC (permalink / raw)
To: Potthuri, Sai Krishna, Adrian Hunter, Simek, Michal, Ulf Hansson
Cc: linux-mmc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, git (AMD-Xilinx),
saikrishna12468@gmail.com
Hi Sai Krishna,
On 25-07-2025 11:19, Potthuri, Sai Krishna wrote:
>> 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.
As SDHCI_CD_STABLE bit is defined in SDHCI specification, why are you
making a driver specific fix? Is this problem specific to Arasan eMMC?
If not, does it make sense to make this a framework level change instead
of a driver specific change?
Given that you are planning to add a quirk, doing this in common code
would be better.
> 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.
> Please let me know if you prefer to increase the timeout or if this logic should be
> enabled by a platform specific quirk.
Thanks,
Prasanna Kumar
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] mmc: sdhci-of-arasan: Ensure CD logic stabilization before power-up
2025-08-01 13:03 ` Prasanna Kumar T S M
@ 2025-08-04 7:11 ` Potthuri, Sai Krishna
0 siblings, 0 replies; 7+ messages in thread
From: Potthuri, Sai Krishna @ 2025-08-04 7:11 UTC (permalink / raw)
To: Prasanna Kumar T S M, Adrian Hunter, Simek, Michal, Ulf Hansson
Cc: linux-mmc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, git (AMD-Xilinx),
saikrishna12468@gmail.com
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Prasanna Kumar,
> -----Original Message-----
> From: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
> Sent: Friday, August 1, 2025 6:34 PM
> To: Potthuri, Sai Krishna <sai.krishna.potthuri@amd.com>; Adrian Hunter
> <adrian.hunter@intel.com>; Simek, Michal <michal.simek@amd.com>; Ulf
> Hansson <ulf.hansson@linaro.org>
> Cc: linux-mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; git (AMD-Xilinx) <git@amd.com>;
> saikrishna12468@gmail.com
> Subject: Re: [PATCH] mmc: sdhci-of-arasan: Ensure CD logic stabilization before
> power-up
>
> Hi Sai Krishna,
>
> On 25-07-2025 11:19, Potthuri, Sai Krishna wrote:
> >> 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.
>
> As SDHCI_CD_STABLE bit is defined in SDHCI specification, why are you
> making a driver specific fix? Is this problem specific to Arasan eMMC?
> If not, does it make sense to make this a framework level change instead
> of a driver specific change?
>
> Given that you are planning to add a quirk, doing this in common code
> would be better.
Agree, with the quirk approach we can move the logic to framework(sdhci.c) and
make it as a common code.
I will move the logic to sdhci_set_power_noreg() function.
Regards
Sai Krishna
>
> > 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.
> > Please let me know if you prefer to increase the timeout or if this logic should be
> > enabled by a platform specific quirk.
>
> Thanks,
>
> Prasanna Kumar
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-08-04 7:11 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-21 9:53 [PATCH] mmc: sdhci-of-arasan: Ensure CD logic stabilization before power-up Sai Krishna Potthuri
2025-07-24 16:20 ` Adrian Hunter
2025-07-25 5:49 ` Potthuri, Sai Krishna
2025-07-25 8:14 ` Adrian Hunter
2025-07-25 10:53 ` Potthuri, Sai Krishna
2025-08-01 13:03 ` Prasanna Kumar T S M
2025-08-04 7:11 ` Potthuri, Sai Krishna
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).