| Message ID | 002201ce9e6b$e0b32850$a21978f0$%jun@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- 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
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 -- 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
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 -- 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
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 -- 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
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; }
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(-)