[PATCH] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions

[PATCH] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions

[Grégory Soutadé]

Create MMC general purpose partitions only if
EXT_CSD_PARTITION_SETTING_COMPLETED bit is set.
Some tools may set general purpose partition size(s) but fail or stop
without finalize.
Another case is to set invalid partition size(s).

Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
---
 drivers/mmc/core/mmc.c  |   15 +++++++++++----
 include/linux/mmc/mmc.h |    2 ++
 2 files changed, 13 insertions(+), 4 deletions(-)

>From commit b6603fe574af289dbe9eb9fb4c540bca04f5a053 in master linux tree.

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 793c6f7..b9fe211 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -471,10 +471,17 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 				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,
-					MMC_BLK_DATA_AREA_GP);
+				if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
+				EXT_CSD_PART_SETTING_COMPLETED) {
+					mmc_part_add(card, part_size << 19,
+						EXT_CSD_PART_CONFIG_ACC_GP0 + idx,
+						"gp%d", idx, false,
+						MMC_BLK_DATA_AREA_GP);
+				} else {
+					pr_warn("%s: has partition size defined, but setting is not complete\n",
+						mmc_hostname(card->host));
+					break;
+				}
 			}
 		}
 		card->ext_csd.sec_trim_mult =
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 64ec963..78753bc 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -281,6 +281,7 @@ struct _mmc_csd {
 #define EXT_CSD_EXP_EVENTS_CTRL		56	/* R/W, 2 bytes */
 #define EXT_CSD_DATA_SECTOR_SIZE	61	/* R */
 #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
+#define EXT_CSD_PARTITION_SETTING_COMPLETED 155	/* R/W */
 #define EXT_CSD_PARTITION_ATTRIBUTE	156	/* R/W */
 #define EXT_CSD_PARTITION_SUPPORT	160	/* RO */
 #define EXT_CSD_HPI_MGMT		161	/* R/W */
@@ -349,6 +350,7 @@ struct _mmc_csd {
 #define EXT_CSD_PART_CONFIG_ACC_RPMB	(0x3)
 #define EXT_CSD_PART_CONFIG_ACC_GP0	(0x4)

+#define EXT_CSD_PART_SETTING_COMPLETED	(0x1)
 #define EXT_CSD_PART_SUPPORT_PART_EN	(0x1)

 #define EXT_CSD_CMD_SET_NORMAL		(1<<0)
-- 
1.7.0.4

[PATCH] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions

[Grégory Soutadé]

Create MMC general purpose partitions only if
EXT_CSD_PARTITION_SETTING_COMPLETED bit is set.
Some tools may set general purpose partition size(s) but fail or stop
without finalize.
Another case is to set invalid partition size(s).

Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
---
 drivers/mmc/core/mmc.c  |   15 +++++++++++----
 include/linux/mmc/mmc.h |    2 ++
 2 files changed, 13 insertions(+), 4 deletions(-)

From commit b6603fe574af289dbe9eb9fb4c540bca04f5a053 in master linux tree.

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 793c6f7..b9fe211 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -471,10 +471,17 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 				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,
-					MMC_BLK_DATA_AREA_GP);
+				if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
+				EXT_CSD_PART_SETTING_COMPLETED) {
+					mmc_part_add(card, part_size << 19,
+						EXT_CSD_PART_CONFIG_ACC_GP0 + idx,
+						"gp%d", idx, false,
+						MMC_BLK_DATA_AREA_GP);
+				} else {
+					pr_warn("%s: has partition size defined, but setting is not complete\n",
+						mmc_hostname(card->host));
+					break;
+				}
 			}
 		}
 		card->ext_csd.sec_trim_mult =
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 64ec963..78753bc 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -281,6 +281,7 @@ struct _mmc_csd {
 #define EXT_CSD_EXP_EVENTS_CTRL		56	/* R/W, 2 bytes */
 #define EXT_CSD_DATA_SECTOR_SIZE	61	/* R */
 #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
+#define EXT_CSD_PARTITION_SETTING_COMPLETED 155	/* R/W */
 #define EXT_CSD_PARTITION_ATTRIBUTE	156	/* R/W */
 #define EXT_CSD_PARTITION_SUPPORT	160	/* RO */
 #define EXT_CSD_HPI_MGMT		161	/* R/W */
@@ -349,6 +350,7 @@ struct _mmc_csd {
 #define EXT_CSD_PART_CONFIG_ACC_RPMB	(0x3)
 #define EXT_CSD_PART_CONFIG_ACC_GP0	(0x4)

+#define EXT_CSD_PART_SETTING_COMPLETED	(0x1)
 #define EXT_CSD_PART_SUPPORT_PART_EN	(0x1)

 #define EXT_CSD_CMD_SET_NORMAL		(1<<0)
-- 
1.7.0.4

Re: [PATCH] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions

[Ulf Hansson]

On 17 July 2014 16:57, Grégory Soutadé <gsoutade@neotion.com> wrote:
> Create MMC general purpose partitions only if
> EXT_CSD_PARTITION_SETTING_COMPLETED bit is set.
> Some tools may set general purpose partition size(s) but fail or stop
> without finalize.
> Another case is to set invalid partition size(s).
>
> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
> ---
>  drivers/mmc/core/mmc.c  |   15 +++++++++++----
>  include/linux/mmc/mmc.h |    2 ++
>  2 files changed, 13 insertions(+), 4 deletions(-)
>
> From commit b6603fe574af289dbe9eb9fb4c540bca04f5a053 in master linux tree.
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 793c6f7..b9fe211 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -471,10 +471,17 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>                                 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,
> -                                       MMC_BLK_DATA_AREA_GP);
> +                               if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
> +                               EXT_CSD_PART_SETTING_COMPLETED) {

Some minor comments here.

I think you could move this if statement up and into the previous "if"
where we check for "ext_csd[EXT_CSD_PARTITION_SUPPORT] &
EXT_CSD_PART_SUPPORT_PART_EN".

Additionally, please run checkpatch.

Kind regards
Uffe

> +                                       mmc_part_add(card, part_size << 19,
> +                                               EXT_CSD_PART_CONFIG_ACC_GP0 + idx,
> +                                               "gp%d", idx, false,
> +                                               MMC_BLK_DATA_AREA_GP);
> +                               } else {
> +                                       pr_warn("%s: has partition size defined, but setting is not complete\n",
> +                                               mmc_hostname(card->host));
> +                                       break;
> +                               }
>                         }
>                 }
>                 card->ext_csd.sec_trim_mult =
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> index 64ec963..78753bc 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -281,6 +281,7 @@ struct _mmc_csd {
>  #define EXT_CSD_EXP_EVENTS_CTRL                56      /* R/W, 2 bytes */
>  #define EXT_CSD_DATA_SECTOR_SIZE       61      /* R */
>  #define EXT_CSD_GP_SIZE_MULT           143     /* R/W */
> +#define EXT_CSD_PARTITION_SETTING_COMPLETED 155        /* R/W */
>  #define EXT_CSD_PARTITION_ATTRIBUTE    156     /* R/W */
>  #define EXT_CSD_PARTITION_SUPPORT      160     /* RO */
>  #define EXT_CSD_HPI_MGMT               161     /* R/W */
> @@ -349,6 +350,7 @@ struct _mmc_csd {
>  #define EXT_CSD_PART_CONFIG_ACC_RPMB   (0x3)
>  #define EXT_CSD_PART_CONFIG_ACC_GP0    (0x4)
>
> +#define EXT_CSD_PART_SETTING_COMPLETED (0x1)
>  #define EXT_CSD_PART_SUPPORT_PART_EN   (0x1)
>
>  #define EXT_CSD_CMD_SET_NORMAL         (1<<0)
> --
> 1.7.0.4

Re: [PATCH] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions

[Grégory Soutadé]

[-- Attachment #1: Type: text/plain, Size: 3979 bytes --]

Le 13/08/2014 10:36, Ulf Hansson a écrit :
> On 17 July 2014 16:57, Grégory Soutadé <gsoutade@neotion.com> wrote:
>> Create MMC general purpose partitions only if
>> EXT_CSD_PARTITION_SETTING_COMPLETED bit is set.
>> Some tools may set general purpose partition size(s) but fail or stop
>> without finalize.
>> Another case is to set invalid partition size(s).
>>
>> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
>> ---
>>  drivers/mmc/core/mmc.c  |   15 +++++++++++----
>>  include/linux/mmc/mmc.h |    2 ++
>>  2 files changed, 13 insertions(+), 4 deletions(-)
>>
>> From commit b6603fe574af289dbe9eb9fb4c540bca04f5a053 in master linux tree.
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 793c6f7..b9fe211 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -471,10 +471,17 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>                                 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,
>> -                                       MMC_BLK_DATA_AREA_GP);
>> +                               if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
>> +                               EXT_CSD_PART_SETTING_COMPLETED) {
> 
> Some minor comments here.
> 
> I think you could move this if statement up and into the previous "if"
> where we check for "ext_csd[EXT_CSD_PARTITION_SUPPORT] &
> EXT_CSD_PART_SUPPORT_PART_EN".
> 
> Additionally, please run checkpatch.
> 
> Kind regards
> Uffe

Hello,

I didn't put the if statement above to warn user in case of bad partitioning.
I don't want the kernel to silently ignore this error.

checkpatch has been run before sending this patch, I know there are two lines
with two extra characters, but names used here are quite long. I hope it will
not block upstream inclusion.


Best regards
Grégory Soutadé

> 
>> +                                       mmc_part_add(card, part_size << 19,
>> +                                               EXT_CSD_PART_CONFIG_ACC_GP0 + idx,
>> +                                               "gp%d", idx, false,
>> +                                               MMC_BLK_DATA_AREA_GP);
>> +                               } else {
>> +                                       pr_warn("%s: has partition size defined, but setting is not complete\n",
>> +                                               mmc_hostname(card->host));
>> +                                       break;
>> +                               }
>>                         }
>>                 }
>>                 card->ext_csd.sec_trim_mult =
>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>> index 64ec963..78753bc 100644
>> --- a/include/linux/mmc/mmc.h
>> +++ b/include/linux/mmc/mmc.h
>> @@ -281,6 +281,7 @@ struct _mmc_csd {
>>  #define EXT_CSD_EXP_EVENTS_CTRL                56      /* R/W, 2 bytes */
>>  #define EXT_CSD_DATA_SECTOR_SIZE       61      /* R */
>>  #define EXT_CSD_GP_SIZE_MULT           143     /* R/W */
>> +#define EXT_CSD_PARTITION_SETTING_COMPLETED 155        /* R/W */
>>  #define EXT_CSD_PARTITION_ATTRIBUTE    156     /* R/W */
>>  #define EXT_CSD_PARTITION_SUPPORT      160     /* RO */
>>  #define EXT_CSD_HPI_MGMT               161     /* R/W */
>> @@ -349,6 +350,7 @@ struct _mmc_csd {
>>  #define EXT_CSD_PART_CONFIG_ACC_RPMB   (0x3)
>>  #define EXT_CSD_PART_CONFIG_ACC_GP0    (0x4)
>>
>> +#define EXT_CSD_PART_SETTING_COMPLETED (0x1)
>>  #define EXT_CSD_PART_SUPPORT_PART_EN   (0x1)
>>
>>  #define EXT_CSD_CMD_SET_NORMAL         (1<<0)
>> --
>> 1.7.0.4
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions

[Ulf Hansson]

On 13 August 2014 11:20, Grégory Soutadé <gsoutade@neotion.com> wrote:
> Le 13/08/2014 10:36, Ulf Hansson a écrit :
>> On 17 July 2014 16:57, Grégory Soutadé <gsoutade@neotion.com> wrote:
>>> Create MMC general purpose partitions only if
>>> EXT_CSD_PARTITION_SETTING_COMPLETED bit is set.
>>> Some tools may set general purpose partition size(s) but fail or stop
>>> without finalize.
>>> Another case is to set invalid partition size(s).
>>>
>>> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
>>> ---
>>>  drivers/mmc/core/mmc.c  |   15 +++++++++++----
>>>  include/linux/mmc/mmc.h |    2 ++
>>>  2 files changed, 13 insertions(+), 4 deletions(-)
>>>
>>> From commit b6603fe574af289dbe9eb9fb4c540bca04f5a053 in master linux tree.
>>>
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index 793c6f7..b9fe211 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -471,10 +471,17 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>>                                 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,
>>> -                                       MMC_BLK_DATA_AREA_GP);
>>> +                               if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
>>> +                               EXT_CSD_PART_SETTING_COMPLETED) {
>>
>> Some minor comments here.
>>
>> I think you could move this if statement up and into the previous "if"
>> where we check for "ext_csd[EXT_CSD_PARTITION_SUPPORT] &
>> EXT_CSD_PART_SUPPORT_PART_EN".
>>
>> Additionally, please run checkpatch.
>>
>> Kind regards
>> Uffe
>
> Hello,
>
> I didn't put the if statement above to warn user in case of bad partitioning.
> I don't want the kernel to silently ignore this error.

Fair enough.

Still I am wondering whether hc_erase_grp_sz, hc_wp_grp_sz and
enhanced_area_en should be updated if
EXT_CSD_PARTITION_SETTING_COMPLETED isn't set.  That's the case in
your patch.

>
> checkpatch has been run before sending this patch, I know there are two lines
> with two extra characters, but names used here are quite long. I hope it will
> not block upstream inclusion.

The mmc_read_ext_csd() is not one of the nicest piece of code, but for
sure we should not move on making it worse. If you need to move code
into separate function to prevent checkpatch warnings, please do so.

Kind regards
Uffe

Re: [PATCH] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions

[Grégory Soutadé]

[-- Attachment #1: Type: text/plain, Size: 3721 bytes --]

Le 14/08/2014 13:46, Ulf Hansson a écrit :
> On 13 August 2014 11:20, Grégory Soutadé <gsoutade@neotion.com> wrote:
>> Le 13/08/2014 10:36, Ulf Hansson a écrit :
>>> On 17 July 2014 16:57, Grégory Soutadé <gsoutade@neotion.com> wrote:
>>>> Create MMC general purpose partitions only if
>>>> EXT_CSD_PARTITION_SETTING_COMPLETED bit is set.
>>>> Some tools may set general purpose partition size(s) but fail or stop
>>>> without finalize.
>>>> Another case is to set invalid partition size(s).
>>>>
>>>> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
>>>> ---
>>>>  drivers/mmc/core/mmc.c  |   15 +++++++++++----
>>>>  include/linux/mmc/mmc.h |    2 ++
>>>>  2 files changed, 13 insertions(+), 4 deletions(-)
>>>>
>>>> From commit b6603fe574af289dbe9eb9fb4c540bca04f5a053 in master linux tree.
>>>>
>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>> index 793c6f7..b9fe211 100644
>>>> --- a/drivers/mmc/core/mmc.c
>>>> +++ b/drivers/mmc/core/mmc.c
>>>> @@ -471,10 +471,17 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>>>                                 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,
>>>> -                                       MMC_BLK_DATA_AREA_GP);
>>>> +                               if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
>>>> +                               EXT_CSD_PART_SETTING_COMPLETED) {
>>>
>>> Some minor comments here.
>>>
>>> I think you could move this if statement up and into the previous "if"
>>> where we check for "ext_csd[EXT_CSD_PARTITION_SUPPORT] &
>>> EXT_CSD_PART_SUPPORT_PART_EN".
>>>
>>> Additionally, please run checkpatch.
>>>
>>> Kind regards
>>> Uffe
>>
>> Hello,
>>
>> I didn't put the if statement above to warn user in case of bad partitioning.
>> I don't want the kernel to silently ignore this error.
> 
> Fair enough.
> 
> Still I am wondering whether hc_erase_grp_sz, hc_wp_grp_sz and
> enhanced_area_en should be updated if
> EXT_CSD_PARTITION_SETTING_COMPLETED isn't set.  That's the case in
> your patch.

I was focused on partitions and I didn't pay attention on enhanced area.

JEDEC says that partitioning implies :
* Configure general purpose partitions attributes and sizes
* Configure enhanced user area offset, attributes and size

and finally set EXT_CSD_PARTITION_SETTING_COMPLETED.

Thus these two parts must checks for setting completed before
computing values.

Plus, "enhanced_area_en" attribute is set whether or not there is an
enhanced area defined. I looked at the code, and the only usage of it
is to set EXT_CSD_ERASE_GROUP_DEF and compute erase size again.
I suggest using "partition_setting_completed" identifier which is common
to the two functions and requires EXT_CSD_ERASE_GROUP_DEF to be set.

If you're ok with that, I'll submit another patch.


Best regards
Grégory Soutadé

> 
>>
>> checkpatch has been run before sending this patch, I know there are two lines
>> with two extra characters, but names used here are quite long. I hope it will
>> not block upstream inclusion.
> 
> The mmc_read_ext_csd() is not one of the nicest piece of code, but for
> sure we should not move on making it worse. If you need to move code
> into separate function to prevent checkpatch warnings, please do so.
> 
> Kind regards
> Uffe
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions

[Ulf Hansson]

On 14 August 2014 15:27, Grégory Soutadé <gsoutade@neotion.com> wrote:
> Le 14/08/2014 13:46, Ulf Hansson a écrit :
>> On 13 August 2014 11:20, Grégory Soutadé <gsoutade@neotion.com> wrote:
>>> Le 13/08/2014 10:36, Ulf Hansson a écrit :
>>>> On 17 July 2014 16:57, Grégory Soutadé <gsoutade@neotion.com> wrote:
>>>>> Create MMC general purpose partitions only if
>>>>> EXT_CSD_PARTITION_SETTING_COMPLETED bit is set.
>>>>> Some tools may set general purpose partition size(s) but fail or stop
>>>>> without finalize.
>>>>> Another case is to set invalid partition size(s).
>>>>>
>>>>> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
>>>>> ---
>>>>>  drivers/mmc/core/mmc.c  |   15 +++++++++++----
>>>>>  include/linux/mmc/mmc.h |    2 ++
>>>>>  2 files changed, 13 insertions(+), 4 deletions(-)
>>>>>
>>>>> From commit b6603fe574af289dbe9eb9fb4c540bca04f5a053 in master linux tree.
>>>>>
>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>> index 793c6f7..b9fe211 100644
>>>>> --- a/drivers/mmc/core/mmc.c
>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>> @@ -471,10 +471,17 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>>>>                                 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,
>>>>> -                                       MMC_BLK_DATA_AREA_GP);
>>>>> +                               if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
>>>>> +                               EXT_CSD_PART_SETTING_COMPLETED) {
>>>>
>>>> Some minor comments here.
>>>>
>>>> I think you could move this if statement up and into the previous "if"
>>>> where we check for "ext_csd[EXT_CSD_PARTITION_SUPPORT] &
>>>> EXT_CSD_PART_SUPPORT_PART_EN".
>>>>
>>>> Additionally, please run checkpatch.
>>>>
>>>> Kind regards
>>>> Uffe
>>>
>>> Hello,
>>>
>>> I didn't put the if statement above to warn user in case of bad partitioning.
>>> I don't want the kernel to silently ignore this error.
>>
>> Fair enough.
>>
>> Still I am wondering whether hc_erase_grp_sz, hc_wp_grp_sz and
>> enhanced_area_en should be updated if
>> EXT_CSD_PARTITION_SETTING_COMPLETED isn't set.  That's the case in
>> your patch.
>
> I was focused on partitions and I didn't pay attention on enhanced area.
>
> JEDEC says that partitioning implies :
> * Configure general purpose partitions attributes and sizes
> * Configure enhanced user area offset, attributes and size
>
> and finally set EXT_CSD_PARTITION_SETTING_COMPLETED.
>
> Thus these two parts must checks for setting completed before
> computing values.
>
> Plus, "enhanced_area_en" attribute is set whether or not there is an
> enhanced area defined. I looked at the code, and the only usage of it
> is to set EXT_CSD_ERASE_GROUP_DEF and compute erase size again.
> I suggest using "partition_setting_completed" identifier which is common
> to the two functions and requires EXT_CSD_ERASE_GROUP_DEF to be set.
>
> If you're ok with that, I'll submit another patch.

Seems reasonable. Please send a v2.

Kind regards
Uffe

>
>
> Best regards
> Grégory Soutadé
>
>>
>>>
>>> checkpatch has been run before sending this patch, I know there are two lines
>>> with two extra characters, but names used here are quite long. I hope it will
>>> not block upstream inclusion.
>>
>> The mmc_read_ext_csd() is not one of the nicest piece of code, but for
>> sure we should not move on making it worse. If you need to move code
>> into separate function to prevent checkpatch warnings, please do so.
>>
>> Kind regards
>> Uffe
>>
>

[PATCH v3 0001/0002] mmc: Replace ext_csd "enhanced_area_en" attribute by "partition_setting_completed"

[Grégory Soutadé]

Replace ext_csd "enhanced_area_en" attribute by "partition_setting_completed".
It was used whether or not enhanced user area is defined and without checks of
EXT_CSD_PARTITION_SETTING_COMPLETED bit.

Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
---
 drivers/mmc/core/mmc.c   |    8 +++++++-
 include/linux/mmc/card.h |    2 +-
 include/linux/mmc/mmc.h  |    2 ++
 3 files changed, 10 insertions(+), 2 deletions(-)

>From commit 33caee39925b887a99a2400dc5c980097c3573f9 in master linux tree.

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 793c6f7..ef25d91 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -403,6 +403,12 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 		ext_csd[EXT_CSD_TRIM_MULT];
 	card->ext_csd.raw_partition_support = ext_csd[EXT_CSD_PARTITION_SUPPORT];
 	if (card->ext_csd.rev >= 4) {
+		if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
+		    EXT_CSD_PART_SETTING_COMPLETED) {
+			card->ext_csd.partition_setting_completed = 1;
+		} else {
+			card->ext_csd.partition_setting_completed = 0;
+		}
 		/*
 		 * Enhanced area feature support -- check whether the eMMC
 		 * card has the Enhanced area enabled.  If so, export enhanced
@@ -1309,7 +1315,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	 * If enhanced_area_en is TRUE, host needs to enable ERASE_GRP_DEF
 	 * bit.  This bit will be lost every time after a reset or power off.
 	 */
-	if (card->ext_csd.enhanced_area_en ||
+	if (card->ext_csd.partition_setting_completed ||
 	    (card->ext_csd.rev >= 3 && (host->caps2 & MMC_CAP2_HC_ERASE_SZ))) {
 		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 				 EXT_CSD_ERASE_GROUP_DEF, 1,
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index d424b9d..38175be 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -74,7 +74,7 @@ struct mmc_ext_csd {
 	unsigned int		sec_trim_mult;	/* Secure trim multiplier  */
 	unsigned int		sec_erase_mult;	/* Secure erase multiplier */
 	unsigned int		trim_timeout;		/* In milliseconds */
-	bool			enhanced_area_en;	/* enable bit */
+	bool			partition_setting_completed;	/* enable bit */
 	unsigned long long	enhanced_area_offset;	/* Units: Byte */
 	unsigned int		enhanced_area_size;	/* Units: KB */
 	unsigned int		cache_size;		/* Units: KB */
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 64ec963..78753bc 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -281,6 +281,7 @@ struct _mmc_csd {
 #define EXT_CSD_EXP_EVENTS_CTRL		56	/* R/W, 2 bytes */
 #define EXT_CSD_DATA_SECTOR_SIZE	61	/* R */
 #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
+#define EXT_CSD_PARTITION_SETTING_COMPLETED 155	/* R/W */
 #define EXT_CSD_PARTITION_ATTRIBUTE	156	/* R/W */
 #define EXT_CSD_PARTITION_SUPPORT	160	/* RO */
 #define EXT_CSD_HPI_MGMT		161	/* R/W */
@@ -349,6 +350,7 @@ struct _mmc_csd {
 #define EXT_CSD_PART_CONFIG_ACC_RPMB	(0x3)
 #define EXT_CSD_PART_CONFIG_ACC_GP0	(0x4)

+#define EXT_CSD_PART_SETTING_COMPLETED	(0x1)
 #define EXT_CSD_PART_SUPPORT_PART_EN	(0x1)

 #define EXT_CSD_CMD_SET_NORMAL		(1<<0)
--
1.7.9.5

Le 15/08/2014 10:17, Ulf Hansson a écrit :
> On 14 August 2014 15:27, Grégory Soutadé <gsoutade@neotion.com> wrote:
>> Le 14/08/2014 13:46, Ulf Hansson a écrit :
>>> On 13 August 2014 11:20, Grégory Soutadé <gsoutade@neotion.com> wrote:
>>>> Le 13/08/2014 10:36, Ulf Hansson a écrit :
>>>>> On 17 July 2014 16:57, Grégory Soutadé <gsoutade@neotion.com> wrote:
>>>>>> Create MMC general purpose partitions only if
>>>>>> EXT_CSD_PARTITION_SETTING_COMPLETED bit is set.
>>>>>> Some tools may set general purpose partition size(s) but fail or stop
>>>>>> without finalize.
>>>>>> Another case is to set invalid partition size(s).
>>>>>>
>>>>>> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
>>>>>> ---
>>>>>>  drivers/mmc/core/mmc.c  |   15 +++++++++++----
>>>>>>  include/linux/mmc/mmc.h |    2 ++
>>>>>>  2 files changed, 13 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> From commit b6603fe574af289dbe9eb9fb4c540bca04f5a053 in master linux tree.
>>>>>>
>>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>>> index 793c6f7..b9fe211 100644
>>>>>> --- a/drivers/mmc/core/mmc.c
>>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>>> @@ -471,10 +471,17 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>>>>>                                 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,
>>>>>> -                                       MMC_BLK_DATA_AREA_GP);
>>>>>> +                               if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
>>>>>> +                               EXT_CSD_PART_SETTING_COMPLETED) {
>>>>>
>>>>> Some minor comments here.
>>>>>
>>>>> I think you could move this if statement up and into the previous "if"
>>>>> where we check for "ext_csd[EXT_CSD_PARTITION_SUPPORT] &
>>>>> EXT_CSD_PART_SUPPORT_PART_EN".
>>>>>
>>>>> Additionally, please run checkpatch.
>>>>>
>>>>> Kind regards
>>>>> Uffe
>>>>
>>>> Hello,
>>>>
>>>> I didn't put the if statement above to warn user in case of bad partitioning.
>>>> I don't want the kernel to silently ignore this error.
>>>
>>> Fair enough.
>>>
>>> Still I am wondering whether hc_erase_grp_sz, hc_wp_grp_sz and
>>> enhanced_area_en should be updated if
>>> EXT_CSD_PARTITION_SETTING_COMPLETED isn't set.  That's the case in
>>> your patch.
>>
>> I was focused on partitions and I didn't pay attention on enhanced area.
>>
>> JEDEC says that partitioning implies :
>> * Configure general purpose partitions attributes and sizes
>> * Configure enhanced user area offset, attributes and size
>>
>> and finally set EXT_CSD_PARTITION_SETTING_COMPLETED.
>>
>> Thus these two parts must checks for setting completed before
>> computing values.
>>
>> Plus, "enhanced_area_en" attribute is set whether or not there is an
>> enhanced area defined. I looked at the code, and the only usage of it
>> is to set EXT_CSD_ERASE_GROUP_DEF and compute erase size again.
>> I suggest using "partition_setting_completed" identifier which is common
>> to the two functions and requires EXT_CSD_ERASE_GROUP_DEF to be set.
>>
>> If you're ok with that, I'll submit another patch.
> 
> Seems reasonable. Please send a v2.
> 
> Kind regards
> Uffe
> 
>>
>>
>> Best regards
>> Grégory Soutadé
>>
>>>
>>>>
>>>> checkpatch has been run before sending this patch, I know there are two lines
>>>> with two extra characters, but names used here are quite long. I hope it will
>>>> not block upstream inclusion.
>>>
>>> The mmc_read_ext_csd() is not one of the nicest piece of code, but for
>>> sure we should not move on making it worse. If you need to move code
>>> into separate function to prevent checkpatch warnings, please do so.
>>>
>>> Kind regards
>>> Uffe
>>>
>>
> 

[PATCH v4 0001/0003] mmc: Move code that manages user area and gp partitions into functions

[Grégory Soutadé]

Move code that manages user area and general purpose
 partitions into functions.

Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
---
 drivers/mmc/core/mmc.c |  162 ++++++++++++++++++++++++++----------------------
 1 file changed, 89 insertions(+), 73 deletions(-)

>From commit 480cadc2b7e0fa2bbab20141efb547dfe0c3707c in master linux tree.

Changelog v3:
	Move code BEFORE fixing bugs.
Changelog v2:
	Move code for user area and general purpose partitions
	into functions.

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 1eda8dd..77b4cf3 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -298,6 +298,93 @@ static void mmc_select_card_type(struct mmc_card *card)
 	card->mmc_avail_type = avail_type;
 }

+static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
+{
+	u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
+
+	/*
+	 * Enhanced area feature support -- check whether the eMMC
+	 * card has the Enhanced area enabled.  If so, export enhanced
+	 * area offset and size to user by adding sysfs interface.
+	 */
+	if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
+	    (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
+		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;
+		/*
+		 * calculate the enhanced data area offset, in bytes
+		 */
+		card->ext_csd.enhanced_area_offset =
+			(ext_csd[139] << 24) + (ext_csd[138] << 16) +
+			(ext_csd[137] << 8) + ext_csd[136];
+		if (mmc_card_blockaddr(card))
+			card->ext_csd.enhanced_area_offset <<= 9;
+		/*
+		 * calculate the enhanced data area size, in kilobytes
+		 */
+		card->ext_csd.enhanced_area_size =
+			(ext_csd[142] << 16) + (ext_csd[141] << 8) +
+			ext_csd[140];
+		card->ext_csd.enhanced_area_size *=
+			(size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
+		card->ext_csd.enhanced_area_size <<= 9;
+	} else {
+		/*
+		 * If the enhanced area is not enabled, disable these
+		 * device attributes.
+		 */
+		card->ext_csd.enhanced_area_offset = -EINVAL;
+		card->ext_csd.enhanced_area_size = -EINVAL;
+	}
+}
+
+static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
+{
+	unsigned int part_size;
+	u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
+	int idx;
+
+	/*
+	 * General purpose partition feature support --
+	 * If ext_csd has 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,
+				MMC_BLK_DATA_AREA_GP);
+		}
+	}
+}
+
 /*
  * Decode extended CSD.
  */
@@ -305,7 +392,6 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 {
 	int err = 0, idx;
 	unsigned int part_size;
-	u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;

 	BUG_ON(!card);

@@ -402,80 +488,10 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 		ext_csd[EXT_CSD_TRIM_MULT];
 	card->ext_csd.raw_partition_support = ext_csd[EXT_CSD_PARTITION_SUPPORT];
 	if (card->ext_csd.rev >= 4) {
-		/*
-		 * Enhanced area feature support -- check whether the eMMC
-		 * card has the Enhanced area enabled.  If so, export enhanced
-		 * area offset and size to user by adding sysfs interface.
-		 */
-		if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
-		    (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
-			hc_erase_grp_sz =
-				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
-			hc_wp_grp_sz =
-				ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+		mmc_manage_enhanced_area(card, ext_csd);

-			card->ext_csd.enhanced_area_en = 1;
-			/*
-			 * calculate the enhanced data area offset, in bytes
-			 */
-			card->ext_csd.enhanced_area_offset =
-				(ext_csd[139] << 24) + (ext_csd[138] << 16) +
-				(ext_csd[137] << 8) + ext_csd[136];
-			if (mmc_card_blockaddr(card))
-				card->ext_csd.enhanced_area_offset <<= 9;
-			/*
-			 * calculate the enhanced data area size, in kilobytes
-			 */
-			card->ext_csd.enhanced_area_size =
-				(ext_csd[142] << 16) + (ext_csd[141] << 8) +
-				ext_csd[140];
-			card->ext_csd.enhanced_area_size *=
-				(size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
-			card->ext_csd.enhanced_area_size <<= 9;
-		} else {
-			/*
-			 * If the enhanced area is not enabled, disable these
-			 * device attributes.
-			 */
-			card->ext_csd.enhanced_area_offset = -EINVAL;
-			card->ext_csd.enhanced_area_size = -EINVAL;
-		}
+		mmc_manage_gp_partitions(card, ext_csd);

-		/*
-		 * General purpose partition feature support --
-		 * If ext_csd has 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,
-					MMC_BLK_DATA_AREA_GP);
-			}
-		}
 		card->ext_csd.sec_trim_mult =
 			ext_csd[EXT_CSD_SEC_TRIM_MULT];
 		card->ext_csd.sec_erase_mult =
-- 
1.7.9.5

> Replace ext_csd "enhanced_area_en" attribute by "partition_setting_completed".
> It was used whether or not enhanced user area is defined and without checks of
> EXT_CSD_PARTITION_SETTING_COMPLETED bit.
> 
> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
> ---
>  drivers/mmc/core/mmc.c   |    8 +++++++-
>  include/linux/mmc/card.h |    2 +-
>  include/linux/mmc/mmc.h  |    2 ++
>  3 files changed, 10 insertions(+), 2 deletions(-)
> 
>>From commit 33caee39925b887a99a2400dc5c980097c3573f9 in master linux tree.
> 
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 793c6f7..ef25d91 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -403,6 +403,12 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>  		ext_csd[EXT_CSD_TRIM_MULT];
>  	card->ext_csd.raw_partition_support = ext_csd[EXT_CSD_PARTITION_SUPPORT];
>  	if (card->ext_csd.rev >= 4) {
> +		if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
> +		    EXT_CSD_PART_SETTING_COMPLETED) {
> +			card->ext_csd.partition_setting_completed = 1;
> +		} else {
> +			card->ext_csd.partition_setting_completed = 0;
> +		}
>  		/*
>  		 * Enhanced area feature support -- check whether the eMMC
>  		 * card has the Enhanced area enabled.  If so, export enhanced
> @@ -1309,7 +1315,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>  	 * If enhanced_area_en is TRUE, host needs to enable ERASE_GRP_DEF
>  	 * bit.  This bit will be lost every time after a reset or power off.
>  	 */
> -	if (card->ext_csd.enhanced_area_en ||
> +	if (card->ext_csd.partition_setting_completed ||
>  	    (card->ext_csd.rev >= 3 && (host->caps2 & MMC_CAP2_HC_ERASE_SZ))) {
>  		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>  				 EXT_CSD_ERASE_GROUP_DEF, 1,
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index d424b9d..38175be 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -74,7 +74,7 @@ struct mmc_ext_csd {
>  	unsigned int		sec_trim_mult;	/* Secure trim multiplier  */
>  	unsigned int		sec_erase_mult;	/* Secure erase multiplier */
>  	unsigned int		trim_timeout;		/* In milliseconds */
> -	bool			enhanced_area_en;	/* enable bit */
> +	bool			partition_setting_completed;	/* enable bit */
>  	unsigned long long	enhanced_area_offset;	/* Units: Byte */
>  	unsigned int		enhanced_area_size;	/* Units: KB */
>  	unsigned int		cache_size;		/* Units: KB */
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> index 64ec963..78753bc 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -281,6 +281,7 @@ struct _mmc_csd {
>  #define EXT_CSD_EXP_EVENTS_CTRL		56	/* R/W, 2 bytes */
>  #define EXT_CSD_DATA_SECTOR_SIZE	61	/* R */
>  #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
> +#define EXT_CSD_PARTITION_SETTING_COMPLETED 155	/* R/W */
>  #define EXT_CSD_PARTITION_ATTRIBUTE	156	/* R/W */
>  #define EXT_CSD_PARTITION_SUPPORT	160	/* RO */
>  #define EXT_CSD_HPI_MGMT		161	/* R/W */
> @@ -349,6 +350,7 @@ struct _mmc_csd {
>  #define EXT_CSD_PART_CONFIG_ACC_RPMB	(0x3)
>  #define EXT_CSD_PART_CONFIG_ACC_GP0	(0x4)
> 
> +#define EXT_CSD_PART_SETTING_COMPLETED	(0x1)
>  #define EXT_CSD_PART_SUPPORT_PART_EN	(0x1)
> 
>  #define EXT_CSD_CMD_SET_NORMAL		(1<<0)
> --
> 1.7.9.5
> 
> Le 15/08/2014 10:17, Ulf Hansson a écrit :
>> On 14 August 2014 15:27, Grégory Soutadé <gsoutade@neotion.com> wrote:
>>> Le 14/08/2014 13:46, Ulf Hansson a écrit :
>>>> On 13 August 2014 11:20, Grégory Soutadé <gsoutade@neotion.com> wrote:
>>>>> Le 13/08/2014 10:36, Ulf Hansson a écrit :
>>>>>> On 17 July 2014 16:57, Grégory Soutadé <gsoutade@neotion.com> wrote:
>>>>>>> Create MMC general purpose partitions only if
>>>>>>> EXT_CSD_PARTITION_SETTING_COMPLETED bit is set.
>>>>>>> Some tools may set general purpose partition size(s) but fail or stop
>>>>>>> without finalize.
>>>>>>> Another case is to set invalid partition size(s).
>>>>>>>
>>>>>>> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
>>>>>>> ---
>>>>>>>  drivers/mmc/core/mmc.c  |   15 +++++++++++----
>>>>>>>  include/linux/mmc/mmc.h |    2 ++
>>>>>>>  2 files changed, 13 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> From commit b6603fe574af289dbe9eb9fb4c540bca04f5a053 in master linux tree.
>>>>>>>
>>>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>>>> index 793c6f7..b9fe211 100644
>>>>>>> --- a/drivers/mmc/core/mmc.c
>>>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>>>> @@ -471,10 +471,17 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>>>>>>                                 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,
>>>>>>> -                                       MMC_BLK_DATA_AREA_GP);
>>>>>>> +                               if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
>>>>>>> +                               EXT_CSD_PART_SETTING_COMPLETED) {
>>>>>>
>>>>>> Some minor comments here.
>>>>>>
>>>>>> I think you could move this if statement up and into the previous "if"
>>>>>> where we check for "ext_csd[EXT_CSD_PARTITION_SUPPORT] &
>>>>>> EXT_CSD_PART_SUPPORT_PART_EN".
>>>>>>
>>>>>> Additionally, please run checkpatch.
>>>>>>
>>>>>> Kind regards
>>>>>> Uffe
>>>>>
>>>>> Hello,
>>>>>
>>>>> I didn't put the if statement above to warn user in case of bad partitioning.
>>>>> I don't want the kernel to silently ignore this error.
>>>>
>>>> Fair enough.
>>>>
>>>> Still I am wondering whether hc_erase_grp_sz, hc_wp_grp_sz and
>>>> enhanced_area_en should be updated if
>>>> EXT_CSD_PARTITION_SETTING_COMPLETED isn't set.  That's the case in
>>>> your patch.
>>>
>>> I was focused on partitions and I didn't pay attention on enhanced area.
>>>
>>> JEDEC says that partitioning implies :
>>> * Configure general purpose partitions attributes and sizes
>>> * Configure enhanced user area offset, attributes and size
>>>
>>> and finally set EXT_CSD_PARTITION_SETTING_COMPLETED.
>>>
>>> Thus these two parts must checks for setting completed before
>>> computing values.
>>>
>>> Plus, "enhanced_area_en" attribute is set whether or not there is an
>>> enhanced area defined. I looked at the code, and the only usage of it
>>> is to set EXT_CSD_ERASE_GROUP_DEF and compute erase size again.
>>> I suggest using "partition_setting_completed" identifier which is common
>>> to the two functions and requires EXT_CSD_ERASE_GROUP_DEF to be set.
>>>
>>> If you're ok with that, I'll submit another patch.
>>
>> Seems reasonable. Please send a v2.
>>
>> Kind regards
>> Uffe
>>
>>>
>>>
>>> Best regards
>>> Grégory Soutadé
>>>
>>>>
>>>>>
>>>>> checkpatch has been run before sending this patch, I know there are two lines
>>>>> with two extra characters, but names used here are quite long. I hope it will
>>>>> not block upstream inclusion.
>>>>
>>>> The mmc_read_ext_csd() is not one of the nicest piece of code, but for
>>>> sure we should not move on making it worse. If you need to move code
>>>> into separate function to prevent checkpatch warnings, please do so.
>>>>
>>>> Kind regards
>>>> Uffe
>>>>
>>>
>>

[PATCH v5 0000/0003] mmc: EXT_CSD_PARTITION_SETTING_COMPLETED bit not checked

[Grégory Soutadé]

JEDEC standard requires that EXT_CSD_PARTITION_SETTING_COMPLETED bit
must be set in order to take in account enhanced area and general purpose
partitions (gp) values.

Current code doesn't checks this bit and blindly trust enhanced area and
gp values. Moreover, "enhanced_area_en" attribute was set according to gp values
but not necessary enhanced area one. It's then used to switch
EXT_CSD_ERASE_GROUP_DEF bit which requires EXT_CSD_PARTITION_SETTING_COMPLETED.
This attribute has been replaced by "partition_setting_completed" that
match the expected behavior.

User is now warned in case of misconfiguration.

Plus, some code has been moved into functions for two reasons :
* functional reason (one behavior per function)
* Deep indentation result

Grégory Soutadé (3):
  mmc: Move code that manages user area and gp partitions into functions
  mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed"
  mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation

 drivers/mmc/core/mmc.c   |  171 +++++++++++++++++++++++++++-------------------
 include/linux/mmc/card.h |    2 +-
 include/linux/mmc/mmc.h  |    2 +
 3 files changed, 102 insertions(+), 73 deletions(-)

>From commit 480cadc2b7e0fa2bbab20141efb547dfe0c3707c in master linux tree.

Changelog v4:
	Second patch in v3 doesn't compile
Changelog v3:
	Move code BEFORE fixing bugs.
Changelog v2:
	Move code for user area and general purpose partitions
	into functions.
-- 
1.7.9.5

> Replace ext_csd "enhanced_area_en" attribute by "partition_setting_completed".
>> It was used whether or not enhanced user area is defined and without checks of
>> EXT_CSD_PARTITION_SETTING_COMPLETED bit.
>> 
>> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
>> ---
>>  drivers/mmc/core/mmc.c   |    8 +++++++-
>>  include/linux/mmc/card.h |    2 +-
>>  include/linux/mmc/mmc.h  |    2 ++
>>  3 files changed, 10 insertions(+), 2 deletions(-)
>> 
>>>From commit 33caee39925b887a99a2400dc5c980097c3573f9 in master linux tree.
>> 
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 793c6f7..ef25d91 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -403,6 +403,12 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>  		ext_csd[EXT_CSD_TRIM_MULT];
>>  	card->ext_csd.raw_partition_support = ext_csd[EXT_CSD_PARTITION_SUPPORT];
>>  	if (card->ext_csd.rev >= 4) {
>> +		if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
>> +		    EXT_CSD_PART_SETTING_COMPLETED) {
>> +			card->ext_csd.partition_setting_completed = 1;
>> +		} else {
>> +			card->ext_csd.partition_setting_completed = 0;
>> +		}
>>  		/*
>>  		 * Enhanced area feature support -- check whether the eMMC
>>  		 * card has the Enhanced area enabled.  If so, export enhanced
>> @@ -1309,7 +1315,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>>  	 * If enhanced_area_en is TRUE, host needs to enable ERASE_GRP_DEF
>>  	 * bit.  This bit will be lost every time after a reset or power off.
>>  	 */
>> -	if (card->ext_csd.enhanced_area_en ||
>> +	if (card->ext_csd.partition_setting_completed ||
>>  	    (card->ext_csd.rev >= 3 && (host->caps2 & MMC_CAP2_HC_ERASE_SZ))) {
>>  		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  				 EXT_CSD_ERASE_GROUP_DEF, 1,
>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> index d424b9d..38175be 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -74,7 +74,7 @@ struct mmc_ext_csd {
>>  	unsigned int		sec_trim_mult;	/* Secure trim multiplier  */
>>  	unsigned int		sec_erase_mult;	/* Secure erase multiplier */
>>  	unsigned int		trim_timeout;		/* In milliseconds */
>> -	bool			enhanced_area_en;	/* enable bit */
>> +	bool			partition_setting_completed;	/* enable bit */
>>  	unsigned long long	enhanced_area_offset;	/* Units: Byte */
>>  	unsigned int		enhanced_area_size;	/* Units: KB */
>>  	unsigned int		cache_size;		/* Units: KB */
>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>> index 64ec963..78753bc 100644
>> --- a/include/linux/mmc/mmc.h
>> +++ b/include/linux/mmc/mmc.h
>> @@ -281,6 +281,7 @@ struct _mmc_csd {
>>  #define EXT_CSD_EXP_EVENTS_CTRL		56	/* R/W, 2 bytes */
>>  #define EXT_CSD_DATA_SECTOR_SIZE	61	/* R */
>>  #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
>> +#define EXT_CSD_PARTITION_SETTING_COMPLETED 155	/* R/W */
>>  #define EXT_CSD_PARTITION_ATTRIBUTE	156	/* R/W */
>>  #define EXT_CSD_PARTITION_SUPPORT	160	/* RO */
>>  #define EXT_CSD_HPI_MGMT		161	/* R/W */
>> @@ -349,6 +350,7 @@ struct _mmc_csd {
>>  #define EXT_CSD_PART_CONFIG_ACC_RPMB	(0x3)
>>  #define EXT_CSD_PART_CONFIG_ACC_GP0	(0x4)
>> 
>> +#define EXT_CSD_PART_SETTING_COMPLETED	(0x1)
>>  #define EXT_CSD_PART_SUPPORT_PART_EN	(0x1)
>> 
>>  #define EXT_CSD_CMD_SET_NORMAL		(1<<0)
>> --
>> 1.7.9.5
>> 
>> Le 15/08/2014 10:17, Ulf Hansson a écrit :
>> >> On 14 August 2014 15:27, Grégory Soutadé <gsoutade@neotion.com> wrote:
>>> >>> Le 14/08/2014 13:46, Ulf Hansson a écrit :
>>>> >>>> On 13 August 2014 11:20, Grégory Soutadé <gsoutade@neotion.com> wrote:
>>>>> >>>>> Le 13/08/2014 10:36, Ulf Hansson a écrit :
>>>>>> >>>>>> On 17 July 2014 16:57, Grégory Soutadé <gsoutade@neotion.com> wrote:
>>>>>>> >>>>>>> Create MMC general purpose partitions only if
>>>>>>> >>>>>>> EXT_CSD_PARTITION_SETTING_COMPLETED bit is set.
>>>>>>> >>>>>>> Some tools may set general purpose partition size(s) but fail or stop
>>>>>>> >>>>>>> without finalize.
>>>>>>> >>>>>>> Another case is to set invalid partition size(s).
>>>>>>> >>>>>>>
>>>>>>> >>>>>>> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
>>>>>>> >>>>>>> ---
>>>>>>> >>>>>>>  drivers/mmc/core/mmc.c  |   15 +++++++++++----
>>>>>>> >>>>>>>  include/linux/mmc/mmc.h |    2 ++
>>>>>>> >>>>>>>  2 files changed, 13 insertions(+), 4 deletions(-)
>>>>>>> >>>>>>>
>>>>>>> >>>>>>> From commit b6603fe574af289dbe9eb9fb4c540bca04f5a053 in master linux tree.
>>>>>>> >>>>>>>
>>>>>>> >>>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>>>> >>>>>>> index 793c6f7..b9fe211 100644
>>>>>>> >>>>>>> --- a/drivers/mmc/core/mmc.c
>>>>>>> >>>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>>>> >>>>>>> @@ -471,10 +471,17 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>>>>>> >>>>>>>                                 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,
>>>>>>> >>>>>>> -                                       MMC_BLK_DATA_AREA_GP);
>>>>>>> >>>>>>> +                               if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
>>>>>>> >>>>>>> +                               EXT_CSD_PART_SETTING_COMPLETED) {
>>>>>> >>>>>>
>>>>>> >>>>>> Some minor comments here.
>>>>>> >>>>>>
>>>>>> >>>>>> I think you could move this if statement up and into the previous "if"
>>>>>> >>>>>> where we check for "ext_csd[EXT_CSD_PARTITION_SUPPORT] &
>>>>>> >>>>>> EXT_CSD_PART_SUPPORT_PART_EN".
>>>>>> >>>>>>
>>>>>> >>>>>> Additionally, please run checkpatch.
>>>>>> >>>>>>
>>>>>> >>>>>> Kind regards
>>>>>> >>>>>> Uffe
>>>>> >>>>>
>>>>> >>>>> Hello,
>>>>> >>>>>
>>>>> >>>>> I didn't put the if statement above to warn user in case of bad partitioning.
>>>>> >>>>> I don't want the kernel to silently ignore this error.
>>>> >>>>
>>>> >>>> Fair enough.
>>>> >>>>
>>>> >>>> Still I am wondering whether hc_erase_grp_sz, hc_wp_grp_sz and
>>>> >>>> enhanced_area_en should be updated if
>>>> >>>> EXT_CSD_PARTITION_SETTING_COMPLETED isn't set.  That's the case in
>>>> >>>> your patch.
>>> >>>
>>> >>> I was focused on partitions and I didn't pay attention on enhanced area.
>>> >>>
>>> >>> JEDEC says that partitioning implies :
>>> >>> * Configure general purpose partitions attributes and sizes
>>> >>> * Configure enhanced user area offset, attributes and size
>>> >>>
>>> >>> and finally set EXT_CSD_PARTITION_SETTING_COMPLETED.
>>> >>>
>>> >>> Thus these two parts must checks for setting completed before
>>> >>> computing values.
>>> >>>
>>> >>> Plus, "enhanced_area_en" attribute is set whether or not there is an
>>> >>> enhanced area defined. I looked at the code, and the only usage of it
>>> >>> is to set EXT_CSD_ERASE_GROUP_DEF and compute erase size again.
>>> >>> I suggest using "partition_setting_completed" identifier which is common
>>> >>> to the two functions and requires EXT_CSD_ERASE_GROUP_DEF to be set.
>>> >>>
>>> >>> If you're ok with that, I'll submit another patch.
>> >>
>> >> Seems reasonable. Please send a v2.
>> >>
>> >> Kind regards
>> >> Uffe
>> >>
>>> >>>
>>> >>>
>>> >>> Best regards
>>> >>> Grégory Soutadé
>>> >>>
>>>> >>>>
>>>>> >>>>>
>>>>> >>>>> checkpatch has been run before sending this patch, I know there are two lines
>>>>> >>>>> with two extra characters, but names used here are quite long. I hope it will
>>>>> >>>>> not block upstream inclusion.
>>>> >>>>
>>>> >>>> The mmc_read_ext_csd() is not one of the nicest piece of code, but for
>>>> >>>> sure we should not move on making it worse. If you need to move code
>>>> >>>> into separate function to prevent checkpatch warnings, please do so.
>>>> >>>>
>>>> >>>> Kind regards
>>>> >>>> Uffe
>>>> >>>>
>>> >>>
>> >>

[PATCH v5 0000/0003] mmc: EXT_CSD_PARTITION_SETTING_COMPLETED bit not checked

[Grégory Soutadé]

JEDEC standard requires that EXT_CSD_PARTITION_SETTING_COMPLETED bit
must be set in order to take in account enhanced area and general purpose
partitions (gp) values.

Current code doesn't checks this bit and blindly trust enhanced area and
gp values. Moreover, "enhanced_area_en" attribute was set according to gp values
but not necessary enhanced area one. It's then used to switch
EXT_CSD_ERASE_GROUP_DEF bit which requires EXT_CSD_PARTITION_SETTING_COMPLETED.
This attribute has been replaced by "partition_setting_completed" that
match the expected behavior.

User is now warned in case of misconfiguration.

Plus, some code has been moved into functions for two reasons :
* functional reason (one behavior per function)
* Deep indentation result

Grégory Soutadé (3):
  mmc: Move code that manages user area and gp partitions into functions
  mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed"
  mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation

 drivers/mmc/core/mmc.c   |  171 +++++++++++++++++++++++++++-------------------
 include/linux/mmc/card.h |    2 +-
 include/linux/mmc/mmc.h  |    2 +
 3 files changed, 102 insertions(+), 73 deletions(-)

From commit 480cadc2b7e0fa2bbab20141efb547dfe0c3707c in master linux tree.

Changelog v4:
	Second patch in v3 doesn't compile
Changelog v3:
	Move code BEFORE fixing bugs.
Changelog v2:
	Move code for user area and general purpose partitions
	into functions.
-- 
1.7.9.5

> Replace ext_csd "enhanced_area_en" attribute by "partition_setting_completed".
>> It was used whether or not enhanced user area is defined and without checks of
>> EXT_CSD_PARTITION_SETTING_COMPLETED bit.
>> 
>> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
>> ---
>>  drivers/mmc/core/mmc.c   |    8 +++++++-
>>  include/linux/mmc/card.h |    2 +-
>>  include/linux/mmc/mmc.h  |    2 ++
>>  3 files changed, 10 insertions(+), 2 deletions(-)
>> 
>>>From commit 33caee39925b887a99a2400dc5c980097c3573f9 in master linux tree.
>> 
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 793c6f7..ef25d91 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -403,6 +403,12 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>  		ext_csd[EXT_CSD_TRIM_MULT];
>>  	card->ext_csd.raw_partition_support = ext_csd[EXT_CSD_PARTITION_SUPPORT];
>>  	if (card->ext_csd.rev >= 4) {
>> +		if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
>> +		    EXT_CSD_PART_SETTING_COMPLETED) {
>> +			card->ext_csd.partition_setting_completed = 1;
>> +		} else {
>> +			card->ext_csd.partition_setting_completed = 0;
>> +		}
>>  		/*
>>  		 * Enhanced area feature support -- check whether the eMMC
>>  		 * card has the Enhanced area enabled.  If so, export enhanced
>> @@ -1309,7 +1315,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>>  	 * If enhanced_area_en is TRUE, host needs to enable ERASE_GRP_DEF
>>  	 * bit.  This bit will be lost every time after a reset or power off.
>>  	 */
>> -	if (card->ext_csd.enhanced_area_en ||
>> +	if (card->ext_csd.partition_setting_completed ||
>>  	    (card->ext_csd.rev >= 3 && (host->caps2 & MMC_CAP2_HC_ERASE_SZ))) {
>>  		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  				 EXT_CSD_ERASE_GROUP_DEF, 1,
>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> index d424b9d..38175be 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -74,7 +74,7 @@ struct mmc_ext_csd {
>>  	unsigned int		sec_trim_mult;	/* Secure trim multiplier  */
>>  	unsigned int		sec_erase_mult;	/* Secure erase multiplier */
>>  	unsigned int		trim_timeout;		/* In milliseconds */
>> -	bool			enhanced_area_en;	/* enable bit */
>> +	bool			partition_setting_completed;	/* enable bit */
>>  	unsigned long long	enhanced_area_offset;	/* Units: Byte */
>>  	unsigned int		enhanced_area_size;	/* Units: KB */
>>  	unsigned int		cache_size;		/* Units: KB */
>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>> index 64ec963..78753bc 100644
>> --- a/include/linux/mmc/mmc.h
>> +++ b/include/linux/mmc/mmc.h
>> @@ -281,6 +281,7 @@ struct _mmc_csd {
>>  #define EXT_CSD_EXP_EVENTS_CTRL		56	/* R/W, 2 bytes */
>>  #define EXT_CSD_DATA_SECTOR_SIZE	61	/* R */
>>  #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
>> +#define EXT_CSD_PARTITION_SETTING_COMPLETED 155	/* R/W */
>>  #define EXT_CSD_PARTITION_ATTRIBUTE	156	/* R/W */
>>  #define EXT_CSD_PARTITION_SUPPORT	160	/* RO */
>>  #define EXT_CSD_HPI_MGMT		161	/* R/W */
>> @@ -349,6 +350,7 @@ struct _mmc_csd {
>>  #define EXT_CSD_PART_CONFIG_ACC_RPMB	(0x3)
>>  #define EXT_CSD_PART_CONFIG_ACC_GP0	(0x4)
>> 
>> +#define EXT_CSD_PART_SETTING_COMPLETED	(0x1)
>>  #define EXT_CSD_PART_SUPPORT_PART_EN	(0x1)
>> 
>>  #define EXT_CSD_CMD_SET_NORMAL		(1<<0)
>> --
>> 1.7.9.5
>> 
>> Le 15/08/2014 10:17, Ulf Hansson a écrit :
>> >> On 14 August 2014 15:27, Grégory Soutadé <gsoutade@neotion.com> wrote:
>>> >>> Le 14/08/2014 13:46, Ulf Hansson a écrit :
>>>> >>>> On 13 August 2014 11:20, Grégory Soutadé <gsoutade@neotion.com> wrote:
>>>>> >>>>> Le 13/08/2014 10:36, Ulf Hansson a écrit :
>>>>>> >>>>>> On 17 July 2014 16:57, Grégory Soutadé <gsoutade@neotion.com> wrote:
>>>>>>> >>>>>>> Create MMC general purpose partitions only if
>>>>>>> >>>>>>> EXT_CSD_PARTITION_SETTING_COMPLETED bit is set.
>>>>>>> >>>>>>> Some tools may set general purpose partition size(s) but fail or stop
>>>>>>> >>>>>>> without finalize.
>>>>>>> >>>>>>> Another case is to set invalid partition size(s).
>>>>>>> >>>>>>>
>>>>>>> >>>>>>> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
>>>>>>> >>>>>>> ---
>>>>>>> >>>>>>>  drivers/mmc/core/mmc.c  |   15 +++++++++++----
>>>>>>> >>>>>>>  include/linux/mmc/mmc.h |    2 ++
>>>>>>> >>>>>>>  2 files changed, 13 insertions(+), 4 deletions(-)
>>>>>>> >>>>>>>
>>>>>>> >>>>>>> From commit b6603fe574af289dbe9eb9fb4c540bca04f5a053 in master linux tree.
>>>>>>> >>>>>>>
>>>>>>> >>>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>>>> >>>>>>> index 793c6f7..b9fe211 100644
>>>>>>> >>>>>>> --- a/drivers/mmc/core/mmc.c
>>>>>>> >>>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>>>> >>>>>>> @@ -471,10 +471,17 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>>>>>> >>>>>>>                                 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,
>>>>>>> >>>>>>> -                                       MMC_BLK_DATA_AREA_GP);
>>>>>>> >>>>>>> +                               if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
>>>>>>> >>>>>>> +                               EXT_CSD_PART_SETTING_COMPLETED) {
>>>>>> >>>>>>
>>>>>> >>>>>> Some minor comments here.
>>>>>> >>>>>>
>>>>>> >>>>>> I think you could move this if statement up and into the previous "if"
>>>>>> >>>>>> where we check for "ext_csd[EXT_CSD_PARTITION_SUPPORT] &
>>>>>> >>>>>> EXT_CSD_PART_SUPPORT_PART_EN".
>>>>>> >>>>>>
>>>>>> >>>>>> Additionally, please run checkpatch.
>>>>>> >>>>>>
>>>>>> >>>>>> Kind regards
>>>>>> >>>>>> Uffe
>>>>> >>>>>
>>>>> >>>>> Hello,
>>>>> >>>>>
>>>>> >>>>> I didn't put the if statement above to warn user in case of bad partitioning.
>>>>> >>>>> I don't want the kernel to silently ignore this error.
>>>> >>>>
>>>> >>>> Fair enough.
>>>> >>>>
>>>> >>>> Still I am wondering whether hc_erase_grp_sz, hc_wp_grp_sz and
>>>> >>>> enhanced_area_en should be updated if
>>>> >>>> EXT_CSD_PARTITION_SETTING_COMPLETED isn't set.  That's the case in
>>>> >>>> your patch.
>>> >>>
>>> >>> I was focused on partitions and I didn't pay attention on enhanced area.
>>> >>>
>>> >>> JEDEC says that partitioning implies :
>>> >>> * Configure general purpose partitions attributes and sizes
>>> >>> * Configure enhanced user area offset, attributes and size
>>> >>>
>>> >>> and finally set EXT_CSD_PARTITION_SETTING_COMPLETED.
>>> >>>
>>> >>> Thus these two parts must checks for setting completed before
>>> >>> computing values.
>>> >>>
>>> >>> Plus, "enhanced_area_en" attribute is set whether or not there is an
>>> >>> enhanced area defined. I looked at the code, and the only usage of it
>>> >>> is to set EXT_CSD_ERASE_GROUP_DEF and compute erase size again.
>>> >>> I suggest using "partition_setting_completed" identifier which is common
>>> >>> to the two functions and requires EXT_CSD_ERASE_GROUP_DEF to be set.
>>> >>>
>>> >>> If you're ok with that, I'll submit another patch.
>> >>
>> >> Seems reasonable. Please send a v2.
>> >>
>> >> Kind regards
>> >> Uffe
>> >>
>>> >>>
>>> >>>
>>> >>> Best regards
>>> >>> Grégory Soutadé
>>> >>>
>>>> >>>>
>>>>> >>>>>
>>>>> >>>>> checkpatch has been run before sending this patch, I know there are two lines
>>>>> >>>>> with two extra characters, but names used here are quite long. I hope it will
>>>>> >>>>> not block upstream inclusion.
>>>> >>>>
>>>> >>>> The mmc_read_ext_csd() is not one of the nicest piece of code, but for
>>>> >>>> sure we should not move on making it worse. If you need to move code
>>>> >>>> into separate function to prevent checkpatch warnings, please do so.
>>>> >>>>
>>>> >>>> Kind regards
>>>> >>>> Uffe
>>>> >>>>
>>> >>>
>> >>

[PATCH v6 0000/0003] mmc: EXT_CSD_PARTITION_SETTING_COMPLETED bit not checked

[Grégory Soutadé]

JEDEC standard requires that EXT_CSD_PARTITION_SETTING_COMPLETED bit
must be set in order to take in account enhanced area and general purpose
partitions (gp) values.

Current code doesn't checks this bit and blindly trust enhanced area and
gp values. Moreover, "enhanced_area_en" attribute was set according to gp values
but not necessary enhanced area one. It's then used to switch
EXT_CSD_ERASE_GROUP_DEF bit which requires EXT_CSD_PARTITION_SETTING_COMPLETED.
This attribute has been replaced by "partition_setting_completed" that
match the expected behavior.

User is now warned in case of misconfiguration.

Plus, some code has been moved into functions for two reasons :
* Functional reason (one behavior per function)
* Deep indentation result

Grégory Soutadé (3):
  mmc: Move code that manages user area and gp partitions into functions
  mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed"
  mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation

 drivers/mmc/core/mmc.c   |  172 ++++++++++++++++++++++++++--------------------
 include/linux/mmc/card.h |    2 +-
 include/linux/mmc/mmc.h  |    2 +
 3 files changed, 102 insertions(+), 74 deletions(-)

>From commit 7ec62d421bdf29cb31101ae2689f7f3a9906289a in master linux tree.

Changelog v5:
	Remove some useless braces
Changelog v4:
	Second patch in v3 doesn't compile
Changelog v3:
	Move code BEFORE fixing bugs.
Changelog v2:
	Move code for user area and general purpose partitions
	into functions.
-- 
1.7.9.5

[PATCH v6 0000/0003] mmc: EXT_CSD_PARTITION_SETTING_COMPLETED bit not checked

[Grégory Soutadé]

JEDEC standard requires that EXT_CSD_PARTITION_SETTING_COMPLETED bit
must be set in order to take in account enhanced area and general purpose
partitions (gp) values.

Current code doesn't checks this bit and blindly trust enhanced area and
gp values. Moreover, "enhanced_area_en" attribute was set according to gp values
but not necessary enhanced area one. It's then used to switch
EXT_CSD_ERASE_GROUP_DEF bit which requires EXT_CSD_PARTITION_SETTING_COMPLETED.
This attribute has been replaced by "partition_setting_completed" that
match the expected behavior.

User is now warned in case of misconfiguration.

Plus, some code has been moved into functions for two reasons :
* Functional reason (one behavior per function)
* Deep indentation result

Grégory Soutadé (3):
  mmc: Move code that manages user area and gp partitions into functions
  mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed"
  mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation

 drivers/mmc/core/mmc.c   |  172 ++++++++++++++++++++++++++--------------------
 include/linux/mmc/card.h |    2 +-
 include/linux/mmc/mmc.h  |    2 +
 3 files changed, 102 insertions(+), 74 deletions(-)

From commit 7ec62d421bdf29cb31101ae2689f7f3a9906289a in master linux tree.

Changelog v5:
	Remove some useless braces
Changelog v4:
	Second patch in v3 doesn't compile
Changelog v3:
	Move code BEFORE fixing bugs.
Changelog v2:
	Move code for user area and general purpose partitions
	into functions.
-- 
1.7.9.5

[PATCH v5 0001/0003] mmc: Move code that manages user area and gp partitions into functions

[Grégory Soutadé]

Move code that manages user area and general purpose
 partitions into functions.

Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
---
 drivers/mmc/core/mmc.c |  162 ++++++++++++++++++++++++++----------------------
 1 file changed, 89 insertions(+), 73 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 1eda8dd..77b4cf3 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -298,6 +298,93 @@ static void mmc_select_card_type(struct mmc_card *card)
 	card->mmc_avail_type = avail_type;
 }

+static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
+{
+	u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
+
+	/*
+	 * Enhanced area feature support -- check whether the eMMC
+	 * card has the Enhanced area enabled.  If so, export enhanced
+	 * area offset and size to user by adding sysfs interface.
+	 */
+	if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
+	    (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
+		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;
+		/*
+		 * calculate the enhanced data area offset, in bytes
+		 */
+		card->ext_csd.enhanced_area_offset =
+			(ext_csd[139] << 24) + (ext_csd[138] << 16) +
+			(ext_csd[137] << 8) + ext_csd[136];
+		if (mmc_card_blockaddr(card))
+			card->ext_csd.enhanced_area_offset <<= 9;
+		/*
+		 * calculate the enhanced data area size, in kilobytes
+		 */
+		card->ext_csd.enhanced_area_size =
+			(ext_csd[142] << 16) + (ext_csd[141] << 8) +
+			ext_csd[140];
+		card->ext_csd.enhanced_area_size *=
+			(size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
+		card->ext_csd.enhanced_area_size <<= 9;
+	} else {
+		/*
+		 * If the enhanced area is not enabled, disable these
+		 * device attributes.
+		 */
+		card->ext_csd.enhanced_area_offset = -EINVAL;
+		card->ext_csd.enhanced_area_size = -EINVAL;
+	}
+}
+
+static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
+{
+	unsigned int part_size;
+	u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
+	int idx;
+
+	/*
+	 * General purpose partition feature support --
+	 * If ext_csd has 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,
+				MMC_BLK_DATA_AREA_GP);
+		}
+	}
+}
+
 /*
  * Decode extended CSD.
  */
@@ -305,7 +392,6 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 {
 	int err = 0, idx;
 	unsigned int part_size;
-	u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;

 	BUG_ON(!card);

@@ -402,80 +488,10 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 		ext_csd[EXT_CSD_TRIM_MULT];
 	card->ext_csd.raw_partition_support = ext_csd[EXT_CSD_PARTITION_SUPPORT];
 	if (card->ext_csd.rev >= 4) {
-		/*
-		 * Enhanced area feature support -- check whether the eMMC
-		 * card has the Enhanced area enabled.  If so, export enhanced
-		 * area offset and size to user by adding sysfs interface.
-		 */
-		if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
-		    (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
-			hc_erase_grp_sz =
-				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
-			hc_wp_grp_sz =
-				ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+		mmc_manage_enhanced_area(card, ext_csd);

-			card->ext_csd.enhanced_area_en = 1;
-			/*
-			 * calculate the enhanced data area offset, in bytes
-			 */
-			card->ext_csd.enhanced_area_offset =
-				(ext_csd[139] << 24) + (ext_csd[138] << 16) +
-				(ext_csd[137] << 8) + ext_csd[136];
-			if (mmc_card_blockaddr(card))
-				card->ext_csd.enhanced_area_offset <<= 9;
-			/*
-			 * calculate the enhanced data area size, in kilobytes
-			 */
-			card->ext_csd.enhanced_area_size =
-				(ext_csd[142] << 16) + (ext_csd[141] << 8) +
-				ext_csd[140];
-			card->ext_csd.enhanced_area_size *=
-				(size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
-			card->ext_csd.enhanced_area_size <<= 9;
-		} else {
-			/*
-			 * If the enhanced area is not enabled, disable these
-			 * device attributes.
-			 */
-			card->ext_csd.enhanced_area_offset = -EINVAL;
-			card->ext_csd.enhanced_area_size = -EINVAL;
-		}
+		mmc_manage_gp_partitions(card, ext_csd);

-		/*
-		 * General purpose partition feature support --
-		 * If ext_csd has 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,
-					MMC_BLK_DATA_AREA_GP);
-			}
-		}
 		card->ext_csd.sec_trim_mult =
 			ext_csd[EXT_CSD_SEC_TRIM_MULT];
 		card->ext_csd.sec_erase_mult =
-- 
1.7.9.5

[PATCH v6 0001/0003] mmc: Move code that manages user area and gp partitions into functions

[Grégory Soutadé]

Move code that manages user area and general purpose
 partitions into functions.

Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
---
 drivers/mmc/core/mmc.c |  162 ++++++++++++++++++++++++++----------------------
 1 file changed, 89 insertions(+), 73 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 1eda8dd..77b4cf3 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -298,6 +298,93 @@ static void mmc_select_card_type(struct mmc_card *card)
 	card->mmc_avail_type = avail_type;
 }

+static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
+{
+	u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
+
+	/*
+	 * Enhanced area feature support -- check whether the eMMC
+	 * card has the Enhanced area enabled.  If so, export enhanced
+	 * area offset and size to user by adding sysfs interface.
+	 */
+	if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
+	    (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
+		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;
+		/*
+		 * calculate the enhanced data area offset, in bytes
+		 */
+		card->ext_csd.enhanced_area_offset =
+			(ext_csd[139] << 24) + (ext_csd[138] << 16) +
+			(ext_csd[137] << 8) + ext_csd[136];
+		if (mmc_card_blockaddr(card))
+			card->ext_csd.enhanced_area_offset <<= 9;
+		/*
+		 * calculate the enhanced data area size, in kilobytes
+		 */
+		card->ext_csd.enhanced_area_size =
+			(ext_csd[142] << 16) + (ext_csd[141] << 8) +
+			ext_csd[140];
+		card->ext_csd.enhanced_area_size *=
+			(size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
+		card->ext_csd.enhanced_area_size <<= 9;
+	} else {
+		/*
+		 * If the enhanced area is not enabled, disable these
+		 * device attributes.
+		 */
+		card->ext_csd.enhanced_area_offset = -EINVAL;
+		card->ext_csd.enhanced_area_size = -EINVAL;
+	}
+}
+
+static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
+{
+	unsigned int part_size;
+	u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
+	int idx;
+
+	/*
+	 * General purpose partition feature support --
+	 * If ext_csd has 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,
+				MMC_BLK_DATA_AREA_GP);
+		}
+	}
+}
+
 /*
  * Decode extended CSD.
  */
@@ -305,7 +392,6 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 {
 	int err = 0, idx;
 	unsigned int part_size;
-	u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;

 	BUG_ON(!card);

@@ -402,80 +488,10 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 		ext_csd[EXT_CSD_TRIM_MULT];
 	card->ext_csd.raw_partition_support = ext_csd[EXT_CSD_PARTITION_SUPPORT];
 	if (card->ext_csd.rev >= 4) {
-		/*
-		 * Enhanced area feature support -- check whether the eMMC
-		 * card has the Enhanced area enabled.  If so, export enhanced
-		 * area offset and size to user by adding sysfs interface.
-		 */
-		if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
-		    (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
-			hc_erase_grp_sz =
-				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
-			hc_wp_grp_sz =
-				ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+		mmc_manage_enhanced_area(card, ext_csd);

-			card->ext_csd.enhanced_area_en = 1;
-			/*
-			 * calculate the enhanced data area offset, in bytes
-			 */
-			card->ext_csd.enhanced_area_offset =
-				(ext_csd[139] << 24) + (ext_csd[138] << 16) +
-				(ext_csd[137] << 8) + ext_csd[136];
-			if (mmc_card_blockaddr(card))
-				card->ext_csd.enhanced_area_offset <<= 9;
-			/*
-			 * calculate the enhanced data area size, in kilobytes
-			 */
-			card->ext_csd.enhanced_area_size =
-				(ext_csd[142] << 16) + (ext_csd[141] << 8) +
-				ext_csd[140];
-			card->ext_csd.enhanced_area_size *=
-				(size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
-			card->ext_csd.enhanced_area_size <<= 9;
-		} else {
-			/*
-			 * If the enhanced area is not enabled, disable these
-			 * device attributes.
-			 */
-			card->ext_csd.enhanced_area_offset = -EINVAL;
-			card->ext_csd.enhanced_area_size = -EINVAL;
-		}
+		mmc_manage_gp_partitions(card, ext_csd);

-		/*
-		 * General purpose partition feature support --
-		 * If ext_csd has 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,
-					MMC_BLK_DATA_AREA_GP);
-			}
-		}
 		card->ext_csd.sec_trim_mult =
 			ext_csd[EXT_CSD_SEC_TRIM_MULT];
 		card->ext_csd.sec_erase_mult =
-- 
1.7.9.5

Re: [PATCH v6 0001/0003] mmc: Move code that manages user area and gp partitions into functions

[Ulf Hansson]

On 15 September 2014 17:47, Grégory Soutadé <gsoutade@neotion.com> wrote:
> Move code that manages user area and general purpose
>  partitions into functions.
>
> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>

Thanks! Applied for next.

Kind regards
Uffe

> ---
>  drivers/mmc/core/mmc.c |  162 ++++++++++++++++++++++++++----------------------
>  1 file changed, 89 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 1eda8dd..77b4cf3 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -298,6 +298,93 @@ static void mmc_select_card_type(struct mmc_card *card)
>         card->mmc_avail_type = avail_type;
>  }
>
> +static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
> +{
> +       u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
> +
> +       /*
> +        * Enhanced area feature support -- check whether the eMMC
> +        * card has the Enhanced area enabled.  If so, export enhanced
> +        * area offset and size to user by adding sysfs interface.
> +        */
> +       if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
> +           (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
> +               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;
> +               /*
> +                * calculate the enhanced data area offset, in bytes
> +                */
> +               card->ext_csd.enhanced_area_offset =
> +                       (ext_csd[139] << 24) + (ext_csd[138] << 16) +
> +                       (ext_csd[137] << 8) + ext_csd[136];
> +               if (mmc_card_blockaddr(card))
> +                       card->ext_csd.enhanced_area_offset <<= 9;
> +               /*
> +                * calculate the enhanced data area size, in kilobytes
> +                */
> +               card->ext_csd.enhanced_area_size =
> +                       (ext_csd[142] << 16) + (ext_csd[141] << 8) +
> +                       ext_csd[140];
> +               card->ext_csd.enhanced_area_size *=
> +                       (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
> +               card->ext_csd.enhanced_area_size <<= 9;
> +       } else {
> +               /*
> +                * If the enhanced area is not enabled, disable these
> +                * device attributes.
> +                */
> +               card->ext_csd.enhanced_area_offset = -EINVAL;
> +               card->ext_csd.enhanced_area_size = -EINVAL;
> +       }
> +}
> +
> +static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
> +{
> +       unsigned int part_size;
> +       u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
> +       int idx;
> +
> +       /*
> +        * General purpose partition feature support --
> +        * If ext_csd has 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,
> +                               MMC_BLK_DATA_AREA_GP);
> +               }
> +       }
> +}
> +
>  /*
>   * Decode extended CSD.
>   */
> @@ -305,7 +392,6 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>  {
>         int err = 0, idx;
>         unsigned int part_size;
> -       u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
>
>         BUG_ON(!card);
>
> @@ -402,80 +488,10 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>                 ext_csd[EXT_CSD_TRIM_MULT];
>         card->ext_csd.raw_partition_support = ext_csd[EXT_CSD_PARTITION_SUPPORT];
>         if (card->ext_csd.rev >= 4) {
> -               /*
> -                * Enhanced area feature support -- check whether the eMMC
> -                * card has the Enhanced area enabled.  If so, export enhanced
> -                * area offset and size to user by adding sysfs interface.
> -                */
> -               if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
> -                   (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
> -                       hc_erase_grp_sz =
> -                               ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
> -                       hc_wp_grp_sz =
> -                               ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
> +               mmc_manage_enhanced_area(card, ext_csd);
>
> -                       card->ext_csd.enhanced_area_en = 1;
> -                       /*
> -                        * calculate the enhanced data area offset, in bytes
> -                        */
> -                       card->ext_csd.enhanced_area_offset =
> -                               (ext_csd[139] << 24) + (ext_csd[138] << 16) +
> -                               (ext_csd[137] << 8) + ext_csd[136];
> -                       if (mmc_card_blockaddr(card))
> -                               card->ext_csd.enhanced_area_offset <<= 9;
> -                       /*
> -                        * calculate the enhanced data area size, in kilobytes
> -                        */
> -                       card->ext_csd.enhanced_area_size =
> -                               (ext_csd[142] << 16) + (ext_csd[141] << 8) +
> -                               ext_csd[140];
> -                       card->ext_csd.enhanced_area_size *=
> -                               (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
> -                       card->ext_csd.enhanced_area_size <<= 9;
> -               } else {
> -                       /*
> -                        * If the enhanced area is not enabled, disable these
> -                        * device attributes.
> -                        */
> -                       card->ext_csd.enhanced_area_offset = -EINVAL;
> -                       card->ext_csd.enhanced_area_size = -EINVAL;
> -               }
> +               mmc_manage_gp_partitions(card, ext_csd);
>
> -               /*
> -                * General purpose partition feature support --
> -                * If ext_csd has 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,
> -                                       MMC_BLK_DATA_AREA_GP);
> -                       }
> -               }
>                 card->ext_csd.sec_trim_mult =
>                         ext_csd[EXT_CSD_SEC_TRIM_MULT];
>                 card->ext_csd.sec_erase_mult =
> --
> 1.7.9.5

[PATCH v5 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed"

[Grégory Soutadé]

Replace ext_csd "enhanced_area_en" attribute by
 "partition_setting_completed". It was used whether or
 not enhanced user area is defined and without checks of
 EXT_CSD_PARTITION_SETTING_COMPLETED bit.

Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
---
 drivers/mmc/core/mmc.c   |   14 +++++++++-----
 include/linux/mmc/card.h |    2 +-
 include/linux/mmc/mmc.h  |    2 ++
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 77b4cf3..d8d3aef 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -314,7 +314,6 @@ static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
 		hc_wp_grp_sz =
 			ext_csd[EXT_CSD_HC_WP_GRP_SIZE];

-		card->ext_csd.enhanced_area_en = 1;
 		/*
 		 * calculate the enhanced data area offset, in bytes
 		 */
@@ -355,13 +354,11 @@ static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
 	 */
 	if (ext_csd[EXT_CSD_PARTITION_SUPPORT] &
 	    EXT_CSD_PART_SUPPORT_PART_EN) {
-		if (card->ext_csd.enhanced_area_en != 1) {
+		if (card->ext_csd.partition_setting_completed) {
 			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++) {
@@ -488,6 +485,13 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 		ext_csd[EXT_CSD_TRIM_MULT];
 	card->ext_csd.raw_partition_support = ext_csd[EXT_CSD_PARTITION_SUPPORT];
 	if (card->ext_csd.rev >= 4) {
+		if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
+		    EXT_CSD_PART_SETTING_COMPLETED) {
+			card->ext_csd.partition_setting_completed = 1;
+		} else {
+			card->ext_csd.partition_setting_completed = 0;
+		}
+
 		mmc_manage_enhanced_area(card, ext_csd);

 		mmc_manage_gp_partitions(card, ext_csd);
@@ -1324,7 +1328,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	 * If enhanced_area_en is TRUE, host needs to enable ERASE_GRP_DEF
 	 * bit.  This bit will be lost every time after a reset or power off.
 	 */
-	if (card->ext_csd.enhanced_area_en ||
+	if (card->ext_csd.partition_setting_completed ||
 	    (card->ext_csd.rev >= 3 && (host->caps2 & MMC_CAP2_HC_ERASE_SZ))) {
 		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 				 EXT_CSD_ERASE_GROUP_DEF, 1,
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index d424b9d..38175be 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -74,7 +74,7 @@ struct mmc_ext_csd {
 	unsigned int		sec_trim_mult;	/* Secure trim multiplier  */
 	unsigned int		sec_erase_mult;	/* Secure erase multiplier */
 	unsigned int		trim_timeout;		/* In milliseconds */
-	bool			enhanced_area_en;	/* enable bit */
+	bool			partition_setting_completed;	/* enable bit */
 	unsigned long long	enhanced_area_offset;	/* Units: Byte */
 	unsigned int		enhanced_area_size;	/* Units: KB */
 	unsigned int		cache_size;		/* Units: KB */
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 64ec963..78753bc 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -281,6 +281,7 @@ struct _mmc_csd {
 #define EXT_CSD_EXP_EVENTS_CTRL		56	/* R/W, 2 bytes */
 #define EXT_CSD_DATA_SECTOR_SIZE	61	/* R */
 #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
+#define EXT_CSD_PARTITION_SETTING_COMPLETED 155	/* R/W */
 #define EXT_CSD_PARTITION_ATTRIBUTE	156	/* R/W */
 #define EXT_CSD_PARTITION_SUPPORT	160	/* RO */
 #define EXT_CSD_HPI_MGMT		161	/* R/W */
@@ -349,6 +350,7 @@ struct _mmc_csd {
 #define EXT_CSD_PART_CONFIG_ACC_RPMB	(0x3)
 #define EXT_CSD_PART_CONFIG_ACC_GP0	(0x4)

+#define EXT_CSD_PART_SETTING_COMPLETED	(0x1)
 #define EXT_CSD_PART_SUPPORT_PART_EN	(0x1)

 #define EXT_CSD_CMD_SET_NORMAL		(1<<0)
-- 
1.7.9.5

Re: [PATCH v5 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed"

[Jaehoon Chung]

On 09/12/2014 11:31 PM, Grégory Soutadé wrote:
> Replace ext_csd "enhanced_area_en" attribute by
>  "partition_setting_completed". It was used whether or
>  not enhanced user area is defined and without checks of
>  EXT_CSD_PARTITION_SETTING_COMPLETED bit.
> 
> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
> ---
>  drivers/mmc/core/mmc.c   |   14 +++++++++-----
>  include/linux/mmc/card.h |    2 +-
>  include/linux/mmc/mmc.h  |    2 ++
>  3 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 77b4cf3..d8d3aef 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -314,7 +314,6 @@ static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
>  		hc_wp_grp_sz =
>  			ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
> 
> -		card->ext_csd.enhanced_area_en = 1;
>  		/*
>  		 * calculate the enhanced data area offset, in bytes
>  		 */
> @@ -355,13 +354,11 @@ static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
>  	 */
>  	if (ext_csd[EXT_CSD_PARTITION_SUPPORT] &
>  	    EXT_CSD_PART_SUPPORT_PART_EN) {
> -		if (card->ext_csd.enhanced_area_en != 1) {
> +		if (card->ext_csd.partition_setting_completed) {
>  			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++) {
> @@ -488,6 +485,13 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>  		ext_csd[EXT_CSD_TRIM_MULT];
>  	card->ext_csd.raw_partition_support = ext_csd[EXT_CSD_PARTITION_SUPPORT];
>  	if (card->ext_csd.rev >= 4) {
> +		if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
> +		    EXT_CSD_PART_SETTING_COMPLETED) {
> +			card->ext_csd.partition_setting_completed = 1;
> +		} else {
> +			card->ext_csd.partition_setting_completed = 0;
> +		}

I remembered that Ulf mentioned these brackets can be removed.

Best Regards,
Jaehoon Chung
> +
>  		mmc_manage_enhanced_area(card, ext_csd);
> 
>  		mmc_manage_gp_partitions(card, ext_csd);
> @@ -1324,7 +1328,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>  	 * If enhanced_area_en is TRUE, host needs to enable ERASE_GRP_DEF
>  	 * bit.  This bit will be lost every time after a reset or power off.
>  	 */
> -	if (card->ext_csd.enhanced_area_en ||
> +	if (card->ext_csd.partition_setting_completed ||
>  	    (card->ext_csd.rev >= 3 && (host->caps2 & MMC_CAP2_HC_ERASE_SZ))) {
>  		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>  				 EXT_CSD_ERASE_GROUP_DEF, 1,
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index d424b9d..38175be 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -74,7 +74,7 @@ struct mmc_ext_csd {
>  	unsigned int		sec_trim_mult;	/* Secure trim multiplier  */
>  	unsigned int		sec_erase_mult;	/* Secure erase multiplier */
>  	unsigned int		trim_timeout;		/* In milliseconds */
> -	bool			enhanced_area_en;	/* enable bit */
> +	bool			partition_setting_completed;	/* enable bit */
>  	unsigned long long	enhanced_area_offset;	/* Units: Byte */
>  	unsigned int		enhanced_area_size;	/* Units: KB */
>  	unsigned int		cache_size;		/* Units: KB */
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> index 64ec963..78753bc 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -281,6 +281,7 @@ struct _mmc_csd {
>  #define EXT_CSD_EXP_EVENTS_CTRL		56	/* R/W, 2 bytes */
>  #define EXT_CSD_DATA_SECTOR_SIZE	61	/* R */
>  #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
> +#define EXT_CSD_PARTITION_SETTING_COMPLETED 155	/* R/W */
>  #define EXT_CSD_PARTITION_ATTRIBUTE	156	/* R/W */
>  #define EXT_CSD_PARTITION_SUPPORT	160	/* RO */
>  #define EXT_CSD_HPI_MGMT		161	/* R/W */
> @@ -349,6 +350,7 @@ struct _mmc_csd {
>  #define EXT_CSD_PART_CONFIG_ACC_RPMB	(0x3)
>  #define EXT_CSD_PART_CONFIG_ACC_GP0	(0x4)
> 
> +#define EXT_CSD_PART_SETTING_COMPLETED	(0x1)
>  #define EXT_CSD_PART_SUPPORT_PART_EN	(0x1)
> 
>  #define EXT_CSD_CMD_SET_NORMAL		(1<<0)
> 

[PATCH v6 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed"

[Grégory Soutadé]

Replace ext_csd "enhanced_area_en" attribute by
 "partition_setting_completed". It was used whether or
 not enhanced user area is defined and without checks of
 EXT_CSD_PARTITION_SETTING_COMPLETED bit.

Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
---
 drivers/mmc/core/mmc.c   |   13 ++++++++-----
 include/linux/mmc/card.h |    2 +-
 include/linux/mmc/mmc.h  |    2 ++
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 77b4cf3..64d0371 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -314,7 +314,6 @@ static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
 		hc_wp_grp_sz =
 			ext_csd[EXT_CSD_HC_WP_GRP_SIZE];

-		card->ext_csd.enhanced_area_en = 1;
 		/*
 		 * calculate the enhanced data area offset, in bytes
 		 */
@@ -355,13 +354,11 @@ static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
 	 */
 	if (ext_csd[EXT_CSD_PARTITION_SUPPORT] &
 	    EXT_CSD_PART_SUPPORT_PART_EN) {
-		if (card->ext_csd.enhanced_area_en != 1) {
+		if (card->ext_csd.partition_setting_completed) {
 			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++) {
@@ -488,6 +485,12 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 		ext_csd[EXT_CSD_TRIM_MULT];
 	card->ext_csd.raw_partition_support = ext_csd[EXT_CSD_PARTITION_SUPPORT];
 	if (card->ext_csd.rev >= 4) {
+		if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
+		    EXT_CSD_PART_SETTING_COMPLETED)
+			card->ext_csd.partition_setting_completed = 1;
+		else
+			card->ext_csd.partition_setting_completed = 0;
+
 		mmc_manage_enhanced_area(card, ext_csd);

 		mmc_manage_gp_partitions(card, ext_csd);
@@ -1324,7 +1327,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	 * If enhanced_area_en is TRUE, host needs to enable ERASE_GRP_DEF
 	 * bit.  This bit will be lost every time after a reset or power off.
 	 */
-	if (card->ext_csd.enhanced_area_en ||
+	if (card->ext_csd.partition_setting_completed ||
 	    (card->ext_csd.rev >= 3 && (host->caps2 & MMC_CAP2_HC_ERASE_SZ))) {
 		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 				 EXT_CSD_ERASE_GROUP_DEF, 1,
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index d424b9d..38175be 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -74,7 +74,7 @@ struct mmc_ext_csd {
 	unsigned int		sec_trim_mult;	/* Secure trim multiplier  */
 	unsigned int		sec_erase_mult;	/* Secure erase multiplier */
 	unsigned int		trim_timeout;		/* In milliseconds */
-	bool			enhanced_area_en;	/* enable bit */
+	bool			partition_setting_completed;	/* enable bit */
 	unsigned long long	enhanced_area_offset;	/* Units: Byte */
 	unsigned int		enhanced_area_size;	/* Units: KB */
 	unsigned int		cache_size;		/* Units: KB */
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 64ec963..78753bc 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -281,6 +281,7 @@ struct _mmc_csd {
 #define EXT_CSD_EXP_EVENTS_CTRL		56	/* R/W, 2 bytes */
 #define EXT_CSD_DATA_SECTOR_SIZE	61	/* R */
 #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
+#define EXT_CSD_PARTITION_SETTING_COMPLETED 155	/* R/W */
 #define EXT_CSD_PARTITION_ATTRIBUTE	156	/* R/W */
 #define EXT_CSD_PARTITION_SUPPORT	160	/* RO */
 #define EXT_CSD_HPI_MGMT		161	/* R/W */
@@ -349,6 +350,7 @@ struct _mmc_csd {
 #define EXT_CSD_PART_CONFIG_ACC_RPMB	(0x3)
 #define EXT_CSD_PART_CONFIG_ACC_GP0	(0x4)

+#define EXT_CSD_PART_SETTING_COMPLETED	(0x1)
 #define EXT_CSD_PART_SUPPORT_PART_EN	(0x1)

 #define EXT_CSD_CMD_SET_NORMAL		(1<<0)
-- 
1.7.9.5

Le 15/09/2014 06:20, Jaehoon Chung a écrit :
> On 09/12/2014 11:31 PM, Grégory Soutadé wrote:
>> Replace ext_csd "enhanced_area_en" attribute by
>>  "partition_setting_completed". It was used whether or
>>  not enhanced user area is defined and without checks of
>>  EXT_CSD_PARTITION_SETTING_COMPLETED bit.
>>
>> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
>> ---
>>  drivers/mmc/core/mmc.c   |   14 +++++++++-----
>>  include/linux/mmc/card.h |    2 +-
>>  include/linux/mmc/mmc.h  |    2 ++
>>  3 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 77b4cf3..d8d3aef 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -314,7 +314,6 @@ static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
>>  		hc_wp_grp_sz =
>>  			ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
>>
>> -		card->ext_csd.enhanced_area_en = 1;
>>  		/*
>>  		 * calculate the enhanced data area offset, in bytes
>>  		 */
>> @@ -355,13 +354,11 @@ static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
>>  	 */
>>  	if (ext_csd[EXT_CSD_PARTITION_SUPPORT] &
>>  	    EXT_CSD_PART_SUPPORT_PART_EN) {
>> -		if (card->ext_csd.enhanced_area_en != 1) {
>> +		if (card->ext_csd.partition_setting_completed) {
>>  			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++) {
>> @@ -488,6 +485,13 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>  		ext_csd[EXT_CSD_TRIM_MULT];
>>  	card->ext_csd.raw_partition_support = ext_csd[EXT_CSD_PARTITION_SUPPORT];
>>  	if (card->ext_csd.rev >= 4) {
>> +		if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
>> +		    EXT_CSD_PART_SETTING_COMPLETED) {
>> +			card->ext_csd.partition_setting_completed = 1;
>> +		} else {
>> +			card->ext_csd.partition_setting_completed = 0;
>> +		}
> 
> I remembered that Ulf mentioned these brackets can be removed.
> 
> Best Regards,
> Jaehoon Chung
>> +
>>  		mmc_manage_enhanced_area(card, ext_csd);
>>
>>  		mmc_manage_gp_partitions(card, ext_csd);
>> @@ -1324,7 +1328,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>>  	 * If enhanced_area_en is TRUE, host needs to enable ERASE_GRP_DEF
>>  	 * bit.  This bit will be lost every time after a reset or power off.
>>  	 */
>> -	if (card->ext_csd.enhanced_area_en ||
>> +	if (card->ext_csd.partition_setting_completed ||
>>  	    (card->ext_csd.rev >= 3 && (host->caps2 & MMC_CAP2_HC_ERASE_SZ))) {
>>  		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>  				 EXT_CSD_ERASE_GROUP_DEF, 1,
>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> index d424b9d..38175be 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -74,7 +74,7 @@ struct mmc_ext_csd {
>>  	unsigned int		sec_trim_mult;	/* Secure trim multiplier  */
>>  	unsigned int		sec_erase_mult;	/* Secure erase multiplier */
>>  	unsigned int		trim_timeout;		/* In milliseconds */
>> -	bool			enhanced_area_en;	/* enable bit */
>> +	bool			partition_setting_completed;	/* enable bit */
>>  	unsigned long long	enhanced_area_offset;	/* Units: Byte */
>>  	unsigned int		enhanced_area_size;	/* Units: KB */
>>  	unsigned int		cache_size;		/* Units: KB */
>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>> index 64ec963..78753bc 100644
>> --- a/include/linux/mmc/mmc.h
>> +++ b/include/linux/mmc/mmc.h
>> @@ -281,6 +281,7 @@ struct _mmc_csd {
>>  #define EXT_CSD_EXP_EVENTS_CTRL		56	/* R/W, 2 bytes */
>>  #define EXT_CSD_DATA_SECTOR_SIZE	61	/* R */
>>  #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
>> +#define EXT_CSD_PARTITION_SETTING_COMPLETED 155	/* R/W */
>>  #define EXT_CSD_PARTITION_ATTRIBUTE	156	/* R/W */
>>  #define EXT_CSD_PARTITION_SUPPORT	160	/* RO */
>>  #define EXT_CSD_HPI_MGMT		161	/* R/W */
>> @@ -349,6 +350,7 @@ struct _mmc_csd {
>>  #define EXT_CSD_PART_CONFIG_ACC_RPMB	(0x3)
>>  #define EXT_CSD_PART_CONFIG_ACC_GP0	(0x4)
>>
>> +#define EXT_CSD_PART_SETTING_COMPLETED	(0x1)
>>  #define EXT_CSD_PART_SUPPORT_PART_EN	(0x1)
>>
>>  #define EXT_CSD_CMD_SET_NORMAL		(1<<0)
>>
> 
> 

Re: [PATCH v6 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed"

[Ulf Hansson]

On 15 September 2014 17:47, Grégory Soutadé <gsoutade@neotion.com> wrote:
> Replace ext_csd "enhanced_area_en" attribute by
>  "partition_setting_completed". It was used whether or
>  not enhanced user area is defined and without checks of
>  EXT_CSD_PARTITION_SETTING_COMPLETED bit.
>
> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>

Thanks! Applied for next.

Kind regards
Uffe


> ---
>  drivers/mmc/core/mmc.c   |   13 ++++++++-----
>  include/linux/mmc/card.h |    2 +-
>  include/linux/mmc/mmc.h  |    2 ++
>  3 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 77b4cf3..64d0371 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -314,7 +314,6 @@ static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
>                 hc_wp_grp_sz =
>                         ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
>
> -               card->ext_csd.enhanced_area_en = 1;
>                 /*
>                  * calculate the enhanced data area offset, in bytes
>                  */
> @@ -355,13 +354,11 @@ static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
>          */
>         if (ext_csd[EXT_CSD_PARTITION_SUPPORT] &
>             EXT_CSD_PART_SUPPORT_PART_EN) {
> -               if (card->ext_csd.enhanced_area_en != 1) {
> +               if (card->ext_csd.partition_setting_completed) {
>                         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++) {
> @@ -488,6 +485,12 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>                 ext_csd[EXT_CSD_TRIM_MULT];
>         card->ext_csd.raw_partition_support = ext_csd[EXT_CSD_PARTITION_SUPPORT];
>         if (card->ext_csd.rev >= 4) {
> +               if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
> +                   EXT_CSD_PART_SETTING_COMPLETED)
> +                       card->ext_csd.partition_setting_completed = 1;
> +               else
> +                       card->ext_csd.partition_setting_completed = 0;
> +
>                 mmc_manage_enhanced_area(card, ext_csd);
>
>                 mmc_manage_gp_partitions(card, ext_csd);
> @@ -1324,7 +1327,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>          * If enhanced_area_en is TRUE, host needs to enable ERASE_GRP_DEF
>          * bit.  This bit will be lost every time after a reset or power off.
>          */
> -       if (card->ext_csd.enhanced_area_en ||
> +       if (card->ext_csd.partition_setting_completed ||
>             (card->ext_csd.rev >= 3 && (host->caps2 & MMC_CAP2_HC_ERASE_SZ))) {
>                 err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>                                  EXT_CSD_ERASE_GROUP_DEF, 1,
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index d424b9d..38175be 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -74,7 +74,7 @@ struct mmc_ext_csd {
>         unsigned int            sec_trim_mult;  /* Secure trim multiplier  */
>         unsigned int            sec_erase_mult; /* Secure erase multiplier */
>         unsigned int            trim_timeout;           /* In milliseconds */
> -       bool                    enhanced_area_en;       /* enable bit */
> +       bool                    partition_setting_completed;    /* enable bit */
>         unsigned long long      enhanced_area_offset;   /* Units: Byte */
>         unsigned int            enhanced_area_size;     /* Units: KB */
>         unsigned int            cache_size;             /* Units: KB */
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> index 64ec963..78753bc 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -281,6 +281,7 @@ struct _mmc_csd {
>  #define EXT_CSD_EXP_EVENTS_CTRL                56      /* R/W, 2 bytes */
>  #define EXT_CSD_DATA_SECTOR_SIZE       61      /* R */
>  #define EXT_CSD_GP_SIZE_MULT           143     /* R/W */
> +#define EXT_CSD_PARTITION_SETTING_COMPLETED 155        /* R/W */
>  #define EXT_CSD_PARTITION_ATTRIBUTE    156     /* R/W */
>  #define EXT_CSD_PARTITION_SUPPORT      160     /* RO */
>  #define EXT_CSD_HPI_MGMT               161     /* R/W */
> @@ -349,6 +350,7 @@ struct _mmc_csd {
>  #define EXT_CSD_PART_CONFIG_ACC_RPMB   (0x3)
>  #define EXT_CSD_PART_CONFIG_ACC_GP0    (0x4)
>
> +#define EXT_CSD_PART_SETTING_COMPLETED (0x1)
>  #define EXT_CSD_PART_SUPPORT_PART_EN   (0x1)
>
>  #define EXT_CSD_CMD_SET_NORMAL         (1<<0)
> --
> 1.7.9.5
>
> Le 15/09/2014 06:20, Jaehoon Chung a écrit :
>> On 09/12/2014 11:31 PM, Grégory Soutadé wrote:
>>> Replace ext_csd "enhanced_area_en" attribute by
>>>  "partition_setting_completed". It was used whether or
>>>  not enhanced user area is defined and without checks of
>>>  EXT_CSD_PARTITION_SETTING_COMPLETED bit.
>>>
>>> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
>>> ---
>>>  drivers/mmc/core/mmc.c   |   14 +++++++++-----
>>>  include/linux/mmc/card.h |    2 +-
>>>  include/linux/mmc/mmc.h  |    2 ++
>>>  3 files changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index 77b4cf3..d8d3aef 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -314,7 +314,6 @@ static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
>>>              hc_wp_grp_sz =
>>>                      ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
>>>
>>> -            card->ext_csd.enhanced_area_en = 1;
>>>              /*
>>>               * calculate the enhanced data area offset, in bytes
>>>               */
>>> @@ -355,13 +354,11 @@ static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
>>>       */
>>>      if (ext_csd[EXT_CSD_PARTITION_SUPPORT] &
>>>          EXT_CSD_PART_SUPPORT_PART_EN) {
>>> -            if (card->ext_csd.enhanced_area_en != 1) {
>>> +            if (card->ext_csd.partition_setting_completed) {
>>>                      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++) {
>>> @@ -488,6 +485,13 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>>              ext_csd[EXT_CSD_TRIM_MULT];
>>>      card->ext_csd.raw_partition_support = ext_csd[EXT_CSD_PARTITION_SUPPORT];
>>>      if (card->ext_csd.rev >= 4) {
>>> +            if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
>>> +                EXT_CSD_PART_SETTING_COMPLETED) {
>>> +                    card->ext_csd.partition_setting_completed = 1;
>>> +            } else {
>>> +                    card->ext_csd.partition_setting_completed = 0;
>>> +            }
>>
>> I remembered that Ulf mentioned these brackets can be removed.
>>
>> Best Regards,
>> Jaehoon Chung
>>> +
>>>              mmc_manage_enhanced_area(card, ext_csd);
>>>
>>>              mmc_manage_gp_partitions(card, ext_csd);
>>> @@ -1324,7 +1328,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>>>       * If enhanced_area_en is TRUE, host needs to enable ERASE_GRP_DEF
>>>       * bit.  This bit will be lost every time after a reset or power off.
>>>       */
>>> -    if (card->ext_csd.enhanced_area_en ||
>>> +    if (card->ext_csd.partition_setting_completed ||
>>>          (card->ext_csd.rev >= 3 && (host->caps2 & MMC_CAP2_HC_ERASE_SZ))) {
>>>              err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>>                               EXT_CSD_ERASE_GROUP_DEF, 1,
>>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>>> index d424b9d..38175be 100644
>>> --- a/include/linux/mmc/card.h
>>> +++ b/include/linux/mmc/card.h
>>> @@ -74,7 +74,7 @@ struct mmc_ext_csd {
>>>      unsigned int            sec_trim_mult;  /* Secure trim multiplier  */
>>>      unsigned int            sec_erase_mult; /* Secure erase multiplier */
>>>      unsigned int            trim_timeout;           /* In milliseconds */
>>> -    bool                    enhanced_area_en;       /* enable bit */
>>> +    bool                    partition_setting_completed;    /* enable bit */
>>>      unsigned long long      enhanced_area_offset;   /* Units: Byte */
>>>      unsigned int            enhanced_area_size;     /* Units: KB */
>>>      unsigned int            cache_size;             /* Units: KB */
>>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>>> index 64ec963..78753bc 100644
>>> --- a/include/linux/mmc/mmc.h
>>> +++ b/include/linux/mmc/mmc.h
>>> @@ -281,6 +281,7 @@ struct _mmc_csd {
>>>  #define EXT_CSD_EXP_EVENTS_CTRL             56      /* R/W, 2 bytes */
>>>  #define EXT_CSD_DATA_SECTOR_SIZE    61      /* R */
>>>  #define EXT_CSD_GP_SIZE_MULT                143     /* R/W */
>>> +#define EXT_CSD_PARTITION_SETTING_COMPLETED 155     /* R/W */
>>>  #define EXT_CSD_PARTITION_ATTRIBUTE 156     /* R/W */
>>>  #define EXT_CSD_PARTITION_SUPPORT   160     /* RO */
>>>  #define EXT_CSD_HPI_MGMT            161     /* R/W */
>>> @@ -349,6 +350,7 @@ struct _mmc_csd {
>>>  #define EXT_CSD_PART_CONFIG_ACC_RPMB        (0x3)
>>>  #define EXT_CSD_PART_CONFIG_ACC_GP0 (0x4)
>>>
>>> +#define EXT_CSD_PART_SETTING_COMPLETED      (0x1)
>>>  #define EXT_CSD_PART_SUPPORT_PART_EN        (0x1)
>>>
>>>  #define EXT_CSD_CMD_SET_NORMAL              (1<<0)
>>>
>>
>>

[PATCH v5 0003/0003] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation

[Grégory Soutadé]

Checks EXT_CSD_PARTITION_SETTING_COMPLETED bit before
 computing enhanced user area offset and size, and
 adding mmc general purpose partitions. The two needs
 EXT_CSD_PARTITION_SETTING_COMPLETED bit be set to be
 valid (as described in JEDEC standard).
Warn user in case of misconfiguration.

Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
---
 drivers/mmc/core/mmc.c |   81 ++++++++++++++++++++++++++----------------------
 1 file changed, 44 insertions(+), 37 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index d8d3aef..a60685a 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -300,7 +300,13 @@ static void mmc_select_card_type(struct mmc_card *card)

 static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
 {
-	u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
+	u8 hc_erase_grp_sz, hc_wp_grp_sz;
+
+	/*
+	 * Disable these attributes by default
+	 */
+	card->ext_csd.enhanced_area_offset = -EINVAL;
+	card->ext_csd.enhanced_area_size = -EINVAL;

 	/*
 	 * Enhanced area feature support -- check whether the eMMC
@@ -309,43 +315,41 @@ static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
 	 */
 	if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
 	    (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
-		hc_erase_grp_sz =
-			ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
-		hc_wp_grp_sz =
-			ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+		if (card->ext_csd.partition_setting_completed) {
+			hc_erase_grp_sz =
+				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
+			hc_wp_grp_sz =
+				ext_csd[EXT_CSD_HC_WP_GRP_SIZE];

-		/*
-		 * calculate the enhanced data area offset, in bytes
-		 */
-		card->ext_csd.enhanced_area_offset =
-			(ext_csd[139] << 24) + (ext_csd[138] << 16) +
-			(ext_csd[137] << 8) + ext_csd[136];
-		if (mmc_card_blockaddr(card))
-			card->ext_csd.enhanced_area_offset <<= 9;
-		/*
-		 * calculate the enhanced data area size, in kilobytes
-		 */
-		card->ext_csd.enhanced_area_size =
-			(ext_csd[142] << 16) + (ext_csd[141] << 8) +
-			ext_csd[140];
-		card->ext_csd.enhanced_area_size *=
-			(size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
-		card->ext_csd.enhanced_area_size <<= 9;
-	} else {
-		/*
-		 * If the enhanced area is not enabled, disable these
-		 * device attributes.
-		 */
-		card->ext_csd.enhanced_area_offset = -EINVAL;
-		card->ext_csd.enhanced_area_size = -EINVAL;
+			/*
+			 * calculate the enhanced data area offset, in bytes
+			 */
+			card->ext_csd.enhanced_area_offset =
+				(ext_csd[139] << 24) + (ext_csd[138] << 16) +
+				(ext_csd[137] << 8) + ext_csd[136];
+			if (mmc_card_blockaddr(card))
+				card->ext_csd.enhanced_area_offset <<= 9;
+			/*
+			 * calculate the enhanced data area size, in kilobytes
+			 */
+			card->ext_csd.enhanced_area_size =
+				(ext_csd[142] << 16) + (ext_csd[141] << 8) +
+				ext_csd[140];
+			card->ext_csd.enhanced_area_size *=
+				(size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
+			card->ext_csd.enhanced_area_size <<= 9;
+		} else {
+			pr_warn("%s: defines enhanced area without partition setting complete\n",
+				mmc_hostname(card->host));
+		}
 	}
 }

 static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
 {
-	unsigned int part_size;
-	u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
 	int idx;
+	u8 hc_erase_grp_sz, hc_wp_grp_sz;
+	unsigned int part_size;

 	/*
 	 * General purpose partition feature support --
@@ -354,18 +358,21 @@ static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
 	 */
 	if (ext_csd[EXT_CSD_PARTITION_SUPPORT] &
 	    EXT_CSD_PART_SUPPORT_PART_EN) {
-		if (card->ext_csd.partition_setting_completed) {
-			hc_erase_grp_sz =
-				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
-			hc_wp_grp_sz =
-				ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
-		}
+		hc_erase_grp_sz =
+			ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
+		hc_wp_grp_sz =
+			ext_csd[EXT_CSD_HC_WP_GRP_SIZE];

 		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;
+			if (card->ext_csd.partition_setting_completed == 0) {
+				pr_warn("%s: has partition size defined without partition complete\n",
+					mmc_hostname(card->host));
+				break;
+			}
 			part_size =
 				(ext_csd[EXT_CSD_GP_SIZE_MULT + idx * 3 + 2]
 				<< 16) +
-- 
1.7.9.5

[PATCH v6 0003/0003] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation

[Grégory Soutadé]

Checks EXT_CSD_PARTITION_SETTING_COMPLETED bit before
 computing enhanced user area offset and size, and
 adding mmc general purpose partitions. The two needs
 EXT_CSD_PARTITION_SETTING_COMPLETED bit be set to be
 valid (as described in JEDEC standard).
Warn user in case of misconfiguration.

Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
---
 drivers/mmc/core/mmc.c |   81 ++++++++++++++++++++++++++----------------------
 1 file changed, 44 insertions(+), 37 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 64d0371..66928b0 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -300,7 +300,13 @@ static void mmc_select_card_type(struct mmc_card *card)

 static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
 {
-	u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
+	u8 hc_erase_grp_sz, hc_wp_grp_sz;
+
+	/*
+	 * Disable these attributes by default
+	 */
+	card->ext_csd.enhanced_area_offset = -EINVAL;
+	card->ext_csd.enhanced_area_size = -EINVAL;

 	/*
 	 * Enhanced area feature support -- check whether the eMMC
@@ -309,43 +315,41 @@ static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
 	 */
 	if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
 	    (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
-		hc_erase_grp_sz =
-			ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
-		hc_wp_grp_sz =
-			ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+		if (card->ext_csd.partition_setting_completed) {
+			hc_erase_grp_sz =
+				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
+			hc_wp_grp_sz =
+				ext_csd[EXT_CSD_HC_WP_GRP_SIZE];

-		/*
-		 * calculate the enhanced data area offset, in bytes
-		 */
-		card->ext_csd.enhanced_area_offset =
-			(ext_csd[139] << 24) + (ext_csd[138] << 16) +
-			(ext_csd[137] << 8) + ext_csd[136];
-		if (mmc_card_blockaddr(card))
-			card->ext_csd.enhanced_area_offset <<= 9;
-		/*
-		 * calculate the enhanced data area size, in kilobytes
-		 */
-		card->ext_csd.enhanced_area_size =
-			(ext_csd[142] << 16) + (ext_csd[141] << 8) +
-			ext_csd[140];
-		card->ext_csd.enhanced_area_size *=
-			(size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
-		card->ext_csd.enhanced_area_size <<= 9;
-	} else {
-		/*
-		 * If the enhanced area is not enabled, disable these
-		 * device attributes.
-		 */
-		card->ext_csd.enhanced_area_offset = -EINVAL;
-		card->ext_csd.enhanced_area_size = -EINVAL;
+			/*
+			 * calculate the enhanced data area offset, in bytes
+			 */
+			card->ext_csd.enhanced_area_offset =
+				(ext_csd[139] << 24) + (ext_csd[138] << 16) +
+				(ext_csd[137] << 8) + ext_csd[136];
+			if (mmc_card_blockaddr(card))
+				card->ext_csd.enhanced_area_offset <<= 9;
+			/*
+			 * calculate the enhanced data area size, in kilobytes
+			 */
+			card->ext_csd.enhanced_area_size =
+				(ext_csd[142] << 16) + (ext_csd[141] << 8) +
+				ext_csd[140];
+			card->ext_csd.enhanced_area_size *=
+				(size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
+			card->ext_csd.enhanced_area_size <<= 9;
+		} else {
+			pr_warn("%s: defines enhanced area without partition setting complete\n",
+				mmc_hostname(card->host));
+		}
 	}
 }

 static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
 {
-	unsigned int part_size;
-	u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
 	int idx;
+	u8 hc_erase_grp_sz, hc_wp_grp_sz;
+	unsigned int part_size;

 	/*
 	 * General purpose partition feature support --
@@ -354,18 +358,21 @@ static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
 	 */
 	if (ext_csd[EXT_CSD_PARTITION_SUPPORT] &
 	    EXT_CSD_PART_SUPPORT_PART_EN) {
-		if (card->ext_csd.partition_setting_completed) {
-			hc_erase_grp_sz =
-				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
-			hc_wp_grp_sz =
-				ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
-		}
+		hc_erase_grp_sz =
+			ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
+		hc_wp_grp_sz =
+			ext_csd[EXT_CSD_HC_WP_GRP_SIZE];

 		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;
+			if (card->ext_csd.partition_setting_completed == 0) {
+				pr_warn("%s: has partition size defined without partition complete\n",
+					mmc_hostname(card->host));
+				break;
+			}
 			part_size =
 				(ext_csd[EXT_CSD_GP_SIZE_MULT + idx * 3 + 2]
 				<< 16) +
-- 
1.7.9.5

Re: [PATCH v6 0003/0003] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation

[Ulf Hansson]

On 15 September 2014 17:47, Grégory Soutadé <gsoutade@neotion.com> wrote:
> Checks EXT_CSD_PARTITION_SETTING_COMPLETED bit before
>  computing enhanced user area offset and size, and
>  adding mmc general purpose partitions. The two needs
>  EXT_CSD_PARTITION_SETTING_COMPLETED bit be set to be
>  valid (as described in JEDEC standard).
> Warn user in case of misconfiguration.
>
> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>

Thanks! Applied for next.

Kind regards
Uffe


> ---
>  drivers/mmc/core/mmc.c |   81 ++++++++++++++++++++++++++----------------------
>  1 file changed, 44 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 64d0371..66928b0 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -300,7 +300,13 @@ static void mmc_select_card_type(struct mmc_card *card)
>
>  static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
>  {
> -       u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
> +       u8 hc_erase_grp_sz, hc_wp_grp_sz;
> +
> +       /*
> +        * Disable these attributes by default
> +        */
> +       card->ext_csd.enhanced_area_offset = -EINVAL;
> +       card->ext_csd.enhanced_area_size = -EINVAL;
>
>         /*
>          * Enhanced area feature support -- check whether the eMMC
> @@ -309,43 +315,41 @@ static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
>          */
>         if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
>             (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
> -               hc_erase_grp_sz =
> -                       ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
> -               hc_wp_grp_sz =
> -                       ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
> +               if (card->ext_csd.partition_setting_completed) {
> +                       hc_erase_grp_sz =
> +                               ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
> +                       hc_wp_grp_sz =
> +                               ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
>
> -               /*
> -                * calculate the enhanced data area offset, in bytes
> -                */
> -               card->ext_csd.enhanced_area_offset =
> -                       (ext_csd[139] << 24) + (ext_csd[138] << 16) +
> -                       (ext_csd[137] << 8) + ext_csd[136];
> -               if (mmc_card_blockaddr(card))
> -                       card->ext_csd.enhanced_area_offset <<= 9;
> -               /*
> -                * calculate the enhanced data area size, in kilobytes
> -                */
> -               card->ext_csd.enhanced_area_size =
> -                       (ext_csd[142] << 16) + (ext_csd[141] << 8) +
> -                       ext_csd[140];
> -               card->ext_csd.enhanced_area_size *=
> -                       (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
> -               card->ext_csd.enhanced_area_size <<= 9;
> -       } else {
> -               /*
> -                * If the enhanced area is not enabled, disable these
> -                * device attributes.
> -                */
> -               card->ext_csd.enhanced_area_offset = -EINVAL;
> -               card->ext_csd.enhanced_area_size = -EINVAL;
> +                       /*
> +                        * calculate the enhanced data area offset, in bytes
> +                        */
> +                       card->ext_csd.enhanced_area_offset =
> +                               (ext_csd[139] << 24) + (ext_csd[138] << 16) +
> +                               (ext_csd[137] << 8) + ext_csd[136];
> +                       if (mmc_card_blockaddr(card))
> +                               card->ext_csd.enhanced_area_offset <<= 9;
> +                       /*
> +                        * calculate the enhanced data area size, in kilobytes
> +                        */
> +                       card->ext_csd.enhanced_area_size =
> +                               (ext_csd[142] << 16) + (ext_csd[141] << 8) +
> +                               ext_csd[140];
> +                       card->ext_csd.enhanced_area_size *=
> +                               (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
> +                       card->ext_csd.enhanced_area_size <<= 9;
> +               } else {
> +                       pr_warn("%s: defines enhanced area without partition setting complete\n",
> +                               mmc_hostname(card->host));
> +               }
>         }
>  }
>
>  static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
>  {
> -       unsigned int part_size;
> -       u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
>         int idx;
> +       u8 hc_erase_grp_sz, hc_wp_grp_sz;
> +       unsigned int part_size;
>
>         /*
>          * General purpose partition feature support --
> @@ -354,18 +358,21 @@ static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
>          */
>         if (ext_csd[EXT_CSD_PARTITION_SUPPORT] &
>             EXT_CSD_PART_SUPPORT_PART_EN) {
> -               if (card->ext_csd.partition_setting_completed) {
> -                       hc_erase_grp_sz =
> -                               ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
> -                       hc_wp_grp_sz =
> -                               ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
> -               }
> +               hc_erase_grp_sz =
> +                       ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
> +               hc_wp_grp_sz =
> +                       ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
>
>                 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;
> +                       if (card->ext_csd.partition_setting_completed == 0) {
> +                               pr_warn("%s: has partition size defined without partition complete\n",
> +                                       mmc_hostname(card->host));
> +                               break;
> +                       }
>                         part_size =
>                                 (ext_csd[EXT_CSD_GP_SIZE_MULT + idx * 3 + 2]
>                                 << 16) +
> --
> 1.7.9.5

[PATCH v4 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed"

[Grégory Soutadé]

Replace ext_csd "enhanced_area_en" attribute by
 "partition_setting_completed". It was used whether or
 not enhanced user area is defined and without checks of
 EXT_CSD_PARTITION_SETTING_COMPLETED bit.

Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
---
 drivers/mmc/core/mmc.c   |    9 ++++++++-
 include/linux/mmc/card.h |    2 +-
 include/linux/mmc/mmc.h  |    2 ++
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 77b4cf3..3578e35 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -488,6 +488,13 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 		ext_csd[EXT_CSD_TRIM_MULT];
 	card->ext_csd.raw_partition_support = ext_csd[EXT_CSD_PARTITION_SUPPORT];
 	if (card->ext_csd.rev >= 4) {
+		if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
+		    EXT_CSD_PART_SETTING_COMPLETED) {
+			card->ext_csd.partition_setting_completed = 1;
+		} else {
+			card->ext_csd.partition_setting_completed = 0;
+		}
+
 		mmc_manage_enhanced_area(card, ext_csd);

 		mmc_manage_gp_partitions(card, ext_csd);
@@ -1324,7 +1331,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	 * If enhanced_area_en is TRUE, host needs to enable ERASE_GRP_DEF
 	 * bit.  This bit will be lost every time after a reset or power off.
 	 */
-	if (card->ext_csd.enhanced_area_en ||
+	if (card->ext_csd.partition_setting_completed ||
 	    (card->ext_csd.rev >= 3 && (host->caps2 & MMC_CAP2_HC_ERASE_SZ))) {
 		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 				 EXT_CSD_ERASE_GROUP_DEF, 1,
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index d424b9d..38175be 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -74,7 +74,7 @@ struct mmc_ext_csd {
 	unsigned int		sec_trim_mult;	/* Secure trim multiplier  */
 	unsigned int		sec_erase_mult;	/* Secure erase multiplier */
 	unsigned int		trim_timeout;		/* In milliseconds */
-	bool			enhanced_area_en;	/* enable bit */
+	bool			partition_setting_completed;	/* enable bit */
 	unsigned long long	enhanced_area_offset;	/* Units: Byte */
 	unsigned int		enhanced_area_size;	/* Units: KB */
 	unsigned int		cache_size;		/* Units: KB */
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 64ec963..78753bc 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -281,6 +281,7 @@ struct _mmc_csd {
 #define EXT_CSD_EXP_EVENTS_CTRL		56	/* R/W, 2 bytes */
 #define EXT_CSD_DATA_SECTOR_SIZE	61	/* R */
 #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
+#define EXT_CSD_PARTITION_SETTING_COMPLETED 155	/* R/W */
 #define EXT_CSD_PARTITION_ATTRIBUTE	156	/* R/W */
 #define EXT_CSD_PARTITION_SUPPORT	160	/* RO */
 #define EXT_CSD_HPI_MGMT		161	/* R/W */
@@ -349,6 +350,7 @@ struct _mmc_csd {
 #define EXT_CSD_PART_CONFIG_ACC_RPMB	(0x3)
 #define EXT_CSD_PART_CONFIG_ACC_GP0	(0x4)

+#define EXT_CSD_PART_SETTING_COMPLETED	(0x1)
 #define EXT_CSD_PART_SUPPORT_PART_EN	(0x1)

 #define EXT_CSD_CMD_SET_NORMAL		(1<<0)
-- 
1.7.9.5

Re: [PATCH v4 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed"

[Ulf Hansson]

On 11 September 2014 08:38, Grégory Soutadé <gsoutade@neotion.com> wrote:
> Replace ext_csd "enhanced_area_en" attribute by
>  "partition_setting_completed". It was used whether or
>  not enhanced user area is defined and without checks of
>  EXT_CSD_PARTITION_SETTING_COMPLETED bit.
>
> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>

This patch doesn't compile.

I also think you could work a bit on the commit messages in this
patchset. Try to describe what the patches do and why. I may help you
- if you like.

> ---
>  drivers/mmc/core/mmc.c   |    9 ++++++++-
>  include/linux/mmc/card.h |    2 +-
>  include/linux/mmc/mmc.h  |    2 ++
>  3 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 77b4cf3..3578e35 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -488,6 +488,13 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>                 ext_csd[EXT_CSD_TRIM_MULT];
>         card->ext_csd.raw_partition_support = ext_csd[EXT_CSD_PARTITION_SUPPORT];
>         if (card->ext_csd.rev >= 4) {
> +               if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
> +                   EXT_CSD_PART_SETTING_COMPLETED) {

No need for brackets if this if-else, please remove.

> +                       card->ext_csd.partition_setting_completed = 1;
> +               } else {
> +                       card->ext_csd.partition_setting_completed = 0;
> +               }
> +
>                 mmc_manage_enhanced_area(card, ext_csd);
>
>                 mmc_manage_gp_partitions(card, ext_csd);

[...]

Kind regards
Uffe

[PATCH v4 0003/0003] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation

[Grégory Soutadé]

Checks EXT_CSD_PARTITION_SETTING_COMPLETED bit before
 computing enhanced user area offset and size, and
 adding mmc general purpose partitions. The two needs
 EXT_CSD_PARTITION_SETTING_COMPLETED bit be set to be
 valid (as described in JEDEC standard).

Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
---
 drivers/mmc/core/mmc.c |   84 +++++++++++++++++++++++++-----------------------
 1 file changed, 44 insertions(+), 40 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 3578e35..a60685a 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -300,7 +300,13 @@ static void mmc_select_card_type(struct mmc_card *card)

 static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
 {
-	u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
+	u8 hc_erase_grp_sz, hc_wp_grp_sz;
+
+	/*
+	 * Disable these attributes by default
+	 */
+	card->ext_csd.enhanced_area_offset = -EINVAL;
+	card->ext_csd.enhanced_area_size = -EINVAL;

 	/*
 	 * Enhanced area feature support -- check whether the eMMC
@@ -309,44 +315,41 @@ static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
 	 */
 	if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
 	    (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
-		hc_erase_grp_sz =
-			ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
-		hc_wp_grp_sz =
-			ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+		if (card->ext_csd.partition_setting_completed) {
+			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;
-		/*
-		 * calculate the enhanced data area offset, in bytes
-		 */
-		card->ext_csd.enhanced_area_offset =
-			(ext_csd[139] << 24) + (ext_csd[138] << 16) +
-			(ext_csd[137] << 8) + ext_csd[136];
-		if (mmc_card_blockaddr(card))
-			card->ext_csd.enhanced_area_offset <<= 9;
-		/*
-		 * calculate the enhanced data area size, in kilobytes
-		 */
-		card->ext_csd.enhanced_area_size =
-			(ext_csd[142] << 16) + (ext_csd[141] << 8) +
-			ext_csd[140];
-		card->ext_csd.enhanced_area_size *=
-			(size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
-		card->ext_csd.enhanced_area_size <<= 9;
-	} else {
-		/*
-		 * If the enhanced area is not enabled, disable these
-		 * device attributes.
-		 */
-		card->ext_csd.enhanced_area_offset = -EINVAL;
-		card->ext_csd.enhanced_area_size = -EINVAL;
+			/*
+			 * calculate the enhanced data area offset, in bytes
+			 */
+			card->ext_csd.enhanced_area_offset =
+				(ext_csd[139] << 24) + (ext_csd[138] << 16) +
+				(ext_csd[137] << 8) + ext_csd[136];
+			if (mmc_card_blockaddr(card))
+				card->ext_csd.enhanced_area_offset <<= 9;
+			/*
+			 * calculate the enhanced data area size, in kilobytes
+			 */
+			card->ext_csd.enhanced_area_size =
+				(ext_csd[142] << 16) + (ext_csd[141] << 8) +
+				ext_csd[140];
+			card->ext_csd.enhanced_area_size *=
+				(size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
+			card->ext_csd.enhanced_area_size <<= 9;
+		} else {
+			pr_warn("%s: defines enhanced area without partition setting complete\n",
+				mmc_hostname(card->host));
+		}
 	}
 }

 static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
 {
-	unsigned int part_size;
-	u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
 	int idx;
+	u8 hc_erase_grp_sz, hc_wp_grp_sz;
+	unsigned int part_size;

 	/*
 	 * General purpose partition feature support --
@@ -355,20 +358,21 @@ static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
 	 */
 	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;
-		}
+		hc_erase_grp_sz =
+			ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
+		hc_wp_grp_sz =
+			ext_csd[EXT_CSD_HC_WP_GRP_SIZE];

 		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;
+			if (card->ext_csd.partition_setting_completed == 0) {
+				pr_warn("%s: has partition size defined without partition complete\n",
+					mmc_hostname(card->host));
+				break;
+			}
 			part_size =
 				(ext_csd[EXT_CSD_GP_SIZE_MULT + idx * 3 + 2]
 				<< 16) +
-- 
1.7.9.5

[PATCH v3 0002/0002] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED bit before partitions computation

[Grégory Soutadé]

Checks EXT_CSD_PARTITION_SETTING_COMPLETED bit before
computing enhanced user area offset and size, and adding
mmc general purpose partitions.
The two needs EXT_CSD_PARTITION_SETTING_COMPLETED bit be set
to be valid (as described in JEDEC standard).

Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
---
 drivers/mmc/core/mmc.c |  167 +++++++++++++++++++++++++++---------------------
 1 file changed, 95 insertions(+), 72 deletions(-)

Changelog v2:
	Move code for user area and general purpose partitions
	into functions.

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index ef25d91..3158470 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -298,6 +298,99 @@ static void mmc_select_card_type(struct mmc_card *card)
 	card->mmc_avail_type = avail_type;
 }

+static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
+{
+	u8 hc_erase_grp_sz, hc_wp_grp_sz;
+
+	/*
+	 * Enhanced area feature support -- check whether the eMMC
+	 * card has the Enhanced area enabled.  If so, export enhanced
+	 * area offset and size to user by adding sysfs interface.
+	 */
+	if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
+	    (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
+		if (card->ext_csd.partition_setting_completed) {
+			hc_erase_grp_sz =
+				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
+			hc_wp_grp_sz =
+				ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+			/*
+			 * calculate the enhanced data area offset, in bytes
+			 */
+			card->ext_csd.enhanced_area_offset =
+				(ext_csd[139] << 24) + (ext_csd[138] << 16) +
+				(ext_csd[137] << 8) + ext_csd[136];
+			if (mmc_card_blockaddr(card))
+				card->ext_csd.enhanced_area_offset <<= 9;
+			/*
+			 * calculate the enhanced data area size, in kilobytes
+			 */
+			card->ext_csd.enhanced_area_size =
+				(ext_csd[142] << 16) + (ext_csd[141] << 8) +
+				ext_csd[140];
+			card->ext_csd.enhanced_area_size *=
+				(size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
+			card->ext_csd.enhanced_area_size <<= 9;
+		} else {
+			pr_warn("%s: defines enhanced area without partition setting complete\n",
+				mmc_hostname(card->host));
+		}
+	} else {
+		/*
+		 * If the enhanced area is not enabled, disable these
+		 * device attributes.
+		 */
+		card->ext_csd.enhanced_area_offset = -EINVAL;
+		card->ext_csd.enhanced_area_size = -EINVAL;
+	}
+}
+
+static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
+{
+	int idx;
+	u8 hc_erase_grp_sz, hc_wp_grp_sz;
+	unsigned int part_size;
+
+	/*
+	 * General purpose partition feature support --
+	 * If ext_csd has 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) {
+		hc_erase_grp_sz =
+			ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
+		hc_wp_grp_sz =
+			ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+
+		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;
+
+			if (card->ext_csd.partition_setting_completed == 0) {
+				pr_warn("%s: has partition size defined without partition complete\n",
+					mmc_hostname(card->host));
+				break;
+			}
+
+			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,
+				MMC_BLK_DATA_AREA_GP);
+		}
+	}
+}
+
 /*
  * Decode extended CSD.
  */
@@ -305,7 +398,6 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 {
 	int err = 0, idx;
 	unsigned int part_size;
-	u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;

 	BUG_ON(!card);

@@ -409,80 +501,11 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 		} else {
 			card->ext_csd.partition_setting_completed = 0;
 		}
-		/*
-		 * Enhanced area feature support -- check whether the eMMC
-		 * card has the Enhanced area enabled.  If so, export enhanced
-		 * area offset and size to user by adding sysfs interface.
-		 */
-		if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
-		    (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
-			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;
-			/*
-			 * calculate the enhanced data area offset, in bytes
-			 */
-			card->ext_csd.enhanced_area_offset =
-				(ext_csd[139] << 24) + (ext_csd[138] << 16) +
-				(ext_csd[137] << 8) + ext_csd[136];
-			if (mmc_card_blockaddr(card))
-				card->ext_csd.enhanced_area_offset <<= 9;
-			/*
-			 * calculate the enhanced data area size, in kilobytes
-			 */
-			card->ext_csd.enhanced_area_size =
-				(ext_csd[142] << 16) + (ext_csd[141] << 8) +
-				ext_csd[140];
-			card->ext_csd.enhanced_area_size *=
-				(size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
-			card->ext_csd.enhanced_area_size <<= 9;
-		} else {
-			/*
-			 * If the enhanced area is not enabled, disable these
-			 * device attributes.
-			 */
-			card->ext_csd.enhanced_area_offset = -EINVAL;
-			card->ext_csd.enhanced_area_size = -EINVAL;
-		}
+		mmc_manage_enhanced_area(card, ext_csd);

-		/*
-		 * General purpose partition feature support --
-		 * If ext_csd has 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;
-			}
+		mmc_manage_gp_partitions(card, ext_csd);

-			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,
-					MMC_BLK_DATA_AREA_GP);
-			}
-		}
 		card->ext_csd.sec_trim_mult =
 			ext_csd[EXT_CSD_SEC_TRIM_MULT];
 		card->ext_csd.sec_erase_mult =
--
1.7.9.5

Re: [PATCH v3 0002/0002] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED bit before partitions computation

[Ulf Hansson]

On 18 August 2014 15:12, Grégory Soutadé <gsoutade@neotion.com> wrote:
> Checks EXT_CSD_PARTITION_SETTING_COMPLETED bit before
> computing enhanced user area offset and size, and adding
> mmc general purpose partitions.
> The two needs EXT_CSD_PARTITION_SETTING_COMPLETED bit be set
> to be valid (as described in JEDEC standard).
>
> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
> ---
>  drivers/mmc/core/mmc.c |  167 +++++++++++++++++++++++++++---------------------
>  1 file changed, 95 insertions(+), 72 deletions(-)
>
> Changelog v2:
>         Move code for user area and general purpose partitions
>         into functions.

Overall I like the end-result of this patch.

What I don't like is that you move code and fix bugs in same patch.
Could possibly, split this up into two patches?

Kind regards
Uffe

[PATCH v2 0001/0002] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions

[Grégory Soutadé]

[-- Attachment #1: Type: text/plain, Size: 3201 bytes --]

Replace "enhanced_area_en" attribute by "partition_setting_completed".
It was used whether or not enhanced user area is defined and without checks of
EXT_CSD_PARTITION_SETTING_COMPLETED bit.

Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
---
 drivers/mmc/core/mmc.c   |    8 +++++++-
 include/linux/mmc/card.h |    2 +-
 include/linux/mmc/mmc.h  |    2 ++
 3 files changed, 10 insertions(+), 2 deletions(-)

>From commit 33caee39925b887a99a2400dc5c980097c3573f9 in master linux tree.

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 793c6f7..ef25d91 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -403,6 +403,12 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 		ext_csd[EXT_CSD_TRIM_MULT];
 	card->ext_csd.raw_partition_support = ext_csd[EXT_CSD_PARTITION_SUPPORT];
 	if (card->ext_csd.rev >= 4) {
+		if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED] &
+		    EXT_CSD_PART_SETTING_COMPLETED) {
+			card->ext_csd.partition_setting_completed = 1;
+		} else {
+			card->ext_csd.partition_setting_completed = 0;
+		}
 		/*
 		 * Enhanced area feature support -- check whether the eMMC
 		 * card has the Enhanced area enabled.  If so, export enhanced
@@ -1309,7 +1315,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	 * If enhanced_area_en is TRUE, host needs to enable ERASE_GRP_DEF
 	 * bit.  This bit will be lost every time after a reset or power off.
 	 */
-	if (card->ext_csd.enhanced_area_en ||
+	if (card->ext_csd.partition_setting_completed ||
 	    (card->ext_csd.rev >= 3 && (host->caps2 & MMC_CAP2_HC_ERASE_SZ))) {
 		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 				 EXT_CSD_ERASE_GROUP_DEF, 1,
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index d424b9d..38175be 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -74,7 +74,7 @@ struct mmc_ext_csd {
 	unsigned int		sec_trim_mult;	/* Secure trim multiplier  */
 	unsigned int		sec_erase_mult;	/* Secure erase multiplier */
 	unsigned int		trim_timeout;		/* In milliseconds */
-	bool			enhanced_area_en;	/* enable bit */
+	bool			partition_setting_completed;	/* enable bit */
 	unsigned long long	enhanced_area_offset;	/* Units: Byte */
 	unsigned int		enhanced_area_size;	/* Units: KB */
 	unsigned int		cache_size;		/* Units: KB */
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 64ec963..78753bc 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -281,6 +281,7 @@ struct _mmc_csd {
 #define EXT_CSD_EXP_EVENTS_CTRL		56	/* R/W, 2 bytes */
 #define EXT_CSD_DATA_SECTOR_SIZE	61	/* R */
 #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
+#define EXT_CSD_PARTITION_SETTING_COMPLETED 155	/* R/W */
 #define EXT_CSD_PARTITION_ATTRIBUTE	156	/* R/W */
 #define EXT_CSD_PARTITION_SUPPORT	160	/* RO */
 #define EXT_CSD_HPI_MGMT		161	/* R/W */
@@ -349,6 +350,7 @@ struct _mmc_csd {
 #define EXT_CSD_PART_CONFIG_ACC_RPMB	(0x3)
 #define EXT_CSD_PART_CONFIG_ACC_GP0	(0x4)

+#define EXT_CSD_PART_SETTING_COMPLETED	(0x1)
 #define EXT_CSD_PART_SUPPORT_PART_EN	(0x1)

 #define EXT_CSD_CMD_SET_NORMAL		(1<<0)
-- 
1.7.9.5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH v2 0002/0002] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions

[Grégory Soutadé]

Checks EXT_CSD_PARTITION_SETTING_COMPLETED bit before
computing enhanced user area and adding mmc partitions
(as described in JEDEC standard).

Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>
---
 drivers/mmc/core/mmc.c |  167 +++++++++++++++++++++++++++---------------------
 1 file changed, 95 insertions(+), 72 deletions(-)

Changelog v2:
	Move code for user area and general purpose partitions
	into functions.

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index ef25d91..3158470 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -298,6 +298,99 @@ static void mmc_select_card_type(struct mmc_card *card)
 	card->mmc_avail_type = avail_type;
 }

+static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
+{
+	u8 hc_erase_grp_sz, hc_wp_grp_sz;
+
+	/*
+	 * Enhanced area feature support -- check whether the eMMC
+	 * card has the Enhanced area enabled.  If so, export enhanced
+	 * area offset and size to user by adding sysfs interface.
+	 */
+	if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
+	    (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
+		if (card->ext_csd.partition_setting_completed) {
+			hc_erase_grp_sz =
+				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
+			hc_wp_grp_sz =
+				ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+			/*
+			 * calculate the enhanced data area offset, in bytes
+			 */
+			card->ext_csd.enhanced_area_offset =
+				(ext_csd[139] << 24) + (ext_csd[138] << 16) +
+				(ext_csd[137] << 8) + ext_csd[136];
+			if (mmc_card_blockaddr(card))
+				card->ext_csd.enhanced_area_offset <<= 9;
+			/*
+			 * calculate the enhanced data area size, in kilobytes
+			 */
+			card->ext_csd.enhanced_area_size =
+				(ext_csd[142] << 16) + (ext_csd[141] << 8) +
+				ext_csd[140];
+			card->ext_csd.enhanced_area_size *=
+				(size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
+			card->ext_csd.enhanced_area_size <<= 9;
+		} else {
+			pr_warn("%s: defines enhanced area without partition setting complete\n",
+				mmc_hostname(card->host));
+		}
+	} else {
+		/*
+		 * If the enhanced area is not enabled, disable these
+		 * device attributes.
+		 */
+		card->ext_csd.enhanced_area_offset = -EINVAL;
+		card->ext_csd.enhanced_area_size = -EINVAL;
+	}
+}
+
+static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
+{
+	int idx;
+	u8 hc_erase_grp_sz, hc_wp_grp_sz;
+	unsigned int part_size;
+
+	/*
+	 * General purpose partition feature support --
+	 * If ext_csd has 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) {
+		hc_erase_grp_sz =
+			ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
+		hc_wp_grp_sz =
+			ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+
+		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;
+
+			if (card->ext_csd.partition_setting_completed == 0) {
+				pr_warn("%s: has partition size defined without partition complete\n",
+					mmc_hostname(card->host));
+				break;
+			}
+
+			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,
+				MMC_BLK_DATA_AREA_GP);
+		}
+	}
+}
+
 /*
  * Decode extended CSD.
  */
@@ -305,7 +398,6 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 {
 	int err = 0, idx;
 	unsigned int part_size;
-	u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;

 	BUG_ON(!card);

@@ -409,80 +501,11 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
 		} else {
 			card->ext_csd.partition_setting_completed = 0;
 		}
-		/*
-		 * Enhanced area feature support -- check whether the eMMC
-		 * card has the Enhanced area enabled.  If so, export enhanced
-		 * area offset and size to user by adding sysfs interface.
-		 */
-		if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
-		    (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
-			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;
-			/*
-			 * calculate the enhanced data area offset, in bytes
-			 */
-			card->ext_csd.enhanced_area_offset =
-				(ext_csd[139] << 24) + (ext_csd[138] << 16) +
-				(ext_csd[137] << 8) + ext_csd[136];
-			if (mmc_card_blockaddr(card))
-				card->ext_csd.enhanced_area_offset <<= 9;
-			/*
-			 * calculate the enhanced data area size, in kilobytes
-			 */
-			card->ext_csd.enhanced_area_size =
-				(ext_csd[142] << 16) + (ext_csd[141] << 8) +
-				ext_csd[140];
-			card->ext_csd.enhanced_area_size *=
-				(size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
-			card->ext_csd.enhanced_area_size <<= 9;
-		} else {
-			/*
-			 * If the enhanced area is not enabled, disable these
-			 * device attributes.
-			 */
-			card->ext_csd.enhanced_area_offset = -EINVAL;
-			card->ext_csd.enhanced_area_size = -EINVAL;
-		}
+		mmc_manage_enhanced_area(card, ext_csd);

-		/*
-		 * General purpose partition feature support --
-		 * If ext_csd has 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;
-			}
+		mmc_manage_gp_partitions(card, ext_csd);

-			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,
-					MMC_BLK_DATA_AREA_GP);
-			}
-		}
 		card->ext_csd.sec_trim_mult =
 			ext_csd[EXT_CSD_SEC_TRIM_MULT];
 		card->ext_csd.sec_erase_mult =
-- 
1.7.9.5

Re: [PATCH v2 0002/0002] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions

[Ulf Hansson]

On 18 August 2014 12:50, Grégory Soutadé <gsoutade@neotion.com> wrote:
> Checks EXT_CSD_PARTITION_SETTING_COMPLETED bit before
> computing enhanced user area and adding mmc partitions
> (as described in JEDEC standard).
>
> Signed-off-by: Grégory Soutadé <gsoutade@neotion.com>

Hi Grégory,

First, could you rename patches so they actually describe what they
actually do. Currently both of them have the same header.
Second, I really appreciate if there could be some version history of
the patches since it helps review process.

Could you please resend them?

Kind regards
Uffe

> ---
>  drivers/mmc/core/mmc.c |  167 +++++++++++++++++++++++++++---------------------
>  1 file changed, 95 insertions(+), 72 deletions(-)
>
> Changelog v2:
>         Move code for user area and general purpose partitions
>         into functions.
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index ef25d91..3158470 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -298,6 +298,99 @@ static void mmc_select_card_type(struct mmc_card *card)
>         card->mmc_avail_type = avail_type;
>  }
>
> +static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
> +{
> +       u8 hc_erase_grp_sz, hc_wp_grp_sz;
> +
> +       /*
> +        * Enhanced area feature support -- check whether the eMMC
> +        * card has the Enhanced area enabled.  If so, export enhanced
> +        * area offset and size to user by adding sysfs interface.
> +        */
> +       if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
> +           (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
> +               if (card->ext_csd.partition_setting_completed) {
> +                       hc_erase_grp_sz =
> +                               ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
> +                       hc_wp_grp_sz =
> +                               ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
> +                       /*
> +                        * calculate the enhanced data area offset, in bytes
> +                        */
> +                       card->ext_csd.enhanced_area_offset =
> +                               (ext_csd[139] << 24) + (ext_csd[138] << 16) +
> +                               (ext_csd[137] << 8) + ext_csd[136];
> +                       if (mmc_card_blockaddr(card))
> +                               card->ext_csd.enhanced_area_offset <<= 9;
> +                       /*
> +                        * calculate the enhanced data area size, in kilobytes
> +                        */
> +                       card->ext_csd.enhanced_area_size =
> +                               (ext_csd[142] << 16) + (ext_csd[141] << 8) +
> +                               ext_csd[140];
> +                       card->ext_csd.enhanced_area_size *=
> +                               (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
> +                       card->ext_csd.enhanced_area_size <<= 9;
> +               } else {
> +                       pr_warn("%s: defines enhanced area without partition setting complete\n",
> +                               mmc_hostname(card->host));
> +               }
> +       } else {
> +               /*
> +                * If the enhanced area is not enabled, disable these
> +                * device attributes.
> +                */
> +               card->ext_csd.enhanced_area_offset = -EINVAL;
> +               card->ext_csd.enhanced_area_size = -EINVAL;
> +       }
> +}
> +
> +static void mmc_manage_gp_partitions(struct mmc_card *card, u8 *ext_csd)
> +{
> +       int idx;
> +       u8 hc_erase_grp_sz, hc_wp_grp_sz;
> +       unsigned int part_size;
> +
> +       /*
> +        * General purpose partition feature support --
> +        * If ext_csd has 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) {
> +               hc_erase_grp_sz =
> +                       ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
> +               hc_wp_grp_sz =
> +                       ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
> +
> +               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;
> +
> +                       if (card->ext_csd.partition_setting_completed == 0) {
> +                               pr_warn("%s: has partition size defined without partition complete\n",
> +                                       mmc_hostname(card->host));
> +                               break;
> +                       }
> +
> +                       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,
> +                               MMC_BLK_DATA_AREA_GP);
> +               }
> +       }
> +}
> +
>  /*
>   * Decode extended CSD.
>   */
> @@ -305,7 +398,6 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>  {
>         int err = 0, idx;
>         unsigned int part_size;
> -       u8 hc_erase_grp_sz = 0, hc_wp_grp_sz = 0;
>
>         BUG_ON(!card);
>
> @@ -409,80 +501,11 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>                 } else {
>                         card->ext_csd.partition_setting_completed = 0;
>                 }
> -               /*
> -                * Enhanced area feature support -- check whether the eMMC
> -                * card has the Enhanced area enabled.  If so, export enhanced
> -                * area offset and size to user by adding sysfs interface.
> -                */
> -               if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
> -                   (ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
> -                       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;
> -                       /*
> -                        * calculate the enhanced data area offset, in bytes
> -                        */
> -                       card->ext_csd.enhanced_area_offset =
> -                               (ext_csd[139] << 24) + (ext_csd[138] << 16) +
> -                               (ext_csd[137] << 8) + ext_csd[136];
> -                       if (mmc_card_blockaddr(card))
> -                               card->ext_csd.enhanced_area_offset <<= 9;
> -                       /*
> -                        * calculate the enhanced data area size, in kilobytes
> -                        */
> -                       card->ext_csd.enhanced_area_size =
> -                               (ext_csd[142] << 16) + (ext_csd[141] << 8) +
> -                               ext_csd[140];
> -                       card->ext_csd.enhanced_area_size *=
> -                               (size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
> -                       card->ext_csd.enhanced_area_size <<= 9;
> -               } else {
> -                       /*
> -                        * If the enhanced area is not enabled, disable these
> -                        * device attributes.
> -                        */
> -                       card->ext_csd.enhanced_area_offset = -EINVAL;
> -                       card->ext_csd.enhanced_area_size = -EINVAL;
> -               }
> +               mmc_manage_enhanced_area(card, ext_csd);
>
> -               /*
> -                * General purpose partition feature support --
> -                * If ext_csd has 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;
> -                       }
> +               mmc_manage_gp_partitions(card, ext_csd);
>
> -                       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,
> -                                       MMC_BLK_DATA_AREA_GP);
> -                       }
> -               }
>                 card->ext_csd.sec_trim_mult =
>                         ext_csd[EXT_CSD_SEC_TRIM_MULT];
>                 card->ext_csd.sec_erase_mult =
> --
> 1.7.9.5