* [PATCH] mmc : general purpose partition support.
@ 2011-09-22 15:34 Namjae Jeon
2011-09-22 22:08 ` J Freyensee
2011-09-23 3:25 ` Andrei Warkentin
0 siblings, 2 replies; 15+ messages in thread
From: Namjae Jeon @ 2011-09-22 15:34 UTC (permalink / raw)
To: cjb, linux-mmc; +Cc: linux-kernel, Namjae Jeon
It allows general purpose parition in MMC Device. If it is enable, it will make mmcblk0gp1,gp2,gp3,gp4 partition like this.
>cat /proc/paritition
179 0 847872 mmcblk0
179 192 4096 mmcblk0gp4
179 160 4096 mmcblk0gp3
179 128 4096 mmcblk0gp2
179 96 1052672 mmcblk0gp1
179 64 1024 mmcblk0boot1
179 32 1024 mmcblk0boot0
Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
---
drivers/mmc/card/block.c | 36 ++++++++++++++++++++++++++++++++++++
drivers/mmc/core/mmc.c | 42 ++++++++++++++++++++++++++++++++++++++++++
include/linux/mmc/card.h | 4 ++++
include/linux/mmc/mmc.h | 4 ++++
4 files changed, 86 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 9b90726..074ec55 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1402,6 +1402,42 @@ static int mmc_blk_alloc_parts(struct mmc_card *card, struct mmc_blk_data *md)
return ret;
}
+ if (card->ext_csd.gp1_size) {
+ ret = mmc_blk_alloc_part(card, md, EXT_CSD_PART_CONFIG_ACC_GP1,
+ card->ext_csd.gp1_size >> 9,
+ false,
+ "gp1");
+ if (ret)
+ return ret;
+ }
+
+ if (card->ext_csd.gp2_size) {
+ ret = mmc_blk_alloc_part(card, md, EXT_CSD_PART_CONFIG_ACC_GP2,
+ card->ext_csd.gp2_size >> 9,
+ false,
+ "gp2");
+ if (ret)
+ return ret;
+ }
+
+ if (card->ext_csd.gp3_size) {
+ ret = mmc_blk_alloc_part(card, md, EXT_CSD_PART_CONFIG_ACC_GP3,
+ card->ext_csd.gp3_size >> 9,
+ false,
+ "gp3");
+ if (ret)
+ return ret;
+ }
+
+ if (card->ext_csd.gp4_size) {
+ ret = mmc_blk_alloc_part(card, md, EXT_CSD_PART_CONFIG_ACC_GP4,
+ card->ext_csd.gp4_size >> 9,
+ false,
+ "gp4");
+ if (ret)
+ return ret;
+ }
+
return ret;
}
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 10f5a19..cc31511 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -393,6 +393,48 @@ 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 Support
+ */
+
+ 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;
+
+ card->ext_csd.gp1_size =
+ (ext_csd[145] << 16) + (ext_csd[144] << 8) +
+ ext_csd[143];
+ card->ext_csd.gp1_size *=
+ (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
+ card->ext_csd.gp1_size <<= 19;
+
+ card->ext_csd.gp2_size =
+ (ext_csd[148] << 16) + (ext_csd[147] << 8) +
+ ext_csd[146];
+ card->ext_csd.gp2_size *=
+ (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
+ card->ext_csd.gp2_size <<= 19;
+
+ card->ext_csd.gp3_size =
+ (ext_csd[151] << 16) + (ext_csd[150] << 8) +
+ ext_csd[149];
+ card->ext_csd.gp3_size *=
+ (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
+ card->ext_csd.gp3_size <<= 19;
+
+ card->ext_csd.gp4_size =
+ (ext_csd[154] << 16) + (ext_csd[153] << 8) +
+ ext_csd[152];
+ card->ext_csd.gp4_size *=
+ (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
+ card->ext_csd.gp4_size <<= 19;
+ }
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..96a98a7 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -64,6 +64,10 @@ struct mmc_ext_csd {
unsigned long long enhanced_area_offset; /* Units: Byte */
unsigned int enhanced_area_size; /* Units: KB */
unsigned int boot_size; /* in bytes */
+ unsigned int gp1_size; /* in bytes */
+ unsigned int gp2_size; /* in bytes */
+ unsigned int gp3_size; /* in bytes */
+ unsigned int gp4_size; /* in bytes */
u8 raw_partition_support; /* 160 */
u8 raw_erased_mem_count; /* 181 */
u8 raw_ext_csd_structure; /* 194 */
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 5a794cb..380720a 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -303,6 +303,10 @@ struct _mmc_csd {
#define EXT_CSD_PART_CONFIG_ACC_MASK (0x7)
#define EXT_CSD_PART_CONFIG_ACC_BOOT0 (0x1)
#define EXT_CSD_PART_CONFIG_ACC_BOOT1 (0x2)
+#define EXT_CSD_PART_CONFIG_ACC_GP1 (0x4)
+#define EXT_CSD_PART_CONFIG_ACC_GP2 (0x5)
+#define EXT_CSD_PART_CONFIG_ACC_GP3 (0x6)
+#define EXT_CSD_PART_CONFIG_ACC_GP4 (0x7)
#define EXT_CSD_CMD_SET_NORMAL (1<<0)
#define EXT_CSD_CMD_SET_SECURE (1<<1)
--
1.7.4.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH] mmc : general purpose partition support.
2011-09-22 15:34 [PATCH] mmc : general purpose partition support Namjae Jeon
@ 2011-09-22 22:08 ` J Freyensee
2011-09-22 23:15 ` NamJae Jeon
2011-09-23 3:25 ` Andrei Warkentin
1 sibling, 1 reply; 15+ messages in thread
From: J Freyensee @ 2011-09-22 22:08 UTC (permalink / raw)
To: Namjae Jeon; +Cc: cjb, linux-mmc, linux-kernel
On 09/22/2011 08:34 AM, Namjae Jeon wrote:
> It allows general purpose parition in MMC Device. If it is enable, it will make mmcblk0gp1,gp2,gp3,gp4 partition like this.
>
>> cat /proc/paritition
> 179 0 847872 mmcblk0
> 179 192 4096 mmcblk0gp4
> 179 160 4096 mmcblk0gp3
> 179 128 4096 mmcblk0gp2
> 179 96 1052672 mmcblk0gp1
> 179 64 1024 mmcblk0boot1
> 179 32 1024 mmcblk0boot0
>
> Signed-off-by: Namjae Jeon<linkinjeon@gmail.com>
> ---
> drivers/mmc/card/block.c | 36 ++++++++++++++++++++++++++++++++++++
> drivers/mmc/core/mmc.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/mmc/card.h | 4 ++++
> include/linux/mmc/mmc.h | 4 ++++
> 4 files changed, 86 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 9b90726..074ec55 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -1402,6 +1402,42 @@ static int mmc_blk_alloc_parts(struct mmc_card *card, struct mmc_blk_data *md)
> return ret;
> }
>
> + if (card->ext_csd.gp1_size) {
> + ret = mmc_blk_alloc_part(card, md, EXT_CSD_PART_CONFIG_ACC_GP1,
> + card->ext_csd.gp1_size>> 9,
> + false,
> + "gp1");
> + if (ret)
> + return ret;
> + }
> +
if mmc_blk_alloc_part() fails for 'if (card->ext_csd.pt1_size)', why
would you want to give this code the opportunity to call
mmc_blk_alloc_part() again with the next if() below? If
mmc_blk_alloc_part() fails, is it a big problem, like kzalloc() failing?
If so, I would think you would want to immediately quit this function
and report an error (either use errno value or pass the value of ret).
> + if (card->ext_csd.gp2_size) {
> + ret = mmc_blk_alloc_part(card, md, EXT_CSD_PART_CONFIG_ACC_GP2,
> + card->ext_csd.gp2_size>> 9,
> + false,
> + "gp2");
> + if (ret)
> + return ret;
> + }
> +
> + if (card->ext_csd.gp3_size) {
> + ret = mmc_blk_alloc_part(card, md, EXT_CSD_PART_CONFIG_ACC_GP3,
> + card->ext_csd.gp3_size>> 9,
> + false,
> + "gp3");
> + if (ret)
> + return ret;
> + }
> +
> + if (card->ext_csd.gp4_size) {
> + ret = mmc_blk_alloc_part(card, md, EXT_CSD_PART_CONFIG_ACC_GP4,
> + card->ext_csd.gp4_size>> 9,
> + false,
> + "gp4");
> + if (ret)
> + return ret;
> + }
> +
> return ret;
> }
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 10f5a19..cc31511 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -393,6 +393,48 @@ 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 Support
> + */
> +
> + 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;
> +
> + card->ext_csd.gp1_size =
> + (ext_csd[145]<< 16) + (ext_csd[144]<< 8) +
> + ext_csd[143];
> + card->ext_csd.gp1_size *=
> + (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
> + card->ext_csd.gp1_size<<= 19;
> +
> + card->ext_csd.gp2_size =
> + (ext_csd[148]<< 16) + (ext_csd[147]<< 8) +
> + ext_csd[146];
> + card->ext_csd.gp2_size *=
> + (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
> + card->ext_csd.gp2_size<<= 19;
> +
> + card->ext_csd.gp3_size =
> + (ext_csd[151]<< 16) + (ext_csd[150]<< 8) +
> + ext_csd[149];
> + card->ext_csd.gp3_size *=
> + (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
> + card->ext_csd.gp3_size<<= 19;
> +
> + card->ext_csd.gp4_size =
> + (ext_csd[154]<< 16) + (ext_csd[153]<< 8) +
> + ext_csd[152];
> + card->ext_csd.gp4_size *=
> + (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
> + card->ext_csd.gp4_size<<= 19;
> + }
> 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..96a98a7 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -64,6 +64,10 @@ struct mmc_ext_csd {
> unsigned long long enhanced_area_offset; /* Units: Byte */
> unsigned int enhanced_area_size; /* Units: KB */
> unsigned int boot_size; /* in bytes */
> + unsigned int gp1_size; /* in bytes */
> + unsigned int gp2_size; /* in bytes */
> + unsigned int gp3_size; /* in bytes */
> + unsigned int gp4_size; /* in bytes */
> u8 raw_partition_support; /* 160 */
> u8 raw_erased_mem_count; /* 181 */
> u8 raw_ext_csd_structure; /* 194 */
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> index 5a794cb..380720a 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -303,6 +303,10 @@ struct _mmc_csd {
> #define EXT_CSD_PART_CONFIG_ACC_MASK (0x7)
> #define EXT_CSD_PART_CONFIG_ACC_BOOT0 (0x1)
> #define EXT_CSD_PART_CONFIG_ACC_BOOT1 (0x2)
> +#define EXT_CSD_PART_CONFIG_ACC_GP1 (0x4)
> +#define EXT_CSD_PART_CONFIG_ACC_GP2 (0x5)
> +#define EXT_CSD_PART_CONFIG_ACC_GP3 (0x6)
> +#define EXT_CSD_PART_CONFIG_ACC_GP4 (0x7)
>
> #define EXT_CSD_CMD_SET_NORMAL (1<<0)
> #define EXT_CSD_CMD_SET_SECURE (1<<1)
--
J (James/Jay) Freyensee
Storage Technology Group
Intel Corporation
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] mmc : general purpose partition support.
2011-09-22 22:08 ` J Freyensee
@ 2011-09-22 23:15 ` NamJae Jeon
0 siblings, 0 replies; 15+ messages in thread
From: NamJae Jeon @ 2011-09-22 23:15 UTC (permalink / raw)
To: J Freyensee; +Cc: cjb, linux-mmc, linux-kernel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 8857 bytes --]
2011/9/23 J Freyensee <james_p_freyensee@linux.intel.com>:
> On 09/22/2011 08:34 AM, Namjae Jeon wrote:
>>
>> It allows general purpose parition in MMC Device. If it is enable, it will
>> make mmcblk0gp1,gp2,gp3,gp4 partition like this.
>>
>>> cat /proc/paritition
>>
>> Â Â Â 179 0 847872 mmcblk0
>> Â Â Â 179 192 4096 mmcblk0gp4
>> Â Â Â 179 160 4096 mmcblk0gp3
>> Â Â Â 179 128 4096 mmcblk0gp2
>> Â Â Â 179 96 Â 1052672 mmcblk0gp1
>> Â Â Â 179 64 Â 1024 mmcblk0boot1
>> Â Â Â 179 32 Â 1024 mmcblk0boot0
>>
>> Signed-off-by: Namjae Jeon<linkinjeon@gmail.com>
>> ---
>> Â drivers/mmc/card/block.c | Â 36 ++++++++++++++++++++++++++++++++++++
>>  drivers/mmc/core/mmc.c  |  42
>> ++++++++++++++++++++++++++++++++++++++++++
>> Â include/linux/mmc/card.h | Â Â 4 ++++
>>  include/linux/mmc/mmc.h  |   4 ++++
>> Â 4 files changed, 86 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>> index 9b90726..074ec55 100644
>> --- a/drivers/mmc/card/block.c
>> +++ b/drivers/mmc/card/block.c
>> @@ -1402,6 +1402,42 @@ static int mmc_blk_alloc_parts(struct mmc_card
>> *card, struct mmc_blk_data *md)
>> Â Â Â Â Â Â Â Â Â Â Â Â return ret;
>> Â Â Â Â }
>>
>> + Â Â Â if (card->ext_csd.gp1_size) {
>> + Â Â Â Â Â Â Â ret = mmc_blk_alloc_part(card, md,
>> EXT_CSD_PART_CONFIG_ACC_GP1,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp1_size>> Â 9,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â false,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "gp1");
>> + Â Â Â Â Â Â Â if (ret)
>> + Â Â Â Â Â Â Â Â Â Â Â return ret;
>> + Â Â Â }
>> +
>
> if mmc_blk_alloc_part() fails for 'if (card->ext_csd.pt1_size)', why would
> you want to give this code the opportunity to call mmc_blk_alloc_part()
> again with the next if() below? Â If mmc_blk_alloc_part() fails, is it a big
> problem, like kzalloc() failing? Â If so, I would think you would want to
> immediately quit this function and report an error (either use errno value
> or pass the value of ret).
-> if card->ext_csd.gp1_size is no zero, it means general purpose
partition is enable.
and it can set up to 4 partition in specification. so we should check
it by next if().
>
>> + Â Â Â if (card->ext_csd.gp2_size) {
>> + Â Â Â Â Â Â Â ret = mmc_blk_alloc_part(card, md,
>> EXT_CSD_PART_CONFIG_ACC_GP2,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp2_size>> Â 9,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â false,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "gp2");
>> + Â Â Â Â Â Â Â if (ret)
>> + Â Â Â Â Â Â Â Â Â Â Â return ret;
>> + Â Â Â }
>> +
>> + Â Â Â if (card->ext_csd.gp3_size) {
>> + Â Â Â Â Â Â Â ret = mmc_blk_alloc_part(card, md,
>> EXT_CSD_PART_CONFIG_ACC_GP3,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp3_size>> Â 9,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â false,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "gp3");
>> + Â Â Â Â Â Â Â if (ret)
>> + Â Â Â Â Â Â Â Â Â Â Â return ret;
>> + Â Â Â }
>> +
>> + Â Â Â if (card->ext_csd.gp4_size) {
>> + Â Â Â Â Â Â Â ret = mmc_blk_alloc_part(card, md,
>> EXT_CSD_PART_CONFIG_ACC_GP4,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp4_size>> Â 9,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â false,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "gp4");
>> + Â Â Â Â Â Â Â if (ret)
>> + Â Â Â Â Â Â Â Â Â Â Â return ret;
>> + Â Â Â }
>> +
>> Â Â Â Â return ret;
>> Â }
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 10f5a19..cc31511 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -393,6 +393,48 @@ 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 Support
>> + Â Â Â Â Â Â Â Â */
>> +
>> + Â Â Â Â Â Â Â 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;
>> +
>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp1_size =
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (ext_csd[145]<< Â 16) + (ext_csd[144]<< Â 8)
>> +
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ext_csd[143];
>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp1_size *=
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp1_size<<= 19;
>> +
>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp2_size =
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (ext_csd[148]<< Â 16) + (ext_csd[147]<< Â 8)
>> +
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ext_csd[146];
>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp2_size *=
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp2_size<<= 19;
>> +
>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp3_size =
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (ext_csd[151]<< Â 16) + (ext_csd[150]<< Â 8)
>> +
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ext_csd[149];
>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp3_size *=
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp3_size<<= 19;
>> +
>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp4_size =
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (ext_csd[154]<< Â 16) + (ext_csd[153]<< Â 8)
>> +
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ext_csd[152];
>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp4_size *=
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp4_size<<= 19;
>> + Â Â Â Â Â Â Â }
>> Â Â Â Â Â Â Â Â 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..96a98a7 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -64,6 +64,10 @@ struct mmc_ext_csd {
>>     unsigned long long    enhanced_area_offset;  /* Units: Byte */
>>     unsigned int       enhanced_area_size;   /* Units: KB */
>>     unsigned int       boot_size;        /* in bytes */
>> +    unsigned int       gp1_size;        /* in bytes */
>> +    unsigned int       gp2_size;        /* in bytes */
>> +    unsigned int       gp3_size;        /* in bytes */
>> +    unsigned int       gp4_size;        /* in bytes */
>> Â Â Â Â u8 Â Â Â Â Â Â Â Â Â Â Â raw_partition_support; Â /* 160 */
>> Â Â Â Â u8 Â Â Â Â Â Â Â Â Â Â Â raw_erased_mem_count; Â /* 181 */
>> Â Â Â Â u8 Â Â Â Â Â Â Â Â Â Â Â raw_ext_csd_structure; Â /* 194 */
>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>> index 5a794cb..380720a 100644
>> --- a/include/linux/mmc/mmc.h
>> +++ b/include/linux/mmc/mmc.h
>> @@ -303,6 +303,10 @@ struct _mmc_csd {
>> Â #define EXT_CSD_PART_CONFIG_ACC_MASK Â (0x7)
>> Â #define EXT_CSD_PART_CONFIG_ACC_BOOT0 (0x1)
>> Â #define EXT_CSD_PART_CONFIG_ACC_BOOT1 (0x2)
>> +#define EXT_CSD_PART_CONFIG_ACC_GP1 Â Â (0x4)
>> +#define EXT_CSD_PART_CONFIG_ACC_GP2 Â Â (0x5)
>> +#define EXT_CSD_PART_CONFIG_ACC_GP3 Â Â (0x6)
>> +#define EXT_CSD_PART_CONFIG_ACC_GP4 Â Â (0x7)
>>
>> Â #define EXT_CSD_CMD_SET_NORMAL Â Â Â Â Â Â Â Â (1<<0)
>> Â #define EXT_CSD_CMD_SET_SECURE Â Â Â Â Â Â Â Â (1<<1)
>
>
> --
> J (James/Jay) Freyensee
> Storage Technology Group
> Intel Corporation
>
ÿôèº{.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¥
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] mmc : general purpose partition support.
@ 2011-09-22 23:15 ` NamJae Jeon
0 siblings, 0 replies; 15+ messages in thread
From: NamJae Jeon @ 2011-09-22 23:15 UTC (permalink / raw)
To: J Freyensee; +Cc: cjb, linux-mmc, linux-kernel
2011/9/23 J Freyensee <james_p_freyensee@linux.intel.com>:
> On 09/22/2011 08:34 AM, Namjae Jeon wrote:
>>
>> It allows general purpose parition in MMC Device. If it is enable, it will
>> make mmcblk0gp1,gp2,gp3,gp4 partition like this.
>>
>>> cat /proc/paritition
>>
>> 179 0 847872 mmcblk0
>> 179 192 4096 mmcblk0gp4
>> 179 160 4096 mmcblk0gp3
>> 179 128 4096 mmcblk0gp2
>> 179 96 1052672 mmcblk0gp1
>> 179 64 1024 mmcblk0boot1
>> 179 32 1024 mmcblk0boot0
>>
>> Signed-off-by: Namjae Jeon<linkinjeon@gmail.com>
>> ---
>> drivers/mmc/card/block.c | 36 ++++++++++++++++++++++++++++++++++++
>> drivers/mmc/core/mmc.c | 42
>> ++++++++++++++++++++++++++++++++++++++++++
>> include/linux/mmc/card.h | 4 ++++
>> include/linux/mmc/mmc.h | 4 ++++
>> 4 files changed, 86 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>> index 9b90726..074ec55 100644
>> --- a/drivers/mmc/card/block.c
>> +++ b/drivers/mmc/card/block.c
>> @@ -1402,6 +1402,42 @@ static int mmc_blk_alloc_parts(struct mmc_card
>> *card, struct mmc_blk_data *md)
>> return ret;
>> }
>>
>> + if (card->ext_csd.gp1_size) {
>> + ret = mmc_blk_alloc_part(card, md,
>> EXT_CSD_PART_CONFIG_ACC_GP1,
>> + card->ext_csd.gp1_size>> 9,
>> + false,
>> + "gp1");
>> + if (ret)
>> + return ret;
>> + }
>> +
>
> if mmc_blk_alloc_part() fails for 'if (card->ext_csd.pt1_size)', why would
> you want to give this code the opportunity to call mmc_blk_alloc_part()
> again with the next if() below? If mmc_blk_alloc_part() fails, is it a big
> problem, like kzalloc() failing? If so, I would think you would want to
> immediately quit this function and report an error (either use errno value
> or pass the value of ret).
-> if card->ext_csd.gp1_size is no zero, it means general purpose
partition is enable.
and it can set up to 4 partition in specification. so we should check
it by next if().
>
>> + if (card->ext_csd.gp2_size) {
>> + ret = mmc_blk_alloc_part(card, md,
>> EXT_CSD_PART_CONFIG_ACC_GP2,
>> + card->ext_csd.gp2_size>> 9,
>> + false,
>> + "gp2");
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + if (card->ext_csd.gp3_size) {
>> + ret = mmc_blk_alloc_part(card, md,
>> EXT_CSD_PART_CONFIG_ACC_GP3,
>> + card->ext_csd.gp3_size>> 9,
>> + false,
>> + "gp3");
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + if (card->ext_csd.gp4_size) {
>> + ret = mmc_blk_alloc_part(card, md,
>> EXT_CSD_PART_CONFIG_ACC_GP4,
>> + card->ext_csd.gp4_size>> 9,
>> + false,
>> + "gp4");
>> + if (ret)
>> + return ret;
>> + }
>> +
>> return ret;
>> }
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 10f5a19..cc31511 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -393,6 +393,48 @@ 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 Support
>> + */
>> +
>> + 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;
>> +
>> + card->ext_csd.gp1_size =
>> + (ext_csd[145]<< 16) + (ext_csd[144]<< 8)
>> +
>> + ext_csd[143];
>> + card->ext_csd.gp1_size *=
>> + (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
>> + card->ext_csd.gp1_size<<= 19;
>> +
>> + card->ext_csd.gp2_size =
>> + (ext_csd[148]<< 16) + (ext_csd[147]<< 8)
>> +
>> + ext_csd[146];
>> + card->ext_csd.gp2_size *=
>> + (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
>> + card->ext_csd.gp2_size<<= 19;
>> +
>> + card->ext_csd.gp3_size =
>> + (ext_csd[151]<< 16) + (ext_csd[150]<< 8)
>> +
>> + ext_csd[149];
>> + card->ext_csd.gp3_size *=
>> + (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
>> + card->ext_csd.gp3_size<<= 19;
>> +
>> + card->ext_csd.gp4_size =
>> + (ext_csd[154]<< 16) + (ext_csd[153]<< 8)
>> +
>> + ext_csd[152];
>> + card->ext_csd.gp4_size *=
>> + (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
>> + card->ext_csd.gp4_size<<= 19;
>> + }
>> 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..96a98a7 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -64,6 +64,10 @@ struct mmc_ext_csd {
>> unsigned long long enhanced_area_offset; /* Units: Byte */
>> unsigned int enhanced_area_size; /* Units: KB */
>> unsigned int boot_size; /* in bytes */
>> + unsigned int gp1_size; /* in bytes */
>> + unsigned int gp2_size; /* in bytes */
>> + unsigned int gp3_size; /* in bytes */
>> + unsigned int gp4_size; /* in bytes */
>> u8 raw_partition_support; /* 160 */
>> u8 raw_erased_mem_count; /* 181 */
>> u8 raw_ext_csd_structure; /* 194 */
>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>> index 5a794cb..380720a 100644
>> --- a/include/linux/mmc/mmc.h
>> +++ b/include/linux/mmc/mmc.h
>> @@ -303,6 +303,10 @@ struct _mmc_csd {
>> #define EXT_CSD_PART_CONFIG_ACC_MASK (0x7)
>> #define EXT_CSD_PART_CONFIG_ACC_BOOT0 (0x1)
>> #define EXT_CSD_PART_CONFIG_ACC_BOOT1 (0x2)
>> +#define EXT_CSD_PART_CONFIG_ACC_GP1 (0x4)
>> +#define EXT_CSD_PART_CONFIG_ACC_GP2 (0x5)
>> +#define EXT_CSD_PART_CONFIG_ACC_GP3 (0x6)
>> +#define EXT_CSD_PART_CONFIG_ACC_GP4 (0x7)
>>
>> #define EXT_CSD_CMD_SET_NORMAL (1<<0)
>> #define EXT_CSD_CMD_SET_SECURE (1<<1)
>
>
> --
> J (James/Jay) Freyensee
> Storage Technology Group
> Intel Corporation
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] mmc : general purpose partition support.
2011-09-22 23:15 ` NamJae Jeon
(?)
@ 2011-09-22 23:25 ` J Freyensee
2011-09-22 23:58 ` NamJae Jeon
-1 siblings, 1 reply; 15+ messages in thread
From: J Freyensee @ 2011-09-22 23:25 UTC (permalink / raw)
To: NamJae Jeon; +Cc: cjb, linux-mmc, linux-kernel
On 09/22/2011 04:15 PM, NamJae Jeon wrote:
> 2011/9/23 J Freyensee<james_p_freyensee@linux.intel.com>:
>> On 09/22/2011 08:34 AM, Namjae Jeon wrote:
>>>
>>> It allows general purpose parition in MMC Device. If it is enable, it will
>>> make mmcblk0gp1,gp2,gp3,gp4 partition like this.
>>>
>>>> cat /proc/paritition
>>>
>>> 179 0 847872 mmcblk0
>>> 179 192 4096 mmcblk0gp4
>>> 179 160 4096 mmcblk0gp3
>>> 179 128 4096 mmcblk0gp2
>>> 179 96 1052672 mmcblk0gp1
>>> 179 64 1024 mmcblk0boot1
>>> 179 32 1024 mmcblk0boot0
>>>
>>> Signed-off-by: Namjae Jeon<linkinjeon@gmail.com>
>>> ---
>>> drivers/mmc/card/block.c | 36 ++++++++++++++++++++++++++++++++++++
>>> drivers/mmc/core/mmc.c | 42
>>> ++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/mmc/card.h | 4 ++++
>>> include/linux/mmc/mmc.h | 4 ++++
>>> 4 files changed, 86 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>> index 9b90726..074ec55 100644
>>> --- a/drivers/mmc/card/block.c
>>> +++ b/drivers/mmc/card/block.c
>>> @@ -1402,6 +1402,42 @@ static int mmc_blk_alloc_parts(struct mmc_card
>>> *card, struct mmc_blk_data *md)
>>> return ret;
>>> }
>>>
>>> + if (card->ext_csd.gp1_size) {
>>> + ret = mmc_blk_alloc_part(card, md,
>>> EXT_CSD_PART_CONFIG_ACC_GP1,
>>> + card->ext_csd.gp1_size>> 9,
>>> + false,
>>> + "gp1");
>>> + if (ret)
>>> + return ret;
>>> + }
>>> +
>>
>> if mmc_blk_alloc_part() fails for 'if (card->ext_csd.pt1_size)', why would
>> you want to give this code the opportunity to call mmc_blk_alloc_part()
>> again with the next if() below? If mmc_blk_alloc_part() fails, is it a big
>> problem, like kzalloc() failing? If so, I would think you would want to
>> immediately quit this function and report an error (either use errno value
>> or pass the value of ret).
> -> if card->ext_csd.gp1_size is no zero, it means general purpose
> partition is enable.
> and it can set up to 4 partition in specification. so we should check
> it by next if().
Please document that in your code. Please add a function header to this
function.
>
>>
>>> + if (card->ext_csd.gp2_size) {
>>> + ret = mmc_blk_alloc_part(card, md,
>>> EXT_CSD_PART_CONFIG_ACC_GP2,
>>> + card->ext_csd.gp2_size>> 9,
>>> + false,
>>> + "gp2");
>>> + if (ret)
>>> + return ret;
>>> + }
>>> +
>>> + if (card->ext_csd.gp3_size) {
>>> + ret = mmc_blk_alloc_part(card, md,
>>> EXT_CSD_PART_CONFIG_ACC_GP3,
>>> + card->ext_csd.gp3_size>> 9,
>>> + false,
>>> + "gp3");
>>> + if (ret)
>>> + return ret;
>>> + }
>>> +
>>> + if (card->ext_csd.gp4_size) {
>>> + ret = mmc_blk_alloc_part(card, md,
>>> EXT_CSD_PART_CONFIG_ACC_GP4,
>>> + card->ext_csd.gp4_size>> 9,
>>> + false,
>>> + "gp4");
>>> + if (ret)
>>> + return ret;
>>> + }
>>> +
>>> return ret;
>>> }
>>>
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index 10f5a19..cc31511 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -393,6 +393,48 @@ 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 Support
>>> + */
>>> +
>>> + 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;
>>> +
>>> + card->ext_csd.gp1_size =
>>> + (ext_csd[145]<< 16) + (ext_csd[144]<< 8)
>>> +
>>> + ext_csd[143];
>>> + card->ext_csd.gp1_size *=
>>> + (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
>>> + card->ext_csd.gp1_size<<= 19;
>>> +
>>> + card->ext_csd.gp2_size =
>>> + (ext_csd[148]<< 16) + (ext_csd[147]<< 8)
>>> +
>>> + ext_csd[146];
>>> + card->ext_csd.gp2_size *=
>>> + (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
>>> + card->ext_csd.gp2_size<<= 19;
>>> +
>>> + card->ext_csd.gp3_size =
>>> + (ext_csd[151]<< 16) + (ext_csd[150]<< 8)
>>> +
>>> + ext_csd[149];
>>> + card->ext_csd.gp3_size *=
>>> + (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
>>> + card->ext_csd.gp3_size<<= 19;
>>> +
>>> + card->ext_csd.gp4_size =
>>> + (ext_csd[154]<< 16) + (ext_csd[153]<< 8)
>>> +
>>> + ext_csd[152];
>>> + card->ext_csd.gp4_size *=
>>> + (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
>>> + card->ext_csd.gp4_size<<= 19;
>>> + }
>>> 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..96a98a7 100644
>>> --- a/include/linux/mmc/card.h
>>> +++ b/include/linux/mmc/card.h
>>> @@ -64,6 +64,10 @@ struct mmc_ext_csd {
>>> unsigned long long enhanced_area_offset; /* Units: Byte */
>>> unsigned int enhanced_area_size; /* Units: KB */
>>> unsigned int boot_size; /* in bytes */
>>> + unsigned int gp1_size; /* in bytes */
>>> + unsigned int gp2_size; /* in bytes */
>>> + unsigned int gp3_size; /* in bytes */
>>> + unsigned int gp4_size; /* in bytes */
>>> u8 raw_partition_support; /* 160 */
>>> u8 raw_erased_mem_count; /* 181 */
>>> u8 raw_ext_csd_structure; /* 194 */
>>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>>> index 5a794cb..380720a 100644
>>> --- a/include/linux/mmc/mmc.h
>>> +++ b/include/linux/mmc/mmc.h
>>> @@ -303,6 +303,10 @@ struct _mmc_csd {
>>> #define EXT_CSD_PART_CONFIG_ACC_MASK (0x7)
>>> #define EXT_CSD_PART_CONFIG_ACC_BOOT0 (0x1)
>>> #define EXT_CSD_PART_CONFIG_ACC_BOOT1 (0x2)
>>> +#define EXT_CSD_PART_CONFIG_ACC_GP1 (0x4)
>>> +#define EXT_CSD_PART_CONFIG_ACC_GP2 (0x5)
>>> +#define EXT_CSD_PART_CONFIG_ACC_GP3 (0x6)
>>> +#define EXT_CSD_PART_CONFIG_ACC_GP4 (0x7)
>>>
>>> #define EXT_CSD_CMD_SET_NORMAL (1<<0)
>>> #define EXT_CSD_CMD_SET_SECURE (1<<1)
>>
>>
>> --
>> J (James/Jay) Freyensee
>> Storage Technology Group
>> Intel Corporation
>>
--
J (James/Jay) Freyensee
Storage Technology Group
Intel Corporation
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] mmc : general purpose partition support.
2011-09-22 23:25 ` J Freyensee
@ 2011-09-22 23:58 ` NamJae Jeon
0 siblings, 0 replies; 15+ messages in thread
From: NamJae Jeon @ 2011-09-22 23:58 UTC (permalink / raw)
To: J Freyensee; +Cc: cjb, linux-mmc, linux-kernel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 9722 bytes --]
2011/9/23 J Freyensee <james_p_freyensee@linux.intel.com>:
> On 09/22/2011 04:15 PM, NamJae Jeon wrote:
>>
>> 2011/9/23 J Freyensee<james_p_freyensee@linux.intel.com>:
>>>
>>> On 09/22/2011 08:34 AM, Namjae Jeon wrote:
>>>>
>>>> It allows general purpose parition in MMC Device. If it is enable, it
>>>> will
>>>> make mmcblk0gp1,gp2,gp3,gp4 partition like this.
>>>>
>>>>> cat /proc/paritition
>>>>
>>>> Â Â Â 179 0 847872 mmcblk0
>>>> Â Â Â 179 192 4096 mmcblk0gp4
>>>> Â Â Â 179 160 4096 mmcblk0gp3
>>>> Â Â Â 179 128 4096 mmcblk0gp2
>>>> Â Â Â 179 96 Â 1052672 mmcblk0gp1
>>>> Â Â Â 179 64 Â 1024 mmcblk0boot1
>>>> Â Â Â 179 32 Â 1024 mmcblk0boot0
>>>>
>>>> Signed-off-by: Namjae Jeon<linkinjeon@gmail.com>
>>>> ---
>>>> Â drivers/mmc/card/block.c | Â 36 ++++++++++++++++++++++++++++++++++++
>>>>  drivers/mmc/core/mmc.c  |  42
>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>> Â include/linux/mmc/card.h | Â Â 4 ++++
>>>>  include/linux/mmc/mmc.h  |   4 ++++
>>>> Â 4 files changed, 86 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>> index 9b90726..074ec55 100644
>>>> --- a/drivers/mmc/card/block.c
>>>> +++ b/drivers/mmc/card/block.c
>>>> @@ -1402,6 +1402,42 @@ static int mmc_blk_alloc_parts(struct mmc_card
>>>> *card, struct mmc_blk_data *md)
>>>> Â Â Â Â Â Â Â Â Â Â Â Â return ret;
>>>> Â Â Â Â }
>>>>
>>>> + Â Â Â if (card->ext_csd.gp1_size) {
>>>> + Â Â Â Â Â Â Â ret = mmc_blk_alloc_part(card, md,
>>>> EXT_CSD_PART_CONFIG_ACC_GP1,
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp1_size>> Â Â 9,
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â false,
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "gp1");
>>>> + Â Â Â Â Â Â Â if (ret)
>>>> + Â Â Â Â Â Â Â Â Â Â Â return ret;
>>>> + Â Â Â }
>>>> +
>>>
>>> if mmc_blk_alloc_part() fails for 'if (card->ext_csd.pt1_size)', why
>>> would
>>> you want to give this code the opportunity to call mmc_blk_alloc_part()
>>> again with the next if() below? Â If mmc_blk_alloc_part() fails, is it a
>>> big
>>> problem, like kzalloc() failing? Â If so, I would think you would want to
>>> immediately quit this function and report an error (either use errno
>>> value
>>> or pass the value of ret).
>>
>> -> Â if card->ext_csd.gp1_size is no zero, it means general purpose
>> partition is enable.
>> and it can set up to 4 partition in specification. so we should check
>> it by next if().
>
> Please document that in your code. Â Please add a function header to this
> function.
I will add comments at this code. I don't understand why a function
header should be added yet.
>
>>
>>>
>>>> + Â Â Â if (card->ext_csd.gp2_size) {
>>>> + Â Â Â Â Â Â Â ret = mmc_blk_alloc_part(card, md,
>>>> EXT_CSD_PART_CONFIG_ACC_GP2,
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp2_size>> Â Â 9,
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â false,
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "gp2");
>>>> + Â Â Â Â Â Â Â if (ret)
>>>> + Â Â Â Â Â Â Â Â Â Â Â return ret;
>>>> + Â Â Â }
>>>> +
>>>> + Â Â Â if (card->ext_csd.gp3_size) {
>>>> + Â Â Â Â Â Â Â ret = mmc_blk_alloc_part(card, md,
>>>> EXT_CSD_PART_CONFIG_ACC_GP3,
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp3_size>> Â Â 9,
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â false,
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "gp3");
>>>> + Â Â Â Â Â Â Â if (ret)
>>>> + Â Â Â Â Â Â Â Â Â Â Â return ret;
>>>> + Â Â Â }
>>>> +
>>>> + Â Â Â if (card->ext_csd.gp4_size) {
>>>> + Â Â Â Â Â Â Â ret = mmc_blk_alloc_part(card, md,
>>>> EXT_CSD_PART_CONFIG_ACC_GP4,
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp4_size>> Â Â 9,
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â false,
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "gp4");
>>>> + Â Â Â Â Â Â Â if (ret)
>>>> + Â Â Â Â Â Â Â Â Â Â Â return ret;
>>>> + Â Â Â }
>>>> +
>>>> Â Â Â Â return ret;
>>>> Â }
>>>>
>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>> index 10f5a19..cc31511 100644
>>>> --- a/drivers/mmc/core/mmc.c
>>>> +++ b/drivers/mmc/core/mmc.c
>>>> @@ -393,6 +393,48 @@ 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 Support
>>>> + Â Â Â Â Â Â Â Â */
>>>> +
>>>> + Â Â Â Â Â Â Â 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;
>>>> +
>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp1_size =
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (ext_csd[145]<< Â Â 16) + (ext_csd[144]<<
>>>> Â Â 8)
>>>> +
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ext_csd[143];
>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp1_size *=
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (size_t)(hc_erase_grp_sz *
>>>> hc_wp_grp_sz);
>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp1_size<<= 19;
>>>> +
>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp2_size =
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (ext_csd[148]<< Â Â 16) + (ext_csd[147]<<
>>>> Â Â 8)
>>>> +
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ext_csd[146];
>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp2_size *=
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (size_t)(hc_erase_grp_sz *
>>>> hc_wp_grp_sz);
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp2_size<<= 19;
>>>> +
>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp3_size =
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (ext_csd[151]<< Â Â 16) + (ext_csd[150]<<
>>>> Â Â 8)
>>>> +
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ext_csd[149];
>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp3_size *=
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (size_t)(hc_erase_grp_sz *
>>>> hc_wp_grp_sz);
>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp3_size<<= 19;
>>>> +
>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp4_size =
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (ext_csd[154]<< Â Â 16) + (ext_csd[153]<<
>>>> Â Â 8)
>>>> +
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ext_csd[152];
>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp4_size *=
>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (size_t)(hc_erase_grp_sz *
>>>> hc_wp_grp_sz);
>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp4_size<<= 19;
>>>> + Â Â Â Â Â Â Â }
>>>> Â Â Â Â Â Â Â Â 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..96a98a7 100644
>>>> --- a/include/linux/mmc/card.h
>>>> +++ b/include/linux/mmc/card.h
>>>> @@ -64,6 +64,10 @@ struct mmc_ext_csd {
>>>>     unsigned long long    enhanced_area_offset;  /* Units: Byte */
>>>>     unsigned int       enhanced_area_size;   /* Units: KB */
>>>>     unsigned int       boot_size;        /* in bytes */
>>>> +    unsigned int       gp1_size;        /* in bytes */
>>>> +    unsigned int       gp2_size;        /* in bytes */
>>>> +    unsigned int       gp3_size;        /* in bytes */
>>>> +    unsigned int       gp4_size;        /* in bytes */
>>>> Â Â Â Â u8 Â Â Â Â Â Â Â Â Â Â Â raw_partition_support; Â /* 160 */
>>>> Â Â Â Â u8 Â Â Â Â Â Â Â Â Â Â Â raw_erased_mem_count; Â /* 181 */
>>>> Â Â Â Â u8 Â Â Â Â Â Â Â Â Â Â Â raw_ext_csd_structure; Â /* 194 */
>>>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>>>> index 5a794cb..380720a 100644
>>>> --- a/include/linux/mmc/mmc.h
>>>> +++ b/include/linux/mmc/mmc.h
>>>> @@ -303,6 +303,10 @@ struct _mmc_csd {
>>>> Â #define EXT_CSD_PART_CONFIG_ACC_MASK Â (0x7)
>>>> Â #define EXT_CSD_PART_CONFIG_ACC_BOOT0 (0x1)
>>>> Â #define EXT_CSD_PART_CONFIG_ACC_BOOT1 (0x2)
>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP1 Â Â (0x4)
>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP2 Â Â (0x5)
>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP3 Â Â (0x6)
>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP4 Â Â (0x7)
>>>>
>>>> Â #define EXT_CSD_CMD_SET_NORMAL Â Â Â Â Â Â Â Â (1<<0)
>>>> Â #define EXT_CSD_CMD_SET_SECURE Â Â Â Â Â Â Â Â (1<<1)
>>>
>>>
>>> --
>>> J (James/Jay) Freyensee
>>> Storage Technology Group
>>> Intel Corporation
>>>
>
>
> --
> J (James/Jay) Freyensee
> Storage Technology Group
> Intel Corporation
>
ÿôèº{.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¥
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] mmc : general purpose partition support.
@ 2011-09-22 23:58 ` NamJae Jeon
0 siblings, 0 replies; 15+ messages in thread
From: NamJae Jeon @ 2011-09-22 23:58 UTC (permalink / raw)
To: J Freyensee; +Cc: cjb, linux-mmc, linux-kernel
2011/9/23 J Freyensee <james_p_freyensee@linux.intel.com>:
> On 09/22/2011 04:15 PM, NamJae Jeon wrote:
>>
>> 2011/9/23 J Freyensee<james_p_freyensee@linux.intel.com>:
>>>
>>> On 09/22/2011 08:34 AM, Namjae Jeon wrote:
>>>>
>>>> It allows general purpose parition in MMC Device. If it is enable, it
>>>> will
>>>> make mmcblk0gp1,gp2,gp3,gp4 partition like this.
>>>>
>>>>> cat /proc/paritition
>>>>
>>>> 179 0 847872 mmcblk0
>>>> 179 192 4096 mmcblk0gp4
>>>> 179 160 4096 mmcblk0gp3
>>>> 179 128 4096 mmcblk0gp2
>>>> 179 96 1052672 mmcblk0gp1
>>>> 179 64 1024 mmcblk0boot1
>>>> 179 32 1024 mmcblk0boot0
>>>>
>>>> Signed-off-by: Namjae Jeon<linkinjeon@gmail.com>
>>>> ---
>>>> drivers/mmc/card/block.c | 36 ++++++++++++++++++++++++++++++++++++
>>>> drivers/mmc/core/mmc.c | 42
>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>> include/linux/mmc/card.h | 4 ++++
>>>> include/linux/mmc/mmc.h | 4 ++++
>>>> 4 files changed, 86 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>> index 9b90726..074ec55 100644
>>>> --- a/drivers/mmc/card/block.c
>>>> +++ b/drivers/mmc/card/block.c
>>>> @@ -1402,6 +1402,42 @@ static int mmc_blk_alloc_parts(struct mmc_card
>>>> *card, struct mmc_blk_data *md)
>>>> return ret;
>>>> }
>>>>
>>>> + if (card->ext_csd.gp1_size) {
>>>> + ret = mmc_blk_alloc_part(card, md,
>>>> EXT_CSD_PART_CONFIG_ACC_GP1,
>>>> + card->ext_csd.gp1_size>> 9,
>>>> + false,
>>>> + "gp1");
>>>> + if (ret)
>>>> + return ret;
>>>> + }
>>>> +
>>>
>>> if mmc_blk_alloc_part() fails for 'if (card->ext_csd.pt1_size)', why
>>> would
>>> you want to give this code the opportunity to call mmc_blk_alloc_part()
>>> again with the next if() below? If mmc_blk_alloc_part() fails, is it a
>>> big
>>> problem, like kzalloc() failing? If so, I would think you would want to
>>> immediately quit this function and report an error (either use errno
>>> value
>>> or pass the value of ret).
>>
>> -> if card->ext_csd.gp1_size is no zero, it means general purpose
>> partition is enable.
>> and it can set up to 4 partition in specification. so we should check
>> it by next if().
>
> Please document that in your code. Please add a function header to this
> function.
I will add comments at this code. I don't understand why a function
header should be added yet.
>
>>
>>>
>>>> + if (card->ext_csd.gp2_size) {
>>>> + ret = mmc_blk_alloc_part(card, md,
>>>> EXT_CSD_PART_CONFIG_ACC_GP2,
>>>> + card->ext_csd.gp2_size>> 9,
>>>> + false,
>>>> + "gp2");
>>>> + if (ret)
>>>> + return ret;
>>>> + }
>>>> +
>>>> + if (card->ext_csd.gp3_size) {
>>>> + ret = mmc_blk_alloc_part(card, md,
>>>> EXT_CSD_PART_CONFIG_ACC_GP3,
>>>> + card->ext_csd.gp3_size>> 9,
>>>> + false,
>>>> + "gp3");
>>>> + if (ret)
>>>> + return ret;
>>>> + }
>>>> +
>>>> + if (card->ext_csd.gp4_size) {
>>>> + ret = mmc_blk_alloc_part(card, md,
>>>> EXT_CSD_PART_CONFIG_ACC_GP4,
>>>> + card->ext_csd.gp4_size>> 9,
>>>> + false,
>>>> + "gp4");
>>>> + if (ret)
>>>> + return ret;
>>>> + }
>>>> +
>>>> return ret;
>>>> }
>>>>
>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>> index 10f5a19..cc31511 100644
>>>> --- a/drivers/mmc/core/mmc.c
>>>> +++ b/drivers/mmc/core/mmc.c
>>>> @@ -393,6 +393,48 @@ 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 Support
>>>> + */
>>>> +
>>>> + 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;
>>>> +
>>>> + card->ext_csd.gp1_size =
>>>> + (ext_csd[145]<< 16) + (ext_csd[144]<<
>>>> 8)
>>>> +
>>>> + ext_csd[143];
>>>> + card->ext_csd.gp1_size *=
>>>> + (size_t)(hc_erase_grp_sz *
>>>> hc_wp_grp_sz);
>>>> + card->ext_csd.gp1_size<<= 19;
>>>> +
>>>> + card->ext_csd.gp2_size =
>>>> + (ext_csd[148]<< 16) + (ext_csd[147]<<
>>>> 8)
>>>> +
>>>> + ext_csd[146];
>>>> + card->ext_csd.gp2_size *=
>>>> + (size_t)(hc_erase_grp_sz *
>>>> hc_wp_grp_sz);
>>>> + card->ext_csd.gp2_size<<= 19;
>>>> +
>>>> + card->ext_csd.gp3_size =
>>>> + (ext_csd[151]<< 16) + (ext_csd[150]<<
>>>> 8)
>>>> +
>>>> + ext_csd[149];
>>>> + card->ext_csd.gp3_size *=
>>>> + (size_t)(hc_erase_grp_sz *
>>>> hc_wp_grp_sz);
>>>> + card->ext_csd.gp3_size<<= 19;
>>>> +
>>>> + card->ext_csd.gp4_size =
>>>> + (ext_csd[154]<< 16) + (ext_csd[153]<<
>>>> 8)
>>>> +
>>>> + ext_csd[152];
>>>> + card->ext_csd.gp4_size *=
>>>> + (size_t)(hc_erase_grp_sz *
>>>> hc_wp_grp_sz);
>>>> + card->ext_csd.gp4_size<<= 19;
>>>> + }
>>>> 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..96a98a7 100644
>>>> --- a/include/linux/mmc/card.h
>>>> +++ b/include/linux/mmc/card.h
>>>> @@ -64,6 +64,10 @@ struct mmc_ext_csd {
>>>> unsigned long long enhanced_area_offset; /* Units: Byte */
>>>> unsigned int enhanced_area_size; /* Units: KB */
>>>> unsigned int boot_size; /* in bytes */
>>>> + unsigned int gp1_size; /* in bytes */
>>>> + unsigned int gp2_size; /* in bytes */
>>>> + unsigned int gp3_size; /* in bytes */
>>>> + unsigned int gp4_size; /* in bytes */
>>>> u8 raw_partition_support; /* 160 */
>>>> u8 raw_erased_mem_count; /* 181 */
>>>> u8 raw_ext_csd_structure; /* 194 */
>>>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>>>> index 5a794cb..380720a 100644
>>>> --- a/include/linux/mmc/mmc.h
>>>> +++ b/include/linux/mmc/mmc.h
>>>> @@ -303,6 +303,10 @@ struct _mmc_csd {
>>>> #define EXT_CSD_PART_CONFIG_ACC_MASK (0x7)
>>>> #define EXT_CSD_PART_CONFIG_ACC_BOOT0 (0x1)
>>>> #define EXT_CSD_PART_CONFIG_ACC_BOOT1 (0x2)
>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP1 (0x4)
>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP2 (0x5)
>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP3 (0x6)
>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP4 (0x7)
>>>>
>>>> #define EXT_CSD_CMD_SET_NORMAL (1<<0)
>>>> #define EXT_CSD_CMD_SET_SECURE (1<<1)
>>>
>>>
>>> --
>>> J (James/Jay) Freyensee
>>> Storage Technology Group
>>> Intel Corporation
>>>
>
>
> --
> J (James/Jay) Freyensee
> Storage Technology Group
> Intel Corporation
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] mmc : general purpose partition support.
2011-09-22 23:58 ` NamJae Jeon
(?)
@ 2011-09-23 0:19 ` J Freyensee
2011-09-23 0:29 ` NamJae Jeon
-1 siblings, 1 reply; 15+ messages in thread
From: J Freyensee @ 2011-09-23 0:19 UTC (permalink / raw)
To: NamJae Jeon; +Cc: cjb, linux-mmc, linux-kernel
On 09/22/2011 04:58 PM, NamJae Jeon wrote:
> 2011/9/23 J Freyensee<james_p_freyensee@linux.intel.com>:
>> On 09/22/2011 04:15 PM, NamJae Jeon wrote:
>>>
>>> 2011/9/23 J Freyensee<james_p_freyensee@linux.intel.com>:
>>>>
>>>> On 09/22/2011 08:34 AM, Namjae Jeon wrote:
>>>>>
>>>>> It allows general purpose parition in MMC Device. If it is enable, it
>>>>> will
>>>>> make mmcblk0gp1,gp2,gp3,gp4 partition like this.
>>>>>
>>>>>> cat /proc/paritition
>>>>>
>>>>> 179 0 847872 mmcblk0
>>>>> 179 192 4096 mmcblk0gp4
>>>>> 179 160 4096 mmcblk0gp3
>>>>> 179 128 4096 mmcblk0gp2
>>>>> 179 96 1052672 mmcblk0gp1
>>>>> 179 64 1024 mmcblk0boot1
>>>>> 179 32 1024 mmcblk0boot0
>>>>>
>>>>> Signed-off-by: Namjae Jeon<linkinjeon@gmail.com>
>>>>> ---
>>>>> drivers/mmc/card/block.c | 36 ++++++++++++++++++++++++++++++++++++
>>>>> drivers/mmc/core/mmc.c | 42
>>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>> include/linux/mmc/card.h | 4 ++++
>>>>> include/linux/mmc/mmc.h | 4 ++++
>>>>> 4 files changed, 86 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>>> index 9b90726..074ec55 100644
>>>>> --- a/drivers/mmc/card/block.c
>>>>> +++ b/drivers/mmc/card/block.c
>>>>> @@ -1402,6 +1402,42 @@ static int mmc_blk_alloc_parts(struct mmc_card
>>>>> *card, struct mmc_blk_data *md)
>>>>> return ret;
>>>>> }
>>>>>
>>>>> + if (card->ext_csd.gp1_size) {
>>>>> + ret = mmc_blk_alloc_part(card, md,
>>>>> EXT_CSD_PART_CONFIG_ACC_GP1,
>>>>> + card->ext_csd.gp1_size>> 9,
>>>>> + false,
>>>>> + "gp1");
>>>>> + if (ret)
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>
>>>> if mmc_blk_alloc_part() fails for 'if (card->ext_csd.pt1_size)', why
>>>> would
>>>> you want to give this code the opportunity to call mmc_blk_alloc_part()
>>>> again with the next if() below? If mmc_blk_alloc_part() fails, is it a
>>>> big
>>>> problem, like kzalloc() failing? If so, I would think you would want to
>>>> immediately quit this function and report an error (either use errno
>>>> value
>>>> or pass the value of ret).
>>>
>>> -> if card->ext_csd.gp1_size is no zero, it means general purpose
>>> partition is enable.
>>> and it can set up to 4 partition in specification. so we should check
>>> it by next if().
>>
>> Please document that in your code. Please add a function header to this
>> function.
> I will add comments at this code. I don't understand why a function
> header should be added yet.
One or the other.
but a function comment header should be added at some point. Yes, the
mmc subsystem is pretty legacy, but any new work that gets added to the
kernel is usually has higher standards for stuff like not just adding
function header comments but putting them in a certain format...at least
this is what I went through when I had things accepted upstream.
>
>>
>>>
>>>>
>>>>> + if (card->ext_csd.gp2_size) {
>>>>> + ret = mmc_blk_alloc_part(card, md,
>>>>> EXT_CSD_PART_CONFIG_ACC_GP2,
>>>>> + card->ext_csd.gp2_size>> 9,
>>>>> + false,
>>>>> + "gp2");
>>>>> + if (ret)
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> + if (card->ext_csd.gp3_size) {
>>>>> + ret = mmc_blk_alloc_part(card, md,
>>>>> EXT_CSD_PART_CONFIG_ACC_GP3,
>>>>> + card->ext_csd.gp3_size>> 9,
>>>>> + false,
>>>>> + "gp3");
>>>>> + if (ret)
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> + if (card->ext_csd.gp4_size) {
>>>>> + ret = mmc_blk_alloc_part(card, md,
>>>>> EXT_CSD_PART_CONFIG_ACC_GP4,
>>>>> + card->ext_csd.gp4_size>> 9,
>>>>> + false,
>>>>> + "gp4");
>>>>> + if (ret)
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> return ret;
>>>>> }
>>>>>
>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>> index 10f5a19..cc31511 100644
>>>>> --- a/drivers/mmc/core/mmc.c
>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>> @@ -393,6 +393,48 @@ 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 Support
>>>>> + */
>>>>> +
>>>>> + 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;
>>>>> +
>>>>> + card->ext_csd.gp1_size =
>>>>> + (ext_csd[145]<< 16) + (ext_csd[144]<<
>>>>> 8)
>>>>> +
>>>>> + ext_csd[143];
>>>>> + card->ext_csd.gp1_size *=
>>>>> + (size_t)(hc_erase_grp_sz *
>>>>> hc_wp_grp_sz);
>>>>> + card->ext_csd.gp1_size<<= 19;
>>>>> +
>>>>> + card->ext_csd.gp2_size =
>>>>> + (ext_csd[148]<< 16) + (ext_csd[147]<<
>>>>> 8)
>>>>> +
>>>>> + ext_csd[146];
>>>>> + card->ext_csd.gp2_size *=
>>>>> + (size_t)(hc_erase_grp_sz *
>>>>> hc_wp_grp_sz);
>>>>> + card->ext_csd.gp2_size<<= 19;
>>>>> +
>>>>> + card->ext_csd.gp3_size =
>>>>> + (ext_csd[151]<< 16) + (ext_csd[150]<<
>>>>> 8)
>>>>> +
>>>>> + ext_csd[149];
>>>>> + card->ext_csd.gp3_size *=
>>>>> + (size_t)(hc_erase_grp_sz *
>>>>> hc_wp_grp_sz);
>>>>> + card->ext_csd.gp3_size<<= 19;
>>>>> +
>>>>> + card->ext_csd.gp4_size =
>>>>> + (ext_csd[154]<< 16) + (ext_csd[153]<<
>>>>> 8)
>>>>> +
>>>>> + ext_csd[152];
>>>>> + card->ext_csd.gp4_size *=
>>>>> + (size_t)(hc_erase_grp_sz *
>>>>> hc_wp_grp_sz);
>>>>> + card->ext_csd.gp4_size<<= 19;
>>>>> + }
>>>>> 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..96a98a7 100644
>>>>> --- a/include/linux/mmc/card.h
>>>>> +++ b/include/linux/mmc/card.h
>>>>> @@ -64,6 +64,10 @@ struct mmc_ext_csd {
>>>>> unsigned long long enhanced_area_offset; /* Units: Byte */
>>>>> unsigned int enhanced_area_size; /* Units: KB */
>>>>> unsigned int boot_size; /* in bytes */
>>>>> + unsigned int gp1_size; /* in bytes */
>>>>> + unsigned int gp2_size; /* in bytes */
>>>>> + unsigned int gp3_size; /* in bytes */
>>>>> + unsigned int gp4_size; /* in bytes */
>>>>> u8 raw_partition_support; /* 160 */
>>>>> u8 raw_erased_mem_count; /* 181 */
>>>>> u8 raw_ext_csd_structure; /* 194 */
>>>>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>>>>> index 5a794cb..380720a 100644
>>>>> --- a/include/linux/mmc/mmc.h
>>>>> +++ b/include/linux/mmc/mmc.h
>>>>> @@ -303,6 +303,10 @@ struct _mmc_csd {
>>>>> #define EXT_CSD_PART_CONFIG_ACC_MASK (0x7)
>>>>> #define EXT_CSD_PART_CONFIG_ACC_BOOT0 (0x1)
>>>>> #define EXT_CSD_PART_CONFIG_ACC_BOOT1 (0x2)
>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP1 (0x4)
>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP2 (0x5)
>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP3 (0x6)
>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP4 (0x7)
>>>>>
>>>>> #define EXT_CSD_CMD_SET_NORMAL (1<<0)
>>>>> #define EXT_CSD_CMD_SET_SECURE (1<<1)
>>>>
>>>>
>>>> --
>>>> J (James/Jay) Freyensee
>>>> Storage Technology Group
>>>> Intel Corporation
>>>>
>>
>>
>> --
>> J (James/Jay) Freyensee
>> Storage Technology Group
>> Intel Corporation
>>
--
J (James/Jay) Freyensee
Storage Technology Group
Intel Corporation
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] mmc : general purpose partition support.
2011-09-23 0:19 ` J Freyensee
@ 2011-09-23 0:29 ` NamJae Jeon
0 siblings, 0 replies; 15+ messages in thread
From: NamJae Jeon @ 2011-09-23 0:29 UTC (permalink / raw)
To: J Freyensee; +Cc: cjb, linux-mmc, linux-kernel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 10866 bytes --]
2011/9/23 J Freyensee <james_p_freyensee@linux.intel.com>
>
> On 09/22/2011 04:58 PM, NamJae Jeon wrote:
>>
>> 2011/9/23 J Freyensee<james_p_freyensee@linux.intel.com>:
>>>
>>> On 09/22/2011 04:15 PM, NamJae Jeon wrote:
>>>>
>>>> 2011/9/23 J Freyensee<james_p_freyensee@linux.intel.com>:
>>>>>
>>>>> On 09/22/2011 08:34 AM, Namjae Jeon wrote:
>>>>>>
>>>>>> It allows general purpose parition in MMC Device. If it is enable, it
>>>>>> will
>>>>>> make mmcblk0gp1,gp2,gp3,gp4 partition like this.
>>>>>>
>>>>>>> cat /proc/paritition
>>>>>>
>>>>>> Â Â Â 179 0 847872 mmcblk0
>>>>>> Â Â Â 179 192 4096 mmcblk0gp4
>>>>>> Â Â Â 179 160 4096 mmcblk0gp3
>>>>>> Â Â Â 179 128 4096 mmcblk0gp2
>>>>>> Â Â Â 179 96 Â 1052672 mmcblk0gp1
>>>>>> Â Â Â 179 64 Â 1024 mmcblk0boot1
>>>>>> Â Â Â 179 32 Â 1024 mmcblk0boot0
>>>>>>
>>>>>> Signed-off-by: Namjae Jeon<linkinjeon@gmail.com>
>>>>>> ---
>>>>>> Â drivers/mmc/card/block.c | Â 36 ++++++++++++++++++++++++++++++++++++
>>>>>>  drivers/mmc/core/mmc.c  |  42
>>>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>>> Â include/linux/mmc/card.h | Â Â 4 ++++
>>>>>>  include/linux/mmc/mmc.h  |   4 ++++
>>>>>> Â 4 files changed, 86 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>>>> index 9b90726..074ec55 100644
>>>>>> --- a/drivers/mmc/card/block.c
>>>>>> +++ b/drivers/mmc/card/block.c
>>>>>> @@ -1402,6 +1402,42 @@ static int mmc_blk_alloc_parts(struct mmc_card
>>>>>> *card, struct mmc_blk_data *md)
>>>>>> Â Â Â Â Â Â Â Â Â Â Â Â return ret;
>>>>>> Â Â Â Â }
>>>>>>
>>>>>> + Â Â Â if (card->ext_csd.gp1_size) {
>>>>>> + Â Â Â Â Â Â Â ret = mmc_blk_alloc_part(card, md,
>>>>>> EXT_CSD_PART_CONFIG_ACC_GP1,
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp1_size>> Â Â Â 9,
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â false,
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "gp1");
>>>>>> + Â Â Â Â Â Â Â if (ret)
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â return ret;
>>>>>> + Â Â Â }
>>>>>> +
>>>>>
>>>>> if mmc_blk_alloc_part() fails for 'if (card->ext_csd.pt1_size)', why
>>>>> would
>>>>> you want to give this code the opportunity to call mmc_blk_alloc_part()
>>>>> again with the next if() below? Â If mmc_blk_alloc_part() fails, is it a
>>>>> big
>>>>> problem, like kzalloc() failing? Â If so, I would think you would want to
>>>>> immediately quit this function and report an error (either use errno
>>>>> value
>>>>> or pass the value of ret).
>>>>
>>>> -> Â Â if card->ext_csd.gp1_size is no zero, it means general purpose
>>>> partition is enable.
>>>> and it can set up to 4 partition in specification. so we should check
>>>> it by next if().
>>>
>>> Please document that in your code. Â Please add a function header to this
>>> function.
>>
>> I will add comments at this code. Â I don't understand why a function
>> header should be added yet.
>
> One or the other.
>
> but a function comment header should be added at some point. Â Yes, the mmc subsystem is pretty legacy, but any new work that gets added to the kernel is usually has higher standards for stuff like not just adding function header comments but putting them in a certain format...at least this is what I went through when I had things accepted upstream.
Okay, I will release patch v2 after adding function header comments again.
Thanks for your review.
>
>>
>>>
>>>>
>>>>>
>>>>>> + Â Â Â if (card->ext_csd.gp2_size) {
>>>>>> + Â Â Â Â Â Â Â ret = mmc_blk_alloc_part(card, md,
>>>>>> EXT_CSD_PART_CONFIG_ACC_GP2,
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp2_size>> Â Â Â 9,
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â false,
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "gp2");
>>>>>> + Â Â Â Â Â Â Â if (ret)
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â return ret;
>>>>>> + Â Â Â }
>>>>>> +
>>>>>> + Â Â Â if (card->ext_csd.gp3_size) {
>>>>>> + Â Â Â Â Â Â Â ret = mmc_blk_alloc_part(card, md,
>>>>>> EXT_CSD_PART_CONFIG_ACC_GP3,
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp3_size>> Â Â Â 9,
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â false,
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "gp3");
>>>>>> + Â Â Â Â Â Â Â if (ret)
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â return ret;
>>>>>> + Â Â Â }
>>>>>> +
>>>>>> + Â Â Â if (card->ext_csd.gp4_size) {
>>>>>> + Â Â Â Â Â Â Â ret = mmc_blk_alloc_part(card, md,
>>>>>> EXT_CSD_PART_CONFIG_ACC_GP4,
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp4_size>> Â Â Â 9,
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â false,
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "gp4");
>>>>>> + Â Â Â Â Â Â Â if (ret)
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â return ret;
>>>>>> + Â Â Â }
>>>>>> +
>>>>>> Â Â Â Â return ret;
>>>>>> Â }
>>>>>>
>>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>>> index 10f5a19..cc31511 100644
>>>>>> --- a/drivers/mmc/core/mmc.c
>>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>>> @@ -393,6 +393,48 @@ 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 Support
>>>>>> + Â Â Â Â Â Â Â Â */
>>>>>> +
>>>>>> + Â Â Â Â Â Â Â 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;
>>>>>> +
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp1_size =
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (ext_csd[145]<< Â Â Â 16) + (ext_csd[144]<<
>>>>>> Â Â 8)
>>>>>> +
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ext_csd[143];
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp1_size *=
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (size_t)(hc_erase_grp_sz *
>>>>>> hc_wp_grp_sz);
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp1_size<<= 19;
>>>>>> +
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp2_size =
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (ext_csd[148]<< Â Â Â 16) + (ext_csd[147]<<
>>>>>> Â Â 8)
>>>>>> +
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ext_csd[146];
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp2_size *=
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (size_t)(hc_erase_grp_sz *
>>>>>> hc_wp_grp_sz);
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp2_size<<= 19;
>>>>>> +
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp3_size =
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (ext_csd[151]<< Â Â Â 16) + (ext_csd[150]<<
>>>>>> Â Â 8)
>>>>>> +
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ext_csd[149];
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp3_size *=
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (size_t)(hc_erase_grp_sz *
>>>>>> hc_wp_grp_sz);
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp3_size<<= 19;
>>>>>> +
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp4_size =
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (ext_csd[154]<< Â Â Â 16) + (ext_csd[153]<<
>>>>>> Â Â 8)
>>>>>> +
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ext_csd[152];
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp4_size *=
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (size_t)(hc_erase_grp_sz *
>>>>>> hc_wp_grp_sz);
>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp4_size<<= 19;
>>>>>> + Â Â Â Â Â Â Â }
>>>>>> Â Â Â Â Â Â Â Â 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..96a98a7 100644
>>>>>> --- a/include/linux/mmc/card.h
>>>>>> +++ b/include/linux/mmc/card.h
>>>>>> @@ -64,6 +64,10 @@ struct mmc_ext_csd {
>>>>>>     unsigned long long    enhanced_area_offset;  /* Units: Byte */
>>>>>>     unsigned int       enhanced_area_size;   /* Units: KB */
>>>>>>     unsigned int       boot_size;        /* in bytes */
>>>>>> +    unsigned int       gp1_size;        /* in bytes */
>>>>>> +    unsigned int       gp2_size;        /* in bytes */
>>>>>> +    unsigned int       gp3_size;        /* in bytes */
>>>>>> +    unsigned int       gp4_size;        /* in bytes */
>>>>>> Â Â Â Â u8 Â Â Â Â Â Â Â Â Â Â Â raw_partition_support; Â /* 160 */
>>>>>> Â Â Â Â u8 Â Â Â Â Â Â Â Â Â Â Â raw_erased_mem_count; Â /* 181 */
>>>>>> Â Â Â Â u8 Â Â Â Â Â Â Â Â Â Â Â raw_ext_csd_structure; Â /* 194 */
>>>>>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>>>>>> index 5a794cb..380720a 100644
>>>>>> --- a/include/linux/mmc/mmc.h
>>>>>> +++ b/include/linux/mmc/mmc.h
>>>>>> @@ -303,6 +303,10 @@ struct _mmc_csd {
>>>>>> Â #define EXT_CSD_PART_CONFIG_ACC_MASK Â (0x7)
>>>>>> Â #define EXT_CSD_PART_CONFIG_ACC_BOOT0 (0x1)
>>>>>> Â #define EXT_CSD_PART_CONFIG_ACC_BOOT1 (0x2)
>>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP1 Â Â (0x4)
>>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP2 Â Â (0x5)
>>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP3 Â Â (0x6)
>>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP4 Â Â (0x7)
>>>>>>
>>>>>> Â #define EXT_CSD_CMD_SET_NORMAL Â Â Â Â Â Â Â Â (1<<0)
>>>>>> Â #define EXT_CSD_CMD_SET_SECURE Â Â Â Â Â Â Â Â (1<<1)
>>>>>
>>>>>
>>>>> --
>>>>> J (James/Jay) Freyensee
>>>>> Storage Technology Group
>>>>> Intel Corporation
>>>>>
>>>
>>>
>>> --
>>> J (James/Jay) Freyensee
>>> Storage Technology Group
>>> Intel Corporation
>>>
>
>
> --
> J (James/Jay) Freyensee
> Storage Technology Group
> Intel Corporation
ÿôèº{.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¥
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] mmc : general purpose partition support.
@ 2011-09-23 0:29 ` NamJae Jeon
0 siblings, 0 replies; 15+ messages in thread
From: NamJae Jeon @ 2011-09-23 0:29 UTC (permalink / raw)
To: J Freyensee; +Cc: cjb, linux-mmc, linux-kernel
2011/9/23 J Freyensee <james_p_freyensee@linux.intel.com>
>
> On 09/22/2011 04:58 PM, NamJae Jeon wrote:
>>
>> 2011/9/23 J Freyensee<james_p_freyensee@linux.intel.com>:
>>>
>>> On 09/22/2011 04:15 PM, NamJae Jeon wrote:
>>>>
>>>> 2011/9/23 J Freyensee<james_p_freyensee@linux.intel.com>:
>>>>>
>>>>> On 09/22/2011 08:34 AM, Namjae Jeon wrote:
>>>>>>
>>>>>> It allows general purpose parition in MMC Device. If it is enable, it
>>>>>> will
>>>>>> make mmcblk0gp1,gp2,gp3,gp4 partition like this.
>>>>>>
>>>>>>> cat /proc/paritition
>>>>>>
>>>>>> 179 0 847872 mmcblk0
>>>>>> 179 192 4096 mmcblk0gp4
>>>>>> 179 160 4096 mmcblk0gp3
>>>>>> 179 128 4096 mmcblk0gp2
>>>>>> 179 96 1052672 mmcblk0gp1
>>>>>> 179 64 1024 mmcblk0boot1
>>>>>> 179 32 1024 mmcblk0boot0
>>>>>>
>>>>>> Signed-off-by: Namjae Jeon<linkinjeon@gmail.com>
>>>>>> ---
>>>>>> drivers/mmc/card/block.c | 36 ++++++++++++++++++++++++++++++++++++
>>>>>> drivers/mmc/core/mmc.c | 42
>>>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>>> include/linux/mmc/card.h | 4 ++++
>>>>>> include/linux/mmc/mmc.h | 4 ++++
>>>>>> 4 files changed, 86 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>>>> index 9b90726..074ec55 100644
>>>>>> --- a/drivers/mmc/card/block.c
>>>>>> +++ b/drivers/mmc/card/block.c
>>>>>> @@ -1402,6 +1402,42 @@ static int mmc_blk_alloc_parts(struct mmc_card
>>>>>> *card, struct mmc_blk_data *md)
>>>>>> return ret;
>>>>>> }
>>>>>>
>>>>>> + if (card->ext_csd.gp1_size) {
>>>>>> + ret = mmc_blk_alloc_part(card, md,
>>>>>> EXT_CSD_PART_CONFIG_ACC_GP1,
>>>>>> + card->ext_csd.gp1_size>> 9,
>>>>>> + false,
>>>>>> + "gp1");
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>> + }
>>>>>> +
>>>>>
>>>>> if mmc_blk_alloc_part() fails for 'if (card->ext_csd.pt1_size)', why
>>>>> would
>>>>> you want to give this code the opportunity to call mmc_blk_alloc_part()
>>>>> again with the next if() below? If mmc_blk_alloc_part() fails, is it a
>>>>> big
>>>>> problem, like kzalloc() failing? If so, I would think you would want to
>>>>> immediately quit this function and report an error (either use errno
>>>>> value
>>>>> or pass the value of ret).
>>>>
>>>> -> if card->ext_csd.gp1_size is no zero, it means general purpose
>>>> partition is enable.
>>>> and it can set up to 4 partition in specification. so we should check
>>>> it by next if().
>>>
>>> Please document that in your code. Please add a function header to this
>>> function.
>>
>> I will add comments at this code. I don't understand why a function
>> header should be added yet.
>
> One or the other.
>
> but a function comment header should be added at some point. Yes, the mmc subsystem is pretty legacy, but any new work that gets added to the kernel is usually has higher standards for stuff like not just adding function header comments but putting them in a certain format...at least this is what I went through when I had things accepted upstream.
Okay, I will release patch v2 after adding function header comments again.
Thanks for your review.
>
>>
>>>
>>>>
>>>>>
>>>>>> + if (card->ext_csd.gp2_size) {
>>>>>> + ret = mmc_blk_alloc_part(card, md,
>>>>>> EXT_CSD_PART_CONFIG_ACC_GP2,
>>>>>> + card->ext_csd.gp2_size>> 9,
>>>>>> + false,
>>>>>> + "gp2");
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>> + }
>>>>>> +
>>>>>> + if (card->ext_csd.gp3_size) {
>>>>>> + ret = mmc_blk_alloc_part(card, md,
>>>>>> EXT_CSD_PART_CONFIG_ACC_GP3,
>>>>>> + card->ext_csd.gp3_size>> 9,
>>>>>> + false,
>>>>>> + "gp3");
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>> + }
>>>>>> +
>>>>>> + if (card->ext_csd.gp4_size) {
>>>>>> + ret = mmc_blk_alloc_part(card, md,
>>>>>> EXT_CSD_PART_CONFIG_ACC_GP4,
>>>>>> + card->ext_csd.gp4_size>> 9,
>>>>>> + false,
>>>>>> + "gp4");
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>> + }
>>>>>> +
>>>>>> return ret;
>>>>>> }
>>>>>>
>>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>>> index 10f5a19..cc31511 100644
>>>>>> --- a/drivers/mmc/core/mmc.c
>>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>>> @@ -393,6 +393,48 @@ 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 Support
>>>>>> + */
>>>>>> +
>>>>>> + 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;
>>>>>> +
>>>>>> + card->ext_csd.gp1_size =
>>>>>> + (ext_csd[145]<< 16) + (ext_csd[144]<<
>>>>>> 8)
>>>>>> +
>>>>>> + ext_csd[143];
>>>>>> + card->ext_csd.gp1_size *=
>>>>>> + (size_t)(hc_erase_grp_sz *
>>>>>> hc_wp_grp_sz);
>>>>>> + card->ext_csd.gp1_size<<= 19;
>>>>>> +
>>>>>> + card->ext_csd.gp2_size =
>>>>>> + (ext_csd[148]<< 16) + (ext_csd[147]<<
>>>>>> 8)
>>>>>> +
>>>>>> + ext_csd[146];
>>>>>> + card->ext_csd.gp2_size *=
>>>>>> + (size_t)(hc_erase_grp_sz *
>>>>>> hc_wp_grp_sz);
>>>>>> + card->ext_csd.gp2_size<<= 19;
>>>>>> +
>>>>>> + card->ext_csd.gp3_size =
>>>>>> + (ext_csd[151]<< 16) + (ext_csd[150]<<
>>>>>> 8)
>>>>>> +
>>>>>> + ext_csd[149];
>>>>>> + card->ext_csd.gp3_size *=
>>>>>> + (size_t)(hc_erase_grp_sz *
>>>>>> hc_wp_grp_sz);
>>>>>> + card->ext_csd.gp3_size<<= 19;
>>>>>> +
>>>>>> + card->ext_csd.gp4_size =
>>>>>> + (ext_csd[154]<< 16) + (ext_csd[153]<<
>>>>>> 8)
>>>>>> +
>>>>>> + ext_csd[152];
>>>>>> + card->ext_csd.gp4_size *=
>>>>>> + (size_t)(hc_erase_grp_sz *
>>>>>> hc_wp_grp_sz);
>>>>>> + card->ext_csd.gp4_size<<= 19;
>>>>>> + }
>>>>>> 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..96a98a7 100644
>>>>>> --- a/include/linux/mmc/card.h
>>>>>> +++ b/include/linux/mmc/card.h
>>>>>> @@ -64,6 +64,10 @@ struct mmc_ext_csd {
>>>>>> unsigned long long enhanced_area_offset; /* Units: Byte */
>>>>>> unsigned int enhanced_area_size; /* Units: KB */
>>>>>> unsigned int boot_size; /* in bytes */
>>>>>> + unsigned int gp1_size; /* in bytes */
>>>>>> + unsigned int gp2_size; /* in bytes */
>>>>>> + unsigned int gp3_size; /* in bytes */
>>>>>> + unsigned int gp4_size; /* in bytes */
>>>>>> u8 raw_partition_support; /* 160 */
>>>>>> u8 raw_erased_mem_count; /* 181 */
>>>>>> u8 raw_ext_csd_structure; /* 194 */
>>>>>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>>>>>> index 5a794cb..380720a 100644
>>>>>> --- a/include/linux/mmc/mmc.h
>>>>>> +++ b/include/linux/mmc/mmc.h
>>>>>> @@ -303,6 +303,10 @@ struct _mmc_csd {
>>>>>> #define EXT_CSD_PART_CONFIG_ACC_MASK (0x7)
>>>>>> #define EXT_CSD_PART_CONFIG_ACC_BOOT0 (0x1)
>>>>>> #define EXT_CSD_PART_CONFIG_ACC_BOOT1 (0x2)
>>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP1 (0x4)
>>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP2 (0x5)
>>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP3 (0x6)
>>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP4 (0x7)
>>>>>>
>>>>>> #define EXT_CSD_CMD_SET_NORMAL (1<<0)
>>>>>> #define EXT_CSD_CMD_SET_SECURE (1<<1)
>>>>>
>>>>>
>>>>> --
>>>>> J (James/Jay) Freyensee
>>>>> Storage Technology Group
>>>>> Intel Corporation
>>>>>
>>>
>>>
>>> --
>>> J (James/Jay) Freyensee
>>> Storage Technology Group
>>> Intel Corporation
>>>
>
>
> --
> J (James/Jay) Freyensee
> Storage Technology Group
> Intel Corporation
^ permalink raw reply [flat|nested] 15+ messages in thread[parent not found: <4E7CB5FC.3090301@linux.intel.com>]
* Re: [PATCH] mmc : general purpose partition support.
[not found] ` <4E7CB5FC.3090301@linux.intel.com>
@ 2011-09-24 5:21 ` NamJae Jeon
0 siblings, 0 replies; 15+ messages in thread
From: NamJae Jeon @ 2011-09-24 5:21 UTC (permalink / raw)
To: J Freyensee; +Cc: cjb, linux-mmc, linux-kernel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 12390 bytes --]
2011/9/24 J Freyensee <james_p_freyensee@linux.intel.com>:
> On 09/22/2011 05:29 PM, NamJae Jeon wrote:
>>
>> 2011/9/23 J Freyensee<james_p_freyensee@linux.intel.com>
>>>
>>> On 09/22/2011 04:58 PM, NamJae Jeon wrote:
>>>>
>>>> 2011/9/23 J Freyensee<james_p_freyensee@linux.intel.com>:
>>>>>
>>>>> On 09/22/2011 04:15 PM, NamJae Jeon wrote:
>>>>>>
>>>>>> 2011/9/23 J Freyensee<james_p_freyensee@linux.intel.com>:
>>>>>>>
>>>>>>> On 09/22/2011 08:34 AM, Namjae Jeon wrote:
>>>>>>>>
>>>>>>>> It allows general purpose parition in MMC Device. If it is enable,
>>>>>>>> it
>>>>>>>> will
>>>>>>>> make mmcblk0gp1,gp2,gp3,gp4 partition like this.
>>>>>>>>
>>>>>>>>> cat /proc/paritition
>>>>>>>>
>>>>>>>> Â Â Â 179 0 847872 mmcblk0
>>>>>>>> Â Â Â 179 192 4096 mmcblk0gp4
>>>>>>>> Â Â Â 179 160 4096 mmcblk0gp3
>>>>>>>> Â Â Â 179 128 4096 mmcblk0gp2
>>>>>>>> Â Â Â 179 96 Â 1052672 mmcblk0gp1
>>>>>>>> Â Â Â 179 64 Â 1024 mmcblk0boot1
>>>>>>>> Â Â Â 179 32 Â 1024 mmcblk0boot0
>>>>>>>>
>>>>>>>> Signed-off-by: Namjae Jeon<linkinjeon@gmail.com>
>>>>>>>> ---
>>>>>>>> Â drivers/mmc/card/block.c | Â 36
>>>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>>>  drivers/mmc/core/mmc.c  |  42
>>>>>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>>>>> Â include/linux/mmc/card.h | Â Â 4 ++++
>>>>>>>>  include/linux/mmc/mmc.h  |   4 ++++
>>>>>>>> Â 4 files changed, 86 insertions(+), 0 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>>>>>> index 9b90726..074ec55 100644
>>>>>>>> --- a/drivers/mmc/card/block.c
>>>>>>>> +++ b/drivers/mmc/card/block.c
>>>>>>>> @@ -1402,6 +1402,42 @@ static int mmc_blk_alloc_parts(struct
>>>>>>>> mmc_card
>>>>>>>> *card, struct mmc_blk_data *md)
>>>>>>>> Â Â Â Â Â Â Â Â Â Â Â Â return ret;
>>>>>>>> Â Â Â Â }
>>>>>>>>
>>>>>>>> + Â Â Â if (card->ext_csd.gp1_size) {
>
> Looks like you may need to also incorporate the ideas Adrian Hunter did in a
> patch that seems similar to yours, mainly also checking
> mmc_boot_partition_access(card->host) in your if() statement?
>
> I attached his patch that was sent today.
>
> Jay
I checked attached patch, see the patch v2. I think that adrian may
modify his patch base on my patch.
Thanks
>>>>>>>>
>>>>>>>> + Â Â Â Â Â Â Â ret = mmc_blk_alloc_part(card, md,
>>>>>>>> EXT_CSD_PART_CONFIG_ACC_GP1,
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp1_size>>
>>>>>>>> Â Â 9,
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â false,
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "gp1");
>>>>>>>> + Â Â Â Â Â Â Â if (ret)
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â return ret;
>>>>>>>> + Â Â Â }
>>>>>>>> +
>>>>>>>
>>>>>>> if mmc_blk_alloc_part() fails for 'if (card->ext_csd.pt1_size)', why
>>>>>>> would
>>>>>>> you want to give this code the opportunity to call
>>>>>>> mmc_blk_alloc_part()
>>>>>>> again with the next if() below? Â If mmc_blk_alloc_part() fails, is it
>>>>>>> a
>>>>>>> big
>>>>>>> problem, like kzalloc() failing? Â If so, I would think you would want
>>>>>>> to
>>>>>>> immediately quit this function and report an error (either use errno
>>>>>>> value
>>>>>>> or pass the value of ret).
>>>>>>
>>>>>> -> Â Â Â if card->ext_csd.gp1_size is no zero, it means general purpose
>>>>>> partition is enable.
>>>>>> and it can set up to 4 partition in specification. so we should check
>>>>>> it by next if().
>>>>>
>>>>> Please document that in your code. Â Please add a function header to
>>>>> this
>>>>> function.
>>>>
>>>> I will add comments at this code. Â I don't understand why a function
>>>> header should be added yet.
>>>
>>> One or the other.
>>>
>>> but a function comment header should be added at some point. Â Yes, the
>>> mmc subsystem is pretty legacy, but any new work that gets added to the
>>> kernel is usually has higher standards for stuff like not just adding
>>> function header comments but putting them in a certain format...at least
>>> this is what I went through when I had things accepted upstream.
>>
>> Okay, I will release patch v2 after adding function header comments again.
>> Thanks for your review.
>>>
>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>>> + Â Â Â if (card->ext_csd.gp2_size) {
>>>>>>>> + Â Â Â Â Â Â Â ret = mmc_blk_alloc_part(card, md,
>>>>>>>> EXT_CSD_PART_CONFIG_ACC_GP2,
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp2_size>>
>>>>>>>> Â Â 9,
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â false,
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "gp2");
>>>>>>>> + Â Â Â Â Â Â Â if (ret)
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â return ret;
>>>>>>>> + Â Â Â }
>>>>>>>> +
>>>>>>>> + Â Â Â if (card->ext_csd.gp3_size) {
>>>>>>>> + Â Â Â Â Â Â Â ret = mmc_blk_alloc_part(card, md,
>>>>>>>> EXT_CSD_PART_CONFIG_ACC_GP3,
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp3_size>>
>>>>>>>> Â Â 9,
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â false,
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "gp3");
>>>>>>>> + Â Â Â Â Â Â Â if (ret)
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â return ret;
>>>>>>>> + Â Â Â }
>>>>>>>> +
>>>>>>>> + Â Â Â if (card->ext_csd.gp4_size) {
>>>>>>>> + Â Â Â Â Â Â Â ret = mmc_blk_alloc_part(card, md,
>>>>>>>> EXT_CSD_PART_CONFIG_ACC_GP4,
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp4_size>>
>>>>>>>> Â Â 9,
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â false,
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "gp4");
>>>>>>>> + Â Â Â Â Â Â Â if (ret)
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â return ret;
>>>>>>>> + Â Â Â }
>>>>>>>> +
>>>>>>>> Â Â Â Â return ret;
>>>>>>>> Â }
>>>>>>>>
>>>>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>>>>> index 10f5a19..cc31511 100644
>>>>>>>> --- a/drivers/mmc/core/mmc.c
>>>>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>>>>> @@ -393,6 +393,48 @@ 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 Support
>>>>>>>> + Â Â Â Â Â Â Â Â */
>>>>>>>> +
>>>>>>>> + Â Â Â Â Â Â Â 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;
>>>>>>>> +
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp1_size =
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (ext_csd[145]<< Â Â Â Â 16) +
>>>>>>>> (ext_csd[144]<<
>>>>>>>> Â Â 8)
>>>>>>>> +
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ext_csd[143];
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp1_size *=
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (size_t)(hc_erase_grp_sz *
>>>>>>>> hc_wp_grp_sz);
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp1_size<<= 19;
>>>>>>>> +
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp2_size =
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (ext_csd[148]<< Â Â Â Â 16) +
>>>>>>>> (ext_csd[147]<<
>>>>>>>> Â Â 8)
>>>>>>>> +
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ext_csd[146];
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp2_size *=
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (size_t)(hc_erase_grp_sz *
>>>>>>>> hc_wp_grp_sz);
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp2_size<<= 19;
>>>>>>>> +
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp3_size =
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (ext_csd[151]<< Â Â Â Â 16) +
>>>>>>>> (ext_csd[150]<<
>>>>>>>> Â Â 8)
>>>>>>>> +
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ext_csd[149];
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp3_size *=
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (size_t)(hc_erase_grp_sz *
>>>>>>>> hc_wp_grp_sz);
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp3_size<<= 19;
>>>>>>>> +
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp4_size =
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (ext_csd[154]<< Â Â Â Â 16) +
>>>>>>>> (ext_csd[153]<<
>>>>>>>> Â Â 8)
>>>>>>>> +
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ext_csd[152];
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp4_size *=
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (size_t)(hc_erase_grp_sz *
>>>>>>>> hc_wp_grp_sz);
>>>>>>>> + Â Â Â Â Â Â Â Â Â Â Â card->ext_csd.gp4_size<<= 19;
>>>>>>>> + Â Â Â Â Â Â Â }
>>>>>>>> Â Â Â Â Â Â Â Â 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..96a98a7 100644
>>>>>>>> --- a/include/linux/mmc/card.h
>>>>>>>> +++ b/include/linux/mmc/card.h
>>>>>>>> @@ -64,6 +64,10 @@ struct mmc_ext_csd {
>>>>>>>>     unsigned long long    enhanced_area_offset;  /* Units:
>>>>>>>> Byte */
>>>>>>>>     unsigned int       enhanced_area_size;   /* Units: KB
>>>>>>>> */
>>>>>>>>     unsigned int       boot_size;        /* in bytes
>>>>>>>> */
>>>>>>>> +    unsigned int       gp1_size;        /* in bytes
>>>>>>>> */
>>>>>>>> +    unsigned int       gp2_size;        /* in bytes
>>>>>>>> */
>>>>>>>> +    unsigned int       gp3_size;        /* in bytes
>>>>>>>> */
>>>>>>>> +    unsigned int       gp4_size;        /* in bytes
>>>>>>>> */
>>>>>>>> Â Â Â Â u8 Â Â Â Â Â Â Â Â Â Â Â raw_partition_support; Â /* 160 */
>>>>>>>> Â Â Â Â u8 Â Â Â Â Â Â Â Â Â Â Â raw_erased_mem_count; Â /* 181 */
>>>>>>>> Â Â Â Â u8 Â Â Â Â Â Â Â Â Â Â Â raw_ext_csd_structure; Â /* 194 */
>>>>>>>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>>>>>>>> index 5a794cb..380720a 100644
>>>>>>>> --- a/include/linux/mmc/mmc.h
>>>>>>>> +++ b/include/linux/mmc/mmc.h
>>>>>>>> @@ -303,6 +303,10 @@ struct _mmc_csd {
>>>>>>>> Â #define EXT_CSD_PART_CONFIG_ACC_MASK Â (0x7)
>>>>>>>> Â #define EXT_CSD_PART_CONFIG_ACC_BOOT0 (0x1)
>>>>>>>> Â #define EXT_CSD_PART_CONFIG_ACC_BOOT1 (0x2)
>>>>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP1 Â Â (0x4)
>>>>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP2 Â Â (0x5)
>>>>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP3 Â Â (0x6)
>>>>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP4 Â Â (0x7)
>>>>>>>>
>>>>>>>> Â #define EXT_CSD_CMD_SET_NORMAL Â Â Â Â Â Â Â Â (1<<0)
>>>>>>>> Â #define EXT_CSD_CMD_SET_SECURE Â Â Â Â Â Â Â Â (1<<1)
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> J (James/Jay) Freyensee
>>>>>>> Storage Technology Group
>>>>>>> Intel Corporation
>>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> J (James/Jay) Freyensee
>>>>> Storage Technology Group
>>>>> Intel Corporation
>>>>>
>>>
>>>
>>> --
>>> J (James/Jay) Freyensee
>>> Storage Technology Group
>>> Intel Corporation
>>
>> N�����r��y���b�X��ǧv�^�)޺{.n�+����{��g"��^n�r��z� ��h����&�� �G���h�
>> (�階�ݢj"�� � m�����z�ޖ���f���h���~�mml==
>
>
> --
> J (James/Jay) Freyensee
> Storage Technology Group
> Intel Corporation
>
ÿôèº{.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¥
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] mmc : general purpose partition support.
@ 2011-09-24 5:21 ` NamJae Jeon
0 siblings, 0 replies; 15+ messages in thread
From: NamJae Jeon @ 2011-09-24 5:21 UTC (permalink / raw)
To: J Freyensee; +Cc: cjb, linux-mmc, linux-kernel
2011/9/24 J Freyensee <james_p_freyensee@linux.intel.com>:
> On 09/22/2011 05:29 PM, NamJae Jeon wrote:
>>
>> 2011/9/23 J Freyensee<james_p_freyensee@linux.intel.com>
>>>
>>> On 09/22/2011 04:58 PM, NamJae Jeon wrote:
>>>>
>>>> 2011/9/23 J Freyensee<james_p_freyensee@linux.intel.com>:
>>>>>
>>>>> On 09/22/2011 04:15 PM, NamJae Jeon wrote:
>>>>>>
>>>>>> 2011/9/23 J Freyensee<james_p_freyensee@linux.intel.com>:
>>>>>>>
>>>>>>> On 09/22/2011 08:34 AM, Namjae Jeon wrote:
>>>>>>>>
>>>>>>>> It allows general purpose parition in MMC Device. If it is enable,
>>>>>>>> it
>>>>>>>> will
>>>>>>>> make mmcblk0gp1,gp2,gp3,gp4 partition like this.
>>>>>>>>
>>>>>>>>> cat /proc/paritition
>>>>>>>>
>>>>>>>> 179 0 847872 mmcblk0
>>>>>>>> 179 192 4096 mmcblk0gp4
>>>>>>>> 179 160 4096 mmcblk0gp3
>>>>>>>> 179 128 4096 mmcblk0gp2
>>>>>>>> 179 96 1052672 mmcblk0gp1
>>>>>>>> 179 64 1024 mmcblk0boot1
>>>>>>>> 179 32 1024 mmcblk0boot0
>>>>>>>>
>>>>>>>> Signed-off-by: Namjae Jeon<linkinjeon@gmail.com>
>>>>>>>> ---
>>>>>>>> drivers/mmc/card/block.c | 36
>>>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>>> drivers/mmc/core/mmc.c | 42
>>>>>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>>>>> include/linux/mmc/card.h | 4 ++++
>>>>>>>> include/linux/mmc/mmc.h | 4 ++++
>>>>>>>> 4 files changed, 86 insertions(+), 0 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>>>>>> index 9b90726..074ec55 100644
>>>>>>>> --- a/drivers/mmc/card/block.c
>>>>>>>> +++ b/drivers/mmc/card/block.c
>>>>>>>> @@ -1402,6 +1402,42 @@ static int mmc_blk_alloc_parts(struct
>>>>>>>> mmc_card
>>>>>>>> *card, struct mmc_blk_data *md)
>>>>>>>> return ret;
>>>>>>>> }
>>>>>>>>
>>>>>>>> + if (card->ext_csd.gp1_size) {
>
> Looks like you may need to also incorporate the ideas Adrian Hunter did in a
> patch that seems similar to yours, mainly also checking
> mmc_boot_partition_access(card->host) in your if() statement?
>
> I attached his patch that was sent today.
>
> Jay
I checked attached patch, see the patch v2. I think that adrian may
modify his patch base on my patch.
Thanks
>>>>>>>>
>>>>>>>> + ret = mmc_blk_alloc_part(card, md,
>>>>>>>> EXT_CSD_PART_CONFIG_ACC_GP1,
>>>>>>>> + card->ext_csd.gp1_size>>
>>>>>>>> 9,
>>>>>>>> + false,
>>>>>>>> + "gp1");
>>>>>>>> + if (ret)
>>>>>>>> + return ret;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>
>>>>>>> if mmc_blk_alloc_part() fails for 'if (card->ext_csd.pt1_size)', why
>>>>>>> would
>>>>>>> you want to give this code the opportunity to call
>>>>>>> mmc_blk_alloc_part()
>>>>>>> again with the next if() below? If mmc_blk_alloc_part() fails, is it
>>>>>>> a
>>>>>>> big
>>>>>>> problem, like kzalloc() failing? If so, I would think you would want
>>>>>>> to
>>>>>>> immediately quit this function and report an error (either use errno
>>>>>>> value
>>>>>>> or pass the value of ret).
>>>>>>
>>>>>> -> if card->ext_csd.gp1_size is no zero, it means general purpose
>>>>>> partition is enable.
>>>>>> and it can set up to 4 partition in specification. so we should check
>>>>>> it by next if().
>>>>>
>>>>> Please document that in your code. Please add a function header to
>>>>> this
>>>>> function.
>>>>
>>>> I will add comments at this code. I don't understand why a function
>>>> header should be added yet.
>>>
>>> One or the other.
>>>
>>> but a function comment header should be added at some point. Yes, the
>>> mmc subsystem is pretty legacy, but any new work that gets added to the
>>> kernel is usually has higher standards for stuff like not just adding
>>> function header comments but putting them in a certain format...at least
>>> this is what I went through when I had things accepted upstream.
>>
>> Okay, I will release patch v2 after adding function header comments again.
>> Thanks for your review.
>>>
>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>>> + if (card->ext_csd.gp2_size) {
>>>>>>>> + ret = mmc_blk_alloc_part(card, md,
>>>>>>>> EXT_CSD_PART_CONFIG_ACC_GP2,
>>>>>>>> + card->ext_csd.gp2_size>>
>>>>>>>> 9,
>>>>>>>> + false,
>>>>>>>> + "gp2");
>>>>>>>> + if (ret)
>>>>>>>> + return ret;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + if (card->ext_csd.gp3_size) {
>>>>>>>> + ret = mmc_blk_alloc_part(card, md,
>>>>>>>> EXT_CSD_PART_CONFIG_ACC_GP3,
>>>>>>>> + card->ext_csd.gp3_size>>
>>>>>>>> 9,
>>>>>>>> + false,
>>>>>>>> + "gp3");
>>>>>>>> + if (ret)
>>>>>>>> + return ret;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + if (card->ext_csd.gp4_size) {
>>>>>>>> + ret = mmc_blk_alloc_part(card, md,
>>>>>>>> EXT_CSD_PART_CONFIG_ACC_GP4,
>>>>>>>> + card->ext_csd.gp4_size>>
>>>>>>>> 9,
>>>>>>>> + false,
>>>>>>>> + "gp4");
>>>>>>>> + if (ret)
>>>>>>>> + return ret;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> return ret;
>>>>>>>> }
>>>>>>>>
>>>>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>>>>> index 10f5a19..cc31511 100644
>>>>>>>> --- a/drivers/mmc/core/mmc.c
>>>>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>>>>> @@ -393,6 +393,48 @@ 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 Support
>>>>>>>> + */
>>>>>>>> +
>>>>>>>> + 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;
>>>>>>>> +
>>>>>>>> + card->ext_csd.gp1_size =
>>>>>>>> + (ext_csd[145]<< 16) +
>>>>>>>> (ext_csd[144]<<
>>>>>>>> 8)
>>>>>>>> +
>>>>>>>> + ext_csd[143];
>>>>>>>> + card->ext_csd.gp1_size *=
>>>>>>>> + (size_t)(hc_erase_grp_sz *
>>>>>>>> hc_wp_grp_sz);
>>>>>>>> + card->ext_csd.gp1_size<<= 19;
>>>>>>>> +
>>>>>>>> + card->ext_csd.gp2_size =
>>>>>>>> + (ext_csd[148]<< 16) +
>>>>>>>> (ext_csd[147]<<
>>>>>>>> 8)
>>>>>>>> +
>>>>>>>> + ext_csd[146];
>>>>>>>> + card->ext_csd.gp2_size *=
>>>>>>>> + (size_t)(hc_erase_grp_sz *
>>>>>>>> hc_wp_grp_sz);
>>>>>>>> + card->ext_csd.gp2_size<<= 19;
>>>>>>>> +
>>>>>>>> + card->ext_csd.gp3_size =
>>>>>>>> + (ext_csd[151]<< 16) +
>>>>>>>> (ext_csd[150]<<
>>>>>>>> 8)
>>>>>>>> +
>>>>>>>> + ext_csd[149];
>>>>>>>> + card->ext_csd.gp3_size *=
>>>>>>>> + (size_t)(hc_erase_grp_sz *
>>>>>>>> hc_wp_grp_sz);
>>>>>>>> + card->ext_csd.gp3_size<<= 19;
>>>>>>>> +
>>>>>>>> + card->ext_csd.gp4_size =
>>>>>>>> + (ext_csd[154]<< 16) +
>>>>>>>> (ext_csd[153]<<
>>>>>>>> 8)
>>>>>>>> +
>>>>>>>> + ext_csd[152];
>>>>>>>> + card->ext_csd.gp4_size *=
>>>>>>>> + (size_t)(hc_erase_grp_sz *
>>>>>>>> hc_wp_grp_sz);
>>>>>>>> + card->ext_csd.gp4_size<<= 19;
>>>>>>>> + }
>>>>>>>> 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..96a98a7 100644
>>>>>>>> --- a/include/linux/mmc/card.h
>>>>>>>> +++ b/include/linux/mmc/card.h
>>>>>>>> @@ -64,6 +64,10 @@ struct mmc_ext_csd {
>>>>>>>> unsigned long long enhanced_area_offset; /* Units:
>>>>>>>> Byte */
>>>>>>>> unsigned int enhanced_area_size; /* Units: KB
>>>>>>>> */
>>>>>>>> unsigned int boot_size; /* in bytes
>>>>>>>> */
>>>>>>>> + unsigned int gp1_size; /* in bytes
>>>>>>>> */
>>>>>>>> + unsigned int gp2_size; /* in bytes
>>>>>>>> */
>>>>>>>> + unsigned int gp3_size; /* in bytes
>>>>>>>> */
>>>>>>>> + unsigned int gp4_size; /* in bytes
>>>>>>>> */
>>>>>>>> u8 raw_partition_support; /* 160 */
>>>>>>>> u8 raw_erased_mem_count; /* 181 */
>>>>>>>> u8 raw_ext_csd_structure; /* 194 */
>>>>>>>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>>>>>>>> index 5a794cb..380720a 100644
>>>>>>>> --- a/include/linux/mmc/mmc.h
>>>>>>>> +++ b/include/linux/mmc/mmc.h
>>>>>>>> @@ -303,6 +303,10 @@ struct _mmc_csd {
>>>>>>>> #define EXT_CSD_PART_CONFIG_ACC_MASK (0x7)
>>>>>>>> #define EXT_CSD_PART_CONFIG_ACC_BOOT0 (0x1)
>>>>>>>> #define EXT_CSD_PART_CONFIG_ACC_BOOT1 (0x2)
>>>>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP1 (0x4)
>>>>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP2 (0x5)
>>>>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP3 (0x6)
>>>>>>>> +#define EXT_CSD_PART_CONFIG_ACC_GP4 (0x7)
>>>>>>>>
>>>>>>>> #define EXT_CSD_CMD_SET_NORMAL (1<<0)
>>>>>>>> #define EXT_CSD_CMD_SET_SECURE (1<<1)
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> J (James/Jay) Freyensee
>>>>>>> Storage Technology Group
>>>>>>> Intel Corporation
>>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> J (James/Jay) Freyensee
>>>>> Storage Technology Group
>>>>> Intel Corporation
>>>>>
>>>
>>>
>>> --
>>> J (James/Jay) Freyensee
>>> Storage Technology Group
>>> Intel Corporation
>>
>> N�����r��y���b�X��ǧv�^�){.n�+����{��g"��^n�r��z� ��h����&�� �G���h�
>> (�階�ݢj"�� � m�����z�ޖ���f���h���~�mml==
>
>
> --
> J (James/Jay) Freyensee
> Storage Technology Group
> Intel Corporation
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mmc : general purpose partition support.
2011-09-22 15:34 [PATCH] mmc : general purpose partition support Namjae Jeon
2011-09-22 22:08 ` J Freyensee
@ 2011-09-23 3:25 ` Andrei Warkentin
2011-09-23 3:56 ` NamJae Jeon
1 sibling, 1 reply; 15+ messages in thread
From: Andrei Warkentin @ 2011-09-23 3:25 UTC (permalink / raw)
To: Namjae Jeon; +Cc: linux-kernel, cjb, linux-mmc
Hi Namjae,
I scanned over your changes to enable GP partitions, and while I
did not yet test your changes, they do appear to be ok.
Please CC me on the next version of your patch so I can give
it a spin and Ack on it.
I do suggest at least thinking about ways to improve on this. Back
when I added boot partition support, I already didn't like the fact
that the block driver had to be aware of the eMMC partitions, with all
the code duplication and such. By adding four more partitions,
you've compounded the problem and now you have six pieces of
code that really look the same.
The core/mmc.c right now scans the size values and stores them
in mmc_card, and the block driver is smart enough to know what
to do with them. Why not make the block code generic at the expense
of keeping an array of structures in mmc_card? The block driver
would then just iterate over these and create the additional devices. The partition
switch routine would then operate on these structures, and could then be pulled out
of the block driver, making it simpler.
Each physical partition could be described by something like -
struct mmc_part {
unsigned int size;
unsigned int cookie; /* for mmc, idx used in mmc_switch, part_type from current mmc_blk_data */
char name[DISK_NAME_LEN];
bool force_ro; /* to make boot parts RO by default */
};
Just an idea. Clearly, if MMC grows by another 4-5 possible partitions, it's not
realistic to be copy-pasting the same block of code, nor would it make sense
if/when these types of devices support an arbitrary amount of physical partitions.
Thanks,
A
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] mmc : general purpose partition support.
2011-09-23 3:25 ` Andrei Warkentin
@ 2011-09-23 3:56 ` NamJae Jeon
0 siblings, 0 replies; 15+ messages in thread
From: NamJae Jeon @ 2011-09-23 3:56 UTC (permalink / raw)
To: Andrei Warkentin; +Cc: linux-kernel, cjb, linux-mmc
2011/9/23 Andrei Warkentin <awarkentin@vmware.com>:
> Hi Namjae,
>
> I scanned over your changes to enable GP partitions, and while I
> did not yet test your changes, they do appear to be ok.
>
> Please CC me on the next version of your patch so I can give
> it a spin and Ack on it.
>
> I do suggest at least thinking about ways to improve on this. Back
> when I added boot partition support, I already didn't like the fact
> that the block driver had to be aware of the eMMC partitions, with all
> the code duplication and such. By adding four more partitions,
> you've compounded the problem and now you have six pieces of
> code that really look the same.
>
> The core/mmc.c right now scans the size values and stores them
> in mmc_card, and the block driver is smart enough to know what
> to do with them. Why not make the block code generic at the expense
> of keeping an array of structures in mmc_card? The block driver
> would then just iterate over these and create the additional devices. The partition
> switch routine would then operate on these structures, and could then be pulled out
> of the block driver, making it simpler.
>
> Each physical partition could be described by something like -
> struct mmc_part {
> unsigned int size;
> unsigned int cookie; /* for mmc, idx used in mmc_switch, part_type from current mmc_blk_data */
> char name[DISK_NAME_LEN];
> bool force_ro; /* to make boot parts RO by default */
> };
>
> Just an idea. Clearly, if MMC grows by another 4-5 possible partitions, it's not
> realistic to be copy-pasting the same block of code, nor would it make sense
> if/when these types of devices support an arbitrary amount of physical partitions.
>
> Thanks,
> A
>
Okay, I understand what you want. I will try to make patch included
your suggestion again.
Thanks for your review.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] mmc : general purpose partition support.
@ 2011-10-04 16:16 Namjae Jeon
0 siblings, 0 replies; 15+ messages in thread
From: Namjae Jeon @ 2011-10-04 16:16 UTC (permalink / raw)
To: cjb, linux-mmc
Cc: linux-kernel, awarkentin, adrian.hunter, linus.walleij,
james_p_freyensee, sebras, Ulf.Hansson, stefan.xk.nilsson,
per.forlin, johan.rudholm, 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 | 31 +++++++++++++++------------
drivers/mmc/core/mmc.c | 52 ++++++++++++++++++++++++++++++++++++++++++---
include/linux/mmc/card.h | 34 +++++++++++++++++++++++++++++-
include/linux/mmc/mmc.h | 5 +++-
4 files changed, 102 insertions(+), 20 deletions(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 1ff5486..8452872 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1377,26 +1377,29 @@ static int mmc_blk_alloc_part(struct mmc_card *card,
return 0;
}
+/* MMC Physical partition consist of two boot partitions and
+ * up to four general purpose partitions.
+ * For each partitions enabled in EXT_CSD
+ * The block device will allocated to provide access to the partition.
+ */
+
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 < card->nr_parts; idx++) {
+ if (card->part[idx].size) {
+ ret = mmc_blk_alloc_part(card, md,
+ card->part[idx].part_cfg,
+ card->part[idx].size >> 9,
+ card->part[idx].force_ro,
+ card->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..478b251 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -239,7 +239,9 @@ 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;
+ unsigned int part_size;
+ u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
BUG_ON(!card);
@@ -340,7 +342,15 @@ 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]) {
+ for (idx = 0; idx < MMC_NUM_BOOT_PARTITION;
+ idx++) {
+ part_size = ext_csd[EXT_CSD_BOOT_MULT] << 17;
+ mmc_part_add(card, part_size,
+ EXT_CSD_PART_CONFIG_ACC_BOOT0 + idx,
+ "boot%d", idx, true);
+ }
+ }
}
card->ext_csd.raw_hc_erase_gap_size =
@@ -361,9 +371,9 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
*/
if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
(ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
- u8 hc_erase_grp_sz =
+ hc_erase_grp_sz =
ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
- u8 hc_wp_grp_sz =
+ hc_wp_grp_sz =
ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
card->ext_csd.enhanced_area_en = 1;
@@ -392,6 +402,40 @@ 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_cfg, partition name in mmc_part.
+ */
+
+ if (ext_csd[EXT_CSD_PARTITION_SUPPORT] &
+ EXT_CSD_PART_SUPPORT_PART_EN) {
+ if (card->ext_csd.enhanced_area_en != 1) {
+ hc_erase_grp_sz =
+ ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
+ hc_wp_grp_sz =
+ ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+
+ card->ext_csd.enhanced_area_en = 1;
+ }
+
+ for (idx = 0; idx < MMC_NUM_GP_PARTITION; idx++) {
+ if (!ext_csd[EXT_CSD_GP_SIZE_MULT + idx * 3] &&
+ !ext_csd[EXT_CSD_GP_SIZE_MULT + idx * 3 + 1] &&
+ !ext_csd[EXT_CSD_GP_SIZE_MULT + idx * 3 + 2])
+ continue;
+ part_size =
+ (ext_csd[EXT_CSD_GP_SIZE_MULT + idx * 3 + 2] << 16) +
+ (ext_csd[EXT_CSD_GP_SIZE_MULT + idx * 3 + 1] << 8) +
+ ext_csd[EXT_CSD_GP_SIZE_MULT + idx * 3];
+ part_size *= (size_t)(hc_erase_grp_sz *
+ hc_wp_grp_sz);
+ mmc_part_add(card, part_size << 19,
+ EXT_CSD_PART_CONFIG_ACC_GP0 + idx,
+ "gp%d", idx, 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..8e42573 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,23 @@ 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
+#define MMC_NUM_PHY_PARTITION 6
+
+/*
+ * MMC Physical partitions
+ */
+struct mmc_part {
+ unsigned int size; /* partition size (in bytes) */
+ unsigned int part_cfg; /* it used to part_type */
+ char name[DISK_NAME_LEN];
+ bool force_ro; /* to make boot parts RO by default */
+};
+
/*
* MMC device
*/
@@ -216,9 +233,24 @@ struct mmc_card {
unsigned int sd_bus_speed; /* Bus Speed Mode set for the card */
struct dentry *debugfs_root;
+ struct mmc_part part[MMC_NUM_PHY_PARTITION]; /* physical partitions */
+ unsigned int nr_parts;
};
/*
+ * This function fill contents in mmc_part.
+ */
+static inline void mmc_part_add(struct mmc_card *card, unsigned int size,
+ unsigned int part_cfg, char *name, int idx, bool ro)
+{
+ card->part[card->nr_parts].size = size;
+ card->part[card->nr_parts].part_cfg = part_cfg;
+ sprintf(card->part[card->nr_parts].name, name, idx);
+ card->part[card->nr_parts].force_ro = ro;
+ card->nr_parts++;
+}
+
+/*
* The world is not perfect and supplies us with broken mmc/sdio devices.
* For at least some of these bugs we need a work-around.
*/
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 5a794cb..ac0422a 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -270,6 +270,7 @@ struct _mmc_csd {
* EXT_CSD fields
*/
+#define EXT_CSD_GP_SIZE_MULT 143 /* R/W */
#define EXT_CSD_PARTITION_ATTRIBUTE 156 /* R/W */
#define EXT_CSD_PARTITION_SUPPORT 160 /* RO */
#define EXT_CSD_WR_REL_PARAM 166 /* RO */
@@ -302,7 +303,9 @@ struct _mmc_csd {
#define EXT_CSD_PART_CONFIG_ACC_MASK (0x7)
#define EXT_CSD_PART_CONFIG_ACC_BOOT0 (0x1)
-#define EXT_CSD_PART_CONFIG_ACC_BOOT1 (0x2)
+#define EXT_CSD_PART_CONFIG_ACC_GP0 (0x4)
+
+#define EXT_CSD_PART_SUPPORT_PART_EN (0x1)
#define EXT_CSD_CMD_SET_NORMAL (1<<0)
#define EXT_CSD_CMD_SET_SECURE (1<<1)
--
1.7.4.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2011-10-04 16:17 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-22 15:34 [PATCH] mmc : general purpose partition support Namjae Jeon
2011-09-22 22:08 ` J Freyensee
2011-09-22 23:15 ` NamJae Jeon
2011-09-22 23:15 ` NamJae Jeon
2011-09-22 23:25 ` J Freyensee
2011-09-22 23:58 ` NamJae Jeon
2011-09-22 23:58 ` NamJae Jeon
2011-09-23 0:19 ` J Freyensee
2011-09-23 0:29 ` NamJae Jeon
2011-09-23 0:29 ` NamJae Jeon
[not found] ` <4E7CB5FC.3090301@linux.intel.com>
2011-09-24 5:21 ` NamJae Jeon
2011-09-24 5:21 ` NamJae Jeon
2011-09-23 3:25 ` Andrei Warkentin
2011-09-23 3:56 ` NamJae Jeon
2011-10-04 16:16 Namjae Jeon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.