[PATCH V1] mmc: Enable the ADMA on esdhc imx driver

[PATCH V1] mmc: Enable the ADMA on esdhc imx driver

[Richard Zhu]

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

Only ADMA2 mode is enabled, MX25/35 can't support the ADMA2 mode.
So this patch is only used for MX51/53.
The ADMA2 mode supported or not can be distinguished by the
Capability Register(offset 0x40) of eSDHC module.
Up to now, only MX51/MX53 set the ADMA2 supported bit(Bit20) in the
Capability Register.

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

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index a19967d..8015e1a 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -31,6 +31,8 @@
 #define SDHCI_VENDOR_SPEC		0xC0
 #define  SDHCI_VENDOR_SPEC_SDIO_QUIRK	0x00000002
 
+#define  SDHCI_SPEC_INT_ADMA_ERR	0x10000000
+
 #define ESDHC_FLAG_GPIO_FOR_CD_WP	(1 << 0)
 /*
  * The CMDTYPE of the CMD register (offset 0xE) should be set to
@@ -80,6 +82,20 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
 			val |= SDHCI_CARD_PRESENT;
 	}
 
+	if (unlikely(reg == SDHCI_CAPABILITIES)) {
+		if (val & SDHCI_CAN_DO_ADMA1) {
+			val &= ~SDHCI_CAN_DO_ADMA1;
+			val |= SDHCI_CAN_DO_ADMA2;
+		}
+	}
+
+	if (unlikely(reg == SDHCI_INT_STATUS)) {
+		if (val & SDHCI_SPEC_INT_ADMA_ERR) {
+			val &= ~SDHCI_SPEC_INT_ADMA_ERR;
+			val |= SDHCI_INT_ADMA_ERROR;
+		}
+	}
+
 	return val;
 }
 
@@ -105,6 +121,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_SPEC_INT_ADMA_ERR;
+		}
+	}
 	writel(val, host->ioaddr + reg);
 }
 
@@ -154,7 +177,7 @@ static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg)
 
 static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
 {
-	u32 new_val;
+	u16 new_val;
 
 	switch (reg) {
 	case SDHCI_POWER_CONTROL:
@@ -168,10 +191,12 @@ static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
 		new_val = val & (SDHCI_CTRL_LED | SDHCI_CTRL_4BITBUS);
 		/* ensure the endianess */
 		new_val |= ESDHC_HOST_CONTROL_LE;
-		/* DMA mode bits are shifted */
-		new_val |= (val & SDHCI_CTRL_DMA_MASK) << 5;
+		if (val & SDHCI_CTRL_DMA_MASK)
+			/* DMA mode bits are shifted */
+			new_val |= (val & SDHCI_CTRL_DMA_MASK) << 5;
 
 		esdhc_clrset_le(host, 0xffff, new_val, reg);
+
 		return;
 	}
 	esdhc_clrset_le(host, 0xff, val, reg);
@@ -322,9 +347,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 V1] mmc: Enable the ADMA on esdhc imx driver

[Richard Zhu]

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

Only ADMA2 mode is enabled, MX25/35 can't support the ADMA2 mode.
So this patch is only used for MX51/53.
The ADMA2 mode supported or not can be distinguished by the
Capability Register(offset 0x40) of eSDHC module.
Up to now, only MX51/MX53 set the ADMA2 supported bit(Bit20) in the
Capability Register.

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

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index a19967d..8015e1a 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -31,6 +31,8 @@
 #define SDHCI_VENDOR_SPEC		0xC0
 #define  SDHCI_VENDOR_SPEC_SDIO_QUIRK	0x00000002
 
+#define  SDHCI_SPEC_INT_ADMA_ERR	0x10000000
+
 #define ESDHC_FLAG_GPIO_FOR_CD_WP	(1 << 0)
 /*
  * The CMDTYPE of the CMD register (offset 0xE) should be set to
@@ -80,6 +82,20 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
 			val |= SDHCI_CARD_PRESENT;
 	}
 
+	if (unlikely(reg == SDHCI_CAPABILITIES)) {
+		if (val & SDHCI_CAN_DO_ADMA1) {
+			val &= ~SDHCI_CAN_DO_ADMA1;
+			val |= SDHCI_CAN_DO_ADMA2;
+		}
+	}
+
+	if (unlikely(reg == SDHCI_INT_STATUS)) {
+		if (val & SDHCI_SPEC_INT_ADMA_ERR) {
+			val &= ~SDHCI_SPEC_INT_ADMA_ERR;
+			val |= SDHCI_INT_ADMA_ERROR;
+		}
+	}
+
 	return val;
 }
 
@@ -105,6 +121,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_SPEC_INT_ADMA_ERR;
+		}
+	}
 	writel(val, host->ioaddr + reg);
 }
 
@@ -154,7 +177,7 @@ static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg)
 
 static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
 {
-	u32 new_val;
+	u16 new_val;
 
 	switch (reg) {
 	case SDHCI_POWER_CONTROL:
@@ -168,10 +191,12 @@ static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
 		new_val = val & (SDHCI_CTRL_LED | SDHCI_CTRL_4BITBUS);
 		/* ensure the endianess */
 		new_val |= ESDHC_HOST_CONTROL_LE;
-		/* DMA mode bits are shifted */
-		new_val |= (val & SDHCI_CTRL_DMA_MASK) << 5;
+		if (val & SDHCI_CTRL_DMA_MASK)
+			/* DMA mode bits are shifted */
+			new_val |= (val & SDHCI_CTRL_DMA_MASK) << 5;
 
 		esdhc_clrset_le(host, 0xffff, new_val, reg);
+
 		return;
 	}
 	esdhc_clrset_le(host, 0xff, val, reg);
@@ -322,9 +347,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

Re: [PATCH V1] mmc: Enable the ADMA on esdhc imx driver

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2814 bytes --]

On Wed, Jun 08, 2011 at 11:16:45AM +0800, Richard Zhu wrote:
> Eanble the ADMA2 mode on freescale esdhc imx driver,
> tested on MX51 and MX53.

What does the new, vendor specific bit in the interrupt register cover what the
old ADMA_ERROR bit did not cover? And is the old one the same as in mx25, so
the new bit can be seen as the fixup which makes ADMA work only on mx51/53?

Information like this should be added to the define, so people will clearly
know later.

> 
> Only ADMA2 mode is enabled, MX25/35 can't support the ADMA2 mode.
> So this patch is only used for MX51/53.
> The ADMA2 mode supported or not can be distinguished by the
> Capability Register(offset 0x40) of eSDHC module.
> Up to now, only MX51/MX53 set the ADMA2 supported bit(Bit20) in the
> Capability Register.
> 
> Signed-off-by: Richard Zhu <richard.zhu@linaro.org>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c |   36 +++++++++++++++++++++++++++++++-----
>  1 files changed, 31 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index a19967d..8015e1a 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -31,6 +31,8 @@
>  #define SDHCI_VENDOR_SPEC		0xC0
>  #define  SDHCI_VENDOR_SPEC_SDIO_QUIRK	0x00000002
>  
> +#define  SDHCI_SPEC_INT_ADMA_ERR	0x10000000

There is no register SDHCI_SPEC_*. I'd suggest
"SDHCI_INT_VENDOR_SPEC_ADMA_ERROR".

> +	if (unlikely(reg == SDHCI_CAPABILITIES)) {
> +		if (val & SDHCI_CAN_DO_ADMA1) {
> +			val &= ~SDHCI_CAN_DO_ADMA1;
> +			val |= SDHCI_CAN_DO_ADMA2;

Why is this needed? Above you say, MX51/53 has ADMA2 bit already set?
And you would set ADMA2 for mx25/35 now?

> @@ -154,7 +177,7 @@ static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg)
>  
>  static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
>  {
> -	u32 new_val;
> +	u16 new_val;

Why? esdhc_clrset_le wants a u32.

>  
>  	switch (reg) {
>  	case SDHCI_POWER_CONTROL:
> @@ -168,10 +191,12 @@ static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
>  		new_val = val & (SDHCI_CTRL_LED | SDHCI_CTRL_4BITBUS);
>  		/* ensure the endianess */
>  		new_val |= ESDHC_HOST_CONTROL_LE;
> -		/* DMA mode bits are shifted */
> -		new_val |= (val & SDHCI_CTRL_DMA_MASK) << 5;
> +		if (val & SDHCI_CTRL_DMA_MASK)
> +			/* DMA mode bits are shifted */
> +			new_val |= (val & SDHCI_CTRL_DMA_MASK) << 5;

I think the if doesn't save much than just doing this simple operation; it also
adds another line and level of indentation.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]