diff mbox

[v2,1/4] enable background operations for supported eMMC card

Message ID 20101203121338.GB18655@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chuanxiao.Dong Dec. 3, 2010, 12:13 p.m. UTC
None

Comments

Per Forlin May 2, 2011, 7:53 p.m. UTC | #1
On Fri, Dec 3, 2010 at 1:13 PM, Chuanxiao Dong <chuanxiao.dong@intel.com> wrote:
> From 984adc755cf2f7966a89e510a50f085e314fe347 Mon Sep 17 00:00:00 2001
> From: Chuanxiao Dong <chuanxiao.dong@intel.com>
> Date: Mon, 22 Nov 2010 16:31:12 +0800
> Subject: [PATCH 1/4] mmc: Enabled background operations feature if eMMC card supports
>
> Background operations is a new feature defined in eMMC4.41 standard.
> Since this feature is opertional for eMMC card, so driver only enable
> for those eMMC card which supports this feature
>
> Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> ---
>  drivers/mmc/core/mmc.c   |   26 ++++++++++++++++++++++++++
>  include/linux/mmc/card.h |    2 ++
>  include/linux/mmc/mmc.h  |    3 +++
>  3 files changed, 31 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 77f93c3..471ed82 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -310,6 +310,14 @@ static int mmc_read_ext_csd(struct mmc_card *card)
>                        ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT];
>                card->ext_csd.trim_timeout = 300 *
>                        ext_csd[EXT_CSD_TRIM_MULT];
> +
> +               /* detect whether the eMMC card support BKOPS */
> +               if (ext_csd[EXT_CSD_BKOPS_SUPPORT] & 0x1) {
> +                       card->ext_csd.bkops = 1;
> +                       card->ext_csd.bkops_en =
> +                               ext_csd[EXT_CSD_BKOPS_EN];
> +               }
> +
>        }
>
>        if (ext_csd[EXT_CSD_ERASED_MEM_CONT])
> @@ -484,6 +492,24 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>        }
>
>        /*
> +        * enable BKOPS if eMMC card supports.
> +        */
> +       if (card->ext_csd.bkops) {
> +               if (!card->ext_csd.bkops_en) {
> +                       err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> +                                       EXT_CSD_BKOPS_EN, 1);
Is it always wise to set BKOPS_EN to 1 if bkops is supported?
BKOPS_EN=1 will enable periodic background operations. The card will
somehow decide periodically when to start background operations. If
BKOPS_EN=0 the background operations need to be started manually. The
mmc framework could make the decision when to start background
operations.
Using dynamic clock control or suspend/resume may cause the periodic
internal operations to be delayed since it can't run without clocking
or power.
Do you have any test results comparing periodic and manual bkops?
On way forward is to add support for both periodic bkops and manual
bkops and let the device platform_data specify what to use?

Regards,
Per
--
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
Per Forlin May 5, 2011, 7:50 a.m. UTC | #2
On Mon, May 2, 2011 at 9:53 PM, Per Forlin <per.lkml@gmail.com> wrote:
> On Fri, Dec 3, 2010 at 1:13 PM, Chuanxiao Dong <chuanxiao.dong@intel.com> wrote:
>> From 984adc755cf2f7966a89e510a50f085e314fe347 Mon Sep 17 00:00:00 2001
>> From: Chuanxiao Dong <chuanxiao.dong@intel.com>
>> Date: Mon, 22 Nov 2010 16:31:12 +0800
>> Subject: [PATCH 1/4] mmc: Enabled background operations feature if eMMC card supports
>>
>> Background operations is a new feature defined in eMMC4.41 standard.
>> Since this feature is opertional for eMMC card, so driver only enable
>> for those eMMC card which supports this feature
>>
>> Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
>> ---
>>  drivers/mmc/core/mmc.c   |   26 ++++++++++++++++++++++++++
>>  include/linux/mmc/card.h |    2 ++
>>  include/linux/mmc/mmc.h  |    3 +++
>>  3 files changed, 31 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 77f93c3..471ed82 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -310,6 +310,14 @@ static int mmc_read_ext_csd(struct mmc_card *card)
>>                        ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT];
>>                card->ext_csd.trim_timeout = 300 *
>>                        ext_csd[EXT_CSD_TRIM_MULT];
>> +
>> +               /* detect whether the eMMC card support BKOPS */
>> +               if (ext_csd[EXT_CSD_BKOPS_SUPPORT] & 0x1) {
>> +                       card->ext_csd.bkops = 1;
>> +                       card->ext_csd.bkops_en =
>> +                               ext_csd[EXT_CSD_BKOPS_EN];
>> +               }
>> +
>>        }
>>
>>        if (ext_csd[EXT_CSD_ERASED_MEM_CONT])
>> @@ -484,6 +492,24 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>>        }
>>
>>        /*
>> +        * enable BKOPS if eMMC card supports.
>> +        */
>> +       if (card->ext_csd.bkops) {
>> +               if (!card->ext_csd.bkops_en) {
>> +                       err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> +                                       EXT_CSD_BKOPS_EN, 1);
> Is it always wise to set BKOPS_EN to 1 if bkops is supported?
> BKOPS_EN=1 will enable periodic background operations. The card will
> somehow decide periodically when to start background operations. If
> BKOPS_EN=0 the background operations need to be started manually. The
> mmc framework could make the decision when to start background
> operations.
BKOP_EN=0 means act like a pre v4.41 device.  No maintenance required.
Thanks Bruce for pointing this out to me.

> Using dynamic clock control or suspend/resume may cause the periodic
> internal operations to be delayed since it can't run without clocking
> or power.
> Do you have any test results comparing periodic and manual bkops?
> On way forward is to add support for both periodic bkops and manual
> bkops and let the device platform_data specify what to use?
Please discard this comment.

My concern is when to fire BKOPS_START.
Would it make sense to check the BKOPS_STATUS before going to sleep
and start bkops even if level is only "non critical"? This would defer
sleep until bkops are completed.

Regards,
Per
--
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 mbox

Patch

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 77f93c3..471ed82 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -310,6 +310,14 @@  static int mmc_read_ext_csd(struct mmc_card *card)
 			ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT];
 		card->ext_csd.trim_timeout = 300 *
 			ext_csd[EXT_CSD_TRIM_MULT];
+
+		/* detect whether the eMMC card support BKOPS */
+		if (ext_csd[EXT_CSD_BKOPS_SUPPORT] & 0x1) {
+			card->ext_csd.bkops = 1;
+			card->ext_csd.bkops_en =
+				ext_csd[EXT_CSD_BKOPS_EN];
+		}
+
 	}
 
 	if (ext_csd[EXT_CSD_ERASED_MEM_CONT])
@@ -484,6 +492,24 @@  static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	}
 
 	/*
+	 * enable BKOPS if eMMC card supports.
+	 */
+	if (card->ext_csd.bkops) {
+		if (!card->ext_csd.bkops_en) {
+			err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+					EXT_CSD_BKOPS_EN, 1);
+			if (err && err != -EBADMSG)
+				goto free_card;
+
+			if (err) {
+				card->ext_csd.bkops_en = 0;
+				err = 0;
+			} else
+				card->ext_csd.bkops_en = 1;
+		}
+	}
+
+	/*
 	 * Activate high speed (if supported)
 	 */
 	if ((card->ext_csd.hs_max_dtr != 0) &&
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 8ce0827..9b755cb 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -54,6 +54,8 @@  struct mmc_ext_csd {
 	unsigned int		sec_trim_mult;	/* Secure trim multiplier  */
 	unsigned int		sec_erase_mult;	/* Secure erase multiplier */
 	unsigned int		trim_timeout;		/* In milliseconds */
+	unsigned int		bkops:1; /* background support bit */
+	unsigned int		bkops_en:1; /* background enable bit */
 };
 
 struct sd_scr {
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 956fbd8..14f7813 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -251,6 +251,8 @@  struct _mmc_csd {
  * EXT_CSD fields
  */
 
+#define EXT_CSD_BKOPS_EN		163	/* R/W */
+#define EXT_CSD_BKOPS_START		164	/* W */
 #define EXT_CSD_ERASE_GROUP_DEF		175	/* R/W */
 #define EXT_CSD_ERASED_MEM_CONT		181	/* RO */
 #define EXT_CSD_BUS_WIDTH		183	/* R/W */
@@ -266,6 +268,7 @@  struct _mmc_csd {
 #define EXT_CSD_SEC_ERASE_MULT		230	/* RO */
 #define EXT_CSD_SEC_FEATURE_SUPPORT	231	/* RO */
 #define EXT_CSD_TRIM_MULT		232	/* RO */
+#define EXT_CSD_BKOPS_SUPPORT	502	/* RO */
 
 /*
  * EXT_CSD field definitions