Skip to contentSkip to conversation

enable background operations for supported eMMC card

PATCH In review Chuanxiao Dong 5 participants 8 messages Dec 3, 2010 – May 5, 2011 MMC, SD and SDIO

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/4 enable background operations for supported eMMC card

Quoted 1 line

From 984adc755cf2f7966a89e510a50f085e314fe347 Mon Sep 17 00:00:00 2001

From: Chuanxiao Dong <[email protected]>
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

---
 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(-)
drivers/mmc/core/mmc.c +26 -0
index 77f93c3..471ed82 100644
@@ -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) &&
include/linux/mmc/card.h +2 -0
index 8ce0827..9b755cb 100644
@@ -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 {
include/linux/mmc/mmc.h +3 -0
index 956fbd8..14f7813 100644
@@ -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
Signature · 1 line
1.6.6.1
#1 lore raw

<Tested-by: Kyungmin Park <[email protected]>

Note that I'm not yet see any performance improvements :)

Quoted 98 lines · Chuanxiao Dong

From 984adc755cf2f7966a89e510a50f085e314fe347 Mon Sep 17 00:00:00 2001
From: Chuanxiao Dong <[email protected]>
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

---
 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(-)
drivers/mmc/core/mmc.c +0 -0
index 77f93c3..471ed82 100644
@@ -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];

drivers/mmc/core/mmc.c +8 -0
+
+               /* 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])

drivers/mmc/core/mmc.c +18 -0
@@ -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) &&

include/linux/mmc/card.h +0 -0
index 8ce0827..9b755cb 100644
@@ -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 {

include/linux/mmc/mmc.h +3 -1
index 956fbd8..14f7813 100644
@@ -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
--

1.6.6.1

#2 lore raw
Quoted 10 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Kyungmin Park
Sent: Monday, December 06, 2010 9:12 AM
To: Dong, Chuanxiao
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v2 1/4]enable background operations for supported eMMC card

Quoted 3 lines

<Tested-by: Kyungmin Park <[email protected]>

Note that I'm not yet see any performance improvements :)

Thanks Park. Can you tell me the testing steps you used?
During the test, has there any background operation been started?
I think two cases can be tested to check whether there is performance improvement or not.
One is doing background operations when card is busy for reading/writing. The other is doing background operations when card is idle.

Thanks
Chuanxiao

#3 lore raw
In reply to Chuanxiao Dong
Quoted 4 lines · Chuanxiao Dong

From 984adc755cf2f7966a89e510a50f085e314fe347 Mon Sep 17 00:00:00 2001
From: Chuanxiao Dong <[email protected]>
Date: Mon, 22 Nov 2010 16:31:12 +0800
Subject: [PATCH 1/4] mmc: Enabled background operations feature if eMMC card supports

These headers don't belong into a submission. If you use git-send-email, they get cut off automatically, otherwise just remove them as you paste the patch into your mail client.

Quoted 2 lines

Background operations is a new feature defined in eMMC4.41 standard. Since this feature is opertional for eMMC card, so driver only enable

s/opertional/optional/

include/linux/mmc/card.h struct mmc_ext_csd { · line 59

Quoted 7 lines
include/linux/mmc/card.h +2 -0
@@ -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 */
 };

Bit fields are not encouraged for kernel internal data structures, just use "bool" variables here.

Arnd

#4 lore raw
Quoted 15 lines

-----Original Message-----
From: Arnd Bergmann [mailto:[email protected]]
Sent: Monday, December 06, 2010 8:29 PM
To: Dong, Chuanxiao
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v2 1/4]enable background operations for supported eMMC card

Quoted 4 lines · Chuanxiao Dong

From 984adc755cf2f7966a89e510a50f085e314fe347 Mon Sep 17 00:00:00 2001
From: Chuanxiao Dong <[email protected]>
Date: Mon, 22 Nov 2010 16:31:12 +0800
Subject: [PATCH 1/4] mmc: Enabled background operations feature if eMMC card

Quoted 6 lines

supports

These headers don't belong into a submission. If you use git-send-email, they get cut off automatically, otherwise just remove them as you paste the patch into your mail client.

OK. I will remove them when the next submission.

Quoted 4 lines
Quoted 2 lines

Background operations is a new feature defined in eMMC4.41 standard. Since this feature is opertional for eMMC card, so driver only enable

s/opertional/optional/

Quoted 10 lines

include/linux/mmc/card.h struct mmc_ext_csd { · line 59

Quoted 7 lines
include/linux/mmc/card.h +2 -0
@@ -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 */
 };

Bit fields are not encouraged for kernel internal data structures, just use "bool" variables here.

Thanks Arnd. I will fix this.

#5 lore raw
Quoted 6 lines
Quoted 5 lines

These headers don't belong into a submission. If you use git-send-email, they get cut off automatically, otherwise just remove them as you paste the patch into your mail client.

OK. I will remove them when the next submission.

'git send-email' takes care of a lot of other things, too (e.g. chaining all following messages to the first one) - strong recommendation :)

Regards,

Wolfram

Signature · 2 lines
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
#6 lore raw
In reply to Chuanxiao Dong
Quoted 36 lines · Chuanxiao Dong

From 984adc755cf2f7966a89e510a50f085e314fe347 Mon Sep 17 00:00:00 2001
From: Chuanxiao Dong <[email protected]>
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

---
 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(-)
drivers/mmc/core/mmc.c +0 -0
index 77f93c3..471ed82 100644
@@ -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];

drivers/mmc/core/mmc.c +8 -0
+
+               /* 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])

drivers/mmc/core/mmc.c static int mmc_init_card(struct mmc_host *host, u32 ocr, · line 500

Quoted 9 lines
drivers/mmc/core/mmc.c +6 -0
@@ -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

#7 lore raw
Quoted 39 lines · Per Forlin
Quoted 38 lines · Chuanxiao Dong

From 984adc755cf2f7966a89e510a50f085e314fe347 Mon Sep 17 00:00:00 2001
From: Chuanxiao Dong <[email protected]>
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

---
 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(-)
drivers/mmc/core/mmc.c +0 -0
index 77f93c3..471ed82 100644
@@ -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];

drivers/mmc/core/mmc.c +8 -0
+
+               /* 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])

drivers/mmc/core/mmc.c +0 -0
@@ -484,6 +492,24 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
        }
 
Quoted 13 lines

drivers/mmc/core/mmc.c static int mmc_init_card(struct mmc_host *host, u32 ocr, · line 500

Quoted 7 lines
drivers/mmc/core/mmc.c +6 -0
        /*
+        * 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.

Quoted 6 lines

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