diff mbox

[21/38] mmc: sdhci: hack up driver to make it more compliant with UHS-1

Message ID 20140425130849.GB15179@pengutronix.de (mailing list archive)
State New, archived
Headers show

Commit Message

Markus Pargmann April 25, 2014, 1:08 p.m. UTC
On Fri, Apr 25, 2014 at 01:49:45PM +0100, Russell King - ARM Linux wrote:
> On Fri, Apr 25, 2014 at 02:38:20PM +0200, Markus Pargmann wrote:
> > Hi,
> > 
> > On Wed, Apr 23, 2014 at 08:07:57PM +0100, Russell King wrote:
> > > Patch suggested by Dong Aisheng <dongas86@gmail.com>, this avoids
> > > additional clock start/stop cycles during the transition to 1.8V
> > > signalling mode.
> > > 
> > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > 
> > I tested the series on imx6s with a RIoT board. With this patch applied
> > the RIoT board emmc does not work. Here is the output of the board:
> 
> Unfortunately, I don't have any emmc sdhci using devices, so this is
> a combination I can't test myself.
> 
> What would be useful is to find out which of the two changes in there
> is the cause - can you try with just the change to
> sdhci_do_start_signal_voltage_switch applied, iow just this change:

I just tried the different parts of the patch. Without the following
change, emmc works:

Markus

Comments

Russell King - ARM Linux April 25, 2014, 1:15 p.m. UTC | #1
On Fri, Apr 25, 2014 at 03:08:49PM +0200, Markus Pargmann wrote:
> I just tried the different parts of the patch. Without the following
> change, emmc works:
> 
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1507,12 +1507,6 @@ static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
>  			host->ops->set_clock(host, host->clock);
>  		}
>  
> -
> -		/* Reset SD Clock Enable */
> -		clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> -		clk &= ~SDHCI_CLOCK_CARD_EN;
> -		sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> -

So the bit which stops us violating the SD spec stops eMMC from working...
that's... just great.

Okay, I'll look deeper at this and see what can be done, but I suspect it
will turn out to require more patches and be more invasive.
Russell King - ARM Linux April 25, 2014, 1:22 p.m. UTC | #2
On Fri, Apr 25, 2014 at 02:15:30PM +0100, Russell King - ARM Linux wrote:
> On Fri, Apr 25, 2014 at 03:08:49PM +0200, Markus Pargmann wrote:
> > I just tried the different parts of the patch. Without the following
> > change, emmc works:
> > 
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -1507,12 +1507,6 @@ static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
> >  			host->ops->set_clock(host, host->clock);
> >  		}
> >  
> > -
> > -		/* Reset SD Clock Enable */
> > -		clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> > -		clk &= ~SDHCI_CLOCK_CARD_EN;
> > -		sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> > -
> 
> So the bit which stops us violating the SD spec stops eMMC from working...
> that's... just great.
> 
> Okay, I'll look deeper at this and see what can be done, but I suspect it
> will turn out to require more patches and be more invasive.

In the mean time, here's the remainder (to patch 33) of the series with
this commit omitted.
Russell King - ARM Linux April 25, 2014, 4:20 p.m. UTC | #3
On Fri, Apr 25, 2014 at 02:15:30PM +0100, Russell King - ARM Linux wrote:
> On Fri, Apr 25, 2014 at 03:08:49PM +0200, Markus Pargmann wrote:
> > I just tried the different parts of the patch. Without the following
> > change, emmc works:
> > 
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -1507,12 +1507,6 @@ static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
> >  			host->ops->set_clock(host, host->clock);
> >  		}
> >  
> > -
> > -		/* Reset SD Clock Enable */
> > -		clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> > -		clk &= ~SDHCI_CLOCK_CARD_EN;
> > -		sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> > -
> 
> So the bit which stops us violating the SD spec stops eMMC from working...
> that's... just great.
> 
> Okay, I'll look deeper at this and see what can be done, but I suspect it
> will turn out to require more patches and be more invasive.

Well, I've given up trying to work out how to clean that mess up -
sdhci will attempt to do all the high speed switching bollocks every
time a set_ios call is made, turning the clock on and off multiple
times in the process.

I've since replaced it with a patch covering just the last hunk of
this patch and killed the other two changes there (and rewritten the
description.)

I'm afraid that it looks like we're going to be stuck with turning the
clock on and off multiple times during the transition to 1.8V signalling,
which is technically against the SD spec.  The spec'd procedure is:

- send command
- check card reports busy
- clock off
- switch to 1.8v signalling
- wait 5ms
- clock on
- wait 1ms
- check card reports non-busy

whereas what we end up doing is:

- send command
- check card reports busy
- clock off
- switch to 1.8v signalling, waiting 5 to 5.5ms
- wait 5ms
- clock on
- clock off
- tweak other settings
- clock on again
- wait 1ms
- check card reports non-busy
Markus Pargmann April 28, 2014, 1:10 p.m. UTC | #4
Hi,

On Fri, Apr 25, 2014 at 02:22:45PM +0100, Russell King - ARM Linux wrote:
> On Fri, Apr 25, 2014 at 02:15:30PM +0100, Russell King - ARM Linux wrote:
> > On Fri, Apr 25, 2014 at 03:08:49PM +0200, Markus Pargmann wrote:
> > > I just tried the different parts of the patch. Without the following
> > > change, emmc works:
> > > 
> > > --- a/drivers/mmc/host/sdhci.c
> > > +++ b/drivers/mmc/host/sdhci.c
> > > @@ -1507,12 +1507,6 @@ static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
> > >  			host->ops->set_clock(host, host->clock);
> > >  		}
> > >  
> > > -
> > > -		/* Reset SD Clock Enable */
> > > -		clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> > > -		clk &= ~SDHCI_CLOCK_CARD_EN;
> > > -		sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> > > -
> > 
> > So the bit which stops us violating the SD spec stops eMMC from working...
> > that's... just great.
> > 
> > Okay, I'll look deeper at this and see what can be done, but I suspect it
> > will turn out to require more patches and be more invasive.
> 
> In the mean time, here's the remainder (to patch 33) of the series with
> this commit omitted.

I tested the whole series again, from 1 to patch 32 and it is working
correctly now on the RIoT board (imx6s) with emmc and sd-card. You can
add my tested-by tag for all core and imx related patches if you want to.

Regards,

Markus
diff mbox

Patch

--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1507,12 +1507,6 @@  static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
 			host->ops->set_clock(host, host->clock);
 		}
 
-
-		/* Reset SD Clock Enable */
-		clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
-		clk &= ~SDHCI_CLOCK_CARD_EN;
-		sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
-

Regards,