Skip to contentSkip to conversation

mmc: Enable the ADMA2 on esdhc imx driver

PATCH Superseded Richard Zhu 4 participants 4 messages Jul 12, 2011 – Jul 13, 2011

Subsystems (4)

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 mmc: Enable the ADMA2 on esdhc imx driver

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

---
 drivers/mmc/host/sdhci-esdhc-imx.c |   45 ++++++++++++++++++++++++++++++++++-
 1 files changed, 43 insertions(+), 2 deletions(-)
drivers/mmc/host/sdhci-esdhc-imx.c +43 -2
index a19967d..46092c7 100644
@@ -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,
Signature · 1 line
1.7.1
maintainer
#1 lore raw

PATCH v3 1/1 mmc: Enable the ADMA2 on esdhc imx driver

Quoted 71 lines · 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

---
 drivers/mmc/host/sdhci-esdhc-imx.c |   45 ++++++++++++++++++++++++++++++++++-
 1 files changed, 43 insertions(+), 2 deletions(-)
drivers/mmc/host/sdhci-esdhc-imx.c +23 -0
index a19967d..46092c7 100644
@@ -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;

drivers/mmc/host/sdhci-esdhc-imx.c static u32 esdhc_readl_le(struct sdhci_host *host, int reg) · line 115

Quoted 10 lines
drivers/mmc/host/sdhci-esdhc-imx.c +10 -0
+		}
+	}
+
+	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

Signature · 4 lines
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 |
#2 lore raw

PATCH v3 1/1 mmc: Enable the ADMA2 on esdhc imx driver

On Tue, Jul 12, 2011 at 03:46:02PM +0200, Sascha Hauer wrote:
[...]

Quoted 3 lines

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,

Signature · 2 lines
Anton Vorontsov
Email: cbouatmailru at gmail.com
#3 lore raw

PATCH v3 1/1 mmc: Enable the ADMA2 on esdhc imx driver

Quoted 12 lines · Anton Vorontsov

On Tue, Jul 12, 2011 at 03:46:02PM +0200, Sascha Hauer wrote:
[...]

Quoted 3 lines

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.

Quoted 4 lines

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