Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753515AbbBTACr (ORCPT ); Thu, 19 Feb 2015 19:02:47 -0500 Received: from pandora.arm.linux.org.uk ([78.32.30.218]:51412 "EHLO pandora.arm.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752018AbbBTACq (ORCPT ); Thu, 19 Feb 2015 19:02:46 -0500 Date: Fri, 20 Feb 2015 00:02:13 +0000 From: Russell King - ARM Linux To: Doug Anderson Cc: Alim Akhtar , Addy Ke , Tao Huang , lintao , Ulf Hansson , Heiko =?iso-8859-1?Q?St=FCbner?= , Andrzej Hajda , "linux-mmc@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Jaehoon Chung , Eddie Cai , Olof Johansson , "open list:ARM/Rockchip SoC..." , Javier Martinez Canillas , "linux-arm-kernel@lists.infradead.org" Subject: Re: [PATCH v4 1/3] mmc: dw_mmc: update clock after host reach a stable voltage Message-ID: <20150220000213.GD8656@n2100.arm.linux.org.uk> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3349 Lines: 84 On Thu, Feb 19, 2015 at 03:49:46PM -0800, 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: > > /* > * 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. This is exactly why I've been saying that we need to get away from the POWER_UP vs POWER_ON stuff. We instead need a _single_ call into drivers to tell them to apply power and bring the card to a state where it can be talked to. That includes waiting for the power to stabilise, and sending the initial clocks to allow the card to initialise. In the case of extra GPIOs, host drivers will need to call back into the MMC core at the point where they have stabilised the power. The current situation where we split the power-up/power-on sequence between the host drivers and the core is really a hinderance to getting a working implementation - it's additional complexity where none is needed. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/