diff mbox

[3/8] sdhci: sdhci-esdhc-imx: support real clock on and off for imx6q

Message ID 1378299257-2980-4-git-send-email-b29396@freescale.com (mailing list archive)
State New, archived
Headers show

Commit Message

Aisheng Dong Sept. 4, 2013, 12:54 p.m. UTC
The signal voltage switch follow requires to shutdown and output
clock in a specific sequence according to standard host controller
v3.0 spec. In that timing, the card must really receive clock or not.

However, for i.MX6Q, the uSDHC will not output clock even the clock
is enabled until there is command or data in transfer on the bus,
which will then cause singal voltage switch always to fail.

For i.MX6Q, we clear ESDHC_VENDOR_SPEC_FRC_SDCLK_ON bit to let
controller to gate off clock automatically and set that bit
to force clock output if clock is on.

This is required by SD3.0 support.

Signed-off-by: Dong Aisheng <b29396@freescale.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c |    3 ---
 drivers/mmc/host/sdhci-esdhc.h     |   29 ++++++++++++++++++++++++++---
 2 files changed, 26 insertions(+), 6 deletions(-)

Comments

Shawn Guo Sept. 5, 2013, 4:32 a.m. UTC | #1
On Wed, Sep 04, 2013 at 08:54:12PM +0800, Dong Aisheng wrote:
> The signal voltage switch follow requires to shutdown and output

s/follow/flow

> clock in a specific sequence according to standard host controller
> v3.0 spec. In that timing, the card must really receive clock or not.
> 
> However, for i.MX6Q, the uSDHC will not output clock even the clock
> is enabled until there is command or data in transfer on the bus,
> which will then cause singal voltage switch always to fail.
> 
> For i.MX6Q, we clear ESDHC_VENDOR_SPEC_FRC_SDCLK_ON bit to let
> controller to gate off clock automatically and set that bit
> to force clock output if clock is on.
> 
> This is required by SD3.0 support.
> 
> Signed-off-by: Dong Aisheng <b29396@freescale.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c |    3 ---
>  drivers/mmc/host/sdhci-esdhc.h     |   29 ++++++++++++++++++++++++++---
>  2 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 1dd5ba8..3118a82 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -31,9 +31,6 @@
>  #include "sdhci-esdhc.h"
>  
>  #define	ESDHC_CTRL_D3CD			0x08
> -/* VENDOR SPEC register */
> -#define ESDHC_VENDOR_SPEC		0xc0
> -#define  ESDHC_VENDOR_SPEC_SDIO_QUIRK	(1 << 1)
>  #define ESDHC_WTMK_LVL			0x44
>  #define ESDHC_MIX_CTRL			0x48
>  #define  ESDHC_MIX_CTRL_AC23EN		(1 << 7)
> diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
> index a2a0642..86fcd5b 100644
> --- a/drivers/mmc/host/sdhci-esdhc.h
> +++ b/drivers/mmc/host/sdhci-esdhc.h
> @@ -14,6 +14,8 @@
>  #ifndef _DRIVERS_MMC_SDHCI_ESDHC_H
>  #define _DRIVERS_MMC_SDHCI_ESDHC_H
>  
> +#include <linux/of.h>
> +
>  /*
>   * Ops and quirks for the Freescale eSDHC controller.
>   */
> @@ -33,6 +35,12 @@
>  #define ESDHC_CLOCK_HCKEN	0x00000002
>  #define ESDHC_CLOCK_IPGEN	0x00000001
>  
> +/* VENDOR SPEC register */
> +#define ESDHC_VENDOR_SPEC		0xc0
> +#define  ESDHC_VENDOR_SPEC_SDIO_QUIRK	(1 << 1)
> +#define  ESDHC_VENDOR_SPEC_VSELECT	(1 << 1)

It would be better to introduce the macro only when it's actually being
used.

> +#define  ESDHC_VENDOR_SPEC_FRC_SDCLK_ON (1 << 8)
> +
>  /* pltfm-specific */
>  #define ESDHC_HOST_CONTROL_LE	0x20
>  
> @@ -52,12 +60,20 @@
>  static inline void esdhc_set_clock(struct sdhci_host *host, unsigned int clock,
>  				   unsigned int host_clock)
>  {
> +	struct device *dev;
>  	int pre_div = 2;
>  	int div = 1;
> -	u32 temp;
> -
> -	if (clock == 0)
> +	u32 temp, val;
> +
> +	dev = mmc_dev(host->mmc);
> +	if (clock == 0) {
> +		if (of_device_is_compatible(dev->of_node, "fsl,imx6q-usdhc")) {
> +			val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> +			writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> +				host->ioaddr + ESDHC_VENDOR_SPEC);
> +		}

To me, maintaining this esdhc_set_clock() for both imx and powerpc makes
no sense now.  I think it might be the time to have different versions
of the function for imx and powerpc, just in esdhc_pltfm_set_clock() and
esdhc_of_set_clock() respectively.

Shawn

>  		goto out;
> +	}
>  
>  	temp = sdhci_readl(host, ESDHC_SYSTEM_CONTROL);
>  	temp &= ~(ESDHC_CLOCK_IPGEN | ESDHC_CLOCK_HCKEN | ESDHC_CLOCK_PEREN
> @@ -81,6 +97,13 @@ static inline void esdhc_set_clock(struct sdhci_host *host, unsigned int clock,
>  		| (div << ESDHC_DIVIDER_SHIFT)
>  		| (pre_div << ESDHC_PREDIV_SHIFT));
>  	sdhci_writel(host, temp, ESDHC_SYSTEM_CONTROL);
> +
> +	if (of_device_is_compatible(dev->of_node, "fsl,imx6q-usdhc")) {
> +		val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> +		writel(val | ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> +			host->ioaddr + ESDHC_VENDOR_SPEC);
> +	}
> +
>  	mdelay(1);
>  out:
>  	host->clock = clock;
> -- 
> 1.7.1
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dong Aisheng Sept. 5, 2013, 2:59 p.m. UTC | #2
On Thu, Sep 5, 2013 at 12:32 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
>
> On Wed, Sep 04, 2013 at 08:54:12PM +0800, Dong Aisheng wrote:
> > The signal voltage switch follow requires to shutdown and output
>
> s/follow/flow
>
> > clock in a specific sequence according to standard host controller
> > v3.0 spec. In that timing, the card must really receive clock or not.
> >
> > However, for i.MX6Q, the uSDHC will not output clock even the clock
> > is enabled until there is command or data in transfer on the bus,
> > which will then cause singal voltage switch always to fail.
> >
> > For i.MX6Q, we clear ESDHC_VENDOR_SPEC_FRC_SDCLK_ON bit to let
> > controller to gate off clock automatically and set that bit
> > to force clock output if clock is on.
> >
> > This is required by SD3.0 support.
> >
> > Signed-off-by: Dong Aisheng <b29396@freescale.com>
> > ---
> >  drivers/mmc/host/sdhci-esdhc-imx.c |    3 ---
> >  drivers/mmc/host/sdhci-esdhc.h     |   29 ++++++++++++++++++++++++++---
> >  2 files changed, 26 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> > index 1dd5ba8..3118a82 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -31,9 +31,6 @@
> >  #include "sdhci-esdhc.h"
> >
> >  #define      ESDHC_CTRL_D3CD                 0x08
> > -/* VENDOR SPEC register */
> > -#define ESDHC_VENDOR_SPEC            0xc0
> > -#define  ESDHC_VENDOR_SPEC_SDIO_QUIRK        (1 << 1)
> >  #define ESDHC_WTMK_LVL                       0x44
> >  #define ESDHC_MIX_CTRL                       0x48
> >  #define  ESDHC_MIX_CTRL_AC23EN               (1 << 7)
> > diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
> > index a2a0642..86fcd5b 100644
> > --- a/drivers/mmc/host/sdhci-esdhc.h
> > +++ b/drivers/mmc/host/sdhci-esdhc.h
> > @@ -14,6 +14,8 @@
> >  #ifndef _DRIVERS_MMC_SDHCI_ESDHC_H
> >  #define _DRIVERS_MMC_SDHCI_ESDHC_H
> >
> > +#include <linux/of.h>
> > +
> >  /*
> >   * Ops and quirks for the Freescale eSDHC controller.
> >   */
> > @@ -33,6 +35,12 @@
> >  #define ESDHC_CLOCK_HCKEN    0x00000002
> >  #define ESDHC_CLOCK_IPGEN    0x00000001
> >
> > +/* VENDOR SPEC register */
> > +#define ESDHC_VENDOR_SPEC            0xc0
> > +#define  ESDHC_VENDOR_SPEC_SDIO_QUIRK        (1 << 1)
> > +#define  ESDHC_VENDOR_SPEC_VSELECT   (1 << 1)
>
> It would be better to introduce the macro only when it's actually being
> used.
>

Correct.

> > +#define  ESDHC_VENDOR_SPEC_FRC_SDCLK_ON (1 << 8)
> > +
> >  /* pltfm-specific */
> >  #define ESDHC_HOST_CONTROL_LE        0x20
> >
> > @@ -52,12 +60,20 @@
> >  static inline void esdhc_set_clock(struct sdhci_host *host, unsigned int clock,
> >                                  unsigned int host_clock)
> >  {
> > +     struct device *dev;
> >       int pre_div = 2;
> >       int div = 1;
> > -     u32 temp;
> > -
> > -     if (clock == 0)
> > +     u32 temp, val;
> > +
> > +     dev = mmc_dev(host->mmc);
> > +     if (clock == 0) {
> > +             if (of_device_is_compatible(dev->of_node, "fsl,imx6q-usdhc")) {
> > +                     val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> > +                     writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> > +                             host->ioaddr + ESDHC_VENDOR_SPEC);
> > +             }
>
> To me, maintaining this esdhc_set_clock() for both imx and powerpc makes
> no sense now.  I think it might be the time to have different versions
> of the function for imx and powerpc, just in esdhc_pltfm_set_clock() and
> esdhc_of_set_clock() respectively.

I agree with you.
Since later i will need add a few more imx6q specific things in this
common headfile
which seems not good.
So it would be better to end this dependency ASAP.
I could move that code into esdhc_pltfm_set_clock in next version
if no objections from others people.

Regards
Dong Aisheng

>
> Shawn
>
> >               goto out;
> > +     }
> >
> >       temp = sdhci_readl(host, ESDHC_SYSTEM_CONTROL);
> >       temp &= ~(ESDHC_CLOCK_IPGEN | ESDHC_CLOCK_HCKEN | ESDHC_CLOCK_PEREN
> > @@ -81,6 +97,13 @@ static inline void esdhc_set_clock(struct sdhci_host *host, unsigned int clock,
> >               | (div << ESDHC_DIVIDER_SHIFT)
> >               | (pre_div << ESDHC_PREDIV_SHIFT));
> >       sdhci_writel(host, temp, ESDHC_SYSTEM_CONTROL);
> > +
> > +     if (of_device_is_compatible(dev->of_node, "fsl,imx6q-usdhc")) {
> > +             val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
> > +             writel(val | ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
> > +                     host->ioaddr + ESDHC_VENDOR_SPEC);
> > +     }
> > +
> >       mdelay(1);
> >  out:
> >       host->clock = clock;
> > --
> > 1.7.1
> >
> >
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 1dd5ba8..3118a82 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -31,9 +31,6 @@ 
 #include "sdhci-esdhc.h"
 
 #define	ESDHC_CTRL_D3CD			0x08
-/* VENDOR SPEC register */
-#define ESDHC_VENDOR_SPEC		0xc0
-#define  ESDHC_VENDOR_SPEC_SDIO_QUIRK	(1 << 1)
 #define ESDHC_WTMK_LVL			0x44
 #define ESDHC_MIX_CTRL			0x48
 #define  ESDHC_MIX_CTRL_AC23EN		(1 << 7)
diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
index a2a0642..86fcd5b 100644
--- a/drivers/mmc/host/sdhci-esdhc.h
+++ b/drivers/mmc/host/sdhci-esdhc.h
@@ -14,6 +14,8 @@ 
 #ifndef _DRIVERS_MMC_SDHCI_ESDHC_H
 #define _DRIVERS_MMC_SDHCI_ESDHC_H
 
+#include <linux/of.h>
+
 /*
  * Ops and quirks for the Freescale eSDHC controller.
  */
@@ -33,6 +35,12 @@ 
 #define ESDHC_CLOCK_HCKEN	0x00000002
 #define ESDHC_CLOCK_IPGEN	0x00000001
 
+/* VENDOR SPEC register */
+#define ESDHC_VENDOR_SPEC		0xc0
+#define  ESDHC_VENDOR_SPEC_SDIO_QUIRK	(1 << 1)
+#define  ESDHC_VENDOR_SPEC_VSELECT	(1 << 1)
+#define  ESDHC_VENDOR_SPEC_FRC_SDCLK_ON (1 << 8)
+
 /* pltfm-specific */
 #define ESDHC_HOST_CONTROL_LE	0x20
 
@@ -52,12 +60,20 @@ 
 static inline void esdhc_set_clock(struct sdhci_host *host, unsigned int clock,
 				   unsigned int host_clock)
 {
+	struct device *dev;
 	int pre_div = 2;
 	int div = 1;
-	u32 temp;
-
-	if (clock == 0)
+	u32 temp, val;
+
+	dev = mmc_dev(host->mmc);
+	if (clock == 0) {
+		if (of_device_is_compatible(dev->of_node, "fsl,imx6q-usdhc")) {
+			val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
+			writel(val & ~ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
+				host->ioaddr + ESDHC_VENDOR_SPEC);
+		}
 		goto out;
+	}
 
 	temp = sdhci_readl(host, ESDHC_SYSTEM_CONTROL);
 	temp &= ~(ESDHC_CLOCK_IPGEN | ESDHC_CLOCK_HCKEN | ESDHC_CLOCK_PEREN
@@ -81,6 +97,13 @@  static inline void esdhc_set_clock(struct sdhci_host *host, unsigned int clock,
 		| (div << ESDHC_DIVIDER_SHIFT)
 		| (pre_div << ESDHC_PREDIV_SHIFT));
 	sdhci_writel(host, temp, ESDHC_SYSTEM_CONTROL);
+
+	if (of_device_is_compatible(dev->of_node, "fsl,imx6q-usdhc")) {
+		val = readl(host->ioaddr + ESDHC_VENDOR_SPEC);
+		writel(val | ESDHC_VENDOR_SPEC_FRC_SDCLK_ON,
+			host->ioaddr + ESDHC_VENDOR_SPEC);
+	}
+
 	mdelay(1);
 out:
 	host->clock = clock;