Jaehoon, On Mon, Jun 30, 2014 at 5:13 AM, Jaehoon Chung <jh80.chung@xxxxxxxxxxx> wrote: > On 06/27/2014 01:18 AM, Doug Anderson wrote: >> Yuvaraj, >> >> On Thu, Jun 26, 2014 at 4:21 AM, Yuvaraj Kumar <yuvaraj.cd@xxxxxxxxx> wrote: >>> Doug >>> >>> On Tue, Jun 24, 2014 at 11:30 PM, Doug Anderson <dianders@xxxxxxxxxxxx> wrote: >>>> Yuvaraj, >>>> >>>> On Mon, Jun 23, 2014 at 3:45 AM, Yuvaraj Kumar C D <yuvaraj.cd@xxxxxxxxx> wrote: >>>>> This patch makes use of mmc_regulator_get_supply() to handle >>>>> the vmmc and vqmmc regulators.Also it moves the code handling >>>>> the these regulators to dw_mci_set_ios().It turned on the vmmc >>>>> and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off >>>>> during MMC_POWER_OFF. >>>>> >>>>> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@xxxxxxxxxxx> >>>>> --- >>>>> drivers/mmc/host/dw_mmc.c | 71 ++++++++++++++++++++++----------------------- >>>>> drivers/mmc/host/dw_mmc.h | 2 ++ >>>>> 2 files changed, 36 insertions(+), 37 deletions(-) >>>> >>>> Perhaps you could CC me on the whole series for the next version since >>>> I was involved in privately reviewing previous versions? >>> It was just accidental missing you in the CC .Surely i will add you in >>> CC for next versions. >>>> >>>> Overall caveat for my review is that I'm nowhere near an SD/MMC expert. >>>> >>>> >>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c >>>>> index 1ac227c..f5cabce 100644 >>>>> --- a/drivers/mmc/host/dw_mmc.c >>>>> +++ b/drivers/mmc/host/dw_mmc.c >>>>> @@ -937,6 +937,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) >>>>> struct dw_mci_slot *slot = mmc_priv(mmc); >>>>> const struct dw_mci_drv_data *drv_data = slot->host->drv_data; >>>>> u32 regs; >>>>> + int ret; >>>>> >>>>> switch (ios->bus_width) { >>>>> case MMC_BUS_WIDTH_4: >>>>> @@ -975,16 +976,41 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) >>>>> >>>>> switch (ios->power_mode) { >>>>> case MMC_POWER_UP: >>>>> + if ((!IS_ERR(mmc->supply.vmmc)) && >>>>> + !test_bit(DW_MMC_CARD_POWERED, &slot->flags)) { >>>>> + ret = regulator_enable(mmc->supply.vmmc); >>>>> + if (!ret) >>>>> + set_bit(DW_MMC_CARD_POWERED, &slot->flags); >>>>> + } >>>> >>>> As per below, I'm not sure why you'd want to turn on vqmmc and vmmc at >>>> different times. >>> As you can see people's have different opinion on this.When i had a >>> look at the other drivers in the subsystem which does in the same flow >>> as above.However i will change in the next version. >> >> Given my self proclaimed lack of SD/MMC knowledge, if others have a >> good reason for doing them separate then you should do it that way. >> So far I haven't heard that reason but I certainly could be wrong. > > At first time, i had believed nothing problem that it turns on vmmc and vqmmc at different time. > It could have the potential problem. OK. As I said I'm nowhere near an expert on SD/MMC, so if there's something out there that says that turning vmmc on before vqmmc is the right thing to do then I guess that's the answer. I would still be very curious to get more details on what the potential problem is. Could you provide a reference to documentation that says vmmc should be on before vqmmc or describe a situation where it's important? Thanks! -Doug -- 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