[PATCH V2] mmc: core: Add host capability check for power class

[PATCH V2] mmc: core: Add host capability check for power class

[Girish K S]

This patch adds a check whether the host supports maximum current value
obtained from the device's extended csd register for a selected interface
voltage and frequency.

cc: Chris Ball <cjb@laptop.org>
Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
---
Changes in v2:
	deleted a unnecessary if else condition identified by subhash J
Changes in v1:
       reduced the number of comparisons as per Hein's suggestion

 drivers/mmc/core/mmc.c   |   19 +++++++++++++++++++
 include/linux/mmc/card.h |    4 ++++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 006e932..b9ef777 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -688,6 +688,25 @@ static int mmc_select_powerclass(struct mmc_card *card,
 		pwrclass_val = (pwrclass_val & EXT_CSD_PWR_CL_4BIT_MASK) >>
 				EXT_CSD_PWR_CL_4BIT_SHIFT;
 
+	if (pwrclass_val >= MMC_MAX_CURRENT_800)
+		pwrclass_val = MMC_MAX_CURRENT_800;
+	else if (pwrclass_val >= MMC_MAX_CURRENT_600)
+		pwrclass_val = MMC_MAX_CURRENT_600;
+	else if (pwrclass_val >= MMC_MAX_CURRENT_400)
+		pwrclass_val = MMC_MAX_CURRENT_400;
+	else
+		pwrclass_val = MMC_MAX_CURRENT_200;
+
+	if ((pwrclass_val == MMC_MAX_CURRENT_800) &&
+	    !(card->host->caps & MMC_CAP_MAX_CURRENT_800))
+		pwrclass_val = MMC_MAX_CURRENT_600;
+	if ((pwrclass_val == MMC_MAX_CURRENT_600) &&
+	    !(card->host->caps & MMC_CAP_MAX_CURRENT_600))
+		pwrclass_val = MMC_MAX_CURRENT_400;
+	if ((pwrclass_val == MMC_MAX_CURRENT_400) &&
+	    !(card->host->caps & MMC_CAP_MAX_CURRENT_400))
+		pwrclass_val = MMC_MAX_CURRENT_200;
+
 	/* If the power class is different from the default value */
 	if (pwrclass_val > 0) {
 		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 9478a6b..c5e031a 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -195,6 +195,10 @@ struct mmc_part {
 #define MMC_BLK_DATA_AREA_GP	(1<<2)
 };
 
+#define MMC_MAX_CURRENT_200	(0)
+#define MMC_MAX_CURRENT_400	(7)
+#define MMC_MAX_CURRENT_600	(11)
+#define MMC_MAX_CURRENT_800	(13)
 /*
  * MMC device
  */
-- 
1.7.1

Re: [PATCH V2] mmc: core: Add host capability check for power class

[amit kachhap]

On Thu, Dec 15, 2011 at 9:28 AM, Girish K S
<girish.shivananjappa@linaro.org> wrote:
> This patch adds a check whether the host supports maximum current value
> obtained from the device's extended csd register for a selected interface
> voltage and frequency.
>
> cc: Chris Ball <cjb@laptop.org>
> Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
> ---
> Changes in v2:
>        deleted a unnecessary if else condition identified by subhash J
> Changes in v1:
>       reduced the number of comparisons as per Hein's suggestion
>
>  drivers/mmc/core/mmc.c   |   19 +++++++++++++++++++
>  include/linux/mmc/card.h |    4 ++++
>  2 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 006e932..b9ef777 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -688,6 +688,25 @@ static int mmc_select_powerclass(struct mmc_card *card,
>                pwrclass_val = (pwrclass_val & EXT_CSD_PWR_CL_4BIT_MASK) >>
>                                EXT_CSD_PWR_CL_4BIT_SHIFT;
>
> +       if (pwrclass_val >= MMC_MAX_CURRENT_800)
Hi girish,

These checks can be made like ,
if (pwrclass_val > MMC_MAX_CURRENT_800)
        pwrclass_val = MMC_MAX_CURRENT_800;
Applicable in all below conditional checks.

Thanks,
Amit D

> +               pwrclass_val = MMC_MAX_CURRENT_800;
> +       else if (pwrclass_val >= MMC_MAX_CURRENT_600)
> +               pwrclass_val = MMC_MAX_CURRENT_600;
> +       else if (pwrclass_val >= MMC_MAX_CURRENT_400)
> +               pwrclass_val = MMC_MAX_CURRENT_400;
> +       else
> +               pwrclass_val = MMC_MAX_CURRENT_200;
> +
> +       if ((pwrclass_val == MMC_MAX_CURRENT_800) &&
> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_800))
> +               pwrclass_val = MMC_MAX_CURRENT_600;
> +       if ((pwrclass_val == MMC_MAX_CURRENT_600) &&
> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_600))
> +               pwrclass_val = MMC_MAX_CURRENT_400;
> +       if ((pwrclass_val == MMC_MAX_CURRENT_400) &&
> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_400))
> +               pwrclass_val = MMC_MAX_CURRENT_200;
> +
>        /* If the power class is different from the default value */
>        if (pwrclass_val > 0) {
>                err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 9478a6b..c5e031a 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -195,6 +195,10 @@ struct mmc_part {
>  #define MMC_BLK_DATA_AREA_GP   (1<<2)
>  };
>
> +#define MMC_MAX_CURRENT_200    (0)
> +#define MMC_MAX_CURRENT_400    (7)
> +#define MMC_MAX_CURRENT_600    (11)
> +#define MMC_MAX_CURRENT_800    (13)
>  /*
>  * MMC device
>  */
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] mmc: core: Add host capability check for power class

[Girish K S]

On 15 December 2011 11:33, amit kachhap <amit.kachhap@linaro.org> wrote:
> On Thu, Dec 15, 2011 at 9:28 AM, Girish K S
> <girish.shivananjappa@linaro.org> wrote:
>> This patch adds a check whether the host supports maximum current value
>> obtained from the device's extended csd register for a selected interface
>> voltage and frequency.
>>
>> cc: Chris Ball <cjb@laptop.org>
>> Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
>> ---
>> Changes in v2:
>>        deleted a unnecessary if else condition identified by subhash J
>> Changes in v1:
>>       reduced the number of comparisons as per Hein's suggestion
>>
>>  drivers/mmc/core/mmc.c   |   19 +++++++++++++++++++
>>  include/linux/mmc/card.h |    4 ++++
>>  2 files changed, 23 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 006e932..b9ef777 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -688,6 +688,25 @@ static int mmc_select_powerclass(struct mmc_card *card,
>>                pwrclass_val = (pwrclass_val & EXT_CSD_PWR_CL_4BIT_MASK) >>
>>                                EXT_CSD_PWR_CL_4BIT_SHIFT;
>>
>> +       if (pwrclass_val >= MMC_MAX_CURRENT_800)
> Hi girish,
>
> These checks can be made like ,
> if (pwrclass_val > MMC_MAX_CURRENT_800)
>        pwrclass_val = MMC_MAX_CURRENT_800;
> Applicable in all below conditional checks.

I feel that should be OK. Since there will be no significant
improvement or side effects with the current implementation.

>
> Thanks,
> Amit D
>
>> +               pwrclass_val = MMC_MAX_CURRENT_800;
>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_600)
>> +               pwrclass_val = MMC_MAX_CURRENT_600;
>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_400)
>> +               pwrclass_val = MMC_MAX_CURRENT_400;
>> +       else
>> +               pwrclass_val = MMC_MAX_CURRENT_200;
>> +
>> +       if ((pwrclass_val == MMC_MAX_CURRENT_800) &&
>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_800))
>> +               pwrclass_val = MMC_MAX_CURRENT_600;
>> +       if ((pwrclass_val == MMC_MAX_CURRENT_600) &&
>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_600))
>> +               pwrclass_val = MMC_MAX_CURRENT_400;
>> +       if ((pwrclass_val == MMC_MAX_CURRENT_400) &&
>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_400))
>> +               pwrclass_val = MMC_MAX_CURRENT_200;
>> +
>>        /* If the power class is different from the default value */
>>        if (pwrclass_val > 0) {
>>                err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> index 9478a6b..c5e031a 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -195,6 +195,10 @@ struct mmc_part {
>>  #define MMC_BLK_DATA_AREA_GP   (1<<2)
>>  };
>>
>> +#define MMC_MAX_CURRENT_200    (0)
>> +#define MMC_MAX_CURRENT_400    (7)
>> +#define MMC_MAX_CURRENT_600    (11)
>> +#define MMC_MAX_CURRENT_800    (13)
>>  /*
>>  * MMC device
>>  */
>> --
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] mmc: core: Add host capability check for power class

[Hein_Tibosch]

On 12/15/2011 2:03 PM, amit kachhap wrote:

> On Thu, Dec 15, 2011 at 9:28 AM, Girish K S
> <girish.shivananjappa@linaro.org> wrote:
>> This patch adds a check whether the host supports maximum current value
>> obtained from the device's extended csd register for a selected interface
>> voltage and frequency.
>>
>> cc: Chris Ball <cjb@laptop.org>
>> Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
>> ---
>> Changes in v2:
>>        deleted a unnecessary if else condition identified by subhash J
>> Changes in v1:
>>       reduced the number of comparisons as per Hein's suggestion
>>
>>  drivers/mmc/core/mmc.c   |   19 +++++++++++++++++++
>>  include/linux/mmc/card.h |    4 ++++
>>  2 files changed, 23 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 006e932..b9ef777 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -688,6 +688,25 @@ static int mmc_select_powerclass(struct mmc_card *card,
>>                pwrclass_val = (pwrclass_val & EXT_CSD_PWR_CL_4BIT_MASK) >>
>>                                EXT_CSD_PWR_CL_4BIT_SHIFT;
>>
>> +       if (pwrclass_val >= MMC_MAX_CURRENT_800)
> Hi girish,
>
> These checks can be made like ,
> if (pwrclass_val > MMC_MAX_CURRENT_800)
>         pwrclass_val = MMC_MAX_CURRENT_800;
> Applicable in all below conditional checks.
Hi both,

Suppose pwrclass_val == MMC_MAX_CURRENT_800 (13),
then with

+       if (pwrclass_val > MMC_MAX_CURRENT_800)
+               pwrclass_val = MMC_MAX_CURRENT_800;
+       else if (pwrclass_val > MMC_MAX_CURRENT_600)
+               pwrclass_val = MMC_MAX_CURRENT_600;

it would get a value of MMC_MAX_CURRENT_600, which is too low

Hein

Re: [PATCH V2] mmc: core: Add host capability check for power class

[Saugata Das]

On 15 December 2011 09:28, Girish K S <girish.shivananjappa@linaro.org> wrote:
> This patch adds a check whether the host supports maximum current value
> obtained from the device's extended csd register for a selected interface
> voltage and frequency.
>
> cc: Chris Ball <cjb@laptop.org>
> Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
> ---
> Changes in v2:
>        deleted a unnecessary if else condition identified by subhash J
> Changes in v1:
>       reduced the number of comparisons as per Hein's suggestion
>
>  drivers/mmc/core/mmc.c   |   19 +++++++++++++++++++
>  include/linux/mmc/card.h |    4 ++++
>  2 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 006e932..b9ef777 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -688,6 +688,25 @@ static int mmc_select_powerclass(struct mmc_card *card,
>                pwrclass_val = (pwrclass_val & EXT_CSD_PWR_CL_4BIT_MASK) >>
>                                EXT_CSD_PWR_CL_4BIT_SHIFT;
>
> +       if (pwrclass_val >= MMC_MAX_CURRENT_800)
> +               pwrclass_val = MMC_MAX_CURRENT_800;
> +       else if (pwrclass_val >= MMC_MAX_CURRENT_600)
> +               pwrclass_val = MMC_MAX_CURRENT_600;
> +       else if (pwrclass_val >= MMC_MAX_CURRENT_400)
> +               pwrclass_val = MMC_MAX_CURRENT_400;
> +       else
> +               pwrclass_val = MMC_MAX_CURRENT_200;
> +
> +       if ((pwrclass_val == MMC_MAX_CURRENT_800) &&
> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_800))
> +               pwrclass_val = MMC_MAX_CURRENT_600;
> +       if ((pwrclass_val == MMC_MAX_CURRENT_600) &&
> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_600))
> +               pwrclass_val = MMC_MAX_CURRENT_400;
> +       if ((pwrclass_val == MMC_MAX_CURRENT_400) &&
> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_400))
> +               pwrclass_val = MMC_MAX_CURRENT_200;
> +
>        /* If the power class is different from the default value */
>        if (pwrclass_val > 0) {
>                err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,

It is not allowed to set the POWER_CLASS with any value other than
what is mentioned in the PWR_CL_ff_vvv or PWR_CL_DDR_ff_vvv  for the
corresponding frequency, voltage. That is, if PWR_CL_200_195 is 14 and
we want to operate at HS200 then the only value allowed for
POWER_CLASS is 14. So, we need to check the PWR_CL numbers and choose
the operating mode (HS200/DDR50/..) based on the platform capability
to support the current consumption and set the corresponding
POWER_CLASS value.

Please refer to section 6.6.5 of the 4.5 spec.


> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 9478a6b..c5e031a 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -195,6 +195,10 @@ struct mmc_part {
>  #define MMC_BLK_DATA_AREA_GP   (1<<2)
>  };
>
> +#define MMC_MAX_CURRENT_200    (0)
> +#define MMC_MAX_CURRENT_400    (7)
> +#define MMC_MAX_CURRENT_600    (11)
> +#define MMC_MAX_CURRENT_800    (13)
>  /*
>  * MMC device
>  */
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] mmc: core: Add host capability check for power class

[Girish K S]

On 15 December 2011 15:34, Saugata Das <saugata.das@linaro.org> wrote:
> On 15 December 2011 09:28, Girish K S <girish.shivananjappa@linaro.org> wrote:
>> This patch adds a check whether the host supports maximum current value
>> obtained from the device's extended csd register for a selected interface
>> voltage and frequency.
>>
>> cc: Chris Ball <cjb@laptop.org>
>> Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
>> ---
>> Changes in v2:
>>        deleted a unnecessary if else condition identified by subhash J
>> Changes in v1:
>>       reduced the number of comparisons as per Hein's suggestion
>>
>>  drivers/mmc/core/mmc.c   |   19 +++++++++++++++++++
>>  include/linux/mmc/card.h |    4 ++++
>>  2 files changed, 23 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 006e932..b9ef777 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -688,6 +688,25 @@ static int mmc_select_powerclass(struct mmc_card *card,
>>                pwrclass_val = (pwrclass_val & EXT_CSD_PWR_CL_4BIT_MASK) >>
>>                                EXT_CSD_PWR_CL_4BIT_SHIFT;
>>
>> +       if (pwrclass_val >= MMC_MAX_CURRENT_800)
>> +               pwrclass_val = MMC_MAX_CURRENT_800;
>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_600)
>> +               pwrclass_val = MMC_MAX_CURRENT_600;
>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_400)
>> +               pwrclass_val = MMC_MAX_CURRENT_400;
>> +       else
>> +               pwrclass_val = MMC_MAX_CURRENT_200;
>> +
>> +       if ((pwrclass_val == MMC_MAX_CURRENT_800) &&
>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_800))
>> +               pwrclass_val = MMC_MAX_CURRENT_600;
>> +       if ((pwrclass_val == MMC_MAX_CURRENT_600) &&
>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_600))
>> +               pwrclass_val = MMC_MAX_CURRENT_400;
>> +       if ((pwrclass_val == MMC_MAX_CURRENT_400) &&
>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_400))
>> +               pwrclass_val = MMC_MAX_CURRENT_200;
>> +
>>        /* If the power class is different from the default value */
>>        if (pwrclass_val > 0) {
>>                err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>
> It is not allowed to set the POWER_CLASS with any value other than
> what is mentioned in the PWR_CL_ff_vvv or PWR_CL_DDR_ff_vvv  for the
> corresponding frequency, voltage. That is, if PWR_CL_200_195 is 14 and
> we want to operate at HS200 then the only value allowed for
> POWER_CLASS is 14. So, we need to check the PWR_CL numbers and choose
> the operating mode (HS200/DDR50/..) based on the platform capability
> to support the current consumption and set the corresponding
> POWER_CLASS value.
>
> Please refer to section 6.6.5 of the 4.5 spec.

The upstreamed code reads the extended csd value based on the already
set voltage level and frequency of host. So it will get the required
power class value which can be set directly. Is my understanding
correct?

>
>
>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> index 9478a6b..c5e031a 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -195,6 +195,10 @@ struct mmc_part {
>>  #define MMC_BLK_DATA_AREA_GP   (1<<2)
>>  };
>>
>> +#define MMC_MAX_CURRENT_200    (0)
>> +#define MMC_MAX_CURRENT_400    (7)
>> +#define MMC_MAX_CURRENT_600    (11)
>> +#define MMC_MAX_CURRENT_800    (13)
>>  /*
>>  * MMC device
>>  */
>> --
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] mmc: core: Add host capability check for power class

[Saugata Das]

On 15 December 2011 16:22, Girish K S <girish.shivananjappa@linaro.org> wrote:
> On 15 December 2011 15:34, Saugata Das <saugata.das@linaro.org> wrote:
>> On 15 December 2011 09:28, Girish K S <girish.shivananjappa@linaro.org> wrote:
>>> This patch adds a check whether the host supports maximum current value
>>> obtained from the device's extended csd register for a selected interface
>>> voltage and frequency.
>>>
>>> cc: Chris Ball <cjb@laptop.org>
>>> Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
>>> ---
>>> Changes in v2:
>>>        deleted a unnecessary if else condition identified by subhash J
>>> Changes in v1:
>>>       reduced the number of comparisons as per Hein's suggestion
>>>
>>>  drivers/mmc/core/mmc.c   |   19 +++++++++++++++++++
>>>  include/linux/mmc/card.h |    4 ++++
>>>  2 files changed, 23 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index 006e932..b9ef777 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -688,6 +688,25 @@ static int mmc_select_powerclass(struct mmc_card *card,
>>>                pwrclass_val = (pwrclass_val & EXT_CSD_PWR_CL_4BIT_MASK) >>
>>>                                EXT_CSD_PWR_CL_4BIT_SHIFT;
>>>
>>> +       if (pwrclass_val >= MMC_MAX_CURRENT_800)
>>> +               pwrclass_val = MMC_MAX_CURRENT_800;
>>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_600)
>>> +               pwrclass_val = MMC_MAX_CURRENT_600;
>>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_400)
>>> +               pwrclass_val = MMC_MAX_CURRENT_400;
>>> +       else
>>> +               pwrclass_val = MMC_MAX_CURRENT_200;
>>> +
>>> +       if ((pwrclass_val == MMC_MAX_CURRENT_800) &&
>>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_800))
>>> +               pwrclass_val = MMC_MAX_CURRENT_600;
>>> +       if ((pwrclass_val == MMC_MAX_CURRENT_600) &&
>>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_600))
>>> +               pwrclass_val = MMC_MAX_CURRENT_400;
>>> +       if ((pwrclass_val == MMC_MAX_CURRENT_400) &&
>>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_400))
>>> +               pwrclass_val = MMC_MAX_CURRENT_200;
>>> +
>>>        /* If the power class is different from the default value */
>>>        if (pwrclass_val > 0) {
>>>                err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>
>> It is not allowed to set the POWER_CLASS with any value other than
>> what is mentioned in the PWR_CL_ff_vvv or PWR_CL_DDR_ff_vvv  for the
>> corresponding frequency, voltage. That is, if PWR_CL_200_195 is 14 and
>> we want to operate at HS200 then the only value allowed for
>> POWER_CLASS is 14. So, we need to check the PWR_CL numbers and choose
>> the operating mode (HS200/DDR50/..) based on the platform capability
>> to support the current consumption and set the corresponding
>> POWER_CLASS value.
>>
>> Please refer to section 6.6.5 of the 4.5 spec.
>
> The upstreamed code reads the extended csd value based on the already
> set voltage level and frequency of host. So it will get the required
> power class value which can be set directly. Is my understanding
> correct?
>

It is not enough to just check the voltage level and frequency.
Consider this example, host has capability to support
MMC_CAP_MAX_CURRENT_400, the PWR_CL_DDR_52_360 has the value 9 (400mA)
and PWR_CL_200_360 has the value 14 (800mA). Then even though the host
might be capable to run 200MHz clock and 3.6V, it can only enable DDR
at 52MHz and set 9 in POWER_CLASS.

I think, in mmc_select_powerclass, we need to loop through the power
classes of all supported modes of transfer (HS200, DDR52, ... ) and
choose the mode which gives maximum bandwidth but falls within host
capability of current consumption. Then set this to POWER_CLASS byte
and also use the same information when setting HS_TIMING in
mmc_init_card.

>>
>>
>>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>>> index 9478a6b..c5e031a 100644
>>> --- a/include/linux/mmc/card.h
>>> +++ b/include/linux/mmc/card.h
>>> @@ -195,6 +195,10 @@ struct mmc_part {
>>>  #define MMC_BLK_DATA_AREA_GP   (1<<2)
>>>  };
>>>
>>> +#define MMC_MAX_CURRENT_200    (0)
>>> +#define MMC_MAX_CURRENT_400    (7)
>>> +#define MMC_MAX_CURRENT_600    (11)
>>> +#define MMC_MAX_CURRENT_800    (13)
>>>  /*
>>>  * MMC device
>>>  */
>>> --
>>> 1.7.1
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH V2] mmc: core: Add host capability check for power class

[Subhash Jadavani]

> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Saugata Das
> Sent: Thursday, December 15, 2011 6:35 PM
> To: Girish K S
> Cc: linux-mmc@vger.kernel.org; patches@linaro.org; linux-samsung-
> soc@vger.kernel.org; subhashj@codeaurora.org; Chris Ball
> Subject: Re: [PATCH V2] mmc: core: Add host capability check for power
class
> 
> On 15 December 2011 16:22, Girish K S <girish.shivananjappa@linaro.org>
> wrote:
> > On 15 December 2011 15:34, Saugata Das <saugata.das@linaro.org> wrote:
> >> On 15 December 2011 09:28, Girish K S <girish.shivananjappa@linaro.org>
> wrote:
> >>> This patch adds a check whether the host supports maximum current
> >>> value obtained from the device's extended csd register for a
> >>> selected interface voltage and frequency.
> >>>
> >>> cc: Chris Ball <cjb@laptop.org>
> >>> Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
> >>> ---
> >>> Changes in v2:
> >>>        deleted a unnecessary if else condition identified by subhash
> >>> J Changes in v1:
> >>>       reduced the number of comparisons as per Hein's suggestion
> >>>
> >>>  drivers/mmc/core/mmc.c   |   19 +++++++++++++++++++
> >>>  include/linux/mmc/card.h |    4 ++++
> >>>  2 files changed, 23 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index
> >>> 006e932..b9ef777 100644
> >>> --- a/drivers/mmc/core/mmc.c
> >>> +++ b/drivers/mmc/core/mmc.c
> >>> @@ -688,6 +688,25 @@ static int mmc_select_powerclass(struct
> >>> mmc_card *card,
> >>>                pwrclass_val = (pwrclass_val &
> >>> EXT_CSD_PWR_CL_4BIT_MASK) >>
> >>>                                EXT_CSD_PWR_CL_4BIT_SHIFT;
> >>>
> >>> +       if (pwrclass_val >= MMC_MAX_CURRENT_800)
> >>> +               pwrclass_val = MMC_MAX_CURRENT_800;
> >>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_600)
> >>> +               pwrclass_val = MMC_MAX_CURRENT_600;
> >>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_400)
> >>> +               pwrclass_val = MMC_MAX_CURRENT_400;
> >>> +       else
> >>> +               pwrclass_val = MMC_MAX_CURRENT_200;
> >>> +
> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_800) &&
> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_800))
> >>> +               pwrclass_val = MMC_MAX_CURRENT_600;
> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_600) &&
> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_600))
> >>> +               pwrclass_val = MMC_MAX_CURRENT_400;
> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_400) &&
> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_400))
> >>> +               pwrclass_val = MMC_MAX_CURRENT_200;
> >>> +
> >>>        /* If the power class is different from the default value */
> >>>        if (pwrclass_val > 0) {
> >>>                err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> >>
> >> It is not allowed to set the POWER_CLASS with any value other than
> >> what is mentioned in the PWR_CL_ff_vvv or PWR_CL_DDR_ff_vvv  for
> the
> >> corresponding frequency, voltage. That is, if PWR_CL_200_195 is 14
> >> and we want to operate at HS200 then the only value allowed for
> >> POWER_CLASS is 14. So, we need to check the PWR_CL numbers and
> choose
> >> the operating mode (HS200/DDR50/..) based on the platform capability
> >> to support the current consumption and set the corresponding
> >> POWER_CLASS value.
> >>
> >> Please refer to section 6.6.5 of the 4.5 spec.
> >
> > The upstreamed code reads the extended csd value based on the already
> > set voltage level and frequency of host. So it will get the required
> > power class value which can be set directly. Is my understanding
> > correct?
> >
> 
> It is not enough to just check the voltage level and frequency.
> Consider this example, host has capability to support
> MMC_CAP_MAX_CURRENT_400, the PWR_CL_DDR_52_360 has the value 9
> (400mA) and PWR_CL_200_360 has the value 14 (800mA). Then even though
> the host might be capable to run 200MHz clock and 3.6V, it can only enable
> DDR at 52MHz and set 9 in POWER_CLASS.
> 
> I think, in mmc_select_powerclass, we need to loop through the power
> classes of all supported modes of transfer (HS200, DDR52, ... ) and choose
the
> mode which gives maximum bandwidth but falls within host capability of
> current consumption. Then set this to POWER_CLASS byte and also use the
> same information when setting HS_TIMING in mmc_init_card.

Hi Saugata,

Does the spec mandates you to set the power class to what is needed by
frequency/voltage combination? I can't see that mentioned anywhere
explicitly in eMMC4.5 spec (if it is mentioned in spec, please let me know
section and line number). It may be still possible to set the power class
lower than what is needed by frequency/voltage combination. Say for example,
8-bit HS200 (200MHz) with high voltage cards may specify power class
(PWR_CL_200_3_6) value of 14 (800 mA) but that doesn't mean if you want the
card to work in 8-bit HS200 mode, its POWER_CLASS value must be 14 (800mA).
If host's VDD regulator is only capable of say 600mA then it may still set
the POWER_CLASS to 12 (600 mA) which should be the indication to card that
host power supply is capable of sourcing only 600mA and I would assume card
will make sure that it doesn't draw anything more than that. 

Hi Girish,

I see couple of other issues with your original power_class selection patch.
1. mmc_select_powerclass() function considers 2.7v to 3.2v VDD range as
invalid. That's not correct. Valid high voltage range is from 2.7v to 3.6v.
Is there a specific reason you have skipped the 2.7v to 3.2v VDD range? If
not, I should post following change:

--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -626,6 +626,11 @@ static int mmc_select_powerclass(struct mmc_card *card,
                else if (host->ios.clock <= 200000000)
                        index = EXT_CSD_PWR_CL_200_195;
                break;
+       case MMC_VDD_27_28:
+       case MMC_VDD_28_29:
+       case MMC_VDD_29_30:
+       case MMC_VDD_30_31:
+       case MMC_VDD_31_32:
        case MMC_VDD_32_33:
        case MMC_VDD_33_34:
        case MMC_VDD_34_35:

2.  Currently in mmc_init_card(), power class selection is done if it's
normal (DS or HS) or DDR or HS200 card.
    If power class selection fails (mmc_select_powerclass() returns error)
for DS/HS/DDR cards, we are just printing the error and going ahead with the
rest of the initialization where as if power class selection fails for HS200
cards, we are simply aborting the  
    initialization and mark it as failed. 

   There are my points:
       1.  Power class failure must be treated in same manner for
DS/HS/DDR/HS200 cards
       2.  If you agree on first point then we need to decide whether power
class selection failure is fatal enough to abort the MMC initialization? or
we can just print the warning and go ahead with rest of the initialization
in same bus speed mode?

Regards,
Subhash

> 
> >>
> >>
> >>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> >>> index 9478a6b..c5e031a 100644
> >>> --- a/include/linux/mmc/card.h
> >>> +++ b/include/linux/mmc/card.h
> >>> @@ -195,6 +195,10 @@ struct mmc_part {
> >>>  #define MMC_BLK_DATA_AREA_GP   (1<<2)
> >>>  };
> >>>
> >>> +#define MMC_MAX_CURRENT_200    (0)
> >>> +#define MMC_MAX_CURRENT_400    (7)
> >>> +#define MMC_MAX_CURRENT_600    (11) #define
> MMC_MAX_CURRENT_800
> >>> +(13)
> >>>  /*
> >>>  * MMC device
> >>>  */
> >>> --
> >>> 1.7.1
> >>>
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-mmc"
> >>> in the body of a message to majordomo@vger.kernel.org More
> majordomo
> >>> info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] mmc: core: Add host capability check for power class

[Girish K S]

On 28 March 2012 16:39, Subhash Jadavani <subhashj@codeaurora.org> wrote:
>
>
>> -----Original Message-----
>> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
>> owner@vger.kernel.org] On Behalf Of Saugata Das
>> Sent: Thursday, December 15, 2011 6:35 PM
>> To: Girish K S
>> Cc: linux-mmc@vger.kernel.org; patches@linaro.org; linux-samsung-
>> soc@vger.kernel.org; subhashj@codeaurora.org; Chris Ball
>> Subject: Re: [PATCH V2] mmc: core: Add host capability check for power
> class
>>
>> On 15 December 2011 16:22, Girish K S <girish.shivananjappa@linaro.org>
>> wrote:
>> > On 15 December 2011 15:34, Saugata Das <saugata.das@linaro.org> wrote:
>> >> On 15 December 2011 09:28, Girish K S <girish.shivananjappa@linaro.org>
>> wrote:
>> >>> This patch adds a check whether the host supports maximum current
>> >>> value obtained from the device's extended csd register for a
>> >>> selected interface voltage and frequency.
>> >>>
>> >>> cc: Chris Ball <cjb@laptop.org>
>> >>> Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
>> >>> ---
>> >>> Changes in v2:
>> >>>        deleted a unnecessary if else condition identified by subhash
>> >>> J Changes in v1:
>> >>>       reduced the number of comparisons as per Hein's suggestion
>> >>>
>> >>>  drivers/mmc/core/mmc.c   |   19 +++++++++++++++++++
>> >>>  include/linux/mmc/card.h |    4 ++++
>> >>>  2 files changed, 23 insertions(+), 0 deletions(-)
>> >>>
>> >>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index
>> >>> 006e932..b9ef777 100644
>> >>> --- a/drivers/mmc/core/mmc.c
>> >>> +++ b/drivers/mmc/core/mmc.c
>> >>> @@ -688,6 +688,25 @@ static int mmc_select_powerclass(struct
>> >>> mmc_card *card,
>> >>>                pwrclass_val = (pwrclass_val &
>> >>> EXT_CSD_PWR_CL_4BIT_MASK) >>
>> >>>                                EXT_CSD_PWR_CL_4BIT_SHIFT;
>> >>>
>> >>> +       if (pwrclass_val >= MMC_MAX_CURRENT_800)
>> >>> +               pwrclass_val = MMC_MAX_CURRENT_800;
>> >>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_600)
>> >>> +               pwrclass_val = MMC_MAX_CURRENT_600;
>> >>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_400)
>> >>> +               pwrclass_val = MMC_MAX_CURRENT_400;
>> >>> +       else
>> >>> +               pwrclass_val = MMC_MAX_CURRENT_200;
>> >>> +
>> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_800) &&
>> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_800))
>> >>> +               pwrclass_val = MMC_MAX_CURRENT_600;
>> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_600) &&
>> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_600))
>> >>> +               pwrclass_val = MMC_MAX_CURRENT_400;
>> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_400) &&
>> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_400))
>> >>> +               pwrclass_val = MMC_MAX_CURRENT_200;
>> >>> +
>> >>>        /* If the power class is different from the default value */
>> >>>        if (pwrclass_val > 0) {
>> >>>                err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> >>
>> >> It is not allowed to set the POWER_CLASS with any value other than
>> >> what is mentioned in the PWR_CL_ff_vvv or PWR_CL_DDR_ff_vvv  for
>> the
>> >> corresponding frequency, voltage. That is, if PWR_CL_200_195 is 14
>> >> and we want to operate at HS200 then the only value allowed for
>> >> POWER_CLASS is 14. So, we need to check the PWR_CL numbers and
>> choose
>> >> the operating mode (HS200/DDR50/..) based on the platform capability
>> >> to support the current consumption and set the corresponding
>> >> POWER_CLASS value.
>> >>
>> >> Please refer to section 6.6.5 of the 4.5 spec.
>> >
>> > The upstreamed code reads the extended csd value based on the already
>> > set voltage level and frequency of host. So it will get the required
>> > power class value which can be set directly. Is my understanding
>> > correct?
>> >
>>
>> It is not enough to just check the voltage level and frequency.
>> Consider this example, host has capability to support
>> MMC_CAP_MAX_CURRENT_400, the PWR_CL_DDR_52_360 has the value 9
>> (400mA) and PWR_CL_200_360 has the value 14 (800mA). Then even though
>> the host might be capable to run 200MHz clock and 3.6V, it can only enable
>> DDR at 52MHz and set 9 in POWER_CLASS.
>>
>> I think, in mmc_select_powerclass, we need to loop through the power
>> classes of all supported modes of transfer (HS200, DDR52, ... ) and choose
> the
>> mode which gives maximum bandwidth but falls within host capability of
>> current consumption. Then set this to POWER_CLASS byte and also use the
>> same information when setting HS_TIMING in mmc_init_card.
>
> Hi Saugata,
>
> Does the spec mandates you to set the power class to what is needed by
> frequency/voltage combination? I can't see that mentioned anywhere
> explicitly in eMMC4.5 spec (if it is mentioned in spec, please let me know
> section and line number). It may be still possible to set the power class
> lower than what is needed by frequency/voltage combination. Say for example,
> 8-bit HS200 (200MHz) with high voltage cards may specify power class
> (PWR_CL_200_3_6) value of 14 (800 mA) but that doesn't mean if you want the
> card to work in 8-bit HS200 mode, its POWER_CLASS value must be 14 (800mA).
> If host's VDD regulator is only capable of say 600mA then it may still set
> the POWER_CLASS to 12 (600 mA) which should be the indication to card that
> host power supply is capable of sourcing only 600mA and I would assume card
> will make sure that it doesn't draw anything more than that.
>
> Hi Girish,
>
> I see couple of other issues with your original power_class selection patch.
> 1. mmc_select_powerclass() function considers 2.7v to 3.2v VDD range as
> invalid. That's not correct. Valid high voltage range is from 2.7v to 3.6v.
> Is there a specific reason you have skipped the 2.7v to 3.2v VDD range? If
> not, I should post following change:

can post a patch for this

>
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -626,6 +626,11 @@ static int mmc_select_powerclass(struct mmc_card *card,
>                else if (host->ios.clock <= 200000000)
>                        index = EXT_CSD_PWR_CL_200_195;
>                break;
> +       case MMC_VDD_27_28:
> +       case MMC_VDD_28_29:
> +       case MMC_VDD_29_30:
> +       case MMC_VDD_30_31:
> +       case MMC_VDD_31_32:
>        case MMC_VDD_32_33:
>        case MMC_VDD_33_34:
>        case MMC_VDD_34_35:
>
> 2.  Currently in mmc_init_card(), power class selection is done if it's
> normal (DS or HS) or DDR or HS200 card.
>    If power class selection fails (mmc_select_powerclass() returns error)
> for DS/HS/DDR cards, we are just printing the error and going ahead with the
> rest of the initialization where as if power class selection fails for HS200
> cards, we are simply aborting the
>    initialization and mark it as failed.
>
>   There are my points:
>       1.  Power class failure must be treated in same manner for
> DS/HS/DDR/HS200 cards
good find.
Yes will modify and repost for the HS200 case.

>       2.  If you agree on first point then we need to decide whether power
> class selection failure is fatal enough to abort the MMC initialization? or
> we can just print the warning and go ahead with rest of the initialization
> in same bus speed mode?

Should not be treated as fatal, should continue the initialization
with default power class

>
> Regards,
> Subhash
>
>>
>> >>
>> >>
>> >>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> >>> index 9478a6b..c5e031a 100644
>> >>> --- a/include/linux/mmc/card.h
>> >>> +++ b/include/linux/mmc/card.h
>> >>> @@ -195,6 +195,10 @@ struct mmc_part {
>> >>>  #define MMC_BLK_DATA_AREA_GP   (1<<2)
>> >>>  };
>> >>>
>> >>> +#define MMC_MAX_CURRENT_200    (0)
>> >>> +#define MMC_MAX_CURRENT_400    (7)
>> >>> +#define MMC_MAX_CURRENT_600    (11) #define
>> MMC_MAX_CURRENT_800
>> >>> +(13)
>> >>>  /*
>> >>>  * MMC device
>> >>>  */
>> >>> --
>> >>> 1.7.1
>> >>>
>> >>> --
>> >>> To unsubscribe from this list: send the line "unsubscribe linux-mmc"
>> >>> in the body of a message to majordomo@vger.kernel.org More
>> majordomo
>> >>> info at  http://vger.kernel.org/majordomo-info.html
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the
>> body of a message to majordomo@vger.kernel.org More majordomo info at
>> http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH V2] mmc: core: Add host capability check for power class

[Girish K S]

On 29 March 2012 11:17, Girish K S <girish.shivananjappa@linaro.org> wrote:
> On 28 March 2012 16:39, Subhash Jadavani <subhashj@codeaurora.org> wrote:
>>
>>
>>> -----Original Message-----
>>> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
>>> owner@vger.kernel.org] On Behalf Of Saugata Das
>>> Sent: Thursday, December 15, 2011 6:35 PM
>>> To: Girish K S
>>> Cc: linux-mmc@vger.kernel.org; patches@linaro.org; linux-samsung-
>>> soc@vger.kernel.org; subhashj@codeaurora.org; Chris Ball
>>> Subject: Re: [PATCH V2] mmc: core: Add host capability check for power
>> class
>>>
>>> On 15 December 2011 16:22, Girish K S <girish.shivananjappa@linaro.org>
>>> wrote:
>>> > On 15 December 2011 15:34, Saugata Das <saugata.das@linaro.org> wrote:
>>> >> On 15 December 2011 09:28, Girish K S <girish.shivananjappa@linaro.org>
>>> wrote:
>>> >>> This patch adds a check whether the host supports maximum current
>>> >>> value obtained from the device's extended csd register for a
>>> >>> selected interface voltage and frequency.
>>> >>>
>>> >>> cc: Chris Ball <cjb@laptop.org>
>>> >>> Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
>>> >>> ---
>>> >>> Changes in v2:
>>> >>>        deleted a unnecessary if else condition identified by subhash
>>> >>> J Changes in v1:
>>> >>>       reduced the number of comparisons as per Hein's suggestion
>>> >>>
>>> >>>  drivers/mmc/core/mmc.c   |   19 +++++++++++++++++++
>>> >>>  include/linux/mmc/card.h |    4 ++++
>>> >>>  2 files changed, 23 insertions(+), 0 deletions(-)
>>> >>>
>>> >>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index
>>> >>> 006e932..b9ef777 100644
>>> >>> --- a/drivers/mmc/core/mmc.c
>>> >>> +++ b/drivers/mmc/core/mmc.c
>>> >>> @@ -688,6 +688,25 @@ static int mmc_select_powerclass(struct
>>> >>> mmc_card *card,
>>> >>>                pwrclass_val = (pwrclass_val &
>>> >>> EXT_CSD_PWR_CL_4BIT_MASK) >>
>>> >>>                                EXT_CSD_PWR_CL_4BIT_SHIFT;
>>> >>>
>>> >>> +       if (pwrclass_val >= MMC_MAX_CURRENT_800)
>>> >>> +               pwrclass_val = MMC_MAX_CURRENT_800;
>>> >>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_600)
>>> >>> +               pwrclass_val = MMC_MAX_CURRENT_600;
>>> >>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_400)
>>> >>> +               pwrclass_val = MMC_MAX_CURRENT_400;
>>> >>> +       else
>>> >>> +               pwrclass_val = MMC_MAX_CURRENT_200;
>>> >>> +
>>> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_800) &&
>>> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_800))
>>> >>> +               pwrclass_val = MMC_MAX_CURRENT_600;
>>> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_600) &&
>>> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_600))
>>> >>> +               pwrclass_val = MMC_MAX_CURRENT_400;
>>> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_400) &&
>>> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_400))
>>> >>> +               pwrclass_val = MMC_MAX_CURRENT_200;
>>> >>> +
>>> >>>        /* If the power class is different from the default value */
>>> >>>        if (pwrclass_val > 0) {
>>> >>>                err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>> >>
>>> >> It is not allowed to set the POWER_CLASS with any value other than
>>> >> what is mentioned in the PWR_CL_ff_vvv or PWR_CL_DDR_ff_vvv  for
>>> the
>>> >> corresponding frequency, voltage. That is, if PWR_CL_200_195 is 14
>>> >> and we want to operate at HS200 then the only value allowed for
>>> >> POWER_CLASS is 14. So, we need to check the PWR_CL numbers and
>>> choose
>>> >> the operating mode (HS200/DDR50/..) based on the platform capability
>>> >> to support the current consumption and set the corresponding
>>> >> POWER_CLASS value.
>>> >>
>>> >> Please refer to section 6.6.5 of the 4.5 spec.
>>> >
>>> > The upstreamed code reads the extended csd value based on the already
>>> > set voltage level and frequency of host. So it will get the required
>>> > power class value which can be set directly. Is my understanding
>>> > correct?
>>> >
>>>
>>> It is not enough to just check the voltage level and frequency.
>>> Consider this example, host has capability to support
>>> MMC_CAP_MAX_CURRENT_400, the PWR_CL_DDR_52_360 has the value 9
>>> (400mA) and PWR_CL_200_360 has the value 14 (800mA). Then even though
>>> the host might be capable to run 200MHz clock and 3.6V, it can only enable
>>> DDR at 52MHz and set 9 in POWER_CLASS.
>>>
>>> I think, in mmc_select_powerclass, we need to loop through the power
>>> classes of all supported modes of transfer (HS200, DDR52, ... ) and choose
>> the
>>> mode which gives maximum bandwidth but falls within host capability of
>>> current consumption. Then set this to POWER_CLASS byte and also use the
>>> same information when setting HS_TIMING in mmc_init_card.
>>
>> Hi Saugata,
>>
>> Does the spec mandates you to set the power class to what is needed by
>> frequency/voltage combination? I can't see that mentioned anywhere
>> explicitly in eMMC4.5 spec (if it is mentioned in spec, please let me know
>> section and line number). It may be still possible to set the power class
>> lower than what is needed by frequency/voltage combination. Say for example,
>> 8-bit HS200 (200MHz) with high voltage cards may specify power class
>> (PWR_CL_200_3_6) value of 14 (800 mA) but that doesn't mean if you want the
>> card to work in 8-bit HS200 mode, its POWER_CLASS value must be 14 (800mA).
>> If host's VDD regulator is only capable of say 600mA then it may still set
>> the POWER_CLASS to 12 (600 mA) which should be the indication to card that
>> host power supply is capable of sourcing only 600mA and I would assume card
>> will make sure that it doesn't draw anything more than that.
>>
>> Hi Girish,
>>
>> I see couple of other issues with your original power_class selection patch.
>> 1. mmc_select_powerclass() function considers 2.7v to 3.2v VDD range as
>> invalid. That's not correct. Valid high voltage range is from 2.7v to 3.6v.
>> Is there a specific reason you have skipped the 2.7v to 3.2v VDD range? If
>> not, I should post following change:
>
> can post a patch for this
>
>>
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -626,6 +626,11 @@ static int mmc_select_powerclass(struct mmc_card *card,
>>                else if (host->ios.clock <= 200000000)
>>                        index = EXT_CSD_PWR_CL_200_195;
>>                break;
>> +       case MMC_VDD_27_28:
>> +       case MMC_VDD_28_29:
>> +       case MMC_VDD_29_30:
>> +       case MMC_VDD_30_31:
>> +       case MMC_VDD_31_32:
>>        case MMC_VDD_32_33:
>>        case MMC_VDD_33_34:
>>        case MMC_VDD_34_35:
>>
>> 2.  Currently in mmc_init_card(), power class selection is done if it's
>> normal (DS or HS) or DDR or HS200 card.
>>    If power class selection fails (mmc_select_powerclass() returns error)
>> for DS/HS/DDR cards, we are just printing the error and going ahead with the
>> rest of the initialization where as if power class selection fails for HS200
>> cards, we are simply aborting the
>>    initialization and mark it as failed.
>>
>>   There are my points:
>>       1.  Power class failure must be treated in same manner for
>> DS/HS/DDR/HS200 cards
> good find.
> Yes will modify and repost for the HS200 case.
>
>>       2.  If you agree on first point then we need to decide whether power
>> class selection failure is fatal enough to abort the MMC initialization? or
>> we can just print the warning and go ahead with rest of the initialization
>> in same bus speed mode?
>
> Should not be treated as fatal, should continue the initialization
> with default power class
Instead of making 2 different patches, can you add this modification
in your above patch. "you need to remove only the return err;
statement"
>
>>
>> Regards,
>> Subhash
>>
>>>
>>> >>
>>> >>
>>> >>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>>> >>> index 9478a6b..c5e031a 100644
>>> >>> --- a/include/linux/mmc/card.h
>>> >>> +++ b/include/linux/mmc/card.h
>>> >>> @@ -195,6 +195,10 @@ struct mmc_part {
>>> >>>  #define MMC_BLK_DATA_AREA_GP   (1<<2)
>>> >>>  };
>>> >>>
>>> >>> +#define MMC_MAX_CURRENT_200    (0)
>>> >>> +#define MMC_MAX_CURRENT_400    (7)
>>> >>> +#define MMC_MAX_CURRENT_600    (11) #define
>>> MMC_MAX_CURRENT_800
>>> >>> +(13)
>>> >>>  /*
>>> >>>  * MMC device
>>> >>>  */
>>> >>> --
>>> >>> 1.7.1
>>> >>>
>>> >>> --
>>> >>> To unsubscribe from this list: send the line "unsubscribe linux-mmc"
>>> >>> in the body of a message to majordomo@vger.kernel.org More
>>> majordomo
>>> >>> info at  http://vger.kernel.org/majordomo-info.html
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the
>>> body of a message to majordomo@vger.kernel.org More majordomo info at
>>> http://vger.kernel.org/majordomo-info.html
>>

RE: [PATCH V2] mmc: core: Add host capability check for power class

[Subhash Jadavani]

> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Girish K S
> Sent: Friday, March 30, 2012 10:29 AM
> To: Subhash Jadavani
> Cc: Saugata Das; linux-mmc@vger.kernel.org; patches@linaro.org; linux-
> samsung-soc@vger.kernel.org; Chris Ball
> Subject: Re: [PATCH V2] mmc: core: Add host capability check for power
class
> 
> On 29 March 2012 11:17, Girish K S <girish.shivananjappa@linaro.org>
wrote:
> > On 28 March 2012 16:39, Subhash Jadavani <subhashj@codeaurora.org>
> wrote:
> >>
> >>
> >>> -----Original Message-----
> >>> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> >>> owner@vger.kernel.org] On Behalf Of Saugata Das
> >>> Sent: Thursday, December 15, 2011 6:35 PM
> >>> To: Girish K S
> >>> Cc: linux-mmc@vger.kernel.org; patches@linaro.org; linux-samsung-
> >>> soc@vger.kernel.org; subhashj@codeaurora.org; Chris Ball
> >>> Subject: Re: [PATCH V2] mmc: core: Add host capability check for
> >>> power
> >> class
> >>>
> >>> On 15 December 2011 16:22, Girish K S
> >>> <girish.shivananjappa@linaro.org>
> >>> wrote:
> >>> > On 15 December 2011 15:34, Saugata Das <saugata.das@linaro.org>
> wrote:
> >>> >> On 15 December 2011 09:28, Girish K S
> >>> >> <girish.shivananjappa@linaro.org>
> >>> wrote:
> >>> >>> This patch adds a check whether the host supports maximum
> >>> >>> current value obtained from the device's extended csd register
> >>> >>> for a selected interface voltage and frequency.
> >>> >>>
> >>> >>> cc: Chris Ball <cjb@laptop.org>
> >>> >>> Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
> >>> >>> ---
> >>> >>> Changes in v2:
> >>> >>>        deleted a unnecessary if else condition identified by
> >>> >>> subhash J Changes in v1:
> >>> >>>       reduced the number of comparisons as per Hein's suggestion
> >>> >>>
> >>> >>>  drivers/mmc/core/mmc.c   |   19 +++++++++++++++++++
> >>> >>>  include/linux/mmc/card.h |    4 ++++
> >>> >>>  2 files changed, 23 insertions(+), 0 deletions(-)
> >>> >>>
> >>> >>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> >>> >>> index
> >>> >>> 006e932..b9ef777 100644
> >>> >>> --- a/drivers/mmc/core/mmc.c
> >>> >>> +++ b/drivers/mmc/core/mmc.c
> >>> >>> @@ -688,6 +688,25 @@ static int mmc_select_powerclass(struct
> >>> >>> mmc_card *card,
> >>> >>>                pwrclass_val = (pwrclass_val &
> >>> >>> EXT_CSD_PWR_CL_4BIT_MASK) >>
> >>> >>>                                EXT_CSD_PWR_CL_4BIT_SHIFT;
> >>> >>>
> >>> >>> +       if (pwrclass_val >= MMC_MAX_CURRENT_800)
> >>> >>> +               pwrclass_val = MMC_MAX_CURRENT_800;
> >>> >>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_600)
> >>> >>> +               pwrclass_val = MMC_MAX_CURRENT_600;
> >>> >>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_400)
> >>> >>> +               pwrclass_val = MMC_MAX_CURRENT_400;
> >>> >>> +       else
> >>> >>> +               pwrclass_val = MMC_MAX_CURRENT_200;
> >>> >>> +
> >>> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_800) &&
> >>> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_800))
> >>> >>> +               pwrclass_val = MMC_MAX_CURRENT_600;
> >>> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_600) &&
> >>> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_600))
> >>> >>> +               pwrclass_val = MMC_MAX_CURRENT_400;
> >>> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_400) &&
> >>> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_400))
> >>> >>> +               pwrclass_val = MMC_MAX_CURRENT_200;
> >>> >>> +
> >>> >>>        /* If the power class is different from the default value
> >>> >>> */
> >>> >>>        if (pwrclass_val > 0) {
> >>> >>>                err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> >>> >>
> >>> >> It is not allowed to set the POWER_CLASS with any value other
> >>> >> than what is mentioned in the PWR_CL_ff_vvv or
> PWR_CL_DDR_ff_vvv
> >>> >> for
> >>> the
> >>> >> corresponding frequency, voltage. That is, if PWR_CL_200_195 is
> >>> >> 14 and we want to operate at HS200 then the only value allowed
> >>> >> for POWER_CLASS is 14. So, we need to check the PWR_CL numbers
> >>> >> and
> >>> choose
> >>> >> the operating mode (HS200/DDR50/..) based on the platform
> >>> >> capability to support the current consumption and set the
> >>> >> corresponding POWER_CLASS value.
> >>> >>
> >>> >> Please refer to section 6.6.5 of the 4.5 spec.
> >>> >
> >>> > The upstreamed code reads the extended csd value based on the
> >>> > already set voltage level and frequency of host. So it will get
> >>> > the required power class value which can be set directly. Is my
> >>> > understanding correct?
> >>> >
> >>>
> >>> It is not enough to just check the voltage level and frequency.
> >>> Consider this example, host has capability to support
> >>> MMC_CAP_MAX_CURRENT_400, the PWR_CL_DDR_52_360 has the
> value 9
> >>> (400mA) and PWR_CL_200_360 has the value 14 (800mA). Then even
> >>> though the host might be capable to run 200MHz clock and 3.6V, it
> >>> can only enable DDR at 52MHz and set 9 in POWER_CLASS.
> >>>
> >>> I think, in mmc_select_powerclass, we need to loop through the power
> >>> classes of all supported modes of transfer (HS200, DDR52, ... ) and
> >>> choose
> >> the
> >>> mode which gives maximum bandwidth but falls within host capability
> >>> of current consumption. Then set this to POWER_CLASS byte and also
> >>> use the same information when setting HS_TIMING in mmc_init_card.
> >>
> >> Hi Saugata,
> >>
> >> Does the spec mandates you to set the power class to what is needed
> >> by frequency/voltage combination? I can't see that mentioned anywhere
> >> explicitly in eMMC4.5 spec (if it is mentioned in spec, please let me
> >> know section and line number). It may be still possible to set the
> >> power class lower than what is needed by frequency/voltage
> >> combination. Say for example, 8-bit HS200 (200MHz) with high voltage
> >> cards may specify power class
> >> (PWR_CL_200_3_6) value of 14 (800 mA) but that doesn't mean if you
> >> want the card to work in 8-bit HS200 mode, its POWER_CLASS value must
> be 14 (800mA).
> >> If host's VDD regulator is only capable of say 600mA then it may
> >> still set the POWER_CLASS to 12 (600 mA) which should be the
> >> indication to card that host power supply is capable of sourcing only
> >> 600mA and I would assume card will make sure that it doesn't draw
> anything more than that.
> >>
> >> Hi Girish,
> >>
> >> I see couple of other issues with your original power_class selection
> patch.
> >> 1. mmc_select_powerclass() function considers 2.7v to 3.2v VDD range
> >> as invalid. That's not correct. Valid high voltage range is from 2.7v
to 3.6v.
> >> Is there a specific reason you have skipped the 2.7v to 3.2v VDD
> >> range? If not, I should post following change:
> >
> > can post a patch for this
> >
> >>
> >> --- a/drivers/mmc/core/mmc.c
> >> +++ b/drivers/mmc/core/mmc.c
> >> @@ -626,6 +626,11 @@ static int mmc_select_powerclass(struct
> mmc_card
> >> *card,
> >>                else if (host->ios.clock <= 200000000)
> >>                        index = EXT_CSD_PWR_CL_200_195;
> >>                break;
> >> +       case MMC_VDD_27_28:
> >> +       case MMC_VDD_28_29:
> >> +       case MMC_VDD_29_30:
> >> +       case MMC_VDD_30_31:
> >> +       case MMC_VDD_31_32:
> >>        case MMC_VDD_32_33:
> >>        case MMC_VDD_33_34:
> >>        case MMC_VDD_34_35:
> >>
> >> 2.  Currently in mmc_init_card(), power class selection is done if
> >> it's normal (DS or HS) or DDR or HS200 card.
> >>    If power class selection fails (mmc_select_powerclass() returns
> >> error) for DS/HS/DDR cards, we are just printing the error and going
> >> ahead with the rest of the initialization where as if power class
> >> selection fails for HS200 cards, we are simply aborting the
> >>    initialization and mark it as failed.
> >>
> >>   There are my points:
> >>       1.  Power class failure must be treated in same manner for
> >> DS/HS/DDR/HS200 cards
> > good find.
> > Yes will modify and repost for the HS200 case.
> >
> >>       2.  If you agree on first point then we need to decide whether
> >> power class selection failure is fatal enough to abort the MMC
> >> initialization? or we can just print the warning and go ahead with
> >> rest of the initialization in same bus speed mode?
> >
> > Should not be treated as fatal, should continue the initialization
> > with default power class
> Instead of making 2 different patches, can you add this modification in
your
> above patch. "you need to remove only the return err; statement"

No issues. I will do that.

> >
> >>
> >> Regards,
> >> Subhash
> >>
> >>>
> >>> >>
> >>> >>
> >>> >>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> >>> >>> index 9478a6b..c5e031a 100644
> >>> >>> --- a/include/linux/mmc/card.h
> >>> >>> +++ b/include/linux/mmc/card.h
> >>> >>> @@ -195,6 +195,10 @@ struct mmc_part {
> >>> >>>  #define MMC_BLK_DATA_AREA_GP   (1<<2)
> >>> >>>  };
> >>> >>>
> >>> >>> +#define MMC_MAX_CURRENT_200    (0) #define
> MMC_MAX_CURRENT_400
> >>> >>> +(7) #define MMC_MAX_CURRENT_600    (11) #define
> >>> MMC_MAX_CURRENT_800
> >>> >>> +(13)
> >>> >>>  /*
> >>> >>>  * MMC device
> >>> >>>  */
> >>> >>> --
> >>> >>> 1.7.1
> >>> >>>
> >>> >>> --
> >>> >>> To unsubscribe from this list: send the line "unsubscribe
linux-mmc"
> >>> >>> in the body of a message to majordomo@vger.kernel.org More
> >>> majordomo
> >>> >>> info at  http://vger.kernel.org/majordomo-info.html
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-mmc"
> >>> in
> >> the
> >>> body of a message to majordomo@vger.kernel.org More majordomo
> info
> >>> at http://vger.kernel.org/majordomo-info.html
> >>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] mmc: core: Add host capability check for power class

[Saugata Das]

On 28 March 2012 16:39, Subhash Jadavani <subhashj@codeaurora.org> wrote:
>
>
>> -----Original Message-----
>> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
>> owner@vger.kernel.org] On Behalf Of Saugata Das
>> Sent: Thursday, December 15, 2011 6:35 PM
>> To: Girish K S
>> Cc: linux-mmc@vger.kernel.org; patches@linaro.org; linux-samsung-
>> soc@vger.kernel.org; subhashj@codeaurora.org; Chris Ball
>> Subject: Re: [PATCH V2] mmc: core: Add host capability check for power
> class
>>
>> On 15 December 2011 16:22, Girish K S <girish.shivananjappa@linaro.org>
>> wrote:
>> > On 15 December 2011 15:34, Saugata Das <saugata.das@linaro.org> wrote:
>> >> On 15 December 2011 09:28, Girish K S <girish.shivananjappa@linaro.org>
>> wrote:
>> >>> This patch adds a check whether the host supports maximum current
>> >>> value obtained from the device's extended csd register for a
>> >>> selected interface voltage and frequency.
>> >>>
>> >>> cc: Chris Ball <cjb@laptop.org>
>> >>> Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
>> >>> ---
>> >>> Changes in v2:
>> >>>        deleted a unnecessary if else condition identified by subhash
>> >>> J Changes in v1:
>> >>>       reduced the number of comparisons as per Hein's suggestion
>> >>>
>> >>>  drivers/mmc/core/mmc.c   |   19 +++++++++++++++++++
>> >>>  include/linux/mmc/card.h |    4 ++++
>> >>>  2 files changed, 23 insertions(+), 0 deletions(-)
>> >>>
>> >>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index
>> >>> 006e932..b9ef777 100644
>> >>> --- a/drivers/mmc/core/mmc.c
>> >>> +++ b/drivers/mmc/core/mmc.c
>> >>> @@ -688,6 +688,25 @@ static int mmc_select_powerclass(struct
>> >>> mmc_card *card,
>> >>>                pwrclass_val = (pwrclass_val &
>> >>> EXT_CSD_PWR_CL_4BIT_MASK) >>
>> >>>                                EXT_CSD_PWR_CL_4BIT_SHIFT;
>> >>>
>> >>> +       if (pwrclass_val >= MMC_MAX_CURRENT_800)
>> >>> +               pwrclass_val = MMC_MAX_CURRENT_800;
>> >>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_600)
>> >>> +               pwrclass_val = MMC_MAX_CURRENT_600;
>> >>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_400)
>> >>> +               pwrclass_val = MMC_MAX_CURRENT_400;
>> >>> +       else
>> >>> +               pwrclass_val = MMC_MAX_CURRENT_200;
>> >>> +
>> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_800) &&
>> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_800))
>> >>> +               pwrclass_val = MMC_MAX_CURRENT_600;
>> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_600) &&
>> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_600))
>> >>> +               pwrclass_val = MMC_MAX_CURRENT_400;
>> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_400) &&
>> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_400))
>> >>> +               pwrclass_val = MMC_MAX_CURRENT_200;
>> >>> +
>> >>>        /* If the power class is different from the default value */
>> >>>        if (pwrclass_val > 0) {
>> >>>                err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> >>
>> >> It is not allowed to set the POWER_CLASS with any value other than
>> >> what is mentioned in the PWR_CL_ff_vvv or PWR_CL_DDR_ff_vvv  for
>> the
>> >> corresponding frequency, voltage. That is, if PWR_CL_200_195 is 14
>> >> and we want to operate at HS200 then the only value allowed for
>> >> POWER_CLASS is 14. So, we need to check the PWR_CL numbers and
>> choose
>> >> the operating mode (HS200/DDR50/..) based on the platform capability
>> >> to support the current consumption and set the corresponding
>> >> POWER_CLASS value.
>> >>
>> >> Please refer to section 6.6.5 of the 4.5 spec.
>> >
>> > The upstreamed code reads the extended csd value based on the already
>> > set voltage level and frequency of host. So it will get the required
>> > power class value which can be set directly. Is my understanding
>> > correct?
>> >
>>
>> It is not enough to just check the voltage level and frequency.
>> Consider this example, host has capability to support
>> MMC_CAP_MAX_CURRENT_400, the PWR_CL_DDR_52_360 has the value 9
>> (400mA) and PWR_CL_200_360 has the value 14 (800mA). Then even though
>> the host might be capable to run 200MHz clock and 3.6V, it can only enable
>> DDR at 52MHz and set 9 in POWER_CLASS.
>>
>> I think, in mmc_select_powerclass, we need to loop through the power
>> classes of all supported modes of transfer (HS200, DDR52, ... ) and choose
> the
>> mode which gives maximum bandwidth but falls within host capability of
>> current consumption. Then set this to POWER_CLASS byte and also use the
>> same information when setting HS_TIMING in mmc_init_card.
>
> Hi Saugata,
>
> Does the spec mandates you to set the power class to what is needed by
> frequency/voltage combination? I can't see that mentioned anywhere
> explicitly in eMMC4.5 spec (if it is mentioned in spec, please let me know
> section and line number). It may be still possible to set the power class
> lower than what is needed by frequency/voltage combination. Say for example,
> 8-bit HS200 (200MHz) with high voltage cards may specify power class
> (PWR_CL_200_3_6) value of 14 (800 mA) but that doesn't mean if you want the
> card to work in 8-bit HS200 mode, its POWER_CLASS value must be 14 (800mA).
> If host's VDD regulator is only capable of say 600mA then it may still set
> the POWER_CLASS to 12 (600 mA) which should be the indication to card that
> host power supply is capable of sourcing only 600mA and I would assume card
> will make sure that it doesn't draw anything more than that.

Please refer to section 6.6.5 of MMC-4.5. It says that the valid
values are defined in the PWR_CL_ff_vvv and PWR_CL_DDR_ff_vvv. We are
allowed to set only that value, depending on the mode, to POWER_CLASS.
Any other value will result in SWITCH_ERROR.


>
> Hi Girish,
>
> I see couple of other issues with your original power_class selection patch.
> 1. mmc_select_powerclass() function considers 2.7v to 3.2v VDD range as
> invalid. That's not correct. Valid high voltage range is from 2.7v to 3.6v.
> Is there a specific reason you have skipped the 2.7v to 3.2v VDD range? If
> not, I should post following change:
>
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -626,6 +626,11 @@ static int mmc_select_powerclass(struct mmc_card *card,
>                else if (host->ios.clock <= 200000000)
>                        index = EXT_CSD_PWR_CL_200_195;
>                break;
> +       case MMC_VDD_27_28:
> +       case MMC_VDD_28_29:
> +       case MMC_VDD_29_30:
> +       case MMC_VDD_30_31:
> +       case MMC_VDD_31_32:
>        case MMC_VDD_32_33:
>        case MMC_VDD_33_34:
>        case MMC_VDD_34_35:
>
> 2.  Currently in mmc_init_card(), power class selection is done if it's
> normal (DS or HS) or DDR or HS200 card.
>    If power class selection fails (mmc_select_powerclass() returns error)
> for DS/HS/DDR cards, we are just printing the error and going ahead with the
> rest of the initialization where as if power class selection fails for HS200
> cards, we are simply aborting the
>    initialization and mark it as failed.
>
>   There are my points:
>       1.  Power class failure must be treated in same manner for
> DS/HS/DDR/HS200 cards
>       2.  If you agree on first point then we need to decide whether power
> class selection failure is fatal enough to abort the MMC initialization? or
> we can just print the warning and go ahead with rest of the initialization
> in same bus speed mode?
>
> Regards,
> Subhash
>
>>
>> >>
>> >>
>> >>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> >>> index 9478a6b..c5e031a 100644
>> >>> --- a/include/linux/mmc/card.h
>> >>> +++ b/include/linux/mmc/card.h
>> >>> @@ -195,6 +195,10 @@ struct mmc_part {
>> >>>  #define MMC_BLK_DATA_AREA_GP   (1<<2)
>> >>>  };
>> >>>
>> >>> +#define MMC_MAX_CURRENT_200    (0)
>> >>> +#define MMC_MAX_CURRENT_400    (7)
>> >>> +#define MMC_MAX_CURRENT_600    (11) #define
>> MMC_MAX_CURRENT_800
>> >>> +(13)
>> >>>  /*
>> >>>  * MMC device
>> >>>  */
>> >>> --
>> >>> 1.7.1
>> >>>
>> >>> --
>> >>> To unsubscribe from this list: send the line "unsubscribe linux-mmc"
>> >>> in the body of a message to majordomo@vger.kernel.org More
>> majordomo
>> >>> info at  http://vger.kernel.org/majordomo-info.html
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the
>> body of a message to majordomo@vger.kernel.org More majordomo info at
>> http://vger.kernel.org/majordomo-info.html
>

RE: [PATCH V2] mmc: core: Add host capability check for power class

[Subhash Jadavani]

> -----Original Message-----
> From: Saugata Das [mailto:saugata.das@linaro.org]
> Sent: Monday, April 02, 2012 1:20 PM
> To: Subhash Jadavani
> Cc: Girish K S; linux-mmc@vger.kernel.org; patches@linaro.org; linux-
> samsung-soc@vger.kernel.org; Chris Ball
> Subject: Re: [PATCH V2] mmc: core: Add host capability check for power
class
> 
> On 28 March 2012 16:39, Subhash Jadavani <subhashj@codeaurora.org>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> >> owner@vger.kernel.org] On Behalf Of Saugata Das
> >> Sent: Thursday, December 15, 2011 6:35 PM
> >> To: Girish K S
> >> Cc: linux-mmc@vger.kernel.org; patches@linaro.org; linux-samsung-
> >> soc@vger.kernel.org; subhashj@codeaurora.org; Chris Ball
> >> Subject: Re: [PATCH V2] mmc: core: Add host capability check for
> >> power
> > class
> >>
> >> On 15 December 2011 16:22, Girish K S
> >> <girish.shivananjappa@linaro.org>
> >> wrote:
> >> > On 15 December 2011 15:34, Saugata Das <saugata.das@linaro.org>
> wrote:
> >> >> On 15 December 2011 09:28, Girish K S
> >> >> <girish.shivananjappa@linaro.org>
> >> wrote:
> >> >>> This patch adds a check whether the host supports maximum current
> >> >>> value obtained from the device's extended csd register for a
> >> >>> selected interface voltage and frequency.
> >> >>>
> >> >>> cc: Chris Ball <cjb@laptop.org>
> >> >>> Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
> >> >>> ---
> >> >>> Changes in v2:
> >> >>>        deleted a unnecessary if else condition identified by
> >> >>> subhash J Changes in v1:
> >> >>>       reduced the number of comparisons as per Hein's suggestion
> >> >>>
> >> >>>  drivers/mmc/core/mmc.c   |   19 +++++++++++++++++++
> >> >>>  include/linux/mmc/card.h |    4 ++++
> >> >>>  2 files changed, 23 insertions(+), 0 deletions(-)
> >> >>>
> >> >>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> >> >>> index
> >> >>> 006e932..b9ef777 100644
> >> >>> --- a/drivers/mmc/core/mmc.c
> >> >>> +++ b/drivers/mmc/core/mmc.c
> >> >>> @@ -688,6 +688,25 @@ static int mmc_select_powerclass(struct
> >> >>> mmc_card *card,
> >> >>>                pwrclass_val = (pwrclass_val &
> >> >>> EXT_CSD_PWR_CL_4BIT_MASK) >>
> >> >>>                                EXT_CSD_PWR_CL_4BIT_SHIFT;
> >> >>>
> >> >>> +       if (pwrclass_val >= MMC_MAX_CURRENT_800)
> >> >>> +               pwrclass_val = MMC_MAX_CURRENT_800;
> >> >>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_600)
> >> >>> +               pwrclass_val = MMC_MAX_CURRENT_600;
> >> >>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_400)
> >> >>> +               pwrclass_val = MMC_MAX_CURRENT_400;
> >> >>> +       else
> >> >>> +               pwrclass_val = MMC_MAX_CURRENT_200;
> >> >>> +
> >> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_800) &&
> >> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_800))
> >> >>> +               pwrclass_val = MMC_MAX_CURRENT_600;
> >> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_600) &&
> >> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_600))
> >> >>> +               pwrclass_val = MMC_MAX_CURRENT_400;
> >> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_400) &&
> >> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_400))
> >> >>> +               pwrclass_val = MMC_MAX_CURRENT_200;
> >> >>> +
> >> >>>        /* If the power class is different from the default value
> >> >>> */
> >> >>>        if (pwrclass_val > 0) {
> >> >>>                err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> >> >>
> >> >> It is not allowed to set the POWER_CLASS with any value other than
> >> >> what is mentioned in the PWR_CL_ff_vvv or PWR_CL_DDR_ff_vvv  for
> >> the
> >> >> corresponding frequency, voltage. That is, if PWR_CL_200_195 is 14
> >> >> and we want to operate at HS200 then the only value allowed for
> >> >> POWER_CLASS is 14. So, we need to check the PWR_CL numbers and
> >> choose
> >> >> the operating mode (HS200/DDR50/..) based on the platform
> >> >> capability to support the current consumption and set the
> >> >> corresponding POWER_CLASS value.
> >> >>
> >> >> Please refer to section 6.6.5 of the 4.5 spec.
> >> >
> >> > The upstreamed code reads the extended csd value based on the
> >> > already set voltage level and frequency of host. So it will get the
> >> > required power class value which can be set directly. Is my
> >> > understanding correct?
> >> >
> >>
> >> It is not enough to just check the voltage level and frequency.
> >> Consider this example, host has capability to support
> >> MMC_CAP_MAX_CURRENT_400, the PWR_CL_DDR_52_360 has the value
> 9
> >> (400mA) and PWR_CL_200_360 has the value 14 (800mA). Then even
> though
> >> the host might be capable to run 200MHz clock and 3.6V, it can only
> >> enable DDR at 52MHz and set 9 in POWER_CLASS.
> >>
> >> I think, in mmc_select_powerclass, we need to loop through the power
> >> classes of all supported modes of transfer (HS200, DDR52, ... ) and
> >> choose
> > the
> >> mode which gives maximum bandwidth but falls within host capability
> >> of current consumption. Then set this to POWER_CLASS byte and also
> >> use the same information when setting HS_TIMING in mmc_init_card.
> >
> > Hi Saugata,
> >
> > Does the spec mandates you to set the power class to what is needed by
> > frequency/voltage combination? I can't see that mentioned anywhere
> > explicitly in eMMC4.5 spec (if it is mentioned in spec, please let me
> > know section and line number). It may be still possible to set the
> > power class lower than what is needed by frequency/voltage
> > combination. Say for example, 8-bit HS200 (200MHz) with high voltage
> > cards may specify power class
> > (PWR_CL_200_3_6) value of 14 (800 mA) but that doesn't mean if you
> > want the card to work in 8-bit HS200 mode, its POWER_CLASS value must
> be 14 (800mA).
> > If host's VDD regulator is only capable of say 600mA then it may still
> > set the POWER_CLASS to 12 (600 mA) which should be the indication to
> > card that host power supply is capable of sourcing only 600mA and I
> > would assume card will make sure that it doesn't draw anything more than
> that.
> 
> Please refer to section 6.6.5 of MMC-4.5. It says that the valid values
are
> defined in the PWR_CL_ff_vvv and PWR_CL_DDR_ff_vvv. We are allowed to
> set only that value, depending on the mode, to POWER_CLASS.
> Any other value will result in SWITCH_ERROR.

Thanks for the details. Just curious, do you have any cards where you have
seen such SWITCH_ERROR while setting the power class?

> 
> 
> >
> > Hi Girish,
> >
> > I see couple of other issues with your original power_class selection
patch.
> > 1. mmc_select_powerclass() function considers 2.7v to 3.2v VDD range
> > as invalid. That's not correct. Valid high voltage range is from 2.7v to
3.6v.
> > Is there a specific reason you have skipped the 2.7v to 3.2v VDD
> > range? If not, I should post following change:
> >
> > --- a/drivers/mmc/core/mmc.c
> > +++ b/drivers/mmc/core/mmc.c
> > @@ -626,6 +626,11 @@ static int mmc_select_powerclass(struct mmc_card
> > *card,
> >                else if (host->ios.clock <= 200000000)
> >                        index = EXT_CSD_PWR_CL_200_195;
> >                break;
> > +       case MMC_VDD_27_28:
> > +       case MMC_VDD_28_29:
> > +       case MMC_VDD_29_30:
> > +       case MMC_VDD_30_31:
> > +       case MMC_VDD_31_32:
> >        case MMC_VDD_32_33:
> >        case MMC_VDD_33_34:
> >        case MMC_VDD_34_35:
> >
> > 2.  Currently in mmc_init_card(), power class selection is done if
> > it's normal (DS or HS) or DDR or HS200 card.
> >    If power class selection fails (mmc_select_powerclass() returns
> > error) for DS/HS/DDR cards, we are just printing the error and going
> > ahead with the rest of the initialization where as if power class
> > selection fails for HS200 cards, we are simply aborting the
> >    initialization and mark it as failed.
> >
> >   There are my points:
> >       1.  Power class failure must be treated in same manner for
> > DS/HS/DDR/HS200 cards
> >       2.  If you agree on first point then we need to decide whether
> > power class selection failure is fatal enough to abort the MMC
> > initialization? or we can just print the warning and go ahead with
> > rest of the initialization in same bus speed mode?
> >
> > Regards,
> > Subhash
> >
> >>
> >> >>
> >> >>
> >> >>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> >> >>> index 9478a6b..c5e031a 100644
> >> >>> --- a/include/linux/mmc/card.h
> >> >>> +++ b/include/linux/mmc/card.h
> >> >>> @@ -195,6 +195,10 @@ struct mmc_part {
> >> >>>  #define MMC_BLK_DATA_AREA_GP   (1<<2)
> >> >>>  };
> >> >>>
> >> >>> +#define MMC_MAX_CURRENT_200    (0) #define
> MMC_MAX_CURRENT_400
> >> >>> +(7) #define MMC_MAX_CURRENT_600    (11) #define
> >> MMC_MAX_CURRENT_800
> >> >>> +(13)
> >> >>>  /*
> >> >>>  * MMC device
> >> >>>  */
> >> >>> --
> >> >>> 1.7.1
> >> >>>
> >> >>> --
> >> >>> To unsubscribe from this list: send the line "unsubscribe
linux-mmc"
> >> >>> in the body of a message to majordomo@vger.kernel.org More
> >> majordomo
> >> >>> info at  http://vger.kernel.org/majordomo-info.html
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-mmc"
> >> in
> > the
> >> body of a message to majordomo@vger.kernel.org More majordomo info
> at
> >> http://vger.kernel.org/majordomo-info.html
> >

Re: [PATCH V2] mmc: core: Add host capability check for power class

[Saugata Das]

On 2 April 2012 16:24, Subhash Jadavani <subhashj@codeaurora.org> wrote:
>
>
>> -----Original Message-----
>> From: Saugata Das [mailto:saugata.das@linaro.org]
>> Sent: Monday, April 02, 2012 1:20 PM
>> To: Subhash Jadavani
>> Cc: Girish K S; linux-mmc@vger.kernel.org; patches@linaro.org; linux-
>> samsung-soc@vger.kernel.org; Chris Ball
>> Subject: Re: [PATCH V2] mmc: core: Add host capability check for power
> class
>>
>> On 28 March 2012 16:39, Subhash Jadavani <subhashj@codeaurora.org>
>> wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
>> >> owner@vger.kernel.org] On Behalf Of Saugata Das
>> >> Sent: Thursday, December 15, 2011 6:35 PM
>> >> To: Girish K S
>> >> Cc: linux-mmc@vger.kernel.org; patches@linaro.org; linux-samsung-
>> >> soc@vger.kernel.org; subhashj@codeaurora.org; Chris Ball
>> >> Subject: Re: [PATCH V2] mmc: core: Add host capability check for
>> >> power
>> > class
>> >>
>> >> On 15 December 2011 16:22, Girish K S
>> >> <girish.shivananjappa@linaro.org>
>> >> wrote:
>> >> > On 15 December 2011 15:34, Saugata Das <saugata.das@linaro.org>
>> wrote:
>> >> >> On 15 December 2011 09:28, Girish K S
>> >> >> <girish.shivananjappa@linaro.org>
>> >> wrote:
>> >> >>> This patch adds a check whether the host supports maximum current
>> >> >>> value obtained from the device's extended csd register for a
>> >> >>> selected interface voltage and frequency.
>> >> >>>
>> >> >>> cc: Chris Ball <cjb@laptop.org>
>> >> >>> Signed-off-by: Girish K S <girish.shivananjappa@linaro.org>
>> >> >>> ---
>> >> >>> Changes in v2:
>> >> >>>        deleted a unnecessary if else condition identified by
>> >> >>> subhash J Changes in v1:
>> >> >>>       reduced the number of comparisons as per Hein's suggestion
>> >> >>>
>> >> >>>  drivers/mmc/core/mmc.c   |   19 +++++++++++++++++++
>> >> >>>  include/linux/mmc/card.h |    4 ++++
>> >> >>>  2 files changed, 23 insertions(+), 0 deletions(-)
>> >> >>>
>> >> >>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> >> >>> index
>> >> >>> 006e932..b9ef777 100644
>> >> >>> --- a/drivers/mmc/core/mmc.c
>> >> >>> +++ b/drivers/mmc/core/mmc.c
>> >> >>> @@ -688,6 +688,25 @@ static int mmc_select_powerclass(struct
>> >> >>> mmc_card *card,
>> >> >>>                pwrclass_val = (pwrclass_val &
>> >> >>> EXT_CSD_PWR_CL_4BIT_MASK) >>
>> >> >>>                                EXT_CSD_PWR_CL_4BIT_SHIFT;
>> >> >>>
>> >> >>> +       if (pwrclass_val >= MMC_MAX_CURRENT_800)
>> >> >>> +               pwrclass_val = MMC_MAX_CURRENT_800;
>> >> >>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_600)
>> >> >>> +               pwrclass_val = MMC_MAX_CURRENT_600;
>> >> >>> +       else if (pwrclass_val >= MMC_MAX_CURRENT_400)
>> >> >>> +               pwrclass_val = MMC_MAX_CURRENT_400;
>> >> >>> +       else
>> >> >>> +               pwrclass_val = MMC_MAX_CURRENT_200;
>> >> >>> +
>> >> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_800) &&
>> >> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_800))
>> >> >>> +               pwrclass_val = MMC_MAX_CURRENT_600;
>> >> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_600) &&
>> >> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_600))
>> >> >>> +               pwrclass_val = MMC_MAX_CURRENT_400;
>> >> >>> +       if ((pwrclass_val == MMC_MAX_CURRENT_400) &&
>> >> >>> +           !(card->host->caps & MMC_CAP_MAX_CURRENT_400))
>> >> >>> +               pwrclass_val = MMC_MAX_CURRENT_200;
>> >> >>> +
>> >> >>>        /* If the power class is different from the default value
>> >> >>> */
>> >> >>>        if (pwrclass_val > 0) {
>> >> >>>                err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> >> >>
>> >> >> It is not allowed to set the POWER_CLASS with any value other than
>> >> >> what is mentioned in the PWR_CL_ff_vvv or PWR_CL_DDR_ff_vvv  for
>> >> the
>> >> >> corresponding frequency, voltage. That is, if PWR_CL_200_195 is 14
>> >> >> and we want to operate at HS200 then the only value allowed for
>> >> >> POWER_CLASS is 14. So, we need to check the PWR_CL numbers and
>> >> choose
>> >> >> the operating mode (HS200/DDR50/..) based on the platform
>> >> >> capability to support the current consumption and set the
>> >> >> corresponding POWER_CLASS value.
>> >> >>
>> >> >> Please refer to section 6.6.5 of the 4.5 spec.
>> >> >
>> >> > The upstreamed code reads the extended csd value based on the
>> >> > already set voltage level and frequency of host. So it will get the
>> >> > required power class value which can be set directly. Is my
>> >> > understanding correct?
>> >> >
>> >>
>> >> It is not enough to just check the voltage level and frequency.
>> >> Consider this example, host has capability to support
>> >> MMC_CAP_MAX_CURRENT_400, the PWR_CL_DDR_52_360 has the value
>> 9
>> >> (400mA) and PWR_CL_200_360 has the value 14 (800mA). Then even
>> though
>> >> the host might be capable to run 200MHz clock and 3.6V, it can only
>> >> enable DDR at 52MHz and set 9 in POWER_CLASS.
>> >>
>> >> I think, in mmc_select_powerclass, we need to loop through the power
>> >> classes of all supported modes of transfer (HS200, DDR52, ... ) and
>> >> choose
>> > the
>> >> mode which gives maximum bandwidth but falls within host capability
>> >> of current consumption. Then set this to POWER_CLASS byte and also
>> >> use the same information when setting HS_TIMING in mmc_init_card.
>> >
>> > Hi Saugata,
>> >
>> > Does the spec mandates you to set the power class to what is needed by
>> > frequency/voltage combination? I can't see that mentioned anywhere
>> > explicitly in eMMC4.5 spec (if it is mentioned in spec, please let me
>> > know section and line number). It may be still possible to set the
>> > power class lower than what is needed by frequency/voltage
>> > combination. Say for example, 8-bit HS200 (200MHz) with high voltage
>> > cards may specify power class
>> > (PWR_CL_200_3_6) value of 14 (800 mA) but that doesn't mean if you
>> > want the card to work in 8-bit HS200 mode, its POWER_CLASS value must
>> be 14 (800mA).
>> > If host's VDD regulator is only capable of say 600mA then it may still
>> > set the POWER_CLASS to 12 (600 mA) which should be the indication to
>> > card that host power supply is capable of sourcing only 600mA and I
>> > would assume card will make sure that it doesn't draw anything more than
>> that.
>>
>> Please refer to section 6.6.5 of MMC-4.5. It says that the valid values
> are
>> defined in the PWR_CL_ff_vvv and PWR_CL_DDR_ff_vvv. We are allowed to
>> set only that value, depending on the mode, to POWER_CLASS.
>> Any other value will result in SWITCH_ERROR.
>
> Thanks for the details. Just curious, do you have any cards where you have
> seen such SWITCH_ERROR while setting the power class?
>

Typically, the host design takes care of the maximum power budget
needed for the highest speed modes. So, I have not encountered such
issues so far.


>>
>>
>> >
>> > Hi Girish,
>> >
>> > I see couple of other issues with your original power_class selection
> patch.
>> > 1. mmc_select_powerclass() function considers 2.7v to 3.2v VDD range
>> > as invalid. That's not correct. Valid high voltage range is from 2.7v to
> 3.6v.
>> > Is there a specific reason you have skipped the 2.7v to 3.2v VDD
>> > range? If not, I should post following change:
>> >
>> > --- a/drivers/mmc/core/mmc.c
>> > +++ b/drivers/mmc/core/mmc.c
>> > @@ -626,6 +626,11 @@ static int mmc_select_powerclass(struct mmc_card
>> > *card,
>> >                else if (host->ios.clock <= 200000000)
>> >                        index = EXT_CSD_PWR_CL_200_195;
>> >                break;
>> > +       case MMC_VDD_27_28:
>> > +       case MMC_VDD_28_29:
>> > +       case MMC_VDD_29_30:
>> > +       case MMC_VDD_30_31:
>> > +       case MMC_VDD_31_32:
>> >        case MMC_VDD_32_33:
>> >        case MMC_VDD_33_34:
>> >        case MMC_VDD_34_35:
>> >
>> > 2.  Currently in mmc_init_card(), power class selection is done if
>> > it's normal (DS or HS) or DDR or HS200 card.
>> >    If power class selection fails (mmc_select_powerclass() returns
>> > error) for DS/HS/DDR cards, we are just printing the error and going
>> > ahead with the rest of the initialization where as if power class
>> > selection fails for HS200 cards, we are simply aborting the
>> >    initialization and mark it as failed.
>> >
>> >   There are my points:
>> >       1.  Power class failure must be treated in same manner for
>> > DS/HS/DDR/HS200 cards
>> >       2.  If you agree on first point then we need to decide whether
>> > power class selection failure is fatal enough to abort the MMC
>> > initialization? or we can just print the warning and go ahead with
>> > rest of the initialization in same bus speed mode?
>> >
>> > Regards,
>> > Subhash
>> >
>> >>
>> >> >>
>> >> >>
>> >> >>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> >> >>> index 9478a6b..c5e031a 100644
>> >> >>> --- a/include/linux/mmc/card.h
>> >> >>> +++ b/include/linux/mmc/card.h
>> >> >>> @@ -195,6 +195,10 @@ struct mmc_part {
>> >> >>>  #define MMC_BLK_DATA_AREA_GP   (1<<2)
>> >> >>>  };
>> >> >>>
>> >> >>> +#define MMC_MAX_CURRENT_200    (0) #define
>> MMC_MAX_CURRENT_400
>> >> >>> +(7) #define MMC_MAX_CURRENT_600    (11) #define
>> >> MMC_MAX_CURRENT_800
>> >> >>> +(13)
>> >> >>>  /*
>> >> >>>  * MMC device
>> >> >>>  */
>> >> >>> --
>> >> >>> 1.7.1
>> >> >>>
>> >> >>> --
>> >> >>> To unsubscribe from this list: send the line "unsubscribe
> linux-mmc"
>> >> >>> in the body of a message to majordomo@vger.kernel.org More
>> >> majordomo
>> >> >>> info at  http://vger.kernel.org/majordomo-info.html
>> >> --
>> >> To unsubscribe from this list: send the line "unsubscribe linux-mmc"
>> >> in
>> > the
>> >> body of a message to majordomo@vger.kernel.org More majordomo info
>> at
>> >> http://vger.kernel.org/majordomo-info.html
>> >
>