[PATCH 0/3] Auto BKOPS PM suspend

[PATCH 0/3] Auto BKOPS PM suspend

[Uri Yanai]

Hi Ulf
Resubmitting three patches for approval, originally submitted by Alex Lemberg <alex.lemberg@sandisk.com>
[1/3] mmc: replacing hardcoded value for runtime PM suspend  		Message ID:<1472739890-3384-2-git-send-email-alex.lemberg@sandisk.com>
[2/3] mmc: Adding AUTO_BKOPS_EN bit set for Auto BKOPS support		Message ID:<1472739890-3384-3-git-send-email-alex.lemberg@sandisk.com>
[3/3] mmc: Checking BKOPS status prior to Suspend 			Message ID:<1472739890-3384-4-git-send-email-alex.lemberg@sandisk.com>

Thanks
Uri Yanai

Uri Yanai (3):
  mmc: replacing hardcoded value for runtime PM suspend
  mmc: Adding AUTO_BKOPS_EN bit set for Auto BKOPS support
  mmc: Checking BKOPS status prior to Suspend

 drivers/mmc/core/block.c |  3 ++-
 drivers/mmc/core/mmc.c   | 45 ++++++++++++++++++++++++++++++++++++---------
 include/linux/mmc/card.h |  1 +
 include/linux/mmc/host.h |  2 ++
 include/linux/mmc/mmc.h  |  3 +++
 5 files changed, 44 insertions(+), 10 deletions(-)

-- 
1.9.1

[PATCH 1/3] mmc: replacing hardcoded value for runtime PM suspend

[Uri Yanai]

Adds define MMC_RUNTIME_SUSPEND_DELAY_MS for setting delay
time(in milliseconds) before entering device runtime suspend.
Setting the default to 3000 milliseconds.

Change-Id: Ifab084cc01d8dff8c79606f8de0858f0821e98bc
Signed-off-by: Uri Yanai <uri.yanai@sandisk.com>
Signed-off-by: Alex Lemberg <alex.lemberg@sandisk.com>
---
 drivers/mmc/core/block.c | 3 ++-
 include/linux/mmc/host.h | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 6648a17..7954f15 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2202,7 +2202,8 @@ static int mmc_blk_probe(struct mmc_card *card)
 			goto out;
 	}
 
-	pm_runtime_set_autosuspend_delay(&card->dev, 3000);
+	pm_runtime_set_autosuspend_delay(&card->dev,
+			MMC_RUNTIME_SUSPEND_DELAY_MS);
 	pm_runtime_use_autosuspend(&card->dev);
 
 	/*
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 8bc8841..4a0b30f 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -22,6 +22,8 @@
 #include <linux/mmc/mmc.h>
 #include <linux/mmc/pm.h>
 
+#define MMC_RUNTIME_SUSPEND_DELAY_MS	3000
+
 struct mmc_ios {
 	unsigned int	clock;			/* clock rate */
 	unsigned short	vdd;
-- 
1.9.1

[PATCH 2/3] mmc: Adding AUTO_BKOPS_EN bit set for Auto BKOPS support

[Uri Yanai]

Read from the device EXT_CSD and set to the card->ext_csd structure

Change-Id: I940b1612f5bac078c330ad3829d7aa5f0ba0989e
Signed-off-by: Uri Yanai <uri.yanai@sandisk.com>
Signed-off-by: Alex Lemberg <alex.lemberg@sandisk.com>
---
 drivers/mmc/core/mmc.c   | 10 ++++++++--
 include/linux/mmc/card.h |  1 +
 include/linux/mmc/mmc.h  |  1 +
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index c0e2507..f70f6a1 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -530,8 +530,14 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd)
 						EXT_CSD_MANUAL_BKOPS_MASK);
 			card->ext_csd.raw_bkops_status =
 				ext_csd[EXT_CSD_BKOPS_STATUS];
-			if (!card->ext_csd.man_bkops_en)
-				pr_debug("%s: MAN_BKOPS_EN bit is not set\n",
+			if (card->ext_csd.man_bkops_en)
+				pr_debug("%s: MAN_BKOPS_EN bit is set\n",
+					mmc_hostname(card->host));
+			card->ext_csd.auto_bkops_en =
+					(ext_csd[EXT_CSD_BKOPS_EN] &
+						EXT_CSD_AUTO_BKOPS_MASK);
+			if (card->ext_csd.auto_bkops_en)
+				pr_debug("%s: AUTO_BKOPS_EN bit is set\n",
 					mmc_hostname(card->host));
 		}
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 00449e5..e3f1031 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -84,6 +84,7 @@ struct mmc_ext_csd {
 	unsigned int		hpi_cmd;		/* cmd used as HPI */
 	bool			bkops;		/* background support bit */
 	bool			man_bkops_en;	/* manual bkops enable bit */
+	bool			auto_bkops_en;	/* auto bkops enable bit */
 	unsigned int            data_sector_size;       /* 512 bytes or 4KB */
 	unsigned int            data_tag_unit_size;     /* DATA TAG UNIT size */
 	unsigned int		boot_ro_lock;		/* ro lock support */
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index a074082..126b8d5 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -449,6 +449,7 @@ struct _mmc_csd {
  * BKOPS modes
  */
 #define EXT_CSD_MANUAL_BKOPS_MASK	0x01
+#define EXT_CSD_AUTO_BKOPS_MASK		0x02
 
 /*
  * Command Queue
-- 
1.9.1

[PATCH 3/3] mmc: Checking BKOPS status prior to Suspend

[Uri Yanai]

If asked to do device Runtime Suspend while
in BKOPS and auto BKOPS is enabled return –EBUSY

Change-Id: Ie4b38a32fe98179f0bca5b57edaaa47a02d0a389
Signed-off-by: Uri Yanai <uri.yanai@sandisk.com>
Signed-off-by: Alex Lemberg <alex.lemberg@sandisk.com>
---
 drivers/mmc/core/mmc.c  | 35 ++++++++++++++++++++++++++++-------
 include/linux/mmc/mmc.h |  2 ++
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f70f6a1..21f5851 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 :
@@ -1953,10 +1954,30 @@ 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_PM_SUSPEND) {
+			/*
+			 * We don’t allow Runtime Suspend while device still
+			 * needs time to complete internal BKOPS, returning
+			 * -EBUSY while BKOPS level is above
+			 *  EXT_CSD_BKOPS_LEVEL_PM_SUSPEND
+			 */
+			err = -EBUSY;
+			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);
@@ -1987,7 +2008,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 +2055,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 +2079,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 126b8d5..2633274 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -445,6 +445,8 @@ struct _mmc_csd {
  */
 #define EXT_CSD_BKOPS_LEVEL_2		0x2
 
+#define EXT_CSD_BKOPS_LEVEL_PM_SUSPEND	EXT_CSD_BKOPS_LEVEL_2
+
 /*
  * BKOPS modes
  */
-- 
1.9.1

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

[Shawn Lin]

On 2017/1/17 20:15, Uri Yanai wrote:
> If asked to do device Runtime Suspend while
> in BKOPS and auto BKOPS is enabled return –EBUSY
>

The commit msg isn't correct as the code just return -EBUSY
when the level of BKOPS is EXT_CSD_BKOPS_LEVEL_PM_SUSPEND.

And how do you decide that EXT_CSD_BKOPS_LEVEL_2 should be used.
 From the spec, it means "performance being impacted". It would be
good to know a bit details inside the eMMC firmware.

BTW, should we use EXT_CSD_BKOPS_LEVEL_2 directly since it looks
more strightforward?


> Change-Id: Ie4b38a32fe98179f0bca5b57edaaa47a02d0a389
> Signed-off-by: Uri Yanai <uri.yanai@sandisk.com>
> Signed-off-by: Alex Lemberg <alex.lemberg@sandisk.com>
> ---
>  drivers/mmc/core/mmc.c  | 35 ++++++++++++++++++++++++++++-------
>  include/linux/mmc/mmc.h |  2 ++
>  2 files changed, 30 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index f70f6a1..21f5851 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 :
> @@ -1953,10 +1954,30 @@ 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_PM_SUSPEND) {
> +			/*
> +			 * We don’t allow Runtime Suspend while device still
> +			 * needs time to complete internal BKOPS, returning
> +			 * -EBUSY while BKOPS level is above
> +			 *  EXT_CSD_BKOPS_LEVEL_PM_SUSPEND
> +			 */
> +			err = -EBUSY;
> +			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);
> @@ -1987,7 +2008,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 +2055,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 +2079,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 126b8d5..2633274 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -445,6 +445,8 @@ struct _mmc_csd {
>   */
>  #define EXT_CSD_BKOPS_LEVEL_2		0x2
>
> +#define EXT_CSD_BKOPS_LEVEL_PM_SUSPEND	EXT_CSD_BKOPS_LEVEL_2
> +
>  /*
>   * BKOPS modes
>   */
>


-- 
Best Regards
Shawn Lin

RE: [PATCH 3/3] mmc: Checking BKOPS status prior to Suspend

[Uri Yanai]

Hi Shawn

Thanks for the review.
1. As you suggested replaced EXT_CSD_BKOPS_LEVEL_PM_SUSPEND  calling EXT_CSD_BKOPS_LEVEL_2 directly.
2. Returning  -EBUSY from the point I do in the code, stops the path to enter suspend( i.e. calling  mmc_power_off() and mmc_card_set_suspended())
Did I get this wrong ?

Thanks
Uri
-----Original Message-----
From: Shawn Lin [mailto:shawn.lin@rock-chips.com] 
Sent: Wednesday, January 18, 2017 4:00 AM
To: Uri Yanai; ulf.hansson@linaro.org
Cc: shawn.lin@rock-chips.com; linux-mmc@vger.kernel.org; chris@printf.net; Alex Lemberg
Subject: Re: [PATCH 3/3] mmc: Checking BKOPS status prior to Suspend

On 2017/1/17 20:15, Uri Yanai wrote:
> If asked to do device Runtime Suspend while in BKOPS and auto BKOPS is 
> enabled return –EBUSY
>

The commit msg isn't correct as the code just return -EBUSY when the level of BKOPS is EXT_CSD_BKOPS_LEVEL_PM_SUSPEND.

And how do you decide that EXT_CSD_BKOPS_LEVEL_2 should be used.
 From the spec, it means "performance being impacted". It would be good to know a bit details inside the eMMC firmware.

BTW, should we use EXT_CSD_BKOPS_LEVEL_2 directly since it looks more strightforward?


> Change-Id: Ie4b38a32fe98179f0bca5b57edaaa47a02d0a389
> Signed-off-by: Uri Yanai <uri.yanai@sandisk.com>
> Signed-off-by: Alex Lemberg <alex.lemberg@sandisk.com>
> ---
>  drivers/mmc/core/mmc.c  | 35 ++++++++++++++++++++++++++++-------
>  include/linux/mmc/mmc.h |  2 ++
>  2 files changed, 30 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 
> f70f6a1..21f5851 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 :
> @@ -1953,10 +1954,30 @@ 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_PM_SUSPEND) {
> +			/*
> +			 * We don’t allow Runtime Suspend while device still
> +			 * needs time to complete internal BKOPS, returning
> +			 * -EBUSY while BKOPS level is above
> +			 *  EXT_CSD_BKOPS_LEVEL_PM_SUSPEND
> +			 */
> +			err = -EBUSY;
> +			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);
> @@ -1987,7 +2008,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 +2055,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 +2079,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 
> 126b8d5..2633274 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -445,6 +445,8 @@ struct _mmc_csd {
>   */
>  #define EXT_CSD_BKOPS_LEVEL_2		0x2
>
> +#define EXT_CSD_BKOPS_LEVEL_PM_SUSPEND	EXT_CSD_BKOPS_LEVEL_2
> +
>  /*
>   * BKOPS modes
>   */
>


--
Best Regards
Shawn Lin

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

[Adrian Hunter]

On 17/01/17 14:15, Uri Yanai wrote:
> If asked to do device Runtime Suspend while
> in BKOPS and auto BKOPS is enabled return –EBUSY

The spec also says

"Before moving to Sleep state hosts may set the POWER_OFF_NOTIFICATION byte
to SLEEP_NOTIFICATION (0x04) if aware that the device is capable of
autonomously initiating background operations for possible performance
improvements."

Have you considered that?

But I wonder if we shouldn't always use SLEEP_NOTIFICATION before sleep?

> 
> Change-Id: Ie4b38a32fe98179f0bca5b57edaaa47a02d0a389
> Signed-off-by: Uri Yanai <uri.yanai@sandisk.com>
> Signed-off-by: Alex Lemberg <alex.lemberg@sandisk.com>
> ---
>  drivers/mmc/core/mmc.c  | 35 ++++++++++++++++++++++++++++-------
>  include/linux/mmc/mmc.h |  2 ++
>  2 files changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index f70f6a1..21f5851 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 :
> @@ -1953,10 +1954,30 @@ 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_PM_SUSPEND) {
> +			/*
> +			 * We don’t allow Runtime Suspend while device still
> +			 * needs time to complete internal BKOPS, returning
> +			 * -EBUSY while BKOPS level is above
> +			 *  EXT_CSD_BKOPS_LEVEL_PM_SUSPEND
> +			 */
> +			err = -EBUSY;
> +			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);
> @@ -1987,7 +2008,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 +2055,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 +2079,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 126b8d5..2633274 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -445,6 +445,8 @@ struct _mmc_csd {
>   */
>  #define EXT_CSD_BKOPS_LEVEL_2		0x2
>  
> +#define EXT_CSD_BKOPS_LEVEL_PM_SUSPEND	EXT_CSD_BKOPS_LEVEL_2
> +
>  /*
>   * BKOPS modes
>   */
> 

RE: [PATCH 3/3] mmc: Checking BKOPS status prior to Suspend

[Uri Yanai]

Hi Adrian

Thanks for your review.
Ulf rejected our approach in the first patch submission calling pm_schedule_suspend()

" 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."

If I understood Ulf correctly, instead of suspending he suggest aborting the suspend altogether returning -EBUSY.

If I understood you correctly, your approach is similar to the one taken in the first patch submission.
Instead of calling pm_schedule_suspend(), you suggest calling  mmc_poweroff_notify(host->card, SLEEP_NOTIFICATION).

Regards
Uri

-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter@intel.com] 
Sent: Friday, January 20, 2017 1:56 PM
To: Uri Yanai; ulf.hansson@linaro.org
Cc: linux-mmc@vger.kernel.org; chris@printf.net; Alex Lemberg
Subject: Re: [PATCH 3/3] mmc: Checking BKOPS status prior to Suspend

On 17/01/17 14:15, Uri Yanai wrote:
> If asked to do device Runtime Suspend while in BKOPS and auto BKOPS is 
> enabled return -EBUSY

The spec also says

"Before moving to Sleep state hosts may set the POWER_OFF_NOTIFICATION byte to SLEEP_NOTIFICATION (0x04) if aware that the device is capable of autonomously initiating background operations for possible performance improvements."

Have you considered that?

But I wonder if we shouldn't always use SLEEP_NOTIFICATION before sleep?

> 
> Change-Id: Ie4b38a32fe98179f0bca5b57edaaa47a02d0a389
> Signed-off-by: Uri Yanai <uri.yanai@sandisk.com>
> Signed-off-by: Alex Lemberg <alex.lemberg@sandisk.com>
> ---
>  drivers/mmc/core/mmc.c  | 35 ++++++++++++++++++++++++++++-------
>  include/linux/mmc/mmc.h |  2 ++
>  2 files changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 
> f70f6a1..21f5851 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 :
> @@ -1953,10 +1954,30 @@ 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_PM_SUSPEND) {
> +			/*
> +			 * We don't allow Runtime Suspend while device still
> +			 * needs time to complete internal BKOPS, returning
> +			 * -EBUSY while BKOPS level is above
> +			 *  EXT_CSD_BKOPS_LEVEL_PM_SUSPEND
> +			 */
> +			err = -EBUSY;
> +			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);
> @@ -1987,7 +2008,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 +2055,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 +2079,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 
> 126b8d5..2633274 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -445,6 +445,8 @@ struct _mmc_csd {
>   */
>  #define EXT_CSD_BKOPS_LEVEL_2		0x2
>  
> +#define EXT_CSD_BKOPS_LEVEL_PM_SUSPEND	EXT_CSD_BKOPS_LEVEL_2
> +
>  /*
>   * BKOPS modes
>   */
> 

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

[Alex Lemberg]

Hi Adrian,

I would like to add several notes on top of Uri’s answer...
I agree with you, by the spec, PON Sleep_Notification should be
set before calling Sleep command.
As you probably remember, we have tried to implement it
in the past and even submitted a patch – “[PATCH] mmc: sleep notification”
https://www.mail-archive.com/linux-mmc@vger.kernel.org/msg30906.html
As you can see from the thread, one of the main issues in Sleep_Notification
is a SLEEP_NOTIFICATION_TIME (defined in EXT_CSD [216]), which need to be
considered in implementation…
Thus, I think postponing Sleep during suspend requires a different from current
AUTO_BKOPS implementation, and I suggest to handle it in separate
patchset, if possible.


Thanks,
Alex

On 1/22/17, 5:55 PM, "Uri Yanai" <Uri.Yanai@sandisk.com> wrote:

    Hi Adrian
    
    Thanks for your review.
    Ulf rejected our approach in the first patch submission calling pm_schedule_suspend()
    
    " 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."
    
    If I understood Ulf correctly, instead of suspending he suggest aborting the suspend altogether returning -EBUSY.
    
    If I understood you correctly, your approach is similar to the one taken in the first patch submission.
    Instead of calling pm_schedule_suspend(), you suggest calling  mmc_poweroff_notify(host->card, SLEEP_NOTIFICATION).
    
    Regards
    Uri
    
    -----Original Message-----
    From: Adrian Hunter [mailto:adrian.hunter@intel.com] 
    Sent: Friday, January 20, 2017 1:56 PM
    To: Uri Yanai; ulf.hansson@linaro.org
    Cc: linux-mmc@vger.kernel.org; chris@printf.net; Alex Lemberg
    Subject: Re: [PATCH 3/3] mmc: Checking BKOPS status prior to Suspend
    
    On 17/01/17 14:15, Uri Yanai wrote:
    > If asked to do device Runtime Suspend while in BKOPS and auto BKOPS is 
    > enabled return –EBUSY
    
    The spec also says
    
    "Before moving to Sleep state hosts may set the POWER_OFF_NOTIFICATION byte to SLEEP_NOTIFICATION (0x04) if aware that the device is capable of autonomously initiating background operations for possible performance improvements."
    
    Have you considered that?
    
    But I wonder if we shouldn't always use SLEEP_NOTIFICATION before sleep?
    
    > 
    > Change-Id: Ie4b38a32fe98179f0bca5b57edaaa47a02d0a389
    > Signed-off-by: Uri Yanai <uri.yanai@sandisk.com>
    > Signed-off-by: Alex Lemberg <alex.lemberg@sandisk.com>
    > ---
    >  drivers/mmc/core/mmc.c  | 35 ++++++++++++++++++++++++++++-------
    >  include/linux/mmc/mmc.h |  2 ++
    >  2 files changed, 30 insertions(+), 7 deletions(-)
    > 
    > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 
    > f70f6a1..21f5851 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 :
    > @@ -1953,10 +1954,30 @@ 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_PM_SUSPEND) {
    > +			/*
    > +			 * We don’t allow Runtime Suspend while device still
    > +			 * needs time to complete internal BKOPS, returning
    > +			 * -EBUSY while BKOPS level is above
    > +			 *  EXT_CSD_BKOPS_LEVEL_PM_SUSPEND
    > +			 */
    > +			err = -EBUSY;
    > +			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);
    > @@ -1987,7 +2008,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 +2055,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 +2079,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 
    > 126b8d5..2633274 100644
    > --- a/include/linux/mmc/mmc.h
    > +++ b/include/linux/mmc/mmc.h
    > @@ -445,6 +445,8 @@ struct _mmc_csd {
    >   */
    >  #define EXT_CSD_BKOPS_LEVEL_2		0x2
    >  
    > +#define EXT_CSD_BKOPS_LEVEL_PM_SUSPEND	EXT_CSD_BKOPS_LEVEL_2
    > +
    >  /*
    >   * BKOPS modes
    >   */
    > 
    
    

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

[Adrian Hunter]

On 23/01/17 19:07, Alex Lemberg wrote:
> Hi Adrian,
> 
> I would like to add several notes on top of Uri’s answer...
> I agree with you, by the spec, PON Sleep_Notification should be
> set before calling Sleep command.
> As you probably remember, we have tried to implement it
> in the past and even submitted a patch – “[PATCH] mmc: sleep notification”
> https://www.mail-archive.com/linux-mmc@vger.kernel.org/msg30906.html
> As you can see from the thread, one of the main issues in Sleep_Notification
> is a SLEEP_NOTIFICATION_TIME (defined in EXT_CSD [216]), which need to be
> considered in implementation…

Does it?  I would tend to assume SLEEP_NOTIFICATION_TIME will be acceptable
during _suspend until proven otherwise.

> Thus, I think postponing Sleep during suspend requires a different from current
> AUTO_BKOPS implementation, and I suggest to handle it in separate
> patchset, if possible.

Not at all sure what you mean by "postponing Sleep"?

> 
> 
> Thanks,
> Alex
> 
> On 1/22/17, 5:55 PM, "Uri Yanai" <Uri.Yanai@sandisk.com> wrote:
> 
>     Hi Adrian
>     
>     Thanks for your review.
>     Ulf rejected our approach in the first patch submission calling pm_schedule_suspend()
>     
>     " 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."
>     
>     If I understood Ulf correctly, instead of suspending he suggest aborting the suspend altogether returning -EBUSY.
>     
>     If I understood you correctly, your approach is similar to the one taken in the first patch submission.
>     Instead of calling pm_schedule_suspend(), you suggest calling  mmc_poweroff_notify(host->card, SLEEP_NOTIFICATION).
>     
>     Regards
>     Uri
>     
>     -----Original Message-----
>     From: Adrian Hunter [mailto:adrian.hunter@intel.com] 
>     Sent: Friday, January 20, 2017 1:56 PM
>     To: Uri Yanai; ulf.hansson@linaro.org
>     Cc: linux-mmc@vger.kernel.org; chris@printf.net; Alex Lemberg
>     Subject: Re: [PATCH 3/3] mmc: Checking BKOPS status prior to Suspend
>     
>     On 17/01/17 14:15, Uri Yanai wrote:
>     > If asked to do device Runtime Suspend while in BKOPS and auto BKOPS is 
>     > enabled return –EBUSY
>     
>     The spec also says
>     
>     "Before moving to Sleep state hosts may set the POWER_OFF_NOTIFICATION byte to SLEEP_NOTIFICATION (0x04) if aware that the device is capable of autonomously initiating background operations for possible performance improvements."
>     
>     Have you considered that?
>     
>     But I wonder if we shouldn't always use SLEEP_NOTIFICATION before sleep?
>     
>     > 
>     > Change-Id: Ie4b38a32fe98179f0bca5b57edaaa47a02d0a389
>     > Signed-off-by: Uri Yanai <uri.yanai@sandisk.com>
>     > Signed-off-by: Alex Lemberg <alex.lemberg@sandisk.com>
>     > ---
>     >  drivers/mmc/core/mmc.c  | 35 ++++++++++++++++++++++++++++-------
>     >  include/linux/mmc/mmc.h |  2 ++
>     >  2 files changed, 30 insertions(+), 7 deletions(-)
>     > 
>     > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 
>     > f70f6a1..21f5851 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 :
>     > @@ -1953,10 +1954,30 @@ 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_PM_SUSPEND) {
>     > +			/*
>     > +			 * We don’t allow Runtime Suspend while device still
>     > +			 * needs time to complete internal BKOPS, returning
>     > +			 * -EBUSY while BKOPS level is above
>     > +			 *  EXT_CSD_BKOPS_LEVEL_PM_SUSPEND
>     > +			 */
>     > +			err = -EBUSY;
>     > +			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);
>     > @@ -1987,7 +2008,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 +2055,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 +2079,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 
>     > 126b8d5..2633274 100644
>     > --- a/include/linux/mmc/mmc.h
>     > +++ b/include/linux/mmc/mmc.h
>     > @@ -445,6 +445,8 @@ struct _mmc_csd {
>     >   */
>     >  #define EXT_CSD_BKOPS_LEVEL_2		0x2
>     >  
>     > +#define EXT_CSD_BKOPS_LEVEL_PM_SUSPEND	EXT_CSD_BKOPS_LEVEL_2
>     > +
>     >  /*
>     >   * BKOPS modes
>     >   */
>     > 
>     
>     
> 

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

[Alex Lemberg]

On 1/24/17, 4:36 PM, "Adrian Hunter" <adrian.hunter@intel.com> wrote:

    On 23/01/17 19:07, Alex Lemberg wrote:
    > Hi Adrian,
    > 
    > I would like to add several notes on top of Uri’s answer...
    > I agree with you, by the spec, PON Sleep_Notification should be
    > set before calling Sleep command.
    > As you probably remember, we have tried to implement it
    > in the past and even submitted a patch – “[PATCH] mmc: sleep notification”
    > https://www.mail-archive.com/linux-mmc@vger.kernel.org/msg30906.html
    > As you can see from the thread, one of the main issues in Sleep_Notification
    > is a SLEEP_NOTIFICATION_TIME (defined in EXT_CSD [216]), which need to be
    > considered in implementation…
    
    Does it?  I would tend to assume SLEEP_NOTIFICATION_TIME will be acceptable
    during _suspend until proven otherwise.

Potentially, by the spec, the Max value of SLEEP_NOTIFICATION_TIME can be 83.88 seconds.
Can we assume that this time is acceptable during _suspend?
    
    > Thus, I think postponing Sleep during suspend requires a different from current
    > AUTO_BKOPS implementation, and I suggest to handle it in separate
    > patchset, if possible.
    
    Not at all sure what you mean by "postponing Sleep"?

I mean letting Sleep_Notification to be completed before Sleep.
 

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

[Adrian Hunter]

On 25/01/17 11:48, Alex Lemberg wrote:
> 
> 
> On 1/24/17, 4:36 PM, "Adrian Hunter" <adrian.hunter@intel.com> wrote:
> 
>     On 23/01/17 19:07, Alex Lemberg wrote:
>     > Hi Adrian,
>     > 
>     > I would like to add several notes on top of Uri’s answer...
>     > I agree with you, by the spec, PON Sleep_Notification should be
>     > set before calling Sleep command.
>     > As you probably remember, we have tried to implement it
>     > in the past and even submitted a patch – “[PATCH] mmc: sleep notification”
>     > https://www.mail-archive.com/linux-mmc@vger.kernel.org/msg30906.html
>     > As you can see from the thread, one of the main issues in Sleep_Notification
>     > is a SLEEP_NOTIFICATION_TIME (defined in EXT_CSD [216]), which need to be
>     > considered in implementation…
>     
>     Does it?  I would tend to assume SLEEP_NOTIFICATION_TIME will be acceptable
>     during _suspend until proven otherwise.
> 
> Potentially, by the spec, the Max value of SLEEP_NOTIFICATION_TIME can be 83.88 seconds.
> Can we assume that this time is acceptable during _suspend?

Do you know of any cards that take that long?

As I wrote, I would assume it is acceptable until we know otherwise.

>     
>     > Thus, I think postponing Sleep during suspend requires a different from current
>     > AUTO_BKOPS implementation, and I suggest to handle it in separate
>     > patchset, if possible.
>     
>     Not at all sure what you mean by "postponing Sleep"?
> 
> I mean letting Sleep_Notification to be completed before Sleep.

ook.

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

[Alex Lemberg]

On 1/25/17, 12:30 PM, "Adrian Hunter" <adrian.hunter@intel.com> wrote:

    On 25/01/17 11:48, Alex Lemberg wrote:
    > 
    > 
    > On 1/24/17, 4:36 PM, "Adrian Hunter" <adrian.hunter@intel.com> wrote:
    > 
    >     On 23/01/17 19:07, Alex Lemberg wrote:
    >     > Hi Adrian,
    >     > 
    >     > I would like to add several notes on top of Uri’s answer...
    >     > I agree with you, by the spec, PON Sleep_Notification should be
    >     > set before calling Sleep command.
    >     > As you probably remember, we have tried to implement it
    >     > in the past and even submitted a patch – “[PATCH] mmc: sleep notification”
    >     > https://www.mail-archive.com/linux-mmc@vger.kernel.org/msg30906.html
    >     > As you can see from the thread, one of the main issues in Sleep_Notification
    >     > is a SLEEP_NOTIFICATION_TIME (defined in EXT_CSD [216]), which need to be
    >     > considered in implementation…
    >     
    >     Does it?  I would tend to assume SLEEP_NOTIFICATION_TIME will be acceptable
    >     during _suspend until proven otherwise.
    > 
    > Potentially, by the spec, the Max value of SLEEP_NOTIFICATION_TIME can be 83.88 seconds.
    > Can we assume that this time is acceptable during _suspend?
    
    Do you know of any cards that take that long?

Yes - for some cards it can take several seconds in some corner cases.

    As I wrote, I would assume it is acceptable until we know otherwise.

I think Sleep_Notification requires more detailed review. 
Since PON Sleep_Notification is a blocking command (BUSY asserted),
in case of getting new request, driver need to send HPI command in order
to interrupt the Sleep_Notification process.
We tried to handle this case in our Sleep_Notification patchset…
Anyway, I think we need to discuss it separately, not related to
the AUTO_BKOPS support…

    >     
    >     > Thus, I think postponing Sleep during suspend requires a different from current
    >     > AUTO_BKOPS implementation, and I suggest to handle it in separate
    >     > patchset, if possible.
    >     
    >     Not at all sure what you mean by "postponing Sleep"?
    > 
    > I mean letting Sleep_Notification to be completed before Sleep.
    
    ook.
    
    

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

[Adrian Hunter]

On 25/01/17 14:40, Alex Lemberg wrote:
> 
> 
> On 1/25/17, 12:30 PM, "Adrian Hunter" <adrian.hunter@intel.com> wrote:
> 
>     On 25/01/17 11:48, Alex Lemberg wrote:
>     > 
>     > 
>     > On 1/24/17, 4:36 PM, "Adrian Hunter" <adrian.hunter@intel.com> wrote:
>     > 
>     >     On 23/01/17 19:07, Alex Lemberg wrote:
>     >     > Hi Adrian,
>     >     > 
>     >     > I would like to add several notes on top of Uri’s answer...
>     >     > I agree with you, by the spec, PON Sleep_Notification should be
>     >     > set before calling Sleep command.
>     >     > As you probably remember, we have tried to implement it
>     >     > in the past and even submitted a patch – “[PATCH] mmc: sleep notification”
>     >     > https://www.mail-archive.com/linux-mmc@vger.kernel.org/msg30906.html
>     >     > As you can see from the thread, one of the main issues in Sleep_Notification
>     >     > is a SLEEP_NOTIFICATION_TIME (defined in EXT_CSD [216]), which need to be
>     >     > considered in implementation…
>     >     
>     >     Does it?  I would tend to assume SLEEP_NOTIFICATION_TIME will be acceptable
>     >     during _suspend until proven otherwise.
>     > 
>     > Potentially, by the spec, the Max value of SLEEP_NOTIFICATION_TIME can be 83.88 seconds.
>     > Can we assume that this time is acceptable during _suspend?
>     
>     Do you know of any cards that take that long?
> 
> Yes - for some cards it can take several seconds in some corner cases.

Wouldn't that mean POWER_OFF_SHORT has the same problem since that is
preparing for *both* Vcc and Vccq to go off?

> 
>     As I wrote, I would assume it is acceptable until we know otherwise.
> 
> I think Sleep_Notification requires more detailed review. 
> Since PON Sleep_Notification is a blocking command (BUSY asserted),
> in case of getting new request, driver need to send HPI command in order
> to interrupt the Sleep_Notification process.
> We tried to handle this case in our Sleep_Notification patchset…
> Anyway, I think we need to discuss it separately, not related to
> the AUTO_BKOPS support…
> 
>     >     
>     >     > Thus, I think postponing Sleep during suspend requires a different from current
>     >     > AUTO_BKOPS implementation, and I suggest to handle it in separate
>     >     > patchset, if possible.
>     >     
>     >     Not at all sure what you mean by "postponing Sleep"?
>     > 
>     > I mean letting Sleep_Notification to be completed before Sleep.
>     
>     ook.
>     
>     
> 

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

[Alex Lemberg]

On 1/25/17, 2:45 PM, "linux-mmc-owner@vger.kernel.org on behalf of Adrian Hunter" <linux-mmc-owner@vger.kernel.org on behalf of adrian.hunter@intel.com> wrote:

 
    >     > Potentially, by the spec, the Max value of SLEEP_NOTIFICATION_TIME can be 83.88 seconds.
    >     > Can we assume that this time is acceptable during _suspend?
    >     
    >     Do you know of any cards that take that long?
    > 
    > Yes - for some cards it can take several seconds in some corner cases.
    
    Wouldn't that mean POWER_OFF_SHORT has the same problem since that is
    preparing for *both* Vcc and Vccq to go off?

POWER_OFF_SHORT should be completed within GENERIC_CMD6_TIME,
which is a standard Switch command time.
 
    > 
    >     As I wrote, I would assume it is acceptable until we know otherwise.
    > 
    > I think Sleep_Notification requires more detailed review. 
    > Since PON Sleep_Notification is a blocking command (BUSY asserted),
    > in case of getting new request, driver need to send HPI command in order
    > to interrupt the Sleep_Notification process.
    > We tried to handle this case in our Sleep_Notification patchset…
    > Anyway, I think we need to discuss it separately, not related to
    > the AUTO_BKOPS support…
    > 
    >     >     
    >     >     > Thus, I think postponing Sleep during suspend requires a different from current
    >     >     > AUTO_BKOPS implementation, and I suggest to handle it in separate
    >     >     > patchset, if possible.
    >     >     
    >     >     Not at all sure what you mean by "postponing Sleep"?
    >     > 
    >     > I mean letting Sleep_Notification to be completed before Sleep.
    >     
    >     ook.
    >     
    >     
    > 
    
    --
    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
    

Re: [PATCH 0/3] Auto BKOPS PM suspend

[Shawn Lin]

Hi Uri,

On 2017/1/17 20:15, Uri Yanai wrote:
> Hi Ulf
> Resubmitting three patches for approval, originally submitted by Alex Lemberg <alex.lemberg@sandisk.com>
> [1/3] mmc: replacing hardcoded value for runtime PM suspend  		Message ID:<1472739890-3384-2-git-send-email-alex.lemberg@sandisk.com>
> [2/3] mmc: Adding AUTO_BKOPS_EN bit set for Auto BKOPS support		Message ID:<1472739890-3384-3-git-send-email-alex.lemberg@sandisk.com>
> [3/3] mmc: Checking BKOPS status prior to Suspend 			Message ID:<1472739890-3384-4-git-send-email-alex.lemberg@sandisk.com>
>
> Thanks
> Uri Yanai

Firstly, the authorship isn't correct and of course the SOB path.
Then, it'd better to add a changelog since you do improve Alex's
patches, and it would help us to better understand what was changed. :)

