* [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