[PATCH V3] mmc: Enable the ADMA2 on esdhc imx driver

[PATCH V3] mmc: Enable the ADMA2 on esdhc imx driver

[Richard Zhu]

Eanble the ADMA2 mode on freescale esdhc imx driver,
tested on MX25, MX51 and MX53.

Only ADMA2 mode is enabled, MX25/35 can't support the ADMA2 mode.
So this patch is only used to enable the ADMA2 for MX51/53 platforms.

The ADMA2 mode supported or not can be distinguished by the
bit20 of Capability Register(offset 0x40) and bit9-8 of
HOST PROTOCOL Register(offset 0x28) in FSL eSDHC module.

BTW:Here are the definition of the Bit9~8 DMAS of HOST PROTOCOL
Reg(offset 0x28). The bit9 couldn't be set to 1 when the SOC can't
support ADMA2. This bit is used to make a double check that the ADMA2
is supported or not in this patch, since the bit20 of Capability Reg
is broken on SOCs.
DMAS definitions:
00: No DMA or Simple DMA is selected
01: ADMA1 is selected
10: ADMA2 is selected
11: reserved

Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
---
 drivers/mmc/host/sdhci-esdhc-imx.c |   45 ++++++++++++++++++++++++++++++++++-
 1 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index a19967d..46092c7 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -31,6 +31,14 @@
 #define SDHCI_VENDOR_SPEC		0xC0
 #define  SDHCI_VENDOR_SPEC_SDIO_QUIRK	0x00000002
 
+/*
+ * There is INT DMA ERR mis-match between eSDHC and STD SDHC SPEC
+ * Bit25 is used in STD SPEC, and is reserved in fsl eSDHC design,
+ * but bit28 is used as the INT DMA ERR in fsl eSDHC design.
+ * Define this macro DMA error INT for fsl eSDHC
+ */
+#define  SDHCI_INT_VENDOR_SPEC_DMA_ERR	0x10000000
+
 #define ESDHC_FLAG_GPIO_FOR_CD_WP	(1 << 0)
 /*
  * The CMDTYPE of the CMD register (offset 0xE) should be set to
@@ -62,6 +70,7 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct pltfm_imx_data *imx_data = pltfm_host->priv;
+	u32 dma_mode;
 
 	/* fake CARD_PRESENT flag on mx25/35 */
 	u32 val = readl(host->ioaddr + reg);
@@ -80,6 +89,30 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
 			val |= SDHCI_CARD_PRESENT;
 	}
 
+	if (unlikely(reg == SDHCI_CAPABILITIES)) {
+		/* In FSL esdhc IC module, only bit20 is used to indicate the
+		 * ADMA2 capability of esdhc, but this bit is messed up on some
+		 * SOCs (e.x MX25, this bit is set, but it can't support the
+		 * ADMA2 actually). So readout HOST_CONTROl register to make a
+		 * double check that the ADMA2 is supported or not.
+		 */
+		dma_mode = readl(host->ioaddr + SDHCI_HOST_CONTROL) >> 5;
+		dma_mode &= SDHCI_CTRL_DMA_MASK;
+
+		if ((val & SDHCI_CAN_DO_ADMA1)
+				&& (dma_mode > SDHCI_CTRL_ADMA1)) {
+			val &= ~SDHCI_CAN_DO_ADMA1;
+			val |= SDHCI_CAN_DO_ADMA2;
+		}
+	}
+
+	if (unlikely(reg == SDHCI_INT_STATUS)) {
+		if (val & SDHCI_INT_VENDOR_SPEC_DMA_ERR) {
+			val &= ~SDHCI_INT_VENDOR_SPEC_DMA_ERR;
+			val |= SDHCI_INT_ADMA_ERROR;
+		}
+	}
+
 	return val;
 }
 
@@ -105,6 +138,13 @@ static void esdhc_writel_le(struct sdhci_host *host, u32 val, int reg)
 			writel(v, host->ioaddr + SDHCI_VENDOR_SPEC);
 	}
 
+	if (unlikely((reg == SDHCI_INT_ENABLE)
+				|| (reg == SDHCI_SIGNAL_ENABLE))) {
+		if (val & SDHCI_INT_ADMA_ERROR) {
+			val &= ~SDHCI_INT_ADMA_ERROR;
+			val |= SDHCI_INT_VENDOR_SPEC_DMA_ERR;
+		}
+	}
 	writel(val, host->ioaddr + reg);
 }
 
@@ -322,9 +362,10 @@ static void esdhc_pltfm_exit(struct sdhci_host *host)
 }
 
 struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
-	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA
+	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_NO_HISPD_BIT
+			| SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC
+			| SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC
 			| SDHCI_QUIRK_BROKEN_CARD_DETECTION,
-	/* ADMA has issues. Might be fixable */
 	.ops = &sdhci_esdhc_ops,
 	.init = esdhc_pltfm_init,
 	.exit = esdhc_pltfm_exit,
-- 
1.7.1

[PATCH V3] mmc: Enable the ADMA2 on esdhc imx driver

[Sascha Hauer]

On Tue, Jul 12, 2011 at 06:13:29PM +0800, Richard Zhu wrote:
> Eanble the ADMA2 mode on freescale esdhc imx driver,
> tested on MX25, MX51 and MX53.
> 
> Only ADMA2 mode is enabled, MX25/35 can't support the ADMA2 mode.
> So this patch is only used to enable the ADMA2 for MX51/53 platforms.
> 
> The ADMA2 mode supported or not can be distinguished by the
> bit20 of Capability Register(offset 0x40) and bit9-8 of
> HOST PROTOCOL Register(offset 0x28) in FSL eSDHC module.
> 
> BTW:Here are the definition of the Bit9~8 DMAS of HOST PROTOCOL
> Reg(offset 0x28). The bit9 couldn't be set to 1 when the SOC can't
> support ADMA2. This bit is used to make a double check that the ADMA2
> is supported or not in this patch, since the bit20 of Capability Reg
> is broken on SOCs.
> DMAS definitions:
> 00: No DMA or Simple DMA is selected
> 01: ADMA1 is selected
> 10: ADMA2 is selected
> 11: reserved
> 
> Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c |   45 ++++++++++++++++++++++++++++++++++-
>  1 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index a19967d..46092c7 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -31,6 +31,14 @@
>  #define SDHCI_VENDOR_SPEC		0xC0
>  #define  SDHCI_VENDOR_SPEC_SDIO_QUIRK	0x00000002
>  
> +/*
> + * There is INT DMA ERR mis-match between eSDHC and STD SDHC SPEC
> + * Bit25 is used in STD SPEC, and is reserved in fsl eSDHC design,
> + * but bit28 is used as the INT DMA ERR in fsl eSDHC design.
> + * Define this macro DMA error INT for fsl eSDHC
> + */
> +#define  SDHCI_INT_VENDOR_SPEC_DMA_ERR	0x10000000
> +
>  #define ESDHC_FLAG_GPIO_FOR_CD_WP	(1 << 0)
>  /*
>   * The CMDTYPE of the CMD register (offset 0xE) should be set to
> @@ -62,6 +70,7 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct pltfm_imx_data *imx_data = pltfm_host->priv;
> +	u32 dma_mode;
>  
>  	/* fake CARD_PRESENT flag on mx25/35 */
>  	u32 val = readl(host->ioaddr + reg);
> @@ -80,6 +89,30 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
>  			val |= SDHCI_CARD_PRESENT;
>  	}
>  
> +	if (unlikely(reg == SDHCI_CAPABILITIES)) {
> +		/* In FSL esdhc IC module, only bit20 is used to indicate the
> +		 * ADMA2 capability of esdhc, but this bit is messed up on some
> +		 * SOCs (e.x MX25, this bit is set, but it can't support the
> +		 * ADMA2 actually). So readout HOST_CONTROl register to make a
> +		 * double check that the ADMA2 is supported or not.
> +		 */
> +		dma_mode = readl(host->ioaddr + SDHCI_HOST_CONTROL) >> 5;
> +		dma_mode &= SDHCI_CTRL_DMA_MASK;
> +
> +		if ((val & SDHCI_CAN_DO_ADMA1)
> +				&& (dma_mode > SDHCI_CTRL_ADMA1)) {
> +			val &= ~SDHCI_CAN_DO_ADMA1;
> +			val |= SDHCI_CAN_DO_ADMA2;
> +		}
> +	}
> +
> +	if (unlikely(reg == SDHCI_INT_STATUS)) {
> +		if (val & SDHCI_INT_VENDOR_SPEC_DMA_ERR) {
> +			val &= ~SDHCI_INT_VENDOR_SPEC_DMA_ERR;
> +			val |= SDHCI_INT_ADMA_ERROR;
> +		}
> +	}
> +

Honestly, putting all kinds of driver logic into the register access
functions will lead to a catastrophe sooner or later. There are too
many quirks in it already, we should not add more of them.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH V3] mmc: Enable the ADMA2 on esdhc imx driver

[Anton Vorontsov]

On Tue, Jul 12, 2011 at 03:46:02PM +0200, Sascha Hauer wrote:
[...]
> Honestly, putting all kinds of driver logic into the register access
> functions will lead to a catastrophe sooner or later. There are too
> many quirks in it already, we should not add more of them.

There aren't many options. You may pollute generic driver with
quirks (which is not an option :-), or you can introduce more
ops for things like sdhci_send_command(), but in that case
you will duplicate the logic just to compensate minor register
differences.

In some cases, e.g. capabilities register, it seems that introducing
get_caps() op would be a logical step, but then you just move the code
under 'if (reg == SDHCI_CAPABILITIES) { ' into a dedicated function.
No big difference.

As for me, I don't see any catastrophe coming because of the register
access fixups. Quite the contrary: from the maintenance stand point,
you just need to know how the generic SDHCI works, and then you can
look into platform drivers to see their differences, which are mostly
minor.

Thanks,

-- 
Anton Vorontsov
Email: cbouatmailru at gmail.com

[PATCH V3] mmc: Enable the ADMA2 on esdhc imx driver

[Pavel Machek]

On Tue 2011-07-12 18:18:43, Anton Vorontsov wrote:
> On Tue, Jul 12, 2011 at 03:46:02PM +0200, Sascha Hauer wrote:
> [...]
> > Honestly, putting all kinds of driver logic into the register access
> > functions will lead to a catastrophe sooner or later. There are too
> > many quirks in it already, we should not add more of them.
> 
> There aren't many options. You may pollute generic driver with
> quirks (which is not an option :-), or you can introduce more
> ops for things like sdhci_send_command(), but in that case
> you will duplicate the logic just to compensate minor register
> differences.
> 
> In some cases, e.g. capabilities register, it seems that introducing
> get_caps() op would be a logical step, but then you just move the code
> under 'if (reg == SDHCI_CAPABILITIES) { ' into a dedicated function.
> No big difference.

Actually, there is huge difference. Generic code does
read-modify-write on the hw registers. And people only implement fixup
in write, for example :-(.
								Pavel