| Message ID | 1472739890-3384-4-git-send-email-alex.lemberg@sandisk.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 1 September 2016 at 16:24, alex lemberg <alex.lemberg@sandisk.com> wrote: > Rescheduling Suspend in case of BKOPS Level >= 1 > in order to let eMMC device to complete its internal GC. > Applicable for Runtime Suspend Only. > > Signed-off-by: alex lemberg <alex.lemberg@sandisk.com> > --- > drivers/mmc/core/mmc.c | 30 +++++++++++++++++++++++------- > include/linux/mmc/mmc.h | 1 + > 2 files changed, 24 insertions(+), 7 deletions(-) > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index e2e987f..c4c6326 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -1904,7 +1904,8 @@ static void mmc_detect(struct mmc_host *host) > } > } > > -static int _mmc_suspend(struct mmc_host *host, bool is_suspend) > +static int _mmc_suspend(struct mmc_host *host, bool is_suspend, > + bool is_runtime_pm) > { > int err = 0; > unsigned int notify_type = is_suspend ? EXT_CSD_POWER_OFF_SHORT : > @@ -1918,10 +1919,25 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend) > if (mmc_card_suspended(host->card)) > goto out; > > - if (mmc_card_doing_bkops(host->card)) { > - err = mmc_stop_bkops(host->card); > - if (err) > + if (mmc_card_doing_bkops(host->card) || > + host->card->ext_csd.auto_bkops_en) { > + err = mmc_read_bkops_status(host->card); > + if (err) { > + pr_err("%s: error %d reading BKOPS Status\n", > + mmc_hostname(host), err); > + goto out; > + } I would appreciate some simple comment of what you want to do here below. > + if (is_runtime_pm && host->card->ext_csd.raw_bkops_status >= > + EXT_CSD_BKOPS_LEVEL_1) { > + pm_schedule_suspend(&host->card->dev, > + MMC_RUNTIME_SUSPEND_DELAY_MS); > goto out; If I understand correctly, you would like to abort the runtime suspend and allow the background operations to complete. That seems reasonable, although in such case you need to return -EBUSY from this function. Moreover, perhaps we should discuss at what BKOPS_LEVEL* we should allow to abort. > + } > + if (mmc_card_doing_bkops(host->card)) { > + err = mmc_stop_bkops(host->card); > + if (err) > + goto out; > + } > } > > err = mmc_flush_cache(host->card); > @@ -1952,7 +1968,7 @@ static int mmc_suspend(struct mmc_host *host) > { > int err; > > - err = _mmc_suspend(host, true); > + err = _mmc_suspend(host, true, false); > if (!err) { > pm_runtime_disable(&host->card->dev); > pm_runtime_set_suspended(&host->card->dev); > @@ -2002,7 +2018,7 @@ static int mmc_shutdown(struct mmc_host *host) > err = _mmc_resume(host); > > if (!err) > - err = _mmc_suspend(host, false); > + err = _mmc_suspend(host, false, false); > > return err; > } > @@ -2026,7 +2042,7 @@ static int mmc_runtime_suspend(struct mmc_host *host) > if (!(host->caps & MMC_CAP_AGGRESSIVE_PM)) > return 0; > > - err = _mmc_suspend(host, true); > + err = _mmc_suspend(host, true, true); > if (err) > pr_err("%s: error %d doing aggressive suspend\n", > mmc_hostname(host), err); > diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h > index 0fe3908..0f08d5c 100644 > --- a/include/linux/mmc/mmc.h > +++ b/include/linux/mmc/mmc.h > @@ -430,6 +430,7 @@ struct _mmc_csd { > /* > * BKOPS status level > */ > +#define EXT_CSD_BKOPS_LEVEL_1 0x1 > #define EXT_CSD_BKOPS_LEVEL_2 0x2 > > /* > -- > 1.9.1 > Another idea I had, was to actually make use of that we know the card will be inactive. So instead of checking if there is an ongoing BKOPS (mmc_card_doing_bkops()), in case of the manual BKOPS, we could potentially check the needed BKOPS LEVEL and schedule a BKOPS to start. In other words, do manual BKOPS when the card is idle. Of course, one need to be able to interrupt/cancel the BKOPS if there is a new request that needs to be managed. I am not sure how we can achieve that, but I hope you get the idea. Kind regards Uffe -- 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
Hi Ulf, Thank you for reviewing the patch! On 10/13/16, 10:36 AM, "Ulf Hansson" <ulf.hansson@linaro.org> wrote: >On 1 September 2016 at 16:24, alex lemberg <alex.lemberg@sandisk.com> wrote: >> Rescheduling Suspend in case of BKOPS Level >= 1 >> in order to let eMMC device to complete its internal GC. >> Applicable for Runtime Suspend Only. >> >> Signed-off-by: alex lemberg <alex.lemberg@sandisk.com> >> --- >> drivers/mmc/core/mmc.c | 30 +++++++++++++++++++++++------- >> include/linux/mmc/mmc.h | 1 + >> 2 files changed, 24 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> index e2e987f..c4c6326 100644 >> --- a/drivers/mmc/core/mmc.c >> +++ b/drivers/mmc/core/mmc.c >> @@ -1904,7 +1904,8 @@ static void mmc_detect(struct mmc_host *host) >> } >> } >> >> -static int _mmc_suspend(struct mmc_host *host, bool is_suspend) >> +static int _mmc_suspend(struct mmc_host *host, bool is_suspend, >> + bool is_runtime_pm) >> { >> int err = 0; >> unsigned int notify_type = is_suspend ? EXT_CSD_POWER_OFF_SHORT : >> @@ -1918,10 +1919,25 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend) >> if (mmc_card_suspended(host->card)) >> goto out; >> >> - if (mmc_card_doing_bkops(host->card)) { >> - err = mmc_stop_bkops(host->card); >> - if (err) >> + if (mmc_card_doing_bkops(host->card) || >> + host->card->ext_csd.auto_bkops_en) { >> + err = mmc_read_bkops_status(host->card); >> + if (err) { >> + pr_err("%s: error %d reading BKOPS Status\n", >> + mmc_hostname(host), err); >> + goto out; >> + } > >I would appreciate some simple comment of what you want to do here below. > >> + if (is_runtime_pm && host->card->ext_csd.raw_bkops_status >= >> + EXT_CSD_BKOPS_LEVEL_1) { >> + pm_schedule_suspend(&host->card->dev, >> + MMC_RUNTIME_SUSPEND_DELAY_MS); >> goto out; > >If I understand correctly, you would like to abort the runtime suspend >and allow the background operations to complete. That seems >reasonable, although in such case you need to return -EBUSY from this >function. In my mind I wanted to reschedule runtime suspend, but your recommendation is makes sense. I will change it and re-submit. >Moreover, perhaps we should discuss at what BKOPS_LEVEL* we should >allow to abort. Agree, the BKOPS_LEVEL value is something that can be interpreted differently by different vendors. Should I define a default value like “BKOPS_LEVEL_TO_USE”, and also maybe add a sysfs entry to allow user configuration? > >> + } >> + if (mmc_card_doing_bkops(host->card)) { >> + err = mmc_stop_bkops(host->card); >> + if (err) >> + goto out; >> + } >> } >> >> err = mmc_flush_cache(host->card); >> @@ -1952,7 +1968,7 @@ static int mmc_suspend(struct mmc_host *host) >> { >> int err; >> >> - err = _mmc_suspend(host, true); >> + err = _mmc_suspend(host, true, false); >> if (!err) { >> pm_runtime_disable(&host->card->dev); >> pm_runtime_set_suspended(&host->card->dev); >> @@ -2002,7 +2018,7 @@ static int mmc_shutdown(struct mmc_host *host) >> err = _mmc_resume(host); >> >> if (!err) >> - err = _mmc_suspend(host, false); >> + err = _mmc_suspend(host, false, false); >> >> return err; >> } >> @@ -2026,7 +2042,7 @@ static int mmc_runtime_suspend(struct mmc_host *host) >> if (!(host->caps & MMC_CAP_AGGRESSIVE_PM)) >> return 0; >> >> - err = _mmc_suspend(host, true); >> + err = _mmc_suspend(host, true, true); >> if (err) >> pr_err("%s: error %d doing aggressive suspend\n", >> mmc_hostname(host), err); >> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h >> index 0fe3908..0f08d5c 100644 >> --- a/include/linux/mmc/mmc.h >> +++ b/include/linux/mmc/mmc.h >> @@ -430,6 +430,7 @@ struct _mmc_csd { >> /* >> * BKOPS status level >> */ >> +#define EXT_CSD_BKOPS_LEVEL_1 0x1 >> #define EXT_CSD_BKOPS_LEVEL_2 0x2 >> >> /* >> -- >> 1.9.1 >> > >Another idea I had, was to actually make use of that we know the card >will be inactive. So instead of checking if there is an ongoing BKOPS >(mmc_card_doing_bkops()), in case of the manual BKOPS, we could >potentially check the needed BKOPS LEVEL and schedule a BKOPS to >start. In other words, do manual BKOPS when the card is idle. If I understand correctly, the idea is to allow MANUAL_ BKOPS in a similar way as it done for AUTO_BKOPS during runtime suspend, right? > >Of course, one need to be able to interrupt/cancel the BKOPS if there >is a new request that needs to be managed. I am not sure how we can >achieve that, but I hope you get the idea. Right, in case of MANUAL_BKOPS, the need the HPI to be issued in order to stop MANUAL_BKOPS immediately on a new request. BTW, the same challenge we had on SLEEP_NOTIFICATION… Since the AUTO_BKOPS is more simple case and doesn’t require the interruption ability from the driver, may I suggest to make it in a separate patchset? > >Kind regards >Uffe
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index e2e987f..c4c6326 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1904,7 +1904,8 @@ static void mmc_detect(struct mmc_host *host) } } -static int _mmc_suspend(struct mmc_host *host, bool is_suspend) +static int _mmc_suspend(struct mmc_host *host, bool is_suspend, + bool is_runtime_pm) { int err = 0; unsigned int notify_type = is_suspend ? EXT_CSD_POWER_OFF_SHORT : @@ -1918,10 +1919,25 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend) if (mmc_card_suspended(host->card)) goto out; - if (mmc_card_doing_bkops(host->card)) { - err = mmc_stop_bkops(host->card); - if (err) + if (mmc_card_doing_bkops(host->card) || + host->card->ext_csd.auto_bkops_en) { + err = mmc_read_bkops_status(host->card); + if (err) { + pr_err("%s: error %d reading BKOPS Status\n", + mmc_hostname(host), err); + goto out; + } + if (is_runtime_pm && host->card->ext_csd.raw_bkops_status >= + EXT_CSD_BKOPS_LEVEL_1) { + pm_schedule_suspend(&host->card->dev, + MMC_RUNTIME_SUSPEND_DELAY_MS); goto out; + } + if (mmc_card_doing_bkops(host->card)) { + err = mmc_stop_bkops(host->card); + if (err) + goto out; + } } err = mmc_flush_cache(host->card); @@ -1952,7 +1968,7 @@ static int mmc_suspend(struct mmc_host *host) { int err; - err = _mmc_suspend(host, true); + err = _mmc_suspend(host, true, false); if (!err) { pm_runtime_disable(&host->card->dev); pm_runtime_set_suspended(&host->card->dev); @@ -2002,7 +2018,7 @@ static int mmc_shutdown(struct mmc_host *host) err = _mmc_resume(host); if (!err) - err = _mmc_suspend(host, false); + err = _mmc_suspend(host, false, false); return err; } @@ -2026,7 +2042,7 @@ static int mmc_runtime_suspend(struct mmc_host *host) if (!(host->caps & MMC_CAP_AGGRESSIVE_PM)) return 0; - err = _mmc_suspend(host, true); + err = _mmc_suspend(host, true, true); if (err) pr_err("%s: error %d doing aggressive suspend\n", mmc_hostname(host), err); diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h index 0fe3908..0f08d5c 100644 --- a/include/linux/mmc/mmc.h +++ b/include/linux/mmc/mmc.h @@ -430,6 +430,7 @@ struct _mmc_csd { /* * BKOPS status level */ +#define EXT_CSD_BKOPS_LEVEL_1 0x1 #define EXT_CSD_BKOPS_LEVEL_2 0x2 /*
Rescheduling Suspend in case of BKOPS Level >= 1 in order to let eMMC device to complete its internal GC. Applicable for Runtime Suspend Only. Signed-off-by: alex lemberg <alex.lemberg@sandisk.com> --- drivers/mmc/core/mmc.c | 30 +++++++++++++++++++++++------- include/linux/mmc/mmc.h | 1 + 2 files changed, 24 insertions(+), 7 deletions(-)