* [PATCH] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions
@ 2014-07-17 14:57 ` Grégory Soutadé
0 siblings, 0 replies; 31+ messages in thread
From: Grégory Soutadé @ 2014-07-17 14:57 UTC (permalink / raw)
To: Chris Ball, Ulf Hansson, Seungwon Jeon, Jaehoon Chung, linux-mmc,
linux-kernel
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions
@ 2014-07-17 14:57 ` Grégory Soutadé
0 siblings, 0 replies; 31+ messages in thread
From: Grégory Soutadé @ 2014-07-17 14:57 UTC (permalink / raw)
To: Chris Ball, Ulf Hansson, Seungwon Jeon, Jaehoon Chung, linux-mmc,
linux-kernel
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions
2014-07-17 14:57 ` Grégory Soutadé
(?)
@ 2014-08-13 8:36 ` Ulf Hansson
2014-08-13 9:20 ` Grégory Soutadé
-1 siblings, 1 reply; 31+ messages in thread
From: Ulf Hansson @ 2014-08-13 8:36 UTC (permalink / raw)
To: Grégory Soutadé
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions
2014-08-13 8:36 ` Ulf Hansson
@ 2014-08-13 9:20 ` Grégory Soutadé
2014-08-14 11:46 ` Ulf Hansson
0 siblings, 1 reply; 31+ messages in thread
From: Grégory Soutadé @ 2014-08-13 9:20 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions
2014-08-13 9:20 ` Grégory Soutadé
@ 2014-08-14 11:46 ` Ulf Hansson
2014-08-14 13:27 ` Grégory Soutadé
0 siblings, 1 reply; 31+ messages in thread
From: Ulf Hansson @ 2014-08-14 11:46 UTC (permalink / raw)
To: Grégory Soutadé
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions
2014-08-14 11:46 ` Ulf Hansson
@ 2014-08-14 13:27 ` Grégory Soutadé
2014-08-15 8:17 ` Ulf Hansson
2014-08-18 10:49 ` [PATCH v2 0001/0002] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions Grégory Soutadé
0 siblings, 2 replies; 31+ messages in thread
From: Grégory Soutadé @ 2014-08-14 13:27 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions
2014-08-14 13:27 ` Grégory Soutadé
@ 2014-08-15 8:17 ` Ulf Hansson
2014-08-18 13:12 ` [PATCH v3 0001/0002] mmc: Replace ext_csd "enhanced_area_en" attribute by "partition_setting_completed" Grégory Soutadé
2014-08-18 13:12 ` [PATCH v3 0002/0002] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED bit " Grégory Soutadé
2014-08-18 10:49 ` [PATCH v2 0001/0002] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions Grégory Soutadé
1 sibling, 2 replies; 31+ messages in thread
From: Ulf Hansson @ 2014-08-15 8:17 UTC (permalink / raw)
To: Grégory Soutadé
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
>>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 0001/0002] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions
2014-08-14 13:27 ` Grégory Soutadé
2014-08-15 8:17 ` Ulf Hansson
@ 2014-08-18 10:49 ` Grégory Soutadé
1 sibling, 0 replies; 31+ messages in thread
From: Grégory Soutadé @ 2014-08-18 10:49 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
[-- 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 --]
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 0002/0002] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions
2014-07-17 14:57 ` Grégory Soutadé
(?)
(?)
@ 2014-08-18 10:50 ` Grégory Soutadé
2014-08-18 12:15 ` Ulf Hansson
-1 siblings, 1 reply; 31+ messages in thread
From: Grégory Soutadé @ 2014-08-18 10:50 UTC (permalink / raw)
To: Chris Ball, Ulf Hansson, Seungwon Jeon, Jaehoon Chung, linux-mmc,
linux-kernel
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v2 0002/0002] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions
2014-08-18 10:50 ` [PATCH v2 0002/0002] " Grégory Soutadé
@ 2014-08-18 12:15 ` Ulf Hansson
0 siblings, 0 replies; 31+ messages in thread
From: Ulf Hansson @ 2014-08-18 12:15 UTC (permalink / raw)
To: Grégory Soutadé
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 0001/0002] mmc: Replace ext_csd "enhanced_area_en" attribute by "partition_setting_completed"
2014-08-15 8:17 ` Ulf Hansson
@ 2014-08-18 13:12 ` Grégory Soutadé
2014-09-11 6:38 ` [PATCH v4 0001/0003] mmc: Move code that manages user area and gp partitions into functions Grégory Soutadé
` (2 more replies)
2014-08-18 13:12 ` [PATCH v3 0002/0002] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED bit " Grégory Soutadé
1 sibling, 3 replies; 31+ messages in thread
From: Grégory Soutadé @ 2014-08-18 13:12 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
>>>
>>
>
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 0002/0002] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED bit before partitions computation
2014-08-15 8:17 ` Ulf Hansson
2014-08-18 13:12 ` [PATCH v3 0001/0002] mmc: Replace ext_csd "enhanced_area_en" attribute by "partition_setting_completed" Grégory Soutadé
@ 2014-08-18 13:12 ` Grégory Soutadé
2014-09-08 12:15 ` Ulf Hansson
1 sibling, 1 reply; 31+ messages in thread
From: Grégory Soutadé @ 2014-08-18 13:12 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v3 0002/0002] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED bit before partitions computation
2014-08-18 13:12 ` [PATCH v3 0002/0002] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED bit " Grégory Soutadé
@ 2014-09-08 12:15 ` Ulf Hansson
0 siblings, 0 replies; 31+ messages in thread
From: Ulf Hansson @ 2014-09-08 12:15 UTC (permalink / raw)
To: Grégory Soutadé
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v4 0001/0003] mmc: Move code that manages user area and gp partitions into functions
2014-08-18 13:12 ` [PATCH v3 0001/0002] mmc: Replace ext_csd "enhanced_area_en" attribute by "partition_setting_completed" Grégory Soutadé
@ 2014-09-11 6:38 ` Grégory Soutadé
2014-09-12 14:31 ` Grégory Soutadé
` (3 more replies)
2014-09-11 6:38 ` [PATCH v4 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed" Grégory Soutadé
2014-09-11 6:38 ` [PATCH v4 0003/0003] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation Grégory Soutadé
2 siblings, 4 replies; 31+ messages in thread
From: Grégory Soutadé @ 2014-09-11 6:38 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
>>>>
>>>
>>
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v4 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed"
2014-08-18 13:12 ` [PATCH v3 0001/0002] mmc: Replace ext_csd "enhanced_area_en" attribute by "partition_setting_completed" Grégory Soutadé
2014-09-11 6:38 ` [PATCH v4 0001/0003] mmc: Move code that manages user area and gp partitions into functions Grégory Soutadé
@ 2014-09-11 6:38 ` Grégory Soutadé
2014-09-11 9:46 ` Ulf Hansson
2014-09-11 6:38 ` [PATCH v4 0003/0003] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation Grégory Soutadé
2 siblings, 1 reply; 31+ messages in thread
From: Grégory Soutadé @ 2014-09-11 6:38 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v4 0003/0003] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation
2014-08-18 13:12 ` [PATCH v3 0001/0002] mmc: Replace ext_csd "enhanced_area_en" attribute by "partition_setting_completed" Grégory Soutadé
2014-09-11 6:38 ` [PATCH v4 0001/0003] mmc: Move code that manages user area and gp partitions into functions Grégory Soutadé
2014-09-11 6:38 ` [PATCH v4 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed" Grégory Soutadé
@ 2014-09-11 6:38 ` Grégory Soutadé
2 siblings, 0 replies; 31+ messages in thread
From: Grégory Soutadé @ 2014-09-11 6:38 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v4 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed"
2014-09-11 6:38 ` [PATCH v4 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed" Grégory Soutadé
@ 2014-09-11 9:46 ` Ulf Hansson
0 siblings, 0 replies; 31+ messages in thread
From: Ulf Hansson @ 2014-09-11 9:46 UTC (permalink / raw)
To: Grégory Soutadé
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v5 0000/0003] mmc: EXT_CSD_PARTITION_SETTING_COMPLETED bit not checked
2014-09-11 6:38 ` [PATCH v4 0001/0003] mmc: Move code that manages user area and gp partitions into functions Grégory Soutadé
@ 2014-09-12 14:31 ` Grégory Soutadé
2014-09-12 14:31 ` [PATCH v5 0001/0003] mmc: Move code that manages user area and gp partitions into functions Grégory Soutadé
` (2 subsequent siblings)
3 siblings, 0 replies; 31+ messages in thread
From: Grégory Soutadé @ 2014-09-12 14:31 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
>>>> >>>>
>>> >>>
>> >>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v5 0000/0003] mmc: EXT_CSD_PARTITION_SETTING_COMPLETED bit not checked
@ 2014-09-12 14:31 ` Grégory Soutadé
0 siblings, 0 replies; 31+ messages in thread
From: Grégory Soutadé @ 2014-09-12 14:31 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
>>>> >>>>
>>> >>>
>> >>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v5 0001/0003] mmc: Move code that manages user area and gp partitions into functions
2014-09-11 6:38 ` [PATCH v4 0001/0003] mmc: Move code that manages user area and gp partitions into functions Grégory Soutadé
2014-09-12 14:31 ` Grégory Soutadé
@ 2014-09-12 14:31 ` Grégory Soutadé
2014-09-15 15:47 ` [PATCH v6 " Grégory Soutadé
2014-09-12 14:31 ` [PATCH v5 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed" Grégory Soutadé
2014-09-12 14:31 ` [PATCH v5 0003/0003] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation Grégory Soutadé
3 siblings, 1 reply; 31+ messages in thread
From: Grégory Soutadé @ 2014-09-12 14:31 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v5 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed"
2014-09-11 6:38 ` [PATCH v4 0001/0003] mmc: Move code that manages user area and gp partitions into functions Grégory Soutadé
2014-09-12 14:31 ` Grégory Soutadé
2014-09-12 14:31 ` [PATCH v5 0001/0003] mmc: Move code that manages user area and gp partitions into functions Grégory Soutadé
@ 2014-09-12 14:31 ` Grégory Soutadé
2014-09-15 4:20 ` Jaehoon Chung
2014-09-12 14:31 ` [PATCH v5 0003/0003] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation Grégory Soutadé
3 siblings, 1 reply; 31+ messages in thread
From: Grégory Soutadé @ 2014-09-12 14:31 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v5 0003/0003] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation
2014-09-11 6:38 ` [PATCH v4 0001/0003] mmc: Move code that manages user area and gp partitions into functions Grégory Soutadé
` (2 preceding siblings ...)
2014-09-12 14:31 ` [PATCH v5 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed" Grégory Soutadé
@ 2014-09-12 14:31 ` Grégory Soutadé
2014-09-15 15:47 ` [PATCH v6 " Grégory Soutadé
3 siblings, 1 reply; 31+ messages in thread
From: Grégory Soutadé @ 2014-09-12 14:31 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v5 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed"
2014-09-12 14:31 ` [PATCH v5 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed" Grégory Soutadé
@ 2014-09-15 4:20 ` Jaehoon Chung
2014-09-15 15:47 ` [PATCH v6 " Grégory Soutadé
0 siblings, 1 reply; 31+ messages in thread
From: Jaehoon Chung @ 2014-09-15 4:20 UTC (permalink / raw)
To: Grégory Soutadé, Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, linux-mmc, linux-kernel
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)
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v6 0000/0003] mmc: EXT_CSD_PARTITION_SETTING_COMPLETED bit not checked
2014-09-12 14:31 ` Grégory Soutadé
@ 2014-09-15 15:47 ` Grégory Soutadé
-1 siblings, 0 replies; 31+ messages in thread
From: Grégory Soutadé @ 2014-09-15 15:47 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v6 0000/0003] mmc: EXT_CSD_PARTITION_SETTING_COMPLETED bit not checked
@ 2014-09-15 15:47 ` Grégory Soutadé
0 siblings, 0 replies; 31+ messages in thread
From: Grégory Soutadé @ 2014-09-15 15:47 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v6 0001/0003] mmc: Move code that manages user area and gp partitions into functions
2014-09-12 14:31 ` [PATCH v5 0001/0003] mmc: Move code that manages user area and gp partitions into functions Grégory Soutadé
@ 2014-09-15 15:47 ` Grégory Soutadé
2014-09-17 22:12 ` Ulf Hansson
0 siblings, 1 reply; 31+ messages in thread
From: Grégory Soutadé @ 2014-09-15 15:47 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v6 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed"
2014-09-15 4:20 ` Jaehoon Chung
@ 2014-09-15 15:47 ` Grégory Soutadé
2014-09-17 22:12 ` Ulf Hansson
0 siblings, 1 reply; 31+ messages in thread
From: Grégory Soutadé @ 2014-09-15 15:47 UTC (permalink / raw)
To: Jaehoon Chung, Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, linux-mmc, linux-kernel
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)
>>
>
>
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v6 0003/0003] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation
2014-09-12 14:31 ` [PATCH v5 0003/0003] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation Grégory Soutadé
@ 2014-09-15 15:47 ` Grégory Soutadé
2014-09-17 22:13 ` Ulf Hansson
0 siblings, 1 reply; 31+ messages in thread
From: Grégory Soutadé @ 2014-09-15 15:47 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v6 0001/0003] mmc: Move code that manages user area and gp partitions into functions
2014-09-15 15:47 ` [PATCH v6 " Grégory Soutadé
@ 2014-09-17 22:12 ` Ulf Hansson
0 siblings, 0 replies; 31+ messages in thread
From: Ulf Hansson @ 2014-09-17 22:12 UTC (permalink / raw)
To: Grégory Soutadé
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed"
2014-09-15 15:47 ` [PATCH v6 " Grégory Soutadé
@ 2014-09-17 22:12 ` Ulf Hansson
0 siblings, 0 replies; 31+ messages in thread
From: Ulf Hansson @ 2014-09-17 22:12 UTC (permalink / raw)
To: Grégory Soutadé
Cc: Jaehoon Chung, Chris Ball, Seungwon Jeon, linux-mmc, linux-kernel
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)
>>>
>>
>>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 0003/0003] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation
2014-09-15 15:47 ` [PATCH v6 " Grégory Soutadé
@ 2014-09-17 22:13 ` Ulf Hansson
0 siblings, 0 replies; 31+ messages in thread
From: Ulf Hansson @ 2014-09-17 22:13 UTC (permalink / raw)
To: Grégory Soutadé
Cc: Chris Ball, Seungwon Jeon, Jaehoon Chung, linux-mmc, linux-kernel
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
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2014-09-17 22:13 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-17 14:57 [PATCH] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions Grégory Soutadé
2014-07-17 14:57 ` Grégory Soutadé
2014-08-13 8:36 ` Ulf Hansson
2014-08-13 9:20 ` Grégory Soutadé
2014-08-14 11:46 ` Ulf Hansson
2014-08-14 13:27 ` Grégory Soutadé
2014-08-15 8:17 ` Ulf Hansson
2014-08-18 13:12 ` [PATCH v3 0001/0002] mmc: Replace ext_csd "enhanced_area_en" attribute by "partition_setting_completed" Grégory Soutadé
2014-09-11 6:38 ` [PATCH v4 0001/0003] mmc: Move code that manages user area and gp partitions into functions Grégory Soutadé
2014-09-12 14:31 ` [PATCH v5 0000/0003] mmc: EXT_CSD_PARTITION_SETTING_COMPLETED bit not checked Grégory Soutadé
2014-09-12 14:31 ` Grégory Soutadé
2014-09-15 15:47 ` [PATCH v6 " Grégory Soutadé
2014-09-15 15:47 ` Grégory Soutadé
2014-09-12 14:31 ` [PATCH v5 0001/0003] mmc: Move code that manages user area and gp partitions into functions Grégory Soutadé
2014-09-15 15:47 ` [PATCH v6 " Grégory Soutadé
2014-09-17 22:12 ` Ulf Hansson
2014-09-12 14:31 ` [PATCH v5 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed" Grégory Soutadé
2014-09-15 4:20 ` Jaehoon Chung
2014-09-15 15:47 ` [PATCH v6 " Grégory Soutadé
2014-09-17 22:12 ` Ulf Hansson
2014-09-12 14:31 ` [PATCH v5 0003/0003] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation Grégory Soutadé
2014-09-15 15:47 ` [PATCH v6 " Grégory Soutadé
2014-09-17 22:13 ` Ulf Hansson
2014-09-11 6:38 ` [PATCH v4 0002/0003] mmc: Replace "enhanced_area_en" attribute by "partition_setting_completed" Grégory Soutadé
2014-09-11 9:46 ` Ulf Hansson
2014-09-11 6:38 ` [PATCH v4 0003/0003] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED before partitions computation Grégory Soutadé
2014-08-18 13:12 ` [PATCH v3 0002/0002] mmc: Checks EXT_CSD_PARTITION_SETTING_COMPLETED bit " Grégory Soutadé
2014-09-08 12:15 ` Ulf Hansson
2014-08-18 10:49 ` [PATCH v2 0001/0002] mmc: check EXT_CSD_PARTITION_SETTING_COMPLETED before creating partitions Grégory Soutadé
2014-08-18 10:50 ` [PATCH v2 0002/0002] " Grégory Soutadé
2014-08-18 12:15 ` Ulf Hansson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.