> -----Original Message----- > From: Saugata Das [mailto:saugata.das@xxxxxxxxxx] > Sent: Tuesday, April 03, 2012 8:55 PM > To: Subhash Jadavani > Cc: linux-mmc@xxxxxxxxxxxxxxx; linux-arm-msm@xxxxxxxxxxxxxxx; > girish.shivananjappa@xxxxxxxxxx > Subject: Re: [PATCH v1 1/1] mmc: core: fix power class selection > > On 3 April 2012 12:25, Subhash Jadavani <subhashj@xxxxxxxxxxxxxx> wrote: > > mmc_select_powerclass() function returns error if eMMC VDD level > > supported by host is between 2.7v to 3.2v. > > > > According to eMMC specification, valid voltage for high voltage cards > > is 2.7v to 3.6v. This patch ensures that 2.7v to 3.6v VDD range is > > treated as valid range. > > > > Also, failure to set the power class shouldn't be treated as fatal > > error because even if setting the power class fails, card can still > > work in default power class. > > If mmc_select_powerclass() returns error, just print the warning > > message and go ahead with rest of the card initialization. > > > > Have you checked why mmc_select_powerclass returned error ? Today, in > the mmc_select_powerclass, it is just setting the value of power class, which > the eMMC expects. So, it should not fail. > > Another point is that, today mmc_select_powerclass is assuming that host > can support the maximum possible power classes and it does not check host > controllers capability in driving higher current (mA). But I think in future we > need to add this check for host controller ability and return error from > mmc_select_powerclass so that mmc_init_card can choose the next speed Agreed with this point. > mode. From that perspective, the approach to ignore the error return from > mmc_select_powerclass looks wrong. My patch was just intended to fix the issue existing power class implementation. As commit text says: - don't consider 2.7v to 3.2v as invalid range - We are already ignoring the error returned by mmc_set_power_class() function for DS/HS/DDR cards. So this patch has extended that to HS200 cards. So as far as this patch is concerned, I don't see anything wrong here. As you have mentioned, currently we are not taking the host controller capability into account (as it doesn't exist as of now) so we should not see mmc_select_powerclass() fail any time. But yes, next thing should be to take into account the host current sourcing capability before deciding which bus speed mode to choose. But that may be a big change and should be as separate patch. Girish had already posted one patch for this which needs to be extended to achieve this. Regards, Subhash > > > > Signed-off-by: Subhash Jadavani <subhashj@xxxxxxxxxxxxxx> > > --- > > drivers/mmc/core/mmc.c | 30 +++++++++++++++++------------- > > 1 files changed, 17 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index > > 02914d6..54df5ad 100644 > > --- a/drivers/mmc/core/mmc.c > > +++ b/drivers/mmc/core/mmc.c > > @@ -695,6 +695,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: > > @@ -1111,11 +1116,10 @@ static int mmc_init_card(struct mmc_host > > *host, u32 ocr, > > ext_csd_bits = (bus_width == MMC_BUS_WIDTH_8) ? > > EXT_CSD_BUS_WIDTH_8 : > > EXT_CSD_BUS_WIDTH_4; > > err = mmc_select_powerclass(card, ext_csd_bits, > > ext_csd); > > - if (err) { > > - pr_err("%s: power class selection to bus width > > %d failed\n", > > - mmc_hostname(card->host), 1 << > > bus_width); > > - goto err; > > - } > > + if (err) > > + pr_warning("%s: power class selection to bus width %d" > > + " failed\n", > > + mmc_hostname(card->host), > > + 1 << bus_width); > > } > > > > /* > > @@ -1147,10 +1151,10 @@ static int mmc_init_card(struct mmc_host > > *host, u32 ocr, > > err = mmc_select_powerclass(card, > > ext_csd_bits[idx][0], > > ext_csd); > > if (err) > > - pr_err("%s: power class selection to " > > - "bus width %d failed\n", > > - mmc_hostname(card->host), > > - 1 << bus_width); > > + pr_warning("%s: power class selection to " > > + "bus width %d failed\n", > > + mmc_hostname(card->host), > > + 1 << bus_width); > > > > err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, > > EXT_CSD_BUS_WIDTH, @@ -1178,10 > > +1182,10 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, > > err = mmc_select_powerclass(card, > > ext_csd_bits[idx][1], > > ext_csd); > > if (err) > > - pr_err("%s: power class selection to " > > - "bus width %d ddr %d failed\n", > > - mmc_hostname(card->host), > > - 1 << bus_width, ddr); > > + pr_warning("%s: power class selection to " > > + "bus width %d ddr %d > > + failed\n", > > + mmc_hostname(card->host), > > + 1 << bus_width, ddr); > > > > err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, > > EXT_CSD_BUS_WIDTH, > > -- > > 1.7.1.1 > > > > -- > > Sent by a consultant of the Qualcomm Innovation Center, Inc. > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora > Forum. > > -- > > 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
