From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752121AbbBYHxS (ORCPT ); Wed, 25 Feb 2015 02:53:18 -0500 Received: from mail-qa0-f42.google.com ([209.85.216.42]:41586 "EHLO mail-qa0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751229AbbBYHxR (ORCPT ); Wed, 25 Feb 2015 02:53:17 -0500 MIME-Version: 1.0 In-Reply-To: References: <1423828368-18456-1-git-send-email-addy.ke@rock-chips.com> <1423894668-8886-1-git-send-email-addy.ke@rock-chips.com> <1423894668-8886-2-git-send-email-addy.ke@rock-chips.com> From: Alim Akhtar Date: Wed, 25 Feb 2015 13:22:36 +0530 Message-ID: Subject: Re: [PATCH v4 1/3] mmc: dw_mmc: update clock after host reach a stable voltage To: Doug Anderson Cc: Addy Ke , Jaehoon Chung , Ulf Hansson , Olof Johansson , Andrzej Hajda , =?UTF-8?Q?Heiko_St=C3=BCbner?= , Eddie Cai , lintao , Tao Huang , "linux-kernel@vger.kernel.org" , "linux-mmc@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "open list:ARM/Rockchip SoC..." , Javier Martinez Canillas Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Doug, On Fri, Feb 20, 2015 at 5:19 AM, Doug Anderson wrote: > Alim and Addy, > > On Sun, Feb 15, 2015 at 3:28 PM, Alim Akhtar wrote: >> Hi Addy, >> >> On Sat, Feb 14, 2015 at 11:47 AM, Addy Ke 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 >>> --- >>> 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 > . > > 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 >> --- >> 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