Also you should remove Change-Id when fetching patches from your
internal code review system.

Anyway, let's talk about the patches themself, I have
a question for your patch 3/3 there.


>
> Uri Yanai (3):
>   mmc: replacing hardcoded value for runtime PM suspend
>   mmc: Adding AUTO_BKOPS_EN bit set for Auto BKOPS support
>   mmc: Checking BKOPS status prior to Suspend
>
>  drivers/mmc/core/block.c |  3 ++-
>  drivers/mmc/core/mmc.c   | 45 ++++++++++++++++++++++++++++++++++++---------
>  include/linux/mmc/card.h |  1 +
>  include/linux/mmc/host.h |  2 ++
>  include/linux/mmc/mmc.h  |  3 +++
>  5 files changed, 44 insertions(+), 10 deletions(-)
>


-- 
Best Regards
Shawn Lin

RE: [PATCH 0/3] Auto BKOPS PM suspend

[Uri Yanai]

Hi Shawn

Thanks for the review, As you request I will add changelog and remove the Change-Id.
I am afraid I didn't quite understand your remark about authorship and  the SOB path, appreciate if you can elaborate on this.

Thanks
Uri

-----Original Message-----
From: Shawn Lin [mailto:shawn.lin@rock-chips.com] 
Sent: Wednesday, January 18, 2017 3:54 AM
To: Uri Yanai
Cc: ulf.hansson@linaro.org; shawn.lin@rock-chips.com; linux-mmc@vger.kernel.org; chris@printf.net; Alex Lemberg
Subject: Re: [PATCH 0/3] Auto BKOPS PM suspend

