* [PATCH 1/3] mmc: core: do power-off with resume failure @ 2013-08-21 12:42 Seungwon Jeon 2013-08-23 8:36 ` Ulf Hansson 0 siblings, 1 reply; 5+ messages in thread From: Seungwon Jeon @ 2013-08-21 12:42 UTC (permalink / raw) To: linux-mmc; +Cc: 'Chris Ball' Currently there is no mmc_power_off() when resume failed. Somehow, state from mmc_power_on() will be kept. This change makes a pair with its use in case of failure. Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> --- drivers/mmc/core/mmc.c | 3 +++ drivers/mmc/core/sd.c | 3 +++ drivers/mmc/core/sdio.c | 3 +++ 3 files changed, 9 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 6d02012..704a561 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1536,6 +1536,9 @@ static int mmc_resume(struct mmc_host *host) mmc_power_up(host); mmc_select_voltage(host, host->ocr); err = mmc_init_card(host, host->ocr, host->card); + if (err) + mmc_power_off(host); + mmc_release_host(host); return err; diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 176d125..2690ae1 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -1099,6 +1099,9 @@ static int mmc_sd_resume(struct mmc_host *host) mmc_power_up(host); mmc_select_voltage(host, host->ocr); err = mmc_sd_init_card(host, host->ocr, host->card); + if (err) + mmc_power_off(host); + mmc_release_host(host); return err; diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index 80d89cf..8c65669 100644 --- a/drivers/mmc/core/sdio.c +++ b/drivers/mmc/core/sdio.c @@ -1033,6 +1033,9 @@ static int mmc_sdio_resume(struct mmc_host *host) } } + if (err && !mmc_card_keep_power(host)) + mmc_power_off(host); + host->pm_flags &= ~MMC_PM_KEEP_POWER; return err; } -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] mmc: core: do power-off with resume failure 2013-08-21 12:42 [PATCH 1/3] mmc: core: do power-off with resume failure Seungwon Jeon @ 2013-08-23 8:36 ` Ulf Hansson 2013-08-26 6:39 ` Seungwon Jeon 0 siblings, 1 reply; 5+ messages in thread From: Ulf Hansson @ 2013-08-23 8:36 UTC (permalink / raw) To: Seungwon Jeon; +Cc: linux-mmc, Chris Ball On 21 August 2013 14:42, Seungwon Jeon <tgih.jun@samsung.com> wrote: > Currently there is no mmc_power_off() when resume failed. > Somehow, state from mmc_power_on() will be kept. This change > makes a pair with its use in case of failure. Hi Seungwon, To be safe, we can not power off, even if a resume failures. The reason is that the bus/card/host is still available for sending commands to it and those will then likely hang since IP controllers internal registers is configured for "power off" state. Instead, we must leave the power off to be handled from the next rescan work when the card is no longer properly "detected". For NONREMOVABLE we will instead at error path try to do a "power_restore_host". Kind regards Ulf Hansson > > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> > --- > drivers/mmc/core/mmc.c | 3 +++ > drivers/mmc/core/sd.c | 3 +++ > drivers/mmc/core/sdio.c | 3 +++ > 3 files changed, 9 insertions(+), 0 deletions(-) > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index 6d02012..704a561 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -1536,6 +1536,9 @@ static int mmc_resume(struct mmc_host *host) > mmc_power_up(host); > mmc_select_voltage(host, host->ocr); > err = mmc_init_card(host, host->ocr, host->card); > + if (err) > + mmc_power_off(host); > + > mmc_release_host(host); > > return err; > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c > index 176d125..2690ae1 100644 > --- a/drivers/mmc/core/sd.c > +++ b/drivers/mmc/core/sd.c > @@ -1099,6 +1099,9 @@ static int mmc_sd_resume(struct mmc_host *host) > mmc_power_up(host); > mmc_select_voltage(host, host->ocr); > err = mmc_sd_init_card(host, host->ocr, host->card); > + if (err) > + mmc_power_off(host); > + > mmc_release_host(host); > > return err; > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c > index 80d89cf..8c65669 100644 > --- a/drivers/mmc/core/sdio.c > +++ b/drivers/mmc/core/sdio.c > @@ -1033,6 +1033,9 @@ static int mmc_sdio_resume(struct mmc_host *host) > } > } > > + if (err && !mmc_card_keep_power(host)) > + mmc_power_off(host); > + > host->pm_flags &= ~MMC_PM_KEEP_POWER; > return err; > } > -- > 1.7.0.4 > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 1/3] mmc: core: do power-off with resume failure 2013-08-23 8:36 ` Ulf Hansson @ 2013-08-26 6:39 ` Seungwon Jeon 2013-08-26 7:11 ` Ulf Hansson 0 siblings, 1 reply; 5+ messages in thread From: Seungwon Jeon @ 2013-08-26 6:39 UTC (permalink / raw) To: 'Ulf Hansson'; +Cc: 'linux-mmc', 'Chris Ball' On Friday, August 23, 2013, Ulf Hansson wrote: > On 21 August 2013 14:42, Seungwon Jeon <tgih.jun@samsung.com> wrote: > > Currently there is no mmc_power_off() when resume failed. > > Somehow, state from mmc_power_on() will be kept. This change > > makes a pair with its use in case of failure. > > Hi Seungwon, > > To be safe, we can not power off, even if a resume failures. The > reason is that the bus/card/host is still available for sending > commands to it and those will then likely hang since IP controllers > internal registers is configured for "power off" state. Generally when host drivers get MMC_POWER_OFF, host doesn't gate clock for host running. MMC_POWER_ON/OFF is related to card side. So, I guess hanging isn't expected because host is kept ready. > > Instead, we must leave the power off to be handled from the next > rescan work when the card is no longer properly "detected". For > NONREMOVABLE we will instead at error path try to do a > "power_restore_host". It might be needed graceful error handling. Ok, I'll consider further. Thanks, Seungwon Jeon > > Kind regards > Ulf Hansson > > > > > > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> > > --- > > drivers/mmc/core/mmc.c | 3 +++ > > drivers/mmc/core/sd.c | 3 +++ > > drivers/mmc/core/sdio.c | 3 +++ > > 3 files changed, 9 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > > index 6d02012..704a561 100644 > > --- a/drivers/mmc/core/mmc.c > > +++ b/drivers/mmc/core/mmc.c > > @@ -1536,6 +1536,9 @@ static int mmc_resume(struct mmc_host *host) > > mmc_power_up(host); > > mmc_select_voltage(host, host->ocr); > > err = mmc_init_card(host, host->ocr, host->card); > > + if (err) > > + mmc_power_off(host); > > + > > mmc_release_host(host); > > > > return err; > > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c > > index 176d125..2690ae1 100644 > > --- a/drivers/mmc/core/sd.c > > +++ b/drivers/mmc/core/sd.c > > @@ -1099,6 +1099,9 @@ static int mmc_sd_resume(struct mmc_host *host) > > mmc_power_up(host); > > mmc_select_voltage(host, host->ocr); > > err = mmc_sd_init_card(host, host->ocr, host->card); > > + if (err) > > + mmc_power_off(host); > > + > > mmc_release_host(host); > > > > return err; > > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c > > index 80d89cf..8c65669 100644 > > --- a/drivers/mmc/core/sdio.c > > +++ b/drivers/mmc/core/sdio.c > > @@ -1033,6 +1033,9 @@ static int mmc_sdio_resume(struct mmc_host *host) > > } > > } > > > > + if (err && !mmc_card_keep_power(host)) > > + mmc_power_off(host); > > + > > host->pm_flags &= ~MMC_PM_KEEP_POWER; > > return err; > > } > > -- > > 1.7.0.4 > > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] mmc: core: do power-off with resume failure 2013-08-26 6:39 ` Seungwon Jeon @ 2013-08-26 7:11 ` Ulf Hansson 2013-08-26 10:56 ` Seungwon Jeon 0 siblings, 1 reply; 5+ messages in thread From: Ulf Hansson @ 2013-08-26 7:11 UTC (permalink / raw) To: Seungwon Jeon; +Cc: linux-mmc, Chris Ball On 26 August 2013 08:39, Seungwon Jeon <tgih.jun@samsung.com> wrote: > On Friday, August 23, 2013, Ulf Hansson wrote: >> On 21 August 2013 14:42, Seungwon Jeon <tgih.jun@samsung.com> wrote: >> > Currently there is no mmc_power_off() when resume failed. >> > Somehow, state from mmc_power_on() will be kept. This change >> > makes a pair with its use in case of failure. >> >> Hi Seungwon, >> >> To be safe, we can not power off, even if a resume failures. The >> reason is that the bus/card/host is still available for sending >> commands to it and those will then likely hang since IP controllers >> internal registers is configured for "power off" state. > Generally when host drivers get MMC_POWER_OFF, host doesn't gate clock for host running. > MMC_POWER_ON/OFF is related to card side. So, I guess hanging isn't expected because host is kept ready. At least for mmci host driver a hang may happen. Internal IP registers needs to be set properly as done for MMC_POWER_ON mode, otherwise a cmd can not be sent to the card without a potential hang, forever waiting for an interrupt. I would be surprised if similar issues could not occur for other host controllers/drivers. Kind regards Ulf Hansson > >> >> Instead, we must leave the power off to be handled from the next >> rescan work when the card is no longer properly "detected". For >> NONREMOVABLE we will instead at error path try to do a >> "power_restore_host". > It might be needed graceful error handling. > Ok, I'll consider further. > > Thanks, > Seungwon Jeon > >> >> Kind regards >> Ulf Hansson >> >> >> > >> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> >> > --- >> > drivers/mmc/core/mmc.c | 3 +++ >> > drivers/mmc/core/sd.c | 3 +++ >> > drivers/mmc/core/sdio.c | 3 +++ >> > 3 files changed, 9 insertions(+), 0 deletions(-) >> > >> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> > index 6d02012..704a561 100644 >> > --- a/drivers/mmc/core/mmc.c >> > +++ b/drivers/mmc/core/mmc.c >> > @@ -1536,6 +1536,9 @@ static int mmc_resume(struct mmc_host *host) >> > mmc_power_up(host); >> > mmc_select_voltage(host, host->ocr); >> > err = mmc_init_card(host, host->ocr, host->card); >> > + if (err) >> > + mmc_power_off(host); >> > + >> > mmc_release_host(host); >> > >> > return err; >> > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c >> > index 176d125..2690ae1 100644 >> > --- a/drivers/mmc/core/sd.c >> > +++ b/drivers/mmc/core/sd.c >> > @@ -1099,6 +1099,9 @@ static int mmc_sd_resume(struct mmc_host *host) >> > mmc_power_up(host); >> > mmc_select_voltage(host, host->ocr); >> > err = mmc_sd_init_card(host, host->ocr, host->card); >> > + if (err) >> > + mmc_power_off(host); >> > + >> > mmc_release_host(host); >> > >> > return err; >> > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c >> > index 80d89cf..8c65669 100644 >> > --- a/drivers/mmc/core/sdio.c >> > +++ b/drivers/mmc/core/sdio.c >> > @@ -1033,6 +1033,9 @@ static int mmc_sdio_resume(struct mmc_host *host) >> > } >> > } >> > >> > + if (err && !mmc_card_keep_power(host)) >> > + mmc_power_off(host); >> > + >> > host->pm_flags &= ~MMC_PM_KEEP_POWER; >> > return err; >> > } >> > -- >> > 1.7.0.4 >> > >> > >> > -- >> > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in >> > the body of a message to majordomo@vger.kernel.org >> > More majordomo info at http://vger.kernel.org/majordomo-info.html >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 1/3] mmc: core: do power-off with resume failure 2013-08-26 7:11 ` Ulf Hansson @ 2013-08-26 10:56 ` Seungwon Jeon 0 siblings, 0 replies; 5+ messages in thread From: Seungwon Jeon @ 2013-08-26 10:56 UTC (permalink / raw) To: 'Ulf Hansson'; +Cc: 'linux-mmc', 'Chris Ball' On Mon, August 26, 2013, Ulf Hansson wrote: > On 26 August 2013 08:39, Seungwon Jeon <tgih.jun@samsung.com> wrote: > > On Friday, August 23, 2013, Ulf Hansson wrote: > >> On 21 August 2013 14:42, Seungwon Jeon <tgih.jun@samsung.com> wrote: > >> > Currently there is no mmc_power_off() when resume failed. > >> > Somehow, state from mmc_power_on() will be kept. This change > >> > makes a pair with its use in case of failure. > >> > >> Hi Seungwon, > >> > >> To be safe, we can not power off, even if a resume failures. The > >> reason is that the bus/card/host is still available for sending > >> commands to it and those will then likely hang since IP controllers > >> internal registers is configured for "power off" state. > > Generally when host drivers get MMC_POWER_OFF, host doesn't gate clock for host running. > > MMC_POWER_ON/OFF is related to card side. So, I guess hanging isn't expected because host is kept > ready. > > At least for mmci host driver a hang may happen. Internal IP registers > needs to be set properly as done for MMC_POWER_ON mode, otherwise a > cmd can not be sent to the card without a potential hang, forever > waiting for an interrupt. I looked into the set_ios function of mmci host. Power for I/O bus lines are controlled there. Then unexpected occurrence from host may be possible. Host can raise interrupt though. Ok. Thanks, Seungwon Jeon > > I would be surprised if similar issues could not occur for other host > controllers/drivers. > > Kind regards > Ulf Hansson > > > > >> > >> Instead, we must leave the power off to be handled from the next > >> rescan work when the card is no longer properly "detected". For > >> NONREMOVABLE we will instead at error path try to do a > >> "power_restore_host". > > It might be needed graceful error handling. > > Ok, I'll consider further. > > > > Thanks, > > Seungwon Jeon > > > >> > >> Kind regards > >> Ulf Hansson > >> > >> > >> > > >> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> > >> > --- > >> > drivers/mmc/core/mmc.c | 3 +++ > >> > drivers/mmc/core/sd.c | 3 +++ > >> > drivers/mmc/core/sdio.c | 3 +++ > >> > 3 files changed, 9 insertions(+), 0 deletions(-) > >> > > >> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > >> > index 6d02012..704a561 100644 > >> > --- a/drivers/mmc/core/mmc.c > >> > +++ b/drivers/mmc/core/mmc.c > >> > @@ -1536,6 +1536,9 @@ static int mmc_resume(struct mmc_host *host) > >> > mmc_power_up(host); > >> > mmc_select_voltage(host, host->ocr); > >> > err = mmc_init_card(host, host->ocr, host->card); > >> > + if (err) > >> > + mmc_power_off(host); > >> > + > >> > mmc_release_host(host); > >> > > >> > return err; > >> > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c > >> > index 176d125..2690ae1 100644 > >> > --- a/drivers/mmc/core/sd.c > >> > +++ b/drivers/mmc/core/sd.c > >> > @@ -1099,6 +1099,9 @@ static int mmc_sd_resume(struct mmc_host *host) > >> > mmc_power_up(host); > >> > mmc_select_voltage(host, host->ocr); > >> > err = mmc_sd_init_card(host, host->ocr, host->card); > >> > + if (err) > >> > + mmc_power_off(host); > >> > + > >> > mmc_release_host(host); > >> > > >> > return err; > >> > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c > >> > index 80d89cf..8c65669 100644 > >> > --- a/drivers/mmc/core/sdio.c > >> > +++ b/drivers/mmc/core/sdio.c > >> > @@ -1033,6 +1033,9 @@ static int mmc_sdio_resume(struct mmc_host *host) > >> > } > >> > } > >> > > >> > + if (err && !mmc_card_keep_power(host)) > >> > + mmc_power_off(host); > >> > + > >> > host->pm_flags &= ~MMC_PM_KEEP_POWER; > >> > return err; > >> > } > >> > -- > >> > 1.7.0.4 > >> > > >> > > >> > -- > >> > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > >> > the body of a message to majordomo@vger.kernel.org > >> > More majordomo info at http://vger.kernel.org/majordomo-info.html > >> -- > >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > >> the body of a message to majordomo@vger.kernel.org > >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-08-26 10:56 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-08-21 12:42 [PATCH 1/3] mmc: core: do power-off with resume failure Seungwon Jeon 2013-08-23 8:36 ` Ulf Hansson 2013-08-26 6:39 ` Seungwon Jeon 2013-08-26 7:11 ` Ulf Hansson 2013-08-26 10:56 ` Seungwon Jeon
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).