Patchwork [3/3,v2] mmc: Add ESDHC weird voltage bits workaround

login
register
mail settings
Submitter Zang Roy-R61911
Date July 30, 2010, 3:52 a.m.
Message ID <1280461977-2023-1-git-send-email-tie-fei.zang@freescale.com>
Download mbox | patch
Permalink /patch/60332/
State Not Applicable
Headers show

Comments

Zang Roy-R61911 - July 30, 2010, 3:52 a.m.
P4080 ESDHC controller does not support 1.8V and 3.0V voltage. but the
host controller capabilities register wrongly set the bits.
This patch adds the workaround to correct the weird voltage setting bits.

Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
---
This is the second version of patch 
http://patchwork.ozlabs.org/patch/60106/
According to the comment, remove some un-necessary setting.

Together with patch
http://patchwork.ozlabs.org/patch/60111/
http://patchwork.ozlabs.org/patch/60116/

This serial patches add mmc support for p4080 silicon

 drivers/mmc/host/sdhci-of-core.c |    4 ++++
 drivers/mmc/host/sdhci.c         |    8 ++++++++
 drivers/mmc/host/sdhci.h         |    4 ++++
 3 files changed, 16 insertions(+), 0 deletions(-)
Anton Vorontsov - July 30, 2010, 7:06 a.m.
On Fri, Jul 30, 2010 at 11:52:57AM +0800, Roy Zang wrote:
> P4080 ESDHC controller does not support 1.8V and 3.0V voltage. but the
> host controller capabilities register wrongly set the bits.
> This patch adds the workaround to correct the weird voltage setting bits.
> 
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> ---
[...]
> diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c
> index 0c30242..1f3913d 100644
> --- a/drivers/mmc/host/sdhci-of-core.c
> +++ b/drivers/mmc/host/sdhci-of-core.c
> @@ -164,6 +164,10 @@ static int __devinit sdhci_of_probe(struct of_device *ofdev,
>  	if (sdhci_of_wp_inverted(np))
>  		host->quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
>  
> +	if (of_device_is_compatible(np, "fsl,p4080-esdhc"))
> +		host->quirks |= (SDHCI_QUIRK_QORIQ_NO_VDD_180
> +				|SDHCI_QUIRK_QORIQ_NO_VDD_300);
> +

It should be two properties, something like sdhci,no-vdd-180
and sdhci,no-vdd-300. But it might be even better: we have
voltage-ranges for mmc-spi case, see
Documentation/powerpc/dts-bindings/mmc-spi-slot.txt.

If voltage-ranges specified, then we use it, not capabilities
register.

For p4080 it will be 'voltage-ranges = <3200 3400>;'. So, with
voltage-ranges we can do fine grained VDD control without
introducing anything new.

As for implementation, you might just factor out voltage-ranges
parsing from drivers/mmc/host/of_mmc_spi.c, and then in sdhci
driver you could do.

if (host->ocr_avail)
	mmc->ocr_avail = host->ocr_avail.

>  	clk = of_get_property(np, "clock-frequency", &size);
>  	if (clk && size == sizeof(*clk) && *clk)
>  		of_host->clock = *clk;
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 1424d08..a667790 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1699,6 +1699,14 @@ int sdhci_add_host(struct sdhci_host *host)
>  
>  	caps = sdhci_readl(host, SDHCI_CAPABILITIES);
>  
> +	 /* Workaround for P4080 host controller capabilities
> +	  * 1.8V and 3.0V do not supported*/
> +	if (host->quirks & SDHCI_QUIRK_QORIQ_NO_VDD_180)

The point of making NO_VDD stuff is to make these quirks
"chip-agnostic". Ideally, sdhci.c should never know about
particular chips.

So, you shouldn't name quirks with QORIQ.
Zang Roy-R61911 - Aug. 2, 2010, 6:19 a.m.
> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru@gmail.com] 
> Sent: Friday, July 30, 2010 15:06 PM
> To: Zang Roy-R61911
> Cc: linux-mmc@vger.kernel.org; linuxppc-dev@ozlabs.org; 
> akpm@linux-foundation.org
> Subject: Re: [PATCH 3/3 v2] mmc: Add ESDHC weird voltage bits 
> workaround
> 
> On Fri, Jul 30, 2010 at 11:52:57AM +0800, Roy Zang wrote:
> > P4080 ESDHC controller does not support 1.8V and 3.0V 
> voltage. but the
> > host controller capabilities register wrongly set the bits.
> > This patch adds the workaround to correct the weird voltage 
> setting bits.
> > 
> > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> > ---
> [...]
> > diff --git a/drivers/mmc/host/sdhci-of-core.c 
> b/drivers/mmc/host/sdhci-of-core.c
> > index 0c30242..1f3913d 100644
> > --- a/drivers/mmc/host/sdhci-of-core.c
> > +++ b/drivers/mmc/host/sdhci-of-core.c
> > @@ -164,6 +164,10 @@ static int __devinit 
> sdhci_of_probe(struct of_device *ofdev,
> >  	if (sdhci_of_wp_inverted(np))
> >  		host->quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
> >  
> > +	if (of_device_is_compatible(np, "fsl,p4080-esdhc"))
> > +		host->quirks |= (SDHCI_QUIRK_QORIQ_NO_VDD_180
> > +				|SDHCI_QUIRK_QORIQ_NO_VDD_300);
> > +
> 
> It should be two properties, something like sdhci,no-vdd-180
> and sdhci,no-vdd-300. But it might be even better: we have
> voltage-ranges for mmc-spi case, see
> Documentation/powerpc/dts-bindings/mmc-spi-slot.txt.
> 
> If voltage-ranges specified, then we use it, not capabilities
> register.
> 
> For p4080 it will be 'voltage-ranges = <3200 3400>;'. So, with
> voltage-ranges we can do fine grained VDD control without
> introducing anything new.
why not
               voltage-ranges = <3300 3300>;
?
Roy
Anton Vorontsov - Aug. 2, 2010, 6:52 a.m.
On Mon, Aug 02, 2010 at 02:19:58PM +0800, Zang Roy-R61911 wrote:
[...]
> > For p4080 it will be 'voltage-ranges = <3200 3400>;'. So, with
> > voltage-ranges we can do fine grained VDD control without
> > introducing anything new.
> why not
>                voltage-ranges = <3300 3300>;

Right you are, both will be 3300.

Thanks,

Patch

diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c
index 0c30242..1f3913d 100644
--- a/drivers/mmc/host/sdhci-of-core.c
+++ b/drivers/mmc/host/sdhci-of-core.c
@@ -164,6 +164,10 @@  static int __devinit sdhci_of_probe(struct of_device *ofdev,
 	if (sdhci_of_wp_inverted(np))
 		host->quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
 
+	if (of_device_is_compatible(np, "fsl,p4080-esdhc"))
+		host->quirks |= (SDHCI_QUIRK_QORIQ_NO_VDD_180
+				|SDHCI_QUIRK_QORIQ_NO_VDD_300);
+
 	clk = of_get_property(np, "clock-frequency", &size);
 	if (clk && size == sizeof(*clk) && *clk)
 		of_host->clock = *clk;
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 1424d08..a667790 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1699,6 +1699,14 @@  int sdhci_add_host(struct sdhci_host *host)
 
 	caps = sdhci_readl(host, SDHCI_CAPABILITIES);
 
+	 /* Workaround for P4080 host controller capabilities
+	  * 1.8V and 3.0V do not supported*/
+	if (host->quirks & SDHCI_QUIRK_QORIQ_NO_VDD_180)
+		caps &= ~SDHCI_CAN_VDD_180;
+
+	if (host->quirks & SDHCI_QUIRK_QORIQ_NO_VDD_300)
+		caps &= ~SDHCI_CAN_VDD_300;
+
 	if (host->quirks & SDHCI_QUIRK_FORCE_DMA)
 		host->flags |= SDHCI_USE_SDMA;
 	else if (!(caps & SDHCI_CAN_DO_SDMA))
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index aa112aa..389b58c 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -243,6 +243,10 @@  struct sdhci_host {
 #define SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC		(1<<26)
 /* Controller uses Auto CMD12 command to stop the transfer */
 #define SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12		(1<<27)
+/* Controller cannot support 1.8V */
+#define SDHCI_QUIRK_QORIQ_NO_VDD_180			(1<<28)
+/* Controller cannot support 3.0V */
+#define SDHCI_QUIRK_QORIQ_NO_VDD_300			(1<<29)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */