| Message ID | 20250211214611.469260-1-erick.shepherd@ni.com |
|---|---|
| State | New |
| Headers | show |
| Series | mmc: core: Wait for Vdd to settle on card power off | expand |
> Subject: [PATCH] mmc: core: Wait for Vdd to settle on card power off ^^^^ Should be a host patch? > > The SD spec version 6.0 section 6.4.1.5 requires that Vdd must be lowered to > less than 0.5V for a minimum of 1 ms when powering off a card. Increase our > wait to 15 ms so that voltage has time to drain down to 0.5V. > > Signed-off-by: Kyle Roeschley <kyle.roeschley@ni.com> > Signed-off-by: Brad Mouring <brad.mouring@ni.com> > Signed-off-by: Erick Shepherd <erick.shepherd@ni.com> Acked-by: Avri Altman <avri.altman@sandisk.com> > --- > drivers/mmc/host/sdhci.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index > f4a7733a8ad2..b15a1f107549 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -2415,6 +2415,9 @@ void sdhci_set_ios(struct mmc_host *mmc, struct > mmc_ios *ios) > else > sdhci_set_power(host, ios->power_mode, ios->vdd); > > + if (ios->power_mode == MMC_POWER_OFF) > + mdelay(15); > + > if (host->ops->platform_send_init_74_clocks) > host->ops->platform_send_init_74_clocks(host, ios- > >power_mode); > > -- > 2.43.0 >
On 11/02/25 23:46, Erick Shepherd wrote: > The SD spec version 6.0 section 6.4.1.5 requires that Vdd must be > lowered to less than 0.5V for a minimum of 1 ms when powering off a > card. Increase our wait to 15 ms so that voltage has time to drain down > to 0.5V. mmc_power_off() has a delay. So does mmc_power_cycle() Why does this need to be in sdhci? Are you experiencing an issue? > > Signed-off-by: Kyle Roeschley <kyle.roeschley@ni.com> > Signed-off-by: Brad Mouring <brad.mouring@ni.com> > Signed-off-by: Erick Shepherd <erick.shepherd@ni.com> > --- > drivers/mmc/host/sdhci.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index f4a7733a8ad2..b15a1f107549 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -2415,6 +2415,9 @@ void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) > else > sdhci_set_power(host, ios->power_mode, ios->vdd); > > + if (ios->power_mode == MMC_POWER_OFF) > + mdelay(15); > + > if (host->ops->platform_send_init_74_clocks) > host->ops->platform_send_init_74_clocks(host, ios->power_mode); >
>> The SD spec version 6.0 section 6.4.1.5 requires that Vdd must be >> lowered to less than 0.5V for a minimum of 1 ms when powering off a >> card. Increase our wait to 15 ms so that voltage has time to drain down >> to 0.5V. > mmc_power_off() has a delay. So does mmc_power_cycle() > Why does this need to be in sdhci? Are you experiencing an > issue? Thank you for taking a look at this. The initial change was made in mmc_power_off() due to an issue we had with some of our devices requiring more time for the Vdd to drain below 0.5V. Ulf gave us this feedback on that change: > No, this isn't the proper place of adding more "magic" delays. > Instead, make sure the related ->set_ios() callback in the mmc host > driver deals with this instead. In case it uses an external regulator, > via the regulator API, then this is something that should be > controlled with the definition of the regulator. Should we take a different approach here? Regards Erick
On 7/03/25 19:46, Erick Shepherd wrote: >>> The SD spec version 6.0 section 6.4.1.5 requires that Vdd must be >>> lowered to less than 0.5V for a minimum of 1 ms when powering off a >>> card. Increase our wait to 15 ms so that voltage has time to drain down >>> to 0.5V. > >> mmc_power_off() has a delay. So does mmc_power_cycle() > >> Why does this need to be in sdhci? Are you experiencing an >> issue? > > Thank you for taking a look at this. The initial change was made in > mmc_power_off() due to an issue we had with some of our devices > requiring more time for the Vdd to drain below 0.5V. Ulf gave us this > feedback on that change: > >> No, this isn't the proper place of adding more "magic" delays. > >> Instead, make sure the related ->set_ios() callback in the mmc host >> driver deals with this instead. In case it uses an external regulator, >> via the regulator API, then this is something that should be >> controlled with the definition of the regulator. > > Should we take a different approach here? It probably should be dealt with in the ->set_power() callback. Is it one of the PCI devices in sdhci-pci-core.c?
On 7/03/25 23:16, Erick Shepherd wrote: >> It probably should be dealt with in the ->set_power() callback. >> Is it one of the PCI devices in sdhci-pci-core.c? > > Sure, I can move the delay to sdhci_set_power(). It looks like that > gets called right before the if-statement in the change I proposed > so the behavior should be the same, unless host->ops->set_power is set. > > I believe we saw this failure on devices using the Intel Atom E3930 > and E3940, which are Apollo Lake. It looks like there is an entry in > sdhci-pci-core.c. Does that change what we should do? What about something like this? diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c index 1f0bd723f011..0789df732e93 100644 --- a/drivers/mmc/host/sdhci-pci-core.c +++ b/drivers/mmc/host/sdhci-pci-core.c @@ -610,8 +610,11 @@ static void sdhci_intel_set_power(struct sdhci_host *host, unsigned char mode, sdhci_set_power(host, mode, vdd); - if (mode == MMC_POWER_OFF) + if (mode == MMC_POWER_OFF) { + if (slot->chip->pdev->device == PCI_DEVICE_ID_INTEL_APL_SD) + usleep_range(15000, 17500); return; + } /* * Bus power might not enable after D3 -> D0 transition due to the
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index f4a7733a8ad2..b15a1f107549 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2415,6 +2415,9 @@ void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) else sdhci_set_power(host, ios->power_mode, ios->vdd); + if (ios->power_mode == MMC_POWER_OFF) + mdelay(15); + if (host->ops->platform_send_init_74_clocks) host->ops->platform_send_init_74_clocks(host, ios->power_mode);