* [PATCH 0/2] mmc: sdhci: Fix for sdhci suspend function @ 2011-12-28 3:11 Aaron Lu 2011-12-28 3:11 ` [PATCH 1/2] mmc: sdhci: Fix tuning timer incorrect setting when suspending host Aaron Lu 2011-12-28 3:11 ` [PATCH 2/2] mmc: sdhci: Deal with failure case in sdhci_suspend_host Aaron Lu 0 siblings, 2 replies; 6+ messages in thread From: Aaron Lu @ 2011-12-28 3:11 UTC (permalink / raw) To: Chris Ball; +Cc: linux-mmc, Philip Rakity, Aaron Lu Fixed a bug in sdhci_suspend_host, the tuning timer should be deactivated instead of reactivated when suspending. Handle failure case in sdhci_suspend_host, so that calling function does not need to take care of the error recovery. mmc: sdhci: Fix tuning timer incorrect setting when suspending host mmc: sdhci: Deal with failure case in sdhci_suspend_host drivers/mmc/host/sdhci-pci.c | 9 ++------- drivers/mmc/host/sdhci.c | 30 ++++++++++++++++++++++-------- 2 files changed, 24 insertions(+), 15 deletions(-) -- 1.7.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] mmc: sdhci: Fix tuning timer incorrect setting when suspending host 2011-12-28 3:11 [PATCH 0/2] mmc: sdhci: Fix for sdhci suspend function Aaron Lu @ 2011-12-28 3:11 ` Aaron Lu 2011-12-28 12:24 ` Adrian Hunter 2011-12-28 3:11 ` [PATCH 2/2] mmc: sdhci: Deal with failure case in sdhci_suspend_host Aaron Lu 1 sibling, 1 reply; 6+ messages in thread From: Aaron Lu @ 2011-12-28 3:11 UTC (permalink / raw) To: Chris Ball; +Cc: linux-mmc, Philip Rakity, Aaron Lu, stable When suspending host, the tuning timer shoule be deactivated. And the HOST_NEEDS_TUNING flag should be set after tuning timer is deactivated. Signed-off-by: Philip Rakity <prakity@marvell.com> Signed-off-by: Aaron Lu <aaron.lu@amd.com> Cc: stable@kernel.org --- drivers/mmc/host/sdhci.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index ab6018f..2007d37 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2346,9 +2346,8 @@ int sdhci_suspend_host(struct sdhci_host *host) /* Disable tuning since we are suspending */ if (host->version >= SDHCI_SPEC_300 && host->tuning_count && host->tuning_mode == SDHCI_TUNING_MODE_1) { + del_timer_sync(&host->tuning_timer); host->flags &= ~SDHCI_NEEDS_RETUNING; - mod_timer(&host->tuning_timer, jiffies + - host->tuning_count * HZ); } ret = mmc_suspend_host(host->mmc); -- 1.7.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] mmc: sdhci: Fix tuning timer incorrect setting when suspending host 2011-12-28 3:11 ` [PATCH 1/2] mmc: sdhci: Fix tuning timer incorrect setting when suspending host Aaron Lu @ 2011-12-28 12:24 ` Adrian Hunter 0 siblings, 0 replies; 6+ messages in thread From: Adrian Hunter @ 2011-12-28 12:24 UTC (permalink / raw) To: Aaron Lu; +Cc: Chris Ball, linux-mmc, Philip Rakity, stable On 28/12/11 05:11, Aaron Lu wrote: > When suspending host, the tuning timer shoule be deactivated. > And the HOST_NEEDS_TUNING flag should be set after tuning timer is > deactivated. > > Signed-off-by: Philip Rakity <prakity@marvell.com> > Signed-off-by: Aaron Lu <aaron.lu@amd.com> > Cc: stable@kernel.org Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/host/sdhci.c | 3 +-- > 1 files changed, 1 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index ab6018f..2007d37 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -2346,9 +2346,8 @@ int sdhci_suspend_host(struct sdhci_host *host) > /* Disable tuning since we are suspending */ > if (host->version >= SDHCI_SPEC_300 && host->tuning_count && > host->tuning_mode == SDHCI_TUNING_MODE_1) { > + del_timer_sync(&host->tuning_timer); > host->flags &= ~SDHCI_NEEDS_RETUNING; > - mod_timer(&host->tuning_timer, jiffies + > - host->tuning_count * HZ); > } > > ret = mmc_suspend_host(host->mmc); ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] mmc: sdhci: Deal with failure case in sdhci_suspend_host 2011-12-28 3:11 [PATCH 0/2] mmc: sdhci: Fix for sdhci suspend function Aaron Lu 2011-12-28 3:11 ` [PATCH 1/2] mmc: sdhci: Fix tuning timer incorrect setting when suspending host Aaron Lu @ 2011-12-28 3:11 ` Aaron Lu 2011-12-28 12:23 ` Adrian Hunter 1 sibling, 1 reply; 6+ messages in thread From: Aaron Lu @ 2011-12-28 3:11 UTC (permalink / raw) To: Chris Ball; +Cc: linux-mmc, Philip Rakity, Aaron Lu If there are errors happened in sdhci_suspend_host, handle it so that when the function returns with error, the host's behaviour is the same before this function call, e.g. card detection is enabled and tuning timer is active, etc. Previously, sdhci_suspend_host will return the error code and rely on the calling function to handle it. sdhci-pci will handle it in sdhci_pci_suspend while sdhci-platform code will not. Signed-off-by: Philip Rakity <prakity@marvell.com> Signed-off-by: Aaron Lu <aaron.lu@amd.com> --- drivers/mmc/host/sdhci-pci.c | 9 ++------- drivers/mmc/host/sdhci.c | 27 +++++++++++++++++++++------ 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c index d2e77fb..494d14b 100644 --- a/drivers/mmc/host/sdhci-pci.c +++ b/drivers/mmc/host/sdhci-pci.c @@ -1013,7 +1013,7 @@ static int sdhci_pci_suspend(struct device *dev) ret = sdhci_suspend_host(slot->host); if (ret) - goto err_pci_suspend; + return ret; slot_pm_flags = slot->host->mmc->pm_flags; if (slot_pm_flags & MMC_PM_WAKE_SDIO_IRQ) @@ -1025,7 +1025,7 @@ static int sdhci_pci_suspend(struct device *dev) if (chip->fixes && chip->fixes->suspend) { ret = chip->fixes->suspend(chip); if (ret) - goto err_pci_suspend; + return ret; } pci_save_state(pdev); @@ -1042,11 +1042,6 @@ static int sdhci_pci_suspend(struct device *dev) } return 0; - -err_pci_suspend: - while (--i >= 0) - sdhci_resume_host(chip->slots[i]->host); - return ret; } static int sdhci_pci_resume(struct device *dev) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 2007d37..37aeb81 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2339,25 +2339,40 @@ out: int sdhci_suspend_host(struct sdhci_host *host) { - int ret; + int ret, has_tuning_timer; sdhci_disable_card_detection(host); /* Disable tuning since we are suspending */ - if (host->version >= SDHCI_SPEC_300 && host->tuning_count && - host->tuning_mode == SDHCI_TUNING_MODE_1) { + has_tuning_timer = host->version >= SDHCI_SPEC_300 && + host->tuning_count && host->tuning_mode == SDHCI_TUNING_MODE_1; + if (has_tuning_timer) { del_timer_sync(&host->tuning_timer); host->flags &= ~SDHCI_NEEDS_RETUNING; } ret = mmc_suspend_host(host->mmc); if (ret) - return ret; + goto err_suspend; + + if (host->vmmc) { + ret = regulator_disable(host->vmmc); + if (ret) + goto err_suspend; + } free_irq(host->irq, host); - if (host->vmmc) - ret = regulator_disable(host->vmmc); + return 0; + +err_suspend: + if (has_tuning_timer) { + host->flags |= SDHCI_NEEDS_RETUNING; + mod_timer(&host->tuning_timer, jiffies + + host->tuning_count * HZ); + } + + sdhci_enable_card_detection(host); return ret; } -- 1.7.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] mmc: sdhci: Deal with failure case in sdhci_suspend_host 2011-12-28 3:11 ` [PATCH 2/2] mmc: sdhci: Deal with failure case in sdhci_suspend_host Aaron Lu @ 2011-12-28 12:23 ` Adrian Hunter 2011-12-29 2:32 ` Aaron Lu 0 siblings, 1 reply; 6+ messages in thread From: Adrian Hunter @ 2011-12-28 12:23 UTC (permalink / raw) To: Aaron Lu; +Cc: Chris Ball, linux-mmc, Philip Rakity On 28/12/11 05:11, Aaron Lu wrote: > If there are errors happened in sdhci_suspend_host, handle it so that > when the function returns with error, the host's behaviour is the same > before this function call, e.g. card detection is enabled and tuning > timer is active, etc. > > Previously, sdhci_suspend_host will return the error code and rely on > the calling function to handle it. sdhci-pci will handle it in > sdhci_pci_suspend while sdhci-platform code will not. > > Signed-off-by: Philip Rakity <prakity@marvell.com> > Signed-off-by: Aaron Lu <aaron.lu@amd.com> > --- > drivers/mmc/host/sdhci-pci.c | 9 ++------- > drivers/mmc/host/sdhci.c | 27 +++++++++++++++++++++------ > 2 files changed, 23 insertions(+), 13 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c > index d2e77fb..494d14b 100644 > --- a/drivers/mmc/host/sdhci-pci.c > +++ b/drivers/mmc/host/sdhci-pci.c > @@ -1013,7 +1013,7 @@ static int sdhci_pci_suspend(struct device *dev) > ret = sdhci_suspend_host(slot->host); > > if (ret) > - goto err_pci_suspend; > + return ret; > > slot_pm_flags = slot->host->mmc->pm_flags; > if (slot_pm_flags & MMC_PM_WAKE_SDIO_IRQ) > @@ -1025,7 +1025,7 @@ static int sdhci_pci_suspend(struct device *dev) > if (chip->fixes && chip->fixes->suspend) { > ret = chip->fixes->suspend(chip); > if (ret) > - goto err_pci_suspend; > + return ret; > } > > pci_save_state(pdev); > @@ -1042,11 +1042,6 @@ static int sdhci_pci_suspend(struct device *dev) > } > > return 0; > - > -err_pci_suspend: > - while (--i >= 0) > - sdhci_resume_host(chip->slots[i]->host); > - return ret; This doesn't look right. This is about having multiple host controllers on the same PCI device. If those hosts have been successfully suspended, then they must be resumed on error. > } > > static int sdhci_pci_resume(struct device *dev) > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 2007d37..37aeb81 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -2339,25 +2339,40 @@ out: > > int sdhci_suspend_host(struct sdhci_host *host) > { > - int ret; > + int ret, has_tuning_timer; > > sdhci_disable_card_detection(host); > > /* Disable tuning since we are suspending */ > - if (host->version >= SDHCI_SPEC_300 && host->tuning_count && > - host->tuning_mode == SDHCI_TUNING_MODE_1) { > + has_tuning_timer = host->version >= SDHCI_SPEC_300 && > + host->tuning_count && host->tuning_mode == SDHCI_TUNING_MODE_1; > + if (has_tuning_timer) { > del_timer_sync(&host->tuning_timer); > host->flags &= ~SDHCI_NEEDS_RETUNING; > } > > ret = mmc_suspend_host(host->mmc); > if (ret) > - return ret; > + goto err_suspend; > + > + if (host->vmmc) { > + ret = regulator_disable(host->vmmc); > + if (ret) > + goto err_suspend; > + } > > free_irq(host->irq, host); > > - if (host->vmmc) > - ret = regulator_disable(host->vmmc); > + return 0; > + > +err_suspend: > + if (has_tuning_timer) { > + host->flags |= SDHCI_NEEDS_RETUNING; > + mod_timer(&host->tuning_timer, jiffies + > + host->tuning_count * HZ); > + } > + > + sdhci_enable_card_detection(host); > > return ret; > } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] mmc: sdhci: Deal with failure case in sdhci_suspend_host 2011-12-28 12:23 ` Adrian Hunter @ 2011-12-29 2:32 ` Aaron Lu 0 siblings, 0 replies; 6+ messages in thread From: Aaron Lu @ 2011-12-29 2:32 UTC (permalink / raw) To: Adrian Hunter; +Cc: Chris Ball, linux-mmc, Philip Rakity On Wed, Dec 28, 2011 at 02:23:32PM +0200, Adrian Hunter wrote: > > - > > -err_pci_suspend: > > - while (--i >= 0) > > - sdhci_resume_host(chip->slots[i]->host); > > - return ret; > > This doesn't look right. This is about having multiple > host controllers on the same PCI device. If those > hosts have been successfully suspended, then they must > be resumed on error. > You are right, I failed to understand the code. Will send the patch again without touching the pci recover code, thanks. > > > } > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-12-29 2:32 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-12-28 3:11 [PATCH 0/2] mmc: sdhci: Fix for sdhci suspend function Aaron Lu 2011-12-28 3:11 ` [PATCH 1/2] mmc: sdhci: Fix tuning timer incorrect setting when suspending host Aaron Lu 2011-12-28 12:24 ` Adrian Hunter 2011-12-28 3:11 ` [PATCH 2/2] mmc: sdhci: Deal with failure case in sdhci_suspend_host Aaron Lu 2011-12-28 12:23 ` Adrian Hunter 2011-12-29 2:32 ` Aaron Lu
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).