> -----Original Message----- > From: linux-mmc-owner@xxxxxxxxxxxxxxx [mailto:linux-mmc- > owner@xxxxxxxxxxxxxxx] On Behalf Of Saugata Das > Sent: Thursday, December 15, 2011 6:35 PM > To: Girish K S > Cc: linux-mmc@xxxxxxxxxxxxxxx; patches@xxxxxxxxxx; linux-samsung- > soc@xxxxxxxxxxxxxxx; subhashj@xxxxxxxxxxxxxx; 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@xxxxxxxxxx> > wrote: > > On 15 December 2011 15:34, Saugata Das <saugata.das@xxxxxxxxxx> wrote: > >> On 15 December 2011 09:28, Girish K S <girish.shivananjappa@xxxxxxxxxx> > 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@xxxxxxxxxx> > >>> Signed-off-by: Girish K S <girish.shivananjappa@xxxxxxxxxx> > >>> --- > >>> 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@xxxxxxxxxxxxxxx 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@xxxxxxxxxxxxxxx More majordomo info at > http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |