Re: [PATCH v4 1/3] mmc: dw_mmc: update clock after host reach a stable voltage

[Russell King - ARM Linux]

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 <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:
> 
>   /*
>    * 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/