Re: [PATCH v2 2/2] mmc: Checking BKOPS status prior to Suspend

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



+ Adrian, Shawn-Lin, Jaehoon, Ritesh

On 22 January 2017 at 10:15, Uri Yanai <uri.yanai@xxxxxxxxxxx> wrote:
> In case of Runtime Suspend, check the device BKOPS level.
> Return –EBUSY if device need more time to complete its internal BKOPS.
>
> Changes in v2:
> - return –EBUSY instead of delaying suspend
> - remove define EXT_CSD_BKOPS_LEVEL_1
>
> Signed-off-by: Uri Yanai <uri.yanai@xxxxxxxxxxx>
> Signed-off-by: Alex Lemberg <alex.lemberg@xxxxxxxxxxx>
> ---
>  drivers/mmc/core/mmc.c | 27 +++++++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index f70f6a1..211b726 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1942,7 +1942,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 :
> @@ -1954,6 +1955,24 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
>                 goto out;
>
>         if (mmc_card_doing_bkops(host->card)) {
> +               if (is_runtime_pm && host->card->ext_csd.auto_bkops_en) {

This don't work. Primarily because mmc_card_doing_bkops(host->card)
don't ever return true, unless manual BKOPS is enabled.

I think you need to start with checking if *auto* BKOPS is enabled,
then you can continue to read the BKOPS status to find out whether you
should return -EBUSY.

> +                       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 (host->card->ext_csd.raw_bkops_status >=
> +                                       EXT_CSD_BKOPS_LEVEL_2) {
> +                               /*
> +                                * We don’t allow Runtime Suspend while device
> +                                * still needs time to complete internal BKOPS
> +                                */

Hmm.

Shouldn't we abort (return -EBUSY) also in the system PM suspend case
and not only for runtime PM suspend?

I think we need to discuss this with some other mmc folkz as well,
looping in Adrian, Shawn, Jaehoon and Ritesh.

Of course, unless we are being clever, in could mean that the system
will never enter system PM suspend state, due to a bad behaving eMMC
always reporting EXT_CSD_BKOPS_LEVEL_2. So we need to be careful here.

Now, assume we may want to abort in the system PM suspend case as
well, then we consider to avoid the "Opportunistic sleep" from
hammering us with new system PM suspend attempts. That can be avoided
by activating a wakeup source (aka wakelock), via calling
pm_wakeup_event(), using the struct device for the mmc_card as the
parameter.

> +                               err = -EBUSY;
> +                               goto out;
> +                       }
> +               }
>                 err = mmc_stop_bkops(host->card);
>                 if (err)
>                         goto out;
> @@ -1987,7 +2006,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);
> @@ -2034,7 +2053,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;
>  }
> @@ -2058,7 +2077,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);
> --
> 1.9.1
>

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux