* [PATCH] mmc: host: sdhci: Fix the incorrect soft reset operation when runtime resuming @ 2019-07-15 10:58 Baolin Wang 2019-07-15 11:19 ` Adrian Hunter 0 siblings, 1 reply; 5+ messages in thread From: Baolin Wang @ 2019-07-15 10:58 UTC (permalink / raw) To: adrian.hunter, ulf.hansson Cc: zhang.lyra, orsonzhai, linus.walleij, baolin.wang, vincent.guittot, linux-mmc, linux-kernel In sdhci_runtime_resume_host() function, we will always do software reset for all, but according to the specification, we should issue reset command and reinitialize the SD/eMMC card. However, we only do reinitialize the SD/eMMC card when the SD/eMMC card are power down during runtime suspend. Thus for those platforms that do not power down the SD/eMMC card during runtime suspend, we should not do software reset for all. To fix this issue, we can add one condition to validate the MMC_CAP_AGGRESSIVE_PM to decide if we can do software reset for all or just reset command and data lines. Signed-off-by: Baolin Wang <baolin.wang@linaro.org> --- drivers/mmc/host/sdhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 9715834..470c5e0 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -3333,7 +3333,7 @@ int sdhci_runtime_resume_host(struct sdhci_host *host) host->ops->enable_dma(host); } - sdhci_init(host, 0); + sdhci_init(host, !(mmc->caps & MMC_CAP_AGGRESSIVE_PM)); if (mmc->ios.power_mode != MMC_POWER_UNDEFINED && mmc->ios.power_mode != MMC_POWER_OFF) { -- 1.7.9.5 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mmc: host: sdhci: Fix the incorrect soft reset operation when runtime resuming 2019-07-15 10:58 [PATCH] mmc: host: sdhci: Fix the incorrect soft reset operation when runtime resuming Baolin Wang @ 2019-07-15 11:19 ` Adrian Hunter 2019-07-15 11:37 ` Baolin Wang 0 siblings, 1 reply; 5+ messages in thread From: Adrian Hunter @ 2019-07-15 11:19 UTC (permalink / raw) To: Baolin Wang, ulf.hansson Cc: zhang.lyra, orsonzhai, linus.walleij, vincent.guittot, linux-mmc, linux-kernel On 15/07/19 1:58 PM, Baolin Wang wrote: > In sdhci_runtime_resume_host() function, we will always do software reset > for all, but according to the specification, we should issue reset command > and reinitialize the SD/eMMC card. Where does it say that? > However, we only do reinitialize the > SD/eMMC card when the SD/eMMC card are power down during runtime suspend. > > Thus for those platforms that do not power down the SD/eMMC card during > runtime suspend, we should not do software reset for all. > To fix this > issue, we can add one condition to validate the MMC_CAP_AGGRESSIVE_PM > to decide if we can do software reset for all or just reset command > and data lines. > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > --- > drivers/mmc/host/sdhci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 9715834..470c5e0 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -3333,7 +3333,7 @@ int sdhci_runtime_resume_host(struct sdhci_host *host) > host->ops->enable_dma(host); > } > > - sdhci_init(host, 0); > + sdhci_init(host, !(mmc->caps & MMC_CAP_AGGRESSIVE_PM)); We have done a full reset for a long time, so it would be surprising to need to change it. What problem is it causing? > > if (mmc->ios.power_mode != MMC_POWER_UNDEFINED && > mmc->ios.power_mode != MMC_POWER_OFF) { > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mmc: host: sdhci: Fix the incorrect soft reset operation when runtime resuming 2019-07-15 11:19 ` Adrian Hunter @ 2019-07-15 11:37 ` Baolin Wang 2019-07-15 12:37 ` Adrian Hunter 0 siblings, 1 reply; 5+ messages in thread From: Baolin Wang @ 2019-07-15 11:37 UTC (permalink / raw) To: Adrian Hunter Cc: Ulf Hansson, Chunyan Zhang, Orson Zhai, Linus Walleij, Vincent Guittot, linux-mmc, LKML Hi Adrian, On Mon, 15 Jul 2019 at 19:20, Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 15/07/19 1:58 PM, Baolin Wang wrote: > > In sdhci_runtime_resume_host() function, we will always do software reset > > for all, but according to the specification, we should issue reset command > > and reinitialize the SD/eMMC card. > > Where does it say that? I checked the SD host controller simplified specification Ver4.20, and in Page 75, Software Reset For All bit, it says "if this bit is set to1, the host driver should issue reset command and reinitialize the SD card". (I did not check other versions). > > > However, we only do reinitialize the > > SD/eMMC card when the SD/eMMC card are power down during runtime suspend. > > > > Thus for those platforms that do not power down the SD/eMMC card during > > runtime suspend, we should not do software reset for all. > > To fix this > > issue, we can add one condition to validate the MMC_CAP_AGGRESSIVE_PM > > to decide if we can do software reset for all or just reset command > > and data lines. > > > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > > --- > > drivers/mmc/host/sdhci.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > > index 9715834..470c5e0 100644 > > --- a/drivers/mmc/host/sdhci.c > > +++ b/drivers/mmc/host/sdhci.c > > @@ -3333,7 +3333,7 @@ int sdhci_runtime_resume_host(struct sdhci_host *host) > > host->ops->enable_dma(host); > > } > > > > - sdhci_init(host, 0); > > + sdhci_init(host, !(mmc->caps & MMC_CAP_AGGRESSIVE_PM)); > > We have done a full reset for a long time, so it would be surprising to need > to change it. > > What problem is it causing? If we did not power down the SD card during runtime suspend, and we reset for all when runtime resume, our SD host controller can not work well, will meet some strange behavior, like: [ 6.525397] mmc0: Got data interrupt 0x00000002 even though no data operation was in progress. [ 6.534189] mmc0: sdhci: ============ SDHCI REGISTER DUMP =========== [ 6.540797] mmc0: sdhci: Sys addr: 0x00000008 | Version: 0x00000004 [ 6.547413] mmc0: sdhci: Blk size: 0x00000200 | Blk cnt: 0x00000000 [ 6.554029] mmc0: sdhci: Argument: 0x03200101 | Trn mode: 0x00000033 [ 6.560645] mmc0: sdhci: Present: 0x01f000f0 | Host ctl: 0x00000030 [ 6.567262] mmc0: sdhci: Power: 0x00000000 | Blk gap: 0x00000000 [ 6.573877] mmc0: sdhci: Wake-up: 0x00000000 | Clock: 0x00000007 [ 6.580493] mmc0: sdhci: Timeout: 0x0000000e | Int stat: 0x00000000 [ 6.587109] mmc0: sdhci: Int enab: 0x037f000b | Sig enab: 0x037f000b [ 6.593726] mmc0: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000 [ 6.600342] mmc0: sdhci: Caps: 0x1c6d0080 | Caps_1: 0x08000007 [ 6.606959] mmc0: sdhci: Cmd: 0x0000061b | Max curr: 0x00ffffff [ 6.613574] mmc0: sdhci: Resp[0]: 0x00001201 | Resp[1]: 0x00000000 [ 6.620190] mmc0: sdhci: Resp[2]: 0x00000000 | Resp[3]: 0x00000000 [ 6.626806] mmc0: sdhci: Host ctl2: 0x00003807 [ 6.631364] mmc0: sdhci: ADMA Err: 0x00000000 | ADMA Ptr: 0x00000000df062000 [ 6.638697] mmc0: sdhci: ============================================ [ 6.645379] mmc0: cache flush error -84 Got data interrupt but no data commands are processing now. With this patch, then our SD host controller can work well. Did I miss anything else? Thanks. -- Baolin Wang Best Regards ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mmc: host: sdhci: Fix the incorrect soft reset operation when runtime resuming 2019-07-15 11:37 ` Baolin Wang @ 2019-07-15 12:37 ` Adrian Hunter 2019-07-16 5:37 ` Baolin Wang 0 siblings, 1 reply; 5+ messages in thread From: Adrian Hunter @ 2019-07-15 12:37 UTC (permalink / raw) To: Baolin Wang Cc: Ulf Hansson, Chunyan Zhang, Orson Zhai, Linus Walleij, Vincent Guittot, linux-mmc, LKML On 15/07/19 2:37 PM, Baolin Wang wrote: > Hi Adrian, > > On Mon, 15 Jul 2019 at 19:20, Adrian Hunter <adrian.hunter@intel.com> wrote: >> >> On 15/07/19 1:58 PM, Baolin Wang wrote: >>> In sdhci_runtime_resume_host() function, we will always do software reset >>> for all, but according to the specification, we should issue reset command >>> and reinitialize the SD/eMMC card. >> >> Where does it say that? > > I checked the SD host controller simplified specification Ver4.20, and > in Page 75, Software Reset For All bit, it says "if this bit is set > to1, the host driver should issue reset command and reinitialize the > SD card". (I did not check other versions). That might simply be assuming that the bus power also controls the card power. > >> >>> However, we only do reinitialize the >>> SD/eMMC card when the SD/eMMC card are power down during runtime suspend. >>> >>> Thus for those platforms that do not power down the SD/eMMC card during >>> runtime suspend, we should not do software reset for all. >>> To fix this >>> issue, we can add one condition to validate the MMC_CAP_AGGRESSIVE_PM >>> to decide if we can do software reset for all or just reset command >>> and data lines. >>> >>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org> >>> --- >>> drivers/mmc/host/sdhci.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >>> index 9715834..470c5e0 100644 >>> --- a/drivers/mmc/host/sdhci.c >>> +++ b/drivers/mmc/host/sdhci.c >>> @@ -3333,7 +3333,7 @@ int sdhci_runtime_resume_host(struct sdhci_host *host) >>> host->ops->enable_dma(host); >>> } >>> >>> - sdhci_init(host, 0); >>> + sdhci_init(host, !(mmc->caps & MMC_CAP_AGGRESSIVE_PM)); >> >> We have done a full reset for a long time, so it would be surprising to need >> to change it. >> >> What problem is it causing? > > If we did not power down the SD card during runtime suspend, and we > reset for all when runtime resume, our SD host controller can not work > well, will meet some strange behavior, like: > > [ 6.525397] mmc0: Got data interrupt 0x00000002 even though no data > operation was in progress. > [ 6.534189] mmc0: sdhci: ============ SDHCI REGISTER DUMP =========== > [ 6.540797] mmc0: sdhci: Sys addr: 0x00000008 | Version: 0x00000004 > [ 6.547413] mmc0: sdhci: Blk size: 0x00000200 | Blk cnt: 0x00000000 > [ 6.554029] mmc0: sdhci: Argument: 0x03200101 | Trn mode: 0x00000033 > [ 6.560645] mmc0: sdhci: Present: 0x01f000f0 | Host ctl: 0x00000030 > [ 6.567262] mmc0: sdhci: Power: 0x00000000 | Blk gap: 0x00000000 > [ 6.573877] mmc0: sdhci: Wake-up: 0x00000000 | Clock: 0x00000007 > [ 6.580493] mmc0: sdhci: Timeout: 0x0000000e | Int stat: 0x00000000 > [ 6.587109] mmc0: sdhci: Int enab: 0x037f000b | Sig enab: 0x037f000b > [ 6.593726] mmc0: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000 > [ 6.600342] mmc0: sdhci: Caps: 0x1c6d0080 | Caps_1: 0x08000007 > [ 6.606959] mmc0: sdhci: Cmd: 0x0000061b | Max curr: 0x00ffffff > [ 6.613574] mmc0: sdhci: Resp[0]: 0x00001201 | Resp[1]: 0x00000000 > [ 6.620190] mmc0: sdhci: Resp[2]: 0x00000000 | Resp[3]: 0x00000000 > [ 6.626806] mmc0: sdhci: Host ctl2: 0x00003807 > [ 6.631364] mmc0: sdhci: ADMA Err: 0x00000000 | ADMA Ptr: 0x00000000df062000 > [ 6.638697] mmc0: sdhci: ============================================ > [ 6.645379] mmc0: cache flush error -84 > > Got data interrupt but no data commands are processing now. With this > patch, then our SD host controller can work well. Did I miss anything > else? Thanks. The response seems to show the card in state 9 bus-testing, which would suggest the use of CMD19 for eMMC. Perhaps the wrong command is used for eMMC re-tuning? The difficulty with changing long standing flow is that it might reveal problems for other existing hardware. Did you consider making a driver-specific change? The ->reset() callback could be used. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mmc: host: sdhci: Fix the incorrect soft reset operation when runtime resuming 2019-07-15 12:37 ` Adrian Hunter @ 2019-07-16 5:37 ` Baolin Wang 0 siblings, 0 replies; 5+ messages in thread From: Baolin Wang @ 2019-07-16 5:37 UTC (permalink / raw) To: Adrian Hunter Cc: Ulf Hansson, Chunyan Zhang, Orson Zhai, Linus Walleij, Vincent Guittot, linux-mmc, LKML Hi Adrian, On Mon, 15 Jul 2019 at 20:39, Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 15/07/19 2:37 PM, Baolin Wang wrote: > > Hi Adrian, > > > > On Mon, 15 Jul 2019 at 19:20, Adrian Hunter <adrian.hunter@intel.com> wrote: > >> > >> On 15/07/19 1:58 PM, Baolin Wang wrote: > >>> In sdhci_runtime_resume_host() function, we will always do software reset > >>> for all, but according to the specification, we should issue reset command > >>> and reinitialize the SD/eMMC card. > >> > >> Where does it say that? > > > > I checked the SD host controller simplified specification Ver4.20, and > > in Page 75, Software Reset For All bit, it says "if this bit is set > > to1, the host driver should issue reset command and reinitialize the > > SD card". (I did not check other versions). > > That might simply be assuming that the bus power also controls the card power. Yes. > > > > >> > >>> However, we only do reinitialize the > >>> SD/eMMC card when the SD/eMMC card are power down during runtime suspend. > >>> > >>> Thus for those platforms that do not power down the SD/eMMC card during > >>> runtime suspend, we should not do software reset for all. > >>> To fix this > >>> issue, we can add one condition to validate the MMC_CAP_AGGRESSIVE_PM > >>> to decide if we can do software reset for all or just reset command > >>> and data lines. > >>> > >>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > >>> --- > >>> drivers/mmc/host/sdhci.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > >>> index 9715834..470c5e0 100644 > >>> --- a/drivers/mmc/host/sdhci.c > >>> +++ b/drivers/mmc/host/sdhci.c > >>> @@ -3333,7 +3333,7 @@ int sdhci_runtime_resume_host(struct sdhci_host *host) > >>> host->ops->enable_dma(host); > >>> } > >>> > >>> - sdhci_init(host, 0); > >>> + sdhci_init(host, !(mmc->caps & MMC_CAP_AGGRESSIVE_PM)); > >> > >> We have done a full reset for a long time, so it would be surprising to need > >> to change it. > >> > >> What problem is it causing? > > > > If we did not power down the SD card during runtime suspend, and we > > reset for all when runtime resume, our SD host controller can not work > > well, will meet some strange behavior, like: > > > > [ 6.525397] mmc0: Got data interrupt 0x00000002 even though no data > > operation was in progress. > > [ 6.534189] mmc0: sdhci: ============ SDHCI REGISTER DUMP =========== > > [ 6.540797] mmc0: sdhci: Sys addr: 0x00000008 | Version: 0x00000004 > > [ 6.547413] mmc0: sdhci: Blk size: 0x00000200 | Blk cnt: 0x00000000 > > [ 6.554029] mmc0: sdhci: Argument: 0x03200101 | Trn mode: 0x00000033 > > [ 6.560645] mmc0: sdhci: Present: 0x01f000f0 | Host ctl: 0x00000030 > > [ 6.567262] mmc0: sdhci: Power: 0x00000000 | Blk gap: 0x00000000 > > [ 6.573877] mmc0: sdhci: Wake-up: 0x00000000 | Clock: 0x00000007 > > [ 6.580493] mmc0: sdhci: Timeout: 0x0000000e | Int stat: 0x00000000 > > [ 6.587109] mmc0: sdhci: Int enab: 0x037f000b | Sig enab: 0x037f000b > > [ 6.593726] mmc0: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000 > > [ 6.600342] mmc0: sdhci: Caps: 0x1c6d0080 | Caps_1: 0x08000007 > > [ 6.606959] mmc0: sdhci: Cmd: 0x0000061b | Max curr: 0x00ffffff > > [ 6.613574] mmc0: sdhci: Resp[0]: 0x00001201 | Resp[1]: 0x00000000 > > [ 6.620190] mmc0: sdhci: Resp[2]: 0x00000000 | Resp[3]: 0x00000000 > > [ 6.626806] mmc0: sdhci: Host ctl2: 0x00003807 > > [ 6.631364] mmc0: sdhci: ADMA Err: 0x00000000 | ADMA Ptr: 0x00000000df062000 > > [ 6.638697] mmc0: sdhci: ============================================ > > [ 6.645379] mmc0: cache flush error -84 > > > > Got data interrupt but no data commands are processing now. With this > > patch, then our SD host controller can work well. Did I miss anything > > else? Thanks. > > The response seems to show the card in state 9 bus-testing, which would > suggest the use of CMD19 for eMMC. Perhaps the wrong command is used for > eMMC re-tuning? So it is strange, for eMMC we use CMD21 for tuning. > > The difficulty with changing long standing flow is that it might reveal > problems for other existing hardware. Did you consider making a > driver-specific change? The ->reset() callback could be used. Understood. I will find a way to fix my issue and do not affect the original logic used by other hardware. Thanks. -- Baolin Wang Best Regards ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-07-16 5:37 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-15 10:58 [PATCH] mmc: host: sdhci: Fix the incorrect soft reset operation when runtime resuming Baolin Wang 2019-07-15 11:19 ` Adrian Hunter 2019-07-15 11:37 ` Baolin Wang 2019-07-15 12:37 ` Adrian Hunter 2019-07-16 5:37 ` Baolin Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®