On 28 March 2012 16:39, Subhash Jadavani <subhashj@xxxxxxxxxxxxxx> wrote: > > >> -----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: 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@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-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
