[PATCH v3] mmc : general purpose partition support.

[PATCH v3] mmc : general purpose partition support.

[Namjae Jeon]

It allows gerneral purpose partitions in MMC Device.
And I try to simpliy make mmc_blk_alloc_parts using mmc_part structure suggested by Andrei Warkentin.
After patching, we can see general purpose partitions like this.
> cat /proc/partitions
              179 0 847872 mmcblk0
              179 192 4096 mmcblk0gp3
              179 160 4096 mmcblk0gp2
              179 128 4096 mmcblk0gp1
              179 96  1052672 mmcblk0gp0
              179 64  1024 mmcblk0boot1
              179 32  1024 mmcblk0boot0

Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
---
 drivers/mmc/card/block.c |   43 ++++++++++++++++++++++++++------------
 drivers/mmc/core/mmc.c   |   51 ++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/mmc/card.h |   20 +++++++++++++++++-
 include/linux/mmc/mmc.h  |   13 +++++++++--
 4 files changed, 107 insertions(+), 20 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 1ff5486..e48529c 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1377,26 +1377,41 @@ static int mmc_blk_alloc_part(struct mmc_card *card,
 	return 0;
 }
 
+/* MMC Physical partition consist of two boot partitons and
+ * four general purpose partitions.
+ * if the register of respective partitions is set in ext_csd,
+ * it allocate block device to be accessed.
+ */
+
 static int mmc_blk_alloc_parts(struct mmc_card *card, struct mmc_blk_data *md)
 {
-	int ret = 0;
+	int idx, ret = 0;
 
 	if (!mmc_card_mmc(card))
 		return 0;
 
-	if (card->ext_csd.boot_size) {
-		ret = mmc_blk_alloc_part(card, md, EXT_CSD_PART_CONFIG_ACC_BOOT0,
-					 card->ext_csd.boot_size >> 9,
-					 true,
-					 "boot0");
-		if (ret)
-			return ret;
-		ret = mmc_blk_alloc_part(card, md, EXT_CSD_PART_CONFIG_ACC_BOOT1,
-					 card->ext_csd.boot_size >> 9,
-					 true,
-					 "boot1");
-		if (ret)
-			return ret;
+	 for (idx = 0; idx < MMC_NUM_BOOT_PARTITION; idx++) {
+		if (card->boot_part[idx].size) {
+			ret = mmc_blk_alloc_part(card, md,
+				card->boot_part[idx].cookie,
+				card->boot_part[idx].size >> 9,
+				card->boot_part[idx].force_ro,
+				card->boot_part[idx].name);
+			if (ret)
+				return ret;
+		}
+	}
+
+	for (idx = 0; idx < MMC_NUM_GP_PARTITION; idx++) {
+		if (card->gp_part[idx].size) {
+			ret = mmc_blk_alloc_part(card, md,
+				card->gp_part[idx].cookie,
+				card->gp_part[idx].size >> 9,
+				card->gp_part[idx].force_ro,
+				card->gp_part[idx].name);
+			if (ret)
+				return ret;
+		}
 	}
 
 	return ret;
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 5700b1c..b36b906 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -239,7 +239,7 @@ static int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd)
  */
 static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 {
-	int err = 0;
+	int err = 0, idx, part_cfg, gp_size_mult;
 
 	BUG_ON(!card);
 
@@ -340,7 +340,17 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 		 * There are two boot regions of equal size, defined in
 		 * multiples of 128K.
 		 */
-		card->ext_csd.boot_size = ext_csd[EXT_CSD_BOOT_MULT] << 17;
+		if (ext_csd[EXT_CSD_BOOT_MULT])	{
+			part_cfg = EXT_CSD_PART_CONFIG_ACC_BOOT0;
+			for (idx = 0; idx < MMC_NUM_BOOT_PARTITION; idx++) {
+				card->boot_part[idx].size =
+					ext_csd[EXT_CSD_BOOT_MULT] << 17;
+				card->boot_part[idx].cookie = part_cfg++;
+				sprintf(card->boot_part[idx].name, "boot%d",
+					idx);
+				card->boot_part[idx].force_ro = true;
+			}
+		}
 	}
 
 	card->ext_csd.raw_hc_erase_gap_size =
@@ -392,6 +402,43 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 			card->ext_csd.enhanced_area_offset = -EINVAL;
 			card->ext_csd.enhanced_area_size = -EINVAL;
 		}
+
+		/*
+		 * General purpose partition feature support --
+		 * If ext_csd have the size of general purpose partitions,
+		 * set size, part_type, partition name in mmc_part.
+		 */
+
+		if (ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x1) {
+			u8 hc_erase_grp_sz =
+				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
+			u8 hc_wp_grp_sz =
+				ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+
+			card->ext_csd.enhanced_area_en = 1;
+
+			part_cfg = EXT_CSD_PART_CONFIG_ACC_GP0;
+			for (idx = 0, gp_size_mult = 143;
+				idx < MMC_NUM_GP_PARTITION; idx++,
+				gp_size_mult += 3) {
+				if (!ext_csd[gp_size_mult] &&
+					!ext_csd[gp_size_mult + 1] &&
+					!ext_csd[gp_size_mult + 2])
+						continue;
+				card->gp_part[idx].size =
+					(ext_csd[gp_size_mult + 2] << 16) +
+					(ext_csd[gp_size_mult + 1] << 8) +
+					ext_csd[gp_size_mult];
+				card->gp_part[idx].size *=
+					(size_t)(hc_erase_grp_sz *
+					hc_wp_grp_sz);
+				card->gp_part[idx].size <<= 19;
+				card->gp_part[idx].cookie = part_cfg++;
+				sprintf(card->gp_part[idx].name,
+					"gp%d", idx);
+				card->gp_part[idx].force_ro = false;
+			}
+		}
 		card->ext_csd.sec_trim_mult =
 			ext_csd[EXT_CSD_SEC_TRIM_MULT];
 		card->ext_csd.sec_erase_mult =
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index b460fc2..aa67bf8 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -12,6 +12,7 @@
 
 #include <linux/mmc/core.h>
 #include <linux/mod_devicetable.h>