Hi Uri,

On 2017/1/17 20:15, Uri Yanai wrote:
> Hi Ulf
> Resubmitting three patches for approval, originally submitted by Alex Lemberg <alex.lemberg@sandisk.com>
> [1/3] mmc: replacing hardcoded value for runtime PM suspend  		Message ID:<1472739890-3384-2-git-send-email-alex.lemberg@sandisk.com>
> [2/3] mmc: Adding AUTO_BKOPS_EN bit set for Auto BKOPS support		Message ID:<1472739890-3384-3-git-send-email-alex.lemberg@sandisk.com>
> [3/3] mmc: Checking BKOPS status prior to Suspend 			Message ID:<1472739890-3384-4-git-send-email-alex.lemberg@sandisk.com>
>
> Thanks
> Uri Yanai

Firstly, the authorship isn't correct and of course the SOB path.
Then, it'd better to add a changelog since you do improve Alex's patches, and it would help us to better understand what was changed. :)

Also you should remove Change-Id when fetching patches from your internal code review system.

Anyway, let's talk about the patches themself, I have a question for your patch 3/3 there.


>
> Uri Yanai (3):
>   mmc: replacing hardcoded value for runtime PM suspend
>   mmc: Adding AUTO_BKOPS_EN bit set for Auto BKOPS support
>   mmc: Checking BKOPS status prior to Suspend
>
>  drivers/mmc/core/block.c |  3 ++-
>  drivers/mmc/core/mmc.c   | 45 ++++++++++++++++++++++++++++++++++++---------
>  include/linux/mmc/card.h |  1 +
>  include/linux/mmc/host.h |  2 ++
>  include/linux/mmc/mmc.h  |  3 +++
>  5 files changed, 44 insertions(+), 10 deletions(-)
>


