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

[PATCH V2] 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 |   34 ++++++++++++++++++++++++++++++++--
 1 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index a19967d..0696e36 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
@@ -80,6 +88,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_INT_VENDOR_SPEC_DMA_ERR) {
+			val &= ~SDHCI_INT_VENDOR_SPEC_DMA_ERR;
+			val |= SDHCI_INT_ADMA_ERROR;
+		}
+	}
+
 	return val;
 }
 
@@ -105,6 +127,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 +351,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 V2] mmc: Enable the ADMA on esdhc imx driver

[Pavel Machek]

Hi!

> 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.

I tried it on mx25 (3-stack), and it unfortunately breaks things:

...
mmc0: unrecognised SCR structure version 12
mmc0: error -22 whilst initialising SD card
...

It may be interfering with my changs, but... 

> +/*
> + * 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
> @@ -80,6 +88,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_INT_VENDOR_SPEC_DMA_ERR) {
> +			val &= ~SDHCI_INT_VENDOR_SPEC_DMA_ERR;
> +			val |= SDHCI_INT_ADMA_ERROR;
> +		}
> +	}
> +
>  	return val;
>  }
>

Unfortunately register differences are common. Is there better
approach than patching it in low level functions like this?

> @@ -322,9 +351,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,

If I re-add "SDHCI_QUIRK_BROKEN_ADMA", it starts working.

								Pavel

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

[Wolfram Sang]

> Unfortunately register differences are common. Is there better
> approach than patching it in low level functions like this?

I am all ears for suggestions...

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110616/3fa3eea8/attachment.sig>

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

[Pavel Machek]

On Thu 2011-06-16 14:00:09, Wolfram Sang wrote:
> 
> > Unfortunately register differences are common. Is there better
> > approach than patching it in low level functions like this?
> 
> I am all ears for suggestions...

One way would be to move functions such as 

sdhci_activate_led()

to the low level driver, and introduce functions such as
write_host_control() -- with no corresponding read_host_control, so
that translation is easy to do...
								Pavel 

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

[Wolfram Sang]

On Thu, Jun 16, 2011 at 02:06:29PM +0200, Pavel Machek wrote:
> On Thu 2011-06-16 14:00:09, Wolfram Sang wrote:
> > 
> > > Unfortunately register differences are common. Is there better
> > > approach than patching it in low level functions like this?
> > 
> > I am all ears for suggestions...
> 
> One way would be to move functions such as 
> 
> sdhci_activate_led()
> 
> to the low level driver,

There is no "low level driver" if the controller is done right.

> and introduce functions such as write_host_control() -- with no
> corresponding read_host_control, so that translation is easy to do...

So, a seperate function for every register which differs from the
standard? There are a lot of SoCs out there, and even more to come...

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110616/09c3d3da/attachment.sig>

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

[Wolfram Sang]

Hi Richard,

>    It's strange that the sd card can't work on MX25 board without ADMA_BROKEN
>    quirks.

BTW how many cards/boards have you tested?

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110617/7bafae68/attachment.sig>