+#include <linux/genhd.h>
 
 struct mmc_cid {
 	unsigned int		manfid;
@@ -63,7 +64,6 @@ struct mmc_ext_csd {
 	bool			enhanced_area_en;	/* enable bit */
 	unsigned long long	enhanced_area_offset;	/* Units: Byte */
 	unsigned int		enhanced_area_size;	/* Units: KB */
-	unsigned int		boot_size;		/* in bytes */
 	u8			raw_partition_support;	/* 160 */
 	u8			raw_erased_mem_count;	/* 181 */
 	u8			raw_ext_csd_structure;	/* 194 */
@@ -157,6 +157,22 @@ struct sdio_func_tuple;
 
 #define SDIO_MAX_FUNCS		7
 
+/* The number of MMC physical partitions
+ * It consist of boot partitions(2), general purpose partitions(4) in MMC v4.4
+ */
+#define MMC_NUM_BOOT_PARTITION	2
+#define MMC_NUM_GP_PARTITION	4
+
+/*
+ * MMC Physical partitions
+ */
+struct mmc_part {
+	unsigned int	size;	/* partition size (in bytes) */
+	unsigned int	cookie;	/* it used to part_type */
+	char	name[DISK_NAME_LEN];
+	bool	force_ro;	/* to make boot parts RO by default */
+};
+
 /*
  * MMC device
  */
@@ -216,6 +232,8 @@ struct mmc_card {
 	unsigned int		sd_bus_speed;	/* Bus Speed Mode set for the card */
 
 	struct dentry		*debugfs_root;
+	struct mmc_part	boot_part[MMC_NUM_BOOT_PARTITION];	/* mmc boot partitions */
+	struct mmc_part	gp_part[MMC_NUM_GP_PARTITION];	/* mmc general purpose partitions */
 };
 
 /*
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 5a794cb..41fe51a 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -300,9 +300,16 @@ struct _mmc_csd {
 
 #define EXT_CSD_WR_REL_PARAM_EN		(1<<2)
 
-#define EXT_CSD_PART_CONFIG_ACC_MASK	(0x7)
-#define EXT_CSD_PART_CONFIG_ACC_BOOT0	(0x1)
-#define EXT_CSD_PART_CONFIG_ACC_BOOT1	(0x2)
+enum {
+	EXT_CSD_PART_CONFIG_ACC_BOOT0 = 0x1,
+	EXT_CSD_PART_CONFIG_ACC_BOOT1,
+	EXT_CSD_PART_CONFIG_ACC_RPMB, /* not used yet. */
+	EXT_CSD_PART_CONFIG_ACC_GP0,
+	EXT_CSD_PART_CONFIG_ACC_GP1,
+	EXT_CSD_PART_CONFIG_ACC_GP2,
+	EXT_CSD_PART_CONFIG_ACC_GP3,
+	EXT_CSD_PART_CONFIG_ACC_MASK = 0x7
+};
 
 #define EXT_CSD_CMD_SET_NORMAL		(1<<0)
 #define EXT_CSD_CMD_SET_SECURE		(1<<1)
-- 
1.7.4.4

Re: [PATCH v3] mmc : general purpose partition support.

[Andrei Warkentin]

Namjae,

----- Original Message -----
> From: "Namjae Jeon" <linkinjeon@gmail.com>
> To: cjb@laptop.org, linux-mmc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org, awarkentin@vmware.com, "adrian hunter" <adrian.hunter@intel.com>, "linus walleij"
> <linus.walleij@linaro.org>, "james p freyensee" <james_p_freyensee@linux.intel.com>, sebras@gmail.com, "Ulf Hansson"
> <Ulf.Hansson@stericsson.com>, "stefan xk nilsson" <stefan.xk.nilsson@stericsson.com>, "per forlin"
> <per.forlin@stericsson.com>, "johan rudholm" <johan.rudholm@stericsson.com>, "Namjae Jeon" <linkinjeon@gmail.com>
> Sent: Thursday, September 29, 2011 11:17:11 AM
> Subject: [PATCH v3] mmc : general purpose partition support.
> 
> It allows gerneral purpose partitions in MMC Device.
> And I try to simpliy make mmc_blk_alloc_parts using mmc_part
> structure suggested by Andrei Warkentin.
> After patching, we can see general purpose partitions like this.
> > cat /proc/partitions
>               179 0 847872 mmcblk0
>               179 192 4096 mmcblk0gp3
>               179 160 4096 mmcblk0gp2
>               179 128 4096 mmcblk0gp1
>               179 96  1052672 mmcblk0gp0
>               179 64  1024 mmcblk0boot1
>               179 32  1024 mmcblk0boot0
> 
> Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>

> +	 for (idx = 0; idx < MMC_NUM_BOOT_PARTITION; idx++) {
> +		if (card->boot_part[idx].size) {
> +			ret = mmc_blk_alloc_part(card, md,
> +				card->boot_part[idx].cookie,
> +				card->boot_part[idx].size >> 9,
> +				card->boot_part[idx].force_ro,
> +				card->boot_part[idx].name);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	for (idx = 0; idx < MMC_NUM_GP_PARTITION; idx++) {
> +		if (card->gp_part[idx].size) {
> +			ret = mmc_blk_alloc_part(card, md,
> +				card->gp_part[idx].cookie,
> +				card->gp_part[idx].size >> 9,
> +				card->gp_part[idx].force_ro,
> +				card->gp_part[idx].name);
> +			if (ret)
> +				return ret;

You were on a better track before. Why does the block drivre need to know about the partition types?
You should parse one array.

> -		card->ext_csd.boot_size = ext_csd[EXT_CSD_BOOT_MULT] << 17;
> +		if (ext_csd[EXT_CSD_BOOT_MULT])	{
> +			part_cfg = EXT_CSD_PART_CONFIG_ACC_BOOT0;
> +			for (idx = 0; idx < MMC_NUM_BOOT_PARTITION; idx++) {
> +				card->boot_part[idx].size =
> +					ext_csd[EXT_CSD_BOOT_MULT] << 17;
> +				card->boot_part[idx].cookie = part_cfg++;
> +				sprintf(card->boot_part[idx].name, "boot%d",
> +					idx);
> +				card->boot_part[idx].force_ro = true;
> +			}
> +		}
>  	}
>  

My earlier comment still stands - make your code look something like -

if (ext_csd[EXT_CSD_BOOT_MULT])	{
	part_cfg = EXT_CSD_PART_CONFIG_ACC_BOOT0;
	for (idx = 0; idx < MMC_NUM_BOOT_PARTITION; idx++) {
		mmc_part_add(card, size, part_cfg++, "boot%d", part_cfg++,
				"boot%d", idx, true)
}

> +	struct mmc_part	boot_part[MMC_NUM_BOOT_PARTITION];	/* mmc boot
> partitions */
> +	struct mmc_part	gp_part[MMC_NUM_GP_PARTITION];	/* mmc general
> purpose partitions */

struct mmc_part parts[MMC_NUM_BOOT_PARTITION + MMC_NUM_GP_PARTITION];
unsigned nr_parts;

A

Re: [PATCH v3] mmc : general purpose partition support.

[NamJae Jeon]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 4618 bytes --]

2011/9/30 Andrei Warkentin <awarkentin@vmware.com>:
> Namjae,
>
> ----- Original Message -----
>> From: "Namjae Jeon" <linkinjeon@gmail.com>
>> To: cjb@laptop.org, linux-mmc@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org, awarkentin@vmware.com, "adrian hunter" <adrian.hunter@intel.com>, "linus walleij"
>> <linus.walleij@linaro.org>, "james p freyensee" <james_p_freyensee@linux.intel.com>, sebras@gmail.com, "Ulf Hansson"
>> <Ulf.Hansson@stericsson.com>, "stefan xk nilsson" <stefan.xk.nilsson@stericsson.com>, "per forlin"
>> <per.forlin@stericsson.com>, "johan rudholm" <johan.rudholm@stericsson.com>, "Namjae Jeon" <linkinjeon@gmail.com>
>> Sent: Thursday, September 29, 2011 11:17:11 AM
>> Subject: [PATCH v3] mmc : general purpose partition support.
>>
>> It allows gerneral purpose partitions in MMC Device.
>> And I try to simpliy make mmc_blk_alloc_parts using mmc_part
>> structure suggested by Andrei Warkentin.
>> After patching, we can see general purpose partitions like this.
>> > cat /proc/partitions
>>               179 0 847872 mmcblk0
>>               179 192 4096 mmcblk0gp3
>>               179 160 4096 mmcblk0gp2
>>               179 128 4096 mmcblk0gp1
>>               179 96  1052672 mmcblk0gp0
>>               179 64  1024 mmcblk0boot1
>>               179 32  1024 mmcblk0boot0
>>
>> Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
>
>> +      for (idx = 0; idx < MMC_NUM_BOOT_PARTITION; idx++) {
>> +             if (card->boot_part[idx].size) {
>> +                     ret = mmc_blk_alloc_part(card, md,
>> +                             card->boot_part[idx].cookie,
>> +                             card->boot_part[idx].size >> 9,
>> +                             card->boot_part[idx].force_ro,
>> +                             card->boot_part[idx].name);
>> +                     if (ret)
>> +                             return ret;
>> +             }
>> +     }
>> +
>> +     for (idx = 0; idx < MMC_NUM_GP_PARTITION; idx++) {
>> +             if (card->gp_part[idx].size) {
>> +                     ret = mmc_blk_alloc_part(card, md,
>> +                             card->gp_part[idx].cookie,
>> +                             card->gp_part[idx].size >> 9,
>> +                             card->gp_part[idx].force_ro,
>> +                             card->gp_part[idx].name);
>> +                     if (ret)
>> +                             return ret;
>
> You were on a better track before. Why does the block drivre need to know about the partition types?
> You should parse one array.
>
>> -             card->ext_csd.boot_size = ext_csd[EXT_CSD_BOOT_MULT] << 17;
>> +             if (ext_csd[EXT_CSD_BOOT_MULT]) {
>> +                     part_cfg = EXT_CSD_PART_CONFIG_ACC_BOOT0;
>> +                     for (idx = 0; idx < MMC_NUM_BOOT_PARTITION; idx++) {
>> +                             card->boot_part[idx].size =
>> +                                     ext_csd[EXT_CSD_BOOT_MULT] << 17;
>> +                             card->boot_part[idx].cookie = part_cfg++;
>> +                             sprintf(card->boot_part[idx].name, "boot%d",
>> +                                     idx);
>> +                             card->boot_part[idx].force_ro = true;
>> +                     }
>> +             }
>>       }
>>
>
> My earlier comment still stands - make your code look something like -
>
> if (ext_csd[EXT_CSD_BOOT_MULT]) {
>        part_cfg = EXT_CSD_PART_CONFIG_ACC_BOOT0;
>        for (idx = 0; idx < MMC_NUM_BOOT_PARTITION; idx++) {
>                mmc_part_add(card, size, part_cfg++, "boot%d", part_cfg++,
>                                "boot%d", idx, true)
> }
>
>> +     struct mmc_part boot_part[MMC_NUM_BOOT_PARTITION];      /* mmc boot
>> partitions */
>> +     struct mmc_part gp_part[MMC_NUM_GP_PARTITION];  /* mmc general
>> purpose partitions */
>
> struct mmc_part parts[MMC_NUM_BOOT_PARTITION + MMC_NUM_GP_PARTITION];
> unsigned nr_parts;
>

Your review will always inspire me.
I will post again.
plz review after..
Thanks.
> A
>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH v3] mmc : general purpose partition support.

[Adrian Hunter]

Perhaps rebase against Chris' mmc-next branch

Re: [PATCH v3] mmc : general purpose partition support.

[NamJae Jeon]

2011/9/30 Adrian Hunter <adrian.hunter@intel.com>:
> Perhaps rebase against Chris' mmc-next branch
I made this patch after rebasing Chris'mmc-next yesterday.
>
>
>

Re: [PATCH v3] mmc : general purpose partition support.

[NamJae Jeon]

2011/9/30 NamJae Jeon <linkinjeon@gmail.com>:
> 2011/9/30 Adrian Hunter <adrian.hunter@intel.com>:
>> Perhaps rebase against Chris' mmc-next branch
> I made this patch after rebasing Chris'mmc-next yesterday.
Hi. Adrian.
There is the overlapped code between your patch and my patch.
What should we do ?
>>
>>
>>
>

Re: [PATCH v3] mmc : general purpose partition support.

[Adrian Hunter]

On 30/09/11 11:11, NamJae Jeon wrote:
> 2011/9/30 NamJae Jeon<linkinjeon@gmail.com>:
>> 2011/9/30 Adrian Hunter<adrian.hunter@intel.com>:
>>> Perhaps rebase against Chris' mmc-next branch
>> I made this patch after rebasing Chris'mmc-next yesterday.
> Hi. Adrian.
> There is the overlapped code between your patch and my patch.
> What should we do ?

Rebase and ensure that boot partitions are only added if
mmc_boot_partition_access(card->host) is true

>>>
>>>
>>>
>>
>