--
Best Regards
Shawn Lin

[PATCH 0/3] mmc: Check BKOPS Level On Runtime Suspend

[alex lemberg]

Flash based storage devices might need to perform the internal
Background Operations (BKOPS) in order to insure better performance
for the next coming IO.
Following the discussion in the mmc thread:
www.spinics.net/lists/linux-mmc/msg37779.html
To let the device complete its BKOPS during Runtime Suspend,
we suggest to check the BKOPS level.
In case the device need to complete its BKOPS, reschedule the Runtime
Suspend.

alex lemberg (3):
  mmc: replacing hardcoded value for runtime PM suspend
  mmc: Adding AUTO_BKOPS_EN bit set for Auto BKOPS support
  mmc: Checking BKOPS status prior to Suspend

 drivers/mmc/card/block.c |  3 ++-
 drivers/mmc/core/mmc.c   | 36 +++++++++++++++++++++++++++++-------
 include/linux/mmc/card.h |  1 +
 include/linux/mmc/host.h |  2 ++
 include/linux/mmc/mmc.h  |  2 ++
 5 files changed, 36 insertions(+), 8 deletions(-)

-- 
1.9.1

[PATCH 3/3] mmc: Checking BKOPS status prior to Suspend

[alex lemberg]

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;
+		}
+		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
 
 /*
-- 
1.9.1

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

[Ulf Hansson]

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

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

[Alex Lemberg]

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