Hi Doug, On Fri, Feb 20, 2015 at 5:19 AM, Doug Anderson <dianders@xxxxxxxxxxxx> wrote: > Alim and Addy, > > On Sun, Feb 15, 2015 at 3:28 PM, Alim Akhtar <alim.akhtar@xxxxxxxxx> wrote: >> Hi Addy, >> >> On Sat, Feb 14, 2015 at 11:47 AM, Addy Ke <addy.ke@xxxxxxxxxxxxxx> wrote: >>> As show in mmc_power_up(), in MMC_POWER_UP state, the voltage isn't >>> stable and we may get 'data busy' which can't be cleaned by resetting >>> all blocks. So we should not send command to update clock in this state. >>> >>> Signed-off-by: Addy Ke <addy.ke@xxxxxxxxxxxxxx> >>> --- >>> drivers/mmc/host/dw_mmc.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c >>> index 4d2e3c2..3472f9b 100644 >>> --- a/drivers/mmc/host/dw_mmc.c >>> +++ b/drivers/mmc/host/dw_mmc.c >>> @@ -1102,7 +1102,8 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) >>> drv_data->set_ios(slot->host, ios); >>> >>> /* Slot specific timing and width adjustment */ >>> - dw_mci_setup_bus(slot, false); >>> + if (ios->power_mode != MMC_POWER_UP) >>> + dw_mci_setup_bus(slot, false); >>> >> This looks a HACK to me. >> If stabilizing host voltage regulator is the problem, can you try out >> below patch, and see if this resolve your issue? > > Actually, IMHO Alim's patch is more of a hack than Addy's. There's > already a 10ms delay between "power up" and "power on" in the MMC core > in mmc_power_up() state. That delay is commented as: > Well, my suggestion (adding 5ms in switch_volatge) was based on DW_MMC databook (V2.41a) section "7.4.1.2 Voltage switch Normal Scenario" step #7 which says:" After the 5ms timer expires, the host voltage regulator is stable". PS: I have limited to no access of my mails and workstation until March 9th, so replies will be slow. > /* > * This delay should be sufficient to allow the power supply > * to reach the minimum voltage. > */ > mmc_delay(10); > > That means that assuming that the voltage is stable in MMC_POWER_UP is > not valid anyway. > > > Addy's patch certainly needs more comments. In another context Olof suggested: > > /* > * Skip bus setup while voltage is still stabilizing. Instead, > * bus setup will be done at MMC_POWER_ON. > */ > > > ...thinking about this more, though: really the voltage should be > stabilized when the regulator call returns (see my comments below). > In actuality the "right" fix might be to just rearrange this function > a little not to turn the clock on _before_ we even try to turn the > voltage on. > > I've got that coded up but I'm still testing it... If you want to try > it too, you can find it at > <https://chromium-review.googlesource.com/251341>. > > Note that without my patch I find that I _really_ need Addy's patch to > make sure that the card isn't busy in setup_bus. With my patch Addy's > code catches the card busy less often. I'm still trying to see if > there's a way to totally remove the need for his setup_bus and still > trying to grok all the patches flying around... > > >> =========== >> [PATCH] mmc: dw_mmc: Wait for host voltage regulator to be stable >> >> Signed-off-by: Alim Akhtar <alim.akhtar@xxxxxxxxxxx> >> --- >> drivers/mmc/host/dw_mmc.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c >> index 4d2e3c2..dc10fbb 100644 >> --- a/drivers/mmc/host/dw_mmc.c >> +++ b/drivers/mmc/host/dw_mmc.c >> @@ -1202,6 +1202,9 @@ static int dw_mci_switch_voltage(struct mmc_host >> *mmc, struct mmc_ios *ios) >> } >> mci_writel(host, UHS_REG, uhs); >> >> + /* wait for 5ms so that host voltage regulator is stable */ >> + usleep_range(5000, 5500); >> + > > Alim: if you have some other instance where you actually need VQMMC to > stabilize it should probably be done in a different way. If I > understand correctly it is the regulator core's job to make sure that > voltage is stable before returning. If you have a gpio-regulator you > may be able to use "startup-delay-us" to specify how long the > regulator takes to come up. You could also look at > 'regulator-enable-ramp-delay' > > -Doug -- Regards, Alim -- 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
