[PATCH 1/3] eSDHC: Access Freescale eSDHC registers by 32-bit

[PATCH 1/3] eSDHC: Access Freescale eSDHC registers by 32-bit

[Roy Zang]

From: Xu lei <B33228@freescale.com>

For Freescale eSDHC registers only support 32-bit accesses,
this patch ensure that all Freescale eSDHC register accesses
are 32-bit.

Signed-off-by: Xu lei <B33228@freescale.com>
Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 drivers/mmc/host/sdhci-of-esdhc.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index ba40d6d..c9a8519 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -1,7 +1,7 @@
 /*
  * Freescale eSDHC controller driver.
  *
- * Copyright (c) 2007 Freescale Semiconductor, Inc.
+ * Copyright (c) 2007, 2010 Freescale Semiconductor, Inc.
  * Copyright (c) 2009 MontaVista Software, Inc.
  *
  * Authors: Xiaobo Xie <X.Xie@freescale.com>
@@ -23,11 +23,21 @@
 static u16 esdhc_readw(struct sdhci_host *host, int reg)
 {
 	u16 ret;
+	int base = reg & ~0x3;
+	int shift = (reg & 0x2) * 8;
 
 	if (unlikely(reg == SDHCI_HOST_VERSION))
-		ret = in_be16(host->ioaddr + reg);
+		ret = in_be32(host->ioaddr + base) & 0xffff;
 	else
-		ret = sdhci_be32bs_readw(host, reg);
+		ret = (in_be32(host->ioaddr + base) >> shift) & 0xffff;
+	return ret;
+}
+
+static u8 esdhc_readb(struct sdhci_host *host, int reg)
+{
+	int base = reg & ~0x3;
+	int shift = (reg & 0x3) * 8;
+	u8 ret = (in_be32(host->ioaddr + base) >> shift) & 0xff;
 	return ret;
 }
 
@@ -79,7 +89,7 @@ struct sdhci_of_data sdhci_esdhc = {
 	.ops = {
 		.read_l = sdhci_be32bs_readl,
 		.read_w = esdhc_readw,
-		.read_b = sdhci_be32bs_readb,
+		.read_b = esdhc_readb,
 		.write_l = sdhci_be32bs_writel,
 		.write_w = esdhc_writew,
 		.write_b = esdhc_writeb,
-- 
1.6.0.6

[PATCH 1/3] eSDHC: Access Freescale eSDHC registers by 32-bit

[Roy Zang]

From: Xu lei <B33228@freescale.com>

For Freescale eSDHC registers only support 32-bit accesses,
this patch ensure that all Freescale eSDHC register accesses
are 32-bit.

Signed-off-by: Xu lei <B33228@freescale.com>
Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 drivers/mmc/host/sdhci-of-esdhc.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index ba40d6d..c9a8519 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -1,7 +1,7 @@
 /*
  * Freescale eSDHC controller driver.
  *
- * Copyright (c) 2007 Freescale Semiconductor, Inc.
+ * Copyright (c) 2007, 2010 Freescale Semiconductor, Inc.
  * Copyright (c) 2009 MontaVista Software, Inc.
  *
  * Authors: Xiaobo Xie <X.Xie@freescale.com>
@@ -23,11 +23,21 @@
 static u16 esdhc_readw(struct sdhci_host *host, int reg)
 {
 	u16 ret;
+	int base = reg & ~0x3;
+	int shift = (reg & 0x2) * 8;
 
 	if (unlikely(reg == SDHCI_HOST_VERSION))
-		ret = in_be16(host->ioaddr + reg);
+		ret = in_be32(host->ioaddr + base) & 0xffff;
 	else
-		ret = sdhci_be32bs_readw(host, reg);
+		ret = (in_be32(host->ioaddr + base) >> shift) & 0xffff;
+	return ret;
+}
+
+static u8 esdhc_readb(struct sdhci_host *host, int reg)
+{
+	int base = reg & ~0x3;
+	int shift = (reg & 0x3) * 8;
+	u8 ret = (in_be32(host->ioaddr + base) >> shift) & 0xff;
 	return ret;
 }
 
@@ -79,7 +89,7 @@ struct sdhci_of_data sdhci_esdhc = {
 	.ops = {
 		.read_l = sdhci_be32bs_readl,
 		.read_w = esdhc_readw,
-		.read_b = sdhci_be32bs_readb,
+		.read_b = esdhc_readb,
 		.write_l = sdhci_be32bs_writel,
 		.write_w = esdhc_writew,
 		.write_b = esdhc_writeb,
-- 
1.6.0.6

[PATCH 2/3] eSDHC: Fix errors when booting kernel with fsl esdhc

[Roy Zang]

From: Xu lei <B33228@freescale.com>

When esdhc module was enabled in p5020, there were following errors:

mmc0: Timeout waiting for hardware interrupt.
mmc0: error -110 whilst initialising SD card
mmc0: Unexpected interrupt 0x02000000.
mmc0: Timeout waiting for hardware interrupt.
mmc0: error -110 whilst initialising SD card
mmc0: Unexpected interrupt 0x02000000.

It is because ESDHC controller has different bit setting for PROCTL
register, when kernel sets Power Control Register by method for standard
SD Host Specification, it would overwritten FSL ESDHC PROCTL[DMAS];
when it set Host Control Registers[DMAS], it sets PROCTL[EMODE] and
PROCTL[D3CD]. These operations will set bad bits for PROCTL Register
on FSL ESDHC Controller and cause errors, so this patch will make esdhc
driver access FSL PROCTL Register according to block guide instead of
standard SD Host Specification.

For some FSL chips, such as MPC8536/P2020, PROCTL[VOLT_SEL] and PROCTL[DMAS]
bits are reserved and even if they are set to wrong bits there is no error.
But considering that all FSL ESDHC Controller register map is not fully
compliant to standard SD Host Specification, we put the patch to all of
FSL ESDHC Controllers.

Signed-off-by: Lei Xu <B33228@freescale.com>
Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 drivers/mmc/host/sdhci-of-core.c |    3 ++
 drivers/mmc/host/sdhci.c         |   62 ++++++++++++++++++++++++++++++-------
 include/linux/mmc/sdhci.h        |    6 ++-
 3 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c
index 60e4186..fede43d 100644
--- a/drivers/mmc/host/sdhci-of-core.c
+++ b/drivers/mmc/host/sdhci-of-core.c
@@ -179,6 +179,9 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev)
 	if (sdhci_of_wp_inverted(np))
 		host->quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
 
+	if (of_device_is_compatible(np, "fsl,esdhc"))
+		host->quirks |= SDHCI_QUIRK_QORIQ_PROCTL_WEIRD;
+
 	clk = of_get_property(np, "clock-frequency", &size);
 	if (clk && size == sizeof(*clk) && *clk)
 		of_host->clock = be32_to_cpup(clk);
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 58d5436..77174e5 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -674,7 +674,7 @@ static void sdhci_set_transfer_irqs(struct sdhci_host *host)
 static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 {
 	u8 count;
-	u8 ctrl;
+	u32 ctrl;
 	struct mmc_data *data = cmd->data;
 	int ret;
 
@@ -807,14 +807,28 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 	 * is ADMA.
 	 */
 	if (host->version >= SDHCI_SPEC_200) {
-		ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
-		ctrl &= ~SDHCI_CTRL_DMA_MASK;
-		if ((host->flags & SDHCI_REQ_USE_DMA) &&
-			(host->flags & SDHCI_USE_ADMA))
-			ctrl |= SDHCI_CTRL_ADMA32;
-		else
-			ctrl |= SDHCI_CTRL_SDMA;
-		sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+		if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD) {
+#define ESDHCI_PROCTL_DMAS_MASK		0x00000300
+#define ESDHCI_PROCTL_ADMA32		0x00000200
+#define ESDHCI_PROCTL_SDMA		0x00000000
+			ctrl = sdhci_readl(host, SDHCI_HOST_CONTROL);
+			ctrl &= ~ESDHCI_PROCTL_DMAS_MASK;
+			if ((host->flags & SDHCI_REQ_USE_DMA) &&
+				(host->flags & SDHCI_USE_ADMA))
+				ctrl |= ESDHCI_PROCTL_ADMA32;
+			else
+				ctrl |= ESDHCI_PROCTL_SDMA;
+			sdhci_writel(host, ctrl, SDHCI_HOST_CONTROL);
+		} else {
+			ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+			ctrl &= ~SDHCI_CTRL_DMA_MASK;
+			if ((host->flags & SDHCI_REQ_USE_DMA) &&
+				(host->flags & SDHCI_USE_ADMA))
+				ctrl |= SDHCI_CTRL_ADMA32;
+			else
+				ctrl |= SDHCI_CTRL_SDMA;
+			sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+		}
 	}
 
 	if (!(host->flags & SDHCI_REQ_USE_DMA)) {
@@ -1138,19 +1152,32 @@ out:
 static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
 {
 	u8 pwr = 0;
+	u8 volt = 0;
 
 	if (power != (unsigned short)-1) {
 		switch (1 << power) {
+#define	ESDHCI_FSL_POWER_MASK	0x40
+#define	ESDHCI_FSL_POWER_180	0x00
+#define	ESDHCI_FSL_POWER_300	0x40
 		case MMC_VDD_165_195:
-			pwr = SDHCI_POWER_180;
+			if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD)
+				pwr = ESDHCI_FSL_POWER_180;
+			else
+				pwr = SDHCI_POWER_180;
 			break;
 		case MMC_VDD_29_30:
 		case MMC_VDD_30_31:
-			pwr = SDHCI_POWER_300;
+			if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD)
+				pwr = ESDHCI_FSL_POWER_300;
+			else
+				pwr = SDHCI_POWER_300;
 			break;
 		case MMC_VDD_32_33:
 		case MMC_VDD_33_34:
-			pwr = SDHCI_POWER_330;
+			if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD)
+				pwr = ESDHCI_FSL_POWER_300;
+			else
+				pwr = SDHCI_POWER_330;
 			break;
 		default:
 			BUG();
@@ -1162,6 +1189,17 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
 
 	host->pwr = pwr;
 
+	/* Now FSL ESDHC Controller has no Bus Power bit,
+	 * and PROCTL[21] bit is for voltage selection */
+	if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD) {
+		volt = sdhci_readb(host, SDHCI_POWER_CONTROL);
+		volt &= ~ESDHCI_FSL_POWER_MASK;
+		volt |= pwr;
+		sdhci_writeb(host, volt, SDHCI_POWER_CONTROL);
+
+		return;
+	}
+
 	if (pwr == 0) {
 		sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
 		return;
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 6a68c4e..d87abc7 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -21,7 +21,7 @@ struct sdhci_host {
 	/* Data set by hardware interface driver */
 	const char *hw_name;	/* Hardware bus name */
 
-	unsigned int quirks;	/* Deviations from spec. */
+	u64 quirks;	/* Deviations from spec. */
 
 /* Controller doesn't honor resets unless we touch the clock register */
 #define SDHCI_QUIRK_CLOCK_BEFORE_RESET			(1<<0)
@@ -86,7 +86,9 @@ struct sdhci_host {
 /* Controller treats ADMA descriptors with length 0000h incorrectly */
 #define SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC		(1<<30)
 /* The read-only detection via SDHCI_PRESENT_STATE register is unstable */
-#define SDHCI_QUIRK_UNSTABLE_RO_DETECT			(1<<31)
+#define SDHCI_QUIRK_UNSTABLE_RO_DETECT			(1U<<31)
+/* Controller has weird bit setting for Protocol Control Register */
+#define SDHCI_QUIRK_QORIQ_PROCTL_WEIRD			(0x100000000U)
 
 	int irq;		/* Device IRQ */
 	void __iomem *ioaddr;	/* Mapped address */
-- 
1.6.0.6

[PATCH 2/3] eSDHC: Fix errors when booting kernel with fsl esdhc

[Roy Zang]

From: Xu lei <B33228@freescale.com>

When esdhc module was enabled in p5020, there were following errors:

mmc0: Timeout waiting for hardware interrupt.
mmc0: error -110 whilst initialising SD card
mmc0: Unexpected interrupt 0x02000000.
mmc0: Timeout waiting for hardware interrupt.
mmc0: error -110 whilst initialising SD card
mmc0: Unexpected interrupt 0x02000000.

It is because ESDHC controller has different bit setting for PROCTL
register, when kernel sets Power Control Register by method for standard
SD Host Specification, it would overwritten FSL ESDHC PROCTL[DMAS];
when it set Host Control Registers[DMAS], it sets PROCTL[EMODE] and
PROCTL[D3CD]. These operations will set bad bits for PROCTL Register
on FSL ESDHC Controller and cause errors, so this patch will make esdhc
driver access FSL PROCTL Register according to block guide instead of
standard SD Host Specification.

For some FSL chips, such as MPC8536/P2020, PROCTL[VOLT_SEL] and PROCTL[DMAS]
bits are reserved and even if they are set to wrong bits there is no error.
But considering that all FSL ESDHC Controller register map is not fully
compliant to standard SD Host Specification, we put the patch to all of
FSL ESDHC Controllers.

Signed-off-by: Lei Xu <B33228@freescale.com>
Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 drivers/mmc/host/sdhci-of-core.c |    3 ++
 drivers/mmc/host/sdhci.c         |   62 ++++++++++++++++++++++++++++++-------
 include/linux/mmc/sdhci.h        |    6 ++-
 3 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c
index 60e4186..fede43d 100644
--- a/drivers/mmc/host/sdhci-of-core.c
+++ b/drivers/mmc/host/sdhci-of-core.c
@@ -179,6 +179,9 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev)
 	if (sdhci_of_wp_inverted(np))
 		host->quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
 
+	if (of_device_is_compatible(np, "fsl,esdhc"))
+		host->quirks |= SDHCI_QUIRK_QORIQ_PROCTL_WEIRD;
+
 	clk = of_get_property(np, "clock-frequency", &size);
 	if (clk && size == sizeof(*clk) && *clk)
 		of_host->clock = be32_to_cpup(clk);
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 58d5436..77174e5 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -674,7 +674,7 @@ static void sdhci_set_transfer_irqs(struct sdhci_host *host)
 static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 {
 	u8 count;
-	u8 ctrl;
+	u32 ctrl;
 	struct mmc_data *data = cmd->data;
 	int ret;
 
@@ -807,14 +807,28 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 	 * is ADMA.
 	 */
 	if (host->version >= SDHCI_SPEC_200) {
-		ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
-		ctrl &= ~SDHCI_CTRL_DMA_MASK;
-		if ((host->flags & SDHCI_REQ_USE_DMA) &&
-			(host->flags & SDHCI_USE_ADMA))
-			ctrl |= SDHCI_CTRL_ADMA32;
-		else
-			ctrl |= SDHCI_CTRL_SDMA;
-		sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+		if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD) {
+#define ESDHCI_PROCTL_DMAS_MASK		0x00000300
+#define ESDHCI_PROCTL_ADMA32		0x00000200
+#define ESDHCI_PROCTL_SDMA		0x00000000
+			ctrl = sdhci_readl(host, SDHCI_HOST_CONTROL);
+			ctrl &= ~ESDHCI_PROCTL_DMAS_MASK;
+			if ((host->flags & SDHCI_REQ_USE_DMA) &&
+				(host->flags & SDHCI_USE_ADMA))
+				ctrl |= ESDHCI_PROCTL_ADMA32;
+			else
+				ctrl |= ESDHCI_PROCTL_SDMA;
+			sdhci_writel(host, ctrl, SDHCI_HOST_CONTROL);
+		} else {
+			ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+			ctrl &= ~SDHCI_CTRL_DMA_MASK;
+			if ((host->flags & SDHCI_REQ_USE_DMA) &&
+				(host->flags & SDHCI_USE_ADMA))
+				ctrl |= SDHCI_CTRL_ADMA32;
+			else
+				ctrl |= SDHCI_CTRL_SDMA;
+			sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+		}
 	}
 
 	if (!(host->flags & SDHCI_REQ_USE_DMA)) {
@@ -1138,19 +1152,32 @@ out:
 static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
 {
 	u8 pwr = 0;
+	u8 volt = 0;
 
 	if (power != (unsigned short)-1) {
 		switch (1 << power) {
+#define	ESDHCI_FSL_POWER_MASK	0x40
+#define	ESDHCI_FSL_POWER_180	0x00
+#define	ESDHCI_FSL_POWER_300	0x40
 		case MMC_VDD_165_195:
-			pwr = SDHCI_POWER_180;
+			if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD)
+				pwr = ESDHCI_FSL_POWER_180;
+			else
+				pwr = SDHCI_POWER_180;
 			break;
 		case MMC_VDD_29_30:
 		case MMC_VDD_30_31:
-			pwr = SDHCI_POWER_300;
+			if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD)
+				pwr = ESDHCI_FSL_POWER_300;
+			else
+				pwr = SDHCI_POWER_300;
 			break;
 		case MMC_VDD_32_33:
 		case MMC_VDD_33_34:
-			pwr = SDHCI_POWER_330;
+			if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD)
+				pwr = ESDHCI_FSL_POWER_300;
+			else
+				pwr = SDHCI_POWER_330;
 			break;
 		default:
 			BUG();
@@ -1162,6 +1189,17 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
 
 	host->pwr = pwr;
 
+	/* Now FSL ESDHC Controller has no Bus Power bit,
+	 * and PROCTL[21] bit is for voltage selection */
+	if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD) {
+		volt = sdhci_readb(host, SDHCI_POWER_CONTROL);
+		volt &= ~ESDHCI_FSL_POWER_MASK;
+		volt |= pwr;
+		sdhci_writeb(host, volt, SDHCI_POWER_CONTROL);
+
+		return;
+	}
+
 	if (pwr == 0) {
 		sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
 		return;
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 6a68c4e..d87abc7 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -21,7 +21,7 @@ struct sdhci_host {
 	/* Data set by hardware interface driver */
 	const char *hw_name;	/* Hardware bus name */
 
-	unsigned int quirks;	/* Deviations from spec. */
+	u64 quirks;	/* Deviations from spec. */
 
 /* Controller doesn't honor resets unless we touch the clock register */
 #define SDHCI_QUIRK_CLOCK_BEFORE_RESET			(1<<0)
@@ -86,7 +86,9 @@ struct sdhci_host {
 /* Controller treats ADMA descriptors with length 0000h incorrectly */
 #define SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC		(1<<30)
 /* The read-only detection via SDHCI_PRESENT_STATE register is unstable */
-#define SDHCI_QUIRK_UNSTABLE_RO_DETECT			(1<<31)
+#define SDHCI_QUIRK_UNSTABLE_RO_DETECT			(1U<<31)
+/* Controller has weird bit setting for Protocol Control Register */
+#define SDHCI_QUIRK_QORIQ_PROCTL_WEIRD			(0x100000000U)
 
 	int irq;		/* Device IRQ */
 	void __iomem *ioaddr;	/* Mapped address */
-- 
1.6.0.6

[PATCH 3/3] eSDHC: fix incorrect default value of the capabilities register on P4080

[Roy Zang]

P4080 eSDHC errata 12 describes incorrect default value of the
the host controller capabilities register.

The default value of the VS18 and VS30 fields in the host controller
capabilities register (HOSTCAPBLT) are incorrect. The default of these bits
should be zero instead of one in the eSDHC logic.

This patch adds the workaround for these errata.

Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
---
 drivers/mmc/host/sdhci-of-core.c |    3 +++
 drivers/mmc/host/sdhci.c         |    6 ++++++
 include/linux/mmc/sdhci.h        |    4 ++++
 3 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c
index fede43d..9bdd30d 100644
--- a/drivers/mmc/host/sdhci-of-core.c
+++ b/drivers/mmc/host/sdhci-of-core.c
@@ -182,6 +182,9 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev)
 	if (of_device_is_compatible(np, "fsl,esdhc"))
 		host->quirks |= SDHCI_QUIRK_QORIQ_PROCTL_WEIRD;
 
+	if (of_device_is_compatible(np, "fsl,p4080-esdhc"))
+		host->quirks |= SDHCI_QUIRK_QORIQ_HOSTCAPBLT_ONLY_VS33;
+
 	clk = of_get_property(np, "clock-frequency", &size);
 	if (clk && size == sizeof(*clk) && *clk)
 		of_host->clock = be32_to_cpup(clk);
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 77174e5..7e0b4cd 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2396,6 +2396,12 @@ int sdhci_add_host(struct sdhci_host *host)
 	caps[1] = (host->version >= SDHCI_SPEC_300) ?
 		sdhci_readl(host, SDHCI_CAPABILITIES_1) : 0;
 
+	 /* Make sure clean the VS18 and VS30 bit. P4080 incorrectly
+	  * set the voltage capability bits
+	  */
+	if (host->quirks & SDHCI_QUIRK_QORIQ_HOSTCAPBLT_ONLY_VS33)
+		caps[0] &= ~(SDHCI_CAN_VDD_180 | SDHCI_CAN_VDD_300);
+
 	if (host->quirks & SDHCI_QUIRK_FORCE_DMA)
 		host->flags |= SDHCI_USE_SDMA;
 	else if (!(caps[0] & SDHCI_CAN_DO_SDMA))
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index d87abc7..7ffd458 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -89,6 +89,10 @@ struct sdhci_host {
 #define SDHCI_QUIRK_UNSTABLE_RO_DETECT			(1U<<31)
 /* Controller has weird bit setting for Protocol Control Register */
 #define SDHCI_QUIRK_QORIQ_PROCTL_WEIRD			(0x100000000U)
+/* Controller can only supports 3.3V, but the capabilities register
+ * has incorrect set 1.8V and 3.0V
+ */
+#define SDHCI_QUIRK_QORIQ_HOSTCAPBLT_ONLY_VS33		(0x200000000U)
 
 	int irq;		/* Device IRQ */
 	void __iomem *ioaddr;	/* Mapped address */
-- 
1.6.0.6

[PATCH 3/3] eSDHC: fix incorrect default value of the capabilities register on P4080

[Roy Zang]

P4080 eSDHC errata 12 describes incorrect default value of the
the host controller capabilities register.

The default value of the VS18 and VS30 fields in the host controller
capabilities register (HOSTCAPBLT) are incorrect. The default of these bits
should be zero instead of one in the eSDHC logic.

This patch adds the workaround for these errata.

Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
---
 drivers/mmc/host/sdhci-of-core.c |    3 +++
 drivers/mmc/host/sdhci.c         |    6 ++++++
 include/linux/mmc/sdhci.h        |    4 ++++
 3 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c
index fede43d..9bdd30d 100644
--- a/drivers/mmc/host/sdhci-of-core.c
+++ b/drivers/mmc/host/sdhci-of-core.c
@@ -182,6 +182,9 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev)
 	if (of_device_is_compatible(np, "fsl,esdhc"))
 		host->quirks |= SDHCI_QUIRK_QORIQ_PROCTL_WEIRD;
 
+	if (of_device_is_compatible(np, "fsl,p4080-esdhc"))
+		host->quirks |= SDHCI_QUIRK_QORIQ_HOSTCAPBLT_ONLY_VS33;
+
 	clk = of_get_property(np, "clock-frequency", &size);
 	if (clk && size == sizeof(*clk) && *clk)
 		of_host->clock = be32_to_cpup(clk);
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 77174e5..7e0b4cd 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2396,6 +2396,12 @@ int sdhci_add_host(struct sdhci_host *host)
 	caps[1] = (host->version >= SDHCI_SPEC_300) ?
 		sdhci_readl(host, SDHCI_CAPABILITIES_1) : 0;
 
+	 /* Make sure clean the VS18 and VS30 bit. P4080 incorrectly
+	  * set the voltage capability bits
+	  */
+	if (host->quirks & SDHCI_QUIRK_QORIQ_HOSTCAPBLT_ONLY_VS33)
+		caps[0] &= ~(SDHCI_CAN_VDD_180 | SDHCI_CAN_VDD_300);
+
 	if (host->quirks & SDHCI_QUIRK_FORCE_DMA)
 		host->flags |= SDHCI_USE_SDMA;
 	else if (!(caps[0] & SDHCI_CAN_DO_SDMA))
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index d87abc7..7ffd458 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -89,6 +89,10 @@ struct sdhci_host {
 #define SDHCI_QUIRK_UNSTABLE_RO_DETECT			(1U<<31)
 /* Controller has weird bit setting for Protocol Control Register */
 #define SDHCI_QUIRK_QORIQ_PROCTL_WEIRD			(0x100000000U)
+/* Controller can only supports 3.3V, but the capabilities register
+ * has incorrect set 1.8V and 3.0V
+ */
+#define SDHCI_QUIRK_QORIQ_HOSTCAPBLT_ONLY_VS33		(0x200000000U)
 
 	int irq;		/* Device IRQ */
 	void __iomem *ioaddr;	/* Mapped address */
-- 
1.6.0.6

Re: [PATCH 2/3] eSDHC: Fix errors when booting kernel with fsl esdhc

[S, Venkatraman]

On Tue, Jul 5, 2011 at 9:49 AM, Roy Zang <tie-fei.zang@freescale.com> wrote:
> From: Xu lei <B33228@freescale.com>
>
> When esdhc module was enabled in p5020, there were following errors:
>
> mmc0: Timeout waiting for hardware interrupt.
> mmc0: error -110 whilst initialising SD card
> mmc0: Unexpected interrupt 0x02000000.
> mmc0: Timeout waiting for hardware interrupt.
> mmc0: error -110 whilst initialising SD card
> mmc0: Unexpected interrupt 0x02000000.
>
> It is because ESDHC controller has different bit setting for PROCTL
> register, when kernel sets Power Control Register by method for standard
> SD Host Specification, it would overwritten FSL ESDHC PROCTL[DMAS];
> when it set Host Control Registers[DMAS], it sets PROCTL[EMODE] and
> PROCTL[D3CD]. These operations will set bad bits for PROCTL Register
> on FSL ESDHC Controller and cause errors, so this patch will make esdhc
> driver access FSL PROCTL Register according to block guide instead of
> standard SD Host Specification.
>
> For some FSL chips, such as MPC8536/P2020, PROCTL[VOLT_SEL] and PROCTL[DMAS]
> bits are reserved and even if they are set to wrong bits there is no error.
> But considering that all FSL ESDHC Controller register map is not fully
> compliant to standard SD Host Specification, we put the patch to all of
> FSL ESDHC Controllers.
>
> Signed-off-by: Lei Xu <B33228@freescale.com>
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> ---
>  drivers/mmc/host/sdhci-of-core.c |    3 ++
>  drivers/mmc/host/sdhci.c         |   62 ++++++++++++++++++++++++++++++-------
>  include/linux/mmc/sdhci.h        |    6 ++-
>  3 files changed, 57 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c
> index 60e4186..fede43d 100644
> --- a/drivers/mmc/host/sdhci-of-core.c
> +++ b/drivers/mmc/host/sdhci-of-core.c
> @@ -179,6 +179,9 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev)
>        if (sdhci_of_wp_inverted(np))
>                host->quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
>
> +       if (of_device_is_compatible(np, "fsl,esdhc"))
> +               host->quirks |= SDHCI_QUIRK_QORIQ_PROCTL_WEIRD;
> +
>        clk = of_get_property(np, "clock-frequency", &size);
>        if (clk && size == sizeof(*clk) && *clk)
>                of_host->clock = be32_to_cpup(clk);
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 58d5436..77174e5 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -674,7 +674,7 @@ static void sdhci_set_transfer_irqs(struct sdhci_host *host)
>  static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
>  {
>        u8 count;
> -       u8 ctrl;
> +       u32 ctrl;
>        struct mmc_data *data = cmd->data;
>        int ret;
>
> @@ -807,14 +807,28 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
>         * is ADMA.
>         */
>        if (host->version >= SDHCI_SPEC_200) {
> -               ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
> -               ctrl &= ~SDHCI_CTRL_DMA_MASK;
> -               if ((host->flags & SDHCI_REQ_USE_DMA) &&
> -                       (host->flags & SDHCI_USE_ADMA))
> -                       ctrl |= SDHCI_CTRL_ADMA32;
> -               else
> -                       ctrl |= SDHCI_CTRL_SDMA;
> -               sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
> +               if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD) {
> +#define ESDHCI_PROCTL_DMAS_MASK                0x00000300
> +#define ESDHCI_PROCTL_ADMA32           0x00000200
> +#define ESDHCI_PROCTL_SDMA             0x00000000

Breaks the code flow / readability. Can be moved to top of the file ?

> +                       ctrl = sdhci_readl(host, SDHCI_HOST_CONTROL);
> +                       ctrl &= ~ESDHCI_PROCTL_DMAS_MASK;
> +                       if ((host->flags & SDHCI_REQ_USE_DMA) &&
> +                               (host->flags & SDHCI_USE_ADMA))
> +                               ctrl |= ESDHCI_PROCTL_ADMA32;
> +                       else
> +                               ctrl |= ESDHCI_PROCTL_SDMA;
> +                       sdhci_writel(host, ctrl, SDHCI_HOST_CONTROL);
> +               } else {
> +                       ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
> +                       ctrl &= ~SDHCI_CTRL_DMA_MASK;
> +                       if ((host->flags & SDHCI_REQ_USE_DMA) &&
> +                               (host->flags & SDHCI_USE_ADMA))
> +                               ctrl |= SDHCI_CTRL_ADMA32;
> +                       else
> +                               ctrl |= SDHCI_CTRL_SDMA;
> +                       sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
> +               }
>        }
>
>        if (!(host->flags & SDHCI_REQ_USE_DMA)) {
> @@ -1138,19 +1152,32 @@ out:
>  static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
>  {
>        u8 pwr = 0;
> +       u8 volt = 0;
>
>        if (power != (unsigned short)-1) {
>                switch (1 << power) {
> +#define        ESDHCI_FSL_POWER_MASK   0x40
> +#define        ESDHCI_FSL_POWER_180    0x00
> +#define        ESDHCI_FSL_POWER_300    0x40

<As above>

>                case MMC_VDD_165_195:
> -                       pwr = SDHCI_POWER_180;
> +                       if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD)
> +                               pwr = ESDHCI_FSL_POWER_180;
> +                       else
> +                               pwr = SDHCI_POWER_180;
>                        break;
>                case MMC_VDD_29_30:
>                case MMC_VDD_30_31:
> -                       pwr = SDHCI_POWER_300;
> +                       if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD)
> +                               pwr = ESDHCI_FSL_POWER_300;
> +                       else
> +                               pwr = SDHCI_POWER_300;
>                        break;
>                case MMC_VDD_32_33:
>                case MMC_VDD_33_34:
> -                       pwr = SDHCI_POWER_330;
> +                       if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD)
> +                               pwr = ESDHCI_FSL_POWER_300;
> +                       else
> +                               pwr = SDHCI_POWER_330;
>                        break;
>                default:
>                        BUG();
> @@ -1162,6 +1189,17 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
>
>        host->pwr = pwr;
>
> +       /* Now FSL ESDHC Controller has no Bus Power bit,
> +        * and PROCTL[21] bit is for voltage selection */

Multiline comment style needed..

> +       if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD) {
> +               volt = sdhci_readb(host, SDHCI_POWER_CONTROL);
> +               volt &= ~ESDHCI_FSL_POWER_MASK;
> +               volt |= pwr;
> +               sdhci_writeb(host, volt, SDHCI_POWER_CONTROL);
> +
> +               return;
> +       }
> +
>        if (pwr == 0) {
>                sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
>                return;
> diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
> index 6a68c4e..d87abc7 100644
> --- a/include/linux/mmc/sdhci.h
> +++ b/include/linux/mmc/sdhci.h
> @@ -21,7 +21,7 @@ struct sdhci_host {
>        /* Data set by hardware interface driver */
>        const char *hw_name;    /* Hardware bus name */
>
> -       unsigned int quirks;    /* Deviations from spec. */
> +       u64 quirks;     /* Deviations from spec. */
>
>  /* Controller doesn't honor resets unless we touch the clock register */
>  #define SDHCI_QUIRK_CLOCK_BEFORE_RESET                 (1<<0)
> @@ -86,7 +86,9 @@ struct sdhci_host {
>  /* Controller treats ADMA descriptors with length 0000h incorrectly */
>  #define SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC           (1<<30)
>  /* The read-only detection via SDHCI_PRESENT_STATE register is unstable */
> -#define SDHCI_QUIRK_UNSTABLE_RO_DETECT                 (1<<31)
> +#define SDHCI_QUIRK_UNSTABLE_RO_DETECT                 (1U<<31)
> +/* Controller has weird bit setting for Protocol Control Register */
> +#define SDHCI_QUIRK_QORIQ_PROCTL_WEIRD                 (0x100000000U)
>
>        int irq;                /* Device IRQ */
>        void __iomem *ioaddr;   /* Mapped address */
> --
> 1.6.0.6
>

Re: [PATCH 2/3] eSDHC: Fix errors when booting kernel with fsl esdhc

[S, Venkatraman]

On Tue, Jul 5, 2011 at 9:49 AM, Roy Zang <tie-fei.zang@freescale.com> wrote=
:
> From: Xu lei <B33228@freescale.com>
>
> When esdhc module was enabled in p5020, there were following errors:
>
> mmc0: Timeout waiting for hardware interrupt.
> mmc0: error -110 whilst initialising SD card
> mmc0: Unexpected interrupt 0x02000000.
> mmc0: Timeout waiting for hardware interrupt.
> mmc0: error -110 whilst initialising SD card
> mmc0: Unexpected interrupt 0x02000000.
>
> It is because ESDHC controller has different bit setting for PROCTL
> register, when kernel sets Power Control Register by method for standard
> SD Host Specification, it would overwritten FSL ESDHC PROCTL[DMAS];
> when it set Host Control Registers[DMAS], it sets PROCTL[EMODE] and
> PROCTL[D3CD]. These operations will set bad bits for PROCTL Register
> on FSL ESDHC Controller and cause errors, so this patch will make esdhc
> driver access FSL PROCTL Register according to block guide instead of
> standard SD Host Specification.
>
> For some FSL chips, such as MPC8536/P2020, PROCTL[VOLT_SEL] and PROCTL[DM=
AS]
> bits are reserved and even if they are set to wrong bits there is no erro=
r.
> But considering that all FSL ESDHC Controller register map is not fully
> compliant to standard SD Host Specification, we put the patch to all of
> FSL ESDHC Controllers.
>
> Signed-off-by: Lei Xu <B33228@freescale.com>
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> ---
> =A0drivers/mmc/host/sdhci-of-core.c | =A0 =A03 ++
> =A0drivers/mmc/host/sdhci.c =A0 =A0 =A0 =A0 | =A0 62 ++++++++++++++++++++=
++++++++++-------
> =A0include/linux/mmc/sdhci.h =A0 =A0 =A0 =A0| =A0 =A06 ++-
> =A03 files changed, 57 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of=
-core.c
> index 60e4186..fede43d 100644
> --- a/drivers/mmc/host/sdhci-of-core.c
> +++ b/drivers/mmc/host/sdhci-of-core.c
> @@ -179,6 +179,9 @@ static int __devinit sdhci_of_probe(struct platform_d=
evice *ofdev)
> =A0 =A0 =A0 =A0if (sdhci_of_wp_inverted(np))
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0host->quirks |=3D SDHCI_QUIRK_INVERTED_WRI=
TE_PROTECT;
>
> + =A0 =A0 =A0 if (of_device_is_compatible(np, "fsl,esdhc"))
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 host->quirks |=3D SDHCI_QUIRK_QORIQ_PROCTL_=
WEIRD;
> +
> =A0 =A0 =A0 =A0clk =3D of_get_property(np, "clock-frequency", &size);
> =A0 =A0 =A0 =A0if (clk && size =3D=3D sizeof(*clk) && *clk)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0of_host->clock =3D be32_to_cpup(clk);
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 58d5436..77174e5 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -674,7 +674,7 @@ static void sdhci_set_transfer_irqs(struct sdhci_host=
 *host)
> =A0static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_com=
mand *cmd)
> =A0{
> =A0 =A0 =A0 =A0u8 count;
> - =A0 =A0 =A0 u8 ctrl;
> + =A0 =A0 =A0 u32 ctrl;
> =A0 =A0 =A0 =A0struct mmc_data *data =3D cmd->data;
> =A0 =A0 =A0 =A0int ret;
>
> @@ -807,14 +807,28 @@ static void sdhci_prepare_data(struct sdhci_host *h=
ost, struct mmc_command *cmd)
> =A0 =A0 =A0 =A0 * is ADMA.
> =A0 =A0 =A0 =A0 */
> =A0 =A0 =A0 =A0if (host->version >=3D SDHCI_SPEC_200) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl =3D sdhci_readb(host, SDHCI_HOST_CONTR=
OL);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl &=3D ~SDHCI_CTRL_DMA_MASK;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if ((host->flags & SDHCI_REQ_USE_DMA) &&
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (host->flags & SDHCI_USE_AD=
MA))
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl |=3D SDHCI_CTRL_ADMA32=
;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl |=3D SDHCI_CTRL_SDMA;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL=
_WEIRD) {
> +#define ESDHCI_PROCTL_DMAS_MASK =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x0000030=
0
> +#define ESDHCI_PROCTL_ADMA32 =A0 =A0 =A0 =A0 =A0 0x00000200
> +#define ESDHCI_PROCTL_SDMA =A0 =A0 =A0 =A0 =A0 =A0 0x00000000

Breaks the code flow / readability. Can be moved to top of the file ?

> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl =3D sdhci_readl(host, =
SDHCI_HOST_CONTROL);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl &=3D ~ESDHCI_PROCTL_DM=
AS_MASK;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if ((host->flags & SDHCI_RE=
Q_USE_DMA) &&
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (host->flag=
s & SDHCI_USE_ADMA))
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl |=3D E=
SDHCI_PROCTL_ADMA32;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl |=3D E=
SDHCI_PROCTL_SDMA;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 sdhci_writel(host, ctrl, SD=
HCI_HOST_CONTROL);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 } else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl =3D sdhci_readb(host, =
SDHCI_HOST_CONTROL);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl &=3D ~SDHCI_CTRL_DMA_M=
ASK;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if ((host->flags & SDHCI_RE=
Q_USE_DMA) &&
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (host->flag=
s & SDHCI_USE_ADMA))
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl |=3D S=
DHCI_CTRL_ADMA32;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl |=3D S=
DHCI_CTRL_SDMA;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 sdhci_writeb(host, ctrl, SD=
HCI_HOST_CONTROL);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> =A0 =A0 =A0 =A0}
>
> =A0 =A0 =A0 =A0if (!(host->flags & SDHCI_REQ_USE_DMA)) {
> @@ -1138,19 +1152,32 @@ out:
> =A0static void sdhci_set_power(struct sdhci_host *host, unsigned short po=
wer)
> =A0{
> =A0 =A0 =A0 =A0u8 pwr =3D 0;
> + =A0 =A0 =A0 u8 volt =3D 0;
>
> =A0 =A0 =A0 =A0if (power !=3D (unsigned short)-1) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0switch (1 << power) {
> +#define =A0 =A0 =A0 =A0ESDHCI_FSL_POWER_MASK =A0 0x40
> +#define =A0 =A0 =A0 =A0ESDHCI_FSL_POWER_180 =A0 =A00x00
> +#define =A0 =A0 =A0 =A0ESDHCI_FSL_POWER_300 =A0 =A00x40

<As above>

> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case MMC_VDD_165_195:
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pwr =3D SDHCI_POWER_180;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (host->quirks & SDHCI_QU=
IRK_QORIQ_PROCTL_WEIRD)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pwr =3D ESD=
HCI_FSL_POWER_180;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pwr =3D SDH=
CI_POWER_180;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case MMC_VDD_29_30:
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case MMC_VDD_30_31:
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pwr =3D SDHCI_POWER_300;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (host->quirks & SDHCI_QU=
IRK_QORIQ_PROCTL_WEIRD)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pwr =3D ESD=
HCI_FSL_POWER_300;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pwr =3D SDH=
CI_POWER_300;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case MMC_VDD_32_33:
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case MMC_VDD_33_34:
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pwr =3D SDHCI_POWER_330;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (host->quirks & SDHCI_QU=
IRK_QORIQ_PROCTL_WEIRD)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pwr =3D ESD=
HCI_FSL_POWER_300;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pwr =3D SDH=
CI_POWER_330;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0default:
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0BUG();
> @@ -1162,6 +1189,17 @@ static void sdhci_set_power(struct sdhci_host *hos=
t, unsigned short power)
>
> =A0 =A0 =A0 =A0host->pwr =3D pwr;
>
> + =A0 =A0 =A0 /* Now FSL ESDHC Controller has no Bus Power bit,
> + =A0 =A0 =A0 =A0* and PROCTL[21] bit is for voltage selection */

Multiline comment style needed..

> + =A0 =A0 =A0 if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 volt =3D sdhci_readb(host, SDHCI_POWER_CONT=
ROL);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 volt &=3D ~ESDHCI_FSL_POWER_MASK;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 volt |=3D pwr;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 sdhci_writeb(host, volt, SDHCI_POWER_CONTRO=
L);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
> + =A0 =A0 =A0 }
> +
> =A0 =A0 =A0 =A0if (pwr =3D=3D 0) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sdhci_writeb(host, 0, SDHCI_POWER_CONTROL)=
;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return;
> diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
> index 6a68c4e..d87abc7 100644
> --- a/include/linux/mmc/sdhci.h
> +++ b/include/linux/mmc/sdhci.h
> @@ -21,7 +21,7 @@ struct sdhci_host {
> =A0 =A0 =A0 =A0/* Data set by hardware interface driver */
> =A0 =A0 =A0 =A0const char *hw_name; =A0 =A0/* Hardware bus name */
>
> - =A0 =A0 =A0 unsigned int quirks; =A0 =A0/* Deviations from spec. */
> + =A0 =A0 =A0 u64 quirks; =A0 =A0 /* Deviations from spec. */
>
> =A0/* Controller doesn't honor resets unless we touch the clock register =
*/
> =A0#define SDHCI_QUIRK_CLOCK_BEFORE_RESET =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 (1<<0)
> @@ -86,7 +86,9 @@ struct sdhci_host {
> =A0/* Controller treats ADMA descriptors with length 0000h incorrectly */
> =A0#define SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC =A0 =A0 =A0 =A0 =A0 (1<<3=
0)
> =A0/* The read-only detection via SDHCI_PRESENT_STATE register is unstabl=
e */
> -#define SDHCI_QUIRK_UNSTABLE_RO_DETECT =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (=
1<<31)
> +#define SDHCI_QUIRK_UNSTABLE_RO_DETECT =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (=
1U<<31)
> +/* Controller has weird bit setting for Protocol Control Register */
> +#define SDHCI_QUIRK_QORIQ_PROCTL_WEIRD =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (=
0x100000000U)
>
> =A0 =A0 =A0 =A0int irq; =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Device IRQ */
> =A0 =A0 =A0 =A0void __iomem *ioaddr; =A0 /* Mapped address */
> --
> 1.6.0.6
>

Re: [PATCH 3/3] eSDHC: fix incorrect default value of the capabilities register on P4080

[Anton Vorontsov]

On Tue, Jul 05, 2011 at 12:19:03PM +0800, Roy Zang wrote:
> P4080 eSDHC errata 12 describes incorrect default value of the
> the host controller capabilities register.
> 
> The default value of the VS18 and VS30 fields in the host controller
> capabilities register (HOSTCAPBLT) are incorrect. The default of these bits
> should be zero instead of one in the eSDHC logic.
> 
> This patch adds the workaround for these errata.
> 
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> ---
>  drivers/mmc/host/sdhci-of-core.c |    3 +++
>  drivers/mmc/host/sdhci.c         |    6 ++++++
>  include/linux/mmc/sdhci.h        |    4 ++++
>  3 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c
> index fede43d..9bdd30d 100644
> --- a/drivers/mmc/host/sdhci-of-core.c
> +++ b/drivers/mmc/host/sdhci-of-core.c
> @@ -182,6 +182,9 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev)
>  	if (of_device_is_compatible(np, "fsl,esdhc"))
>  		host->quirks |= SDHCI_QUIRK_QORIQ_PROCTL_WEIRD;
>  
> +	if (of_device_is_compatible(np, "fsl,p4080-esdhc"))
> +		host->quirks |= SDHCI_QUIRK_QORIQ_HOSTCAPBLT_ONLY_VS33;

Should really use voltage-ranges, not quirks.

http://www.spinics.net/lists/linux-mmc/msg02785.html

Thanks,

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com

Re: [PATCH 3/3] eSDHC: fix incorrect default value of the capabilities register on P4080

[Anton Vorontsov]

On Tue, Jul 05, 2011 at 12:19:03PM +0800, Roy Zang wrote:
> P4080 eSDHC errata 12 describes incorrect default value of the
> the host controller capabilities register.
> 
> The default value of the VS18 and VS30 fields in the host controller
> capabilities register (HOSTCAPBLT) are incorrect. The default of these bits
> should be zero instead of one in the eSDHC logic.
> 
> This patch adds the workaround for these errata.
> 
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> ---
>  drivers/mmc/host/sdhci-of-core.c |    3 +++
>  drivers/mmc/host/sdhci.c         |    6 ++++++
>  include/linux/mmc/sdhci.h        |    4 ++++
>  3 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c
> index fede43d..9bdd30d 100644
> --- a/drivers/mmc/host/sdhci-of-core.c
> +++ b/drivers/mmc/host/sdhci-of-core.c
> @@ -182,6 +182,9 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev)
>  	if (of_device_is_compatible(np, "fsl,esdhc"))
>  		host->quirks |= SDHCI_QUIRK_QORIQ_PROCTL_WEIRD;
>  
> +	if (of_device_is_compatible(np, "fsl,p4080-esdhc"))
> +		host->quirks |= SDHCI_QUIRK_QORIQ_HOSTCAPBLT_ONLY_VS33;

Should really use voltage-ranges, not quirks.

http://www.spinics.net/lists/linux-mmc/msg02785.html

Thanks,

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com

RE: [PATCH 3/3] eSDHC: fix incorrect default value of the capabilities register on P4080

[Zang Roy-R61911]

> -----Original Message-----
> From: Anton Vorontsov [mailto:avorontsov@mvista.com]
> Sent: Tuesday, July 05, 2011 18:18 PM
> To: Zang Roy-R61911
> Cc: linux-mmc@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; akpm@linux-
> foundation.org
> Subject: Re: [PATCH 3/3] eSDHC: fix incorrect default value of the
> capabilities register on P4080
> 
> On Tue, Jul 05, 2011 at 12:19:03PM +0800, Roy Zang wrote:
> > P4080 eSDHC errata 12 describes incorrect default value of the
> > the host controller capabilities register.
> >
> > The default value of the VS18 and VS30 fields in the host controller
> > capabilities register (HOSTCAPBLT) are incorrect. The default of these bits
> > should be zero instead of one in the eSDHC logic.
> >
> > This patch adds the workaround for these errata.
> >
> > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> > ---
> >  drivers/mmc/host/sdhci-of-core.c |    3 +++
> >  drivers/mmc/host/sdhci.c         |    6 ++++++
> >  include/linux/mmc/sdhci.h        |    4 ++++
> >  3 files changed, 13 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-
> core.c
> > index fede43d..9bdd30d 100644
> > --- a/drivers/mmc/host/sdhci-of-core.c
> > +++ b/drivers/mmc/host/sdhci-of-core.c
> > @@ -182,6 +182,9 @@ static int __devinit sdhci_of_probe(struct
> platform_device *ofdev)
> >  	if (of_device_is_compatible(np, "fsl,esdhc"))
> >  		host->quirks |= SDHCI_QUIRK_QORIQ_PROCTL_WEIRD;
> >
> > +	if (of_device_is_compatible(np, "fsl,p4080-esdhc"))
> > +		host->quirks |= SDHCI_QUIRK_QORIQ_HOSTCAPBLT_ONLY_VS33;
> 
> Should really use voltage-ranges, not quirks.
> 
> http://www.spinics.net/lists/linux-mmc/msg02785.html
Make sense to me. Thanks.
Roy

RE: [PATCH 3/3] eSDHC: fix incorrect default value of the capabilities register on P4080

[Zang Roy-R61911]

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQW50b24gVm9yb250c292
IFttYWlsdG86YXZvcm9udHNvdkBtdmlzdGEuY29tXQ0KPiBTZW50OiBUdWVzZGF5LCBKdWx5IDA1
LCAyMDExIDE4OjE4IFBNDQo+IFRvOiBaYW5nIFJveS1SNjE5MTENCj4gQ2M6IGxpbnV4LW1tY0B2
Z2VyLmtlcm5lbC5vcmc7IGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnOyBha3BtQGxpbnV4
LQ0KPiBmb3VuZGF0aW9uLm9yZw0KPiBTdWJqZWN0OiBSZTogW1BBVENIIDMvM10gZVNESEM6IGZp
eCBpbmNvcnJlY3QgZGVmYXVsdCB2YWx1ZSBvZiB0aGUNCj4gY2FwYWJpbGl0aWVzIHJlZ2lzdGVy
IG9uIFA0MDgwDQo+IA0KPiBPbiBUdWUsIEp1bCAwNSwgMjAxMSBhdCAxMjoxOTowM1BNICswODAw
LCBSb3kgWmFuZyB3cm90ZToNCj4gPiBQNDA4MCBlU0RIQyBlcnJhdGEgMTIgZGVzY3JpYmVzIGlu
Y29ycmVjdCBkZWZhdWx0IHZhbHVlIG9mIHRoZQ0KPiA+IHRoZSBob3N0IGNvbnRyb2xsZXIgY2Fw
YWJpbGl0aWVzIHJlZ2lzdGVyLg0KPiA+DQo+ID4gVGhlIGRlZmF1bHQgdmFsdWUgb2YgdGhlIFZT
MTggYW5kIFZTMzAgZmllbGRzIGluIHRoZSBob3N0IGNvbnRyb2xsZXINCj4gPiBjYXBhYmlsaXRp
ZXMgcmVnaXN0ZXIgKEhPU1RDQVBCTFQpIGFyZSBpbmNvcnJlY3QuIFRoZSBkZWZhdWx0IG9mIHRo
ZXNlIGJpdHMNCj4gPiBzaG91bGQgYmUgemVybyBpbnN0ZWFkIG9mIG9uZSBpbiB0aGUgZVNESEMg
bG9naWMuDQo+ID4NCj4gPiBUaGlzIHBhdGNoIGFkZHMgdGhlIHdvcmthcm91bmQgZm9yIHRoZXNl
IGVycmF0YS4NCj4gPg0KPiA+IFNpZ25lZC1vZmYtYnk6IFJveSBaYW5nIDx0aWUtZmVpLnphbmdA
ZnJlZXNjYWxlLmNvbT4NCj4gPiAtLS0NCj4gPiAgZHJpdmVycy9tbWMvaG9zdC9zZGhjaS1vZi1j
b3JlLmMgfCAgICAzICsrKw0KPiA+ICBkcml2ZXJzL21tYy9ob3N0L3NkaGNpLmMgICAgICAgICB8
ICAgIDYgKysrKysrDQo+ID4gIGluY2x1ZGUvbGludXgvbW1jL3NkaGNpLmggICAgICAgIHwgICAg
NCArKysrDQo+ID4gIDMgZmlsZXMgY2hhbmdlZCwgMTMgaW5zZXJ0aW9ucygrKSwgMCBkZWxldGlv
bnMoLSkNCj4gPg0KPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL21tYy9ob3N0L3NkaGNpLW9mLWNv
cmUuYyBiL2RyaXZlcnMvbW1jL2hvc3Qvc2RoY2ktb2YtDQo+IGNvcmUuYw0KPiA+IGluZGV4IGZl
ZGU0M2QuLjliZGQzMGQgMTAwNjQ0DQo+ID4gLS0tIGEvZHJpdmVycy9tbWMvaG9zdC9zZGhjaS1v
Zi1jb3JlLmMNCj4gPiArKysgYi9kcml2ZXJzL21tYy9ob3N0L3NkaGNpLW9mLWNvcmUuYw0KPiA+
IEBAIC0xODIsNiArMTgyLDkgQEAgc3RhdGljIGludCBfX2RldmluaXQgc2RoY2lfb2ZfcHJvYmUo
c3RydWN0DQo+IHBsYXRmb3JtX2RldmljZSAqb2ZkZXYpDQo+ID4gIAlpZiAob2ZfZGV2aWNlX2lz
X2NvbXBhdGlibGUobnAsICJmc2wsZXNkaGMiKSkNCj4gPiAgCQlob3N0LT5xdWlya3MgfD0gU0RI
Q0lfUVVJUktfUU9SSVFfUFJPQ1RMX1dFSVJEOw0KPiA+DQo+ID4gKwlpZiAob2ZfZGV2aWNlX2lz
X2NvbXBhdGlibGUobnAsICJmc2wscDQwODAtZXNkaGMiKSkNCj4gPiArCQlob3N0LT5xdWlya3Mg
fD0gU0RIQ0lfUVVJUktfUU9SSVFfSE9TVENBUEJMVF9PTkxZX1ZTMzM7DQo+IA0KPiBTaG91bGQg
cmVhbGx5IHVzZSB2b2x0YWdlLXJhbmdlcywgbm90IHF1aXJrcy4NCj4gDQo+IGh0dHA6Ly93d3cu
c3Bpbmljcy5uZXQvbGlzdHMvbGludXgtbW1jL21zZzAyNzg1Lmh0bWwNCk1ha2Ugc2Vuc2UgdG8g
bWUuIFRoYW5rcy4NClJveQ0K

RE: [PATCH 2/3] eSDHC: Fix errors when booting kernel with fsl esdhc

[Zang Roy-R61911]

> -----Original Message-----
> From: S, Venkatraman [mailto:svenkatr@ti.com]
> Sent: Tuesday, July 05, 2011 14:17 PM
> To: Zang Roy-R61911
> Cc: linux-mmc; linuxppc-dev; cbouatmailru; akpm; Xu Lei-B33228; Kumar Gala
> Subject: Re: [PATCH 2/3] eSDHC: Fix errors when booting kernel with fsl esdhc
> 
> On Tue, Jul 5, 2011 at 9:49 AM, Roy Zang <tie-fei.zang@freescale.com> wrote:
> > From: Xu lei <B33228@freescale.com>
> >
> > When esdhc module was enabled in p5020, there were following errors:
> >
> > mmc0: Timeout waiting for hardware interrupt.
> > mmc0: error -110 whilst initialising SD card
> > mmc0: Unexpected interrupt 0x02000000.
> > mmc0: Timeout waiting for hardware interrupt.
> > mmc0: error -110 whilst initialising SD card
> > mmc0: Unexpected interrupt 0x02000000.
> >
> > It is because ESDHC controller has different bit setting for PROCTL
> > register, when kernel sets Power Control Register by method for standard
> > SD Host Specification, it would overwritten FSL ESDHC PROCTL[DMAS];
> > when it set Host Control Registers[DMAS], it sets PROCTL[EMODE] and
> > PROCTL[D3CD]. These operations will set bad bits for PROCTL Register
> > on FSL ESDHC Controller and cause errors, so this patch will make esdhc
> > driver access FSL PROCTL Register according to block guide instead of
> > standard SD Host Specification.
> >
> > For some FSL chips, such as MPC8536/P2020, PROCTL[VOLT_SEL] and PROCTL[DMAS]
> > bits are reserved and even if they are set to wrong bits there is no error.
> > But considering that all FSL ESDHC Controller register map is not fully
> > compliant to standard SD Host Specification, we put the patch to all of
> > FSL ESDHC Controllers.
> >
> > Signed-off-by: Lei Xu <B33228@freescale.com>
> > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> > Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> > ---
> >  drivers/mmc/host/sdhci-of-core.c |    3 ++
> >  drivers/mmc/host/sdhci.c         |   62 ++++++++++++++++++++++++++++++-----
> --
> >  include/linux/mmc/sdhci.h        |    6 ++-
> >  3 files changed, 57 insertions(+), 14 deletions(-)
[snip]

> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index 58d5436..77174e5 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -674,7 +674,7 @@ static void sdhci_set_transfer_irqs(struct sdhci_host
> *host)
> >  static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command
> *cmd)
> >  {
> >        u8 count;
> > -       u8 ctrl;
> > +       u32 ctrl;
> >        struct mmc_data *data = cmd->data;
> >        int ret;
> >
> > @@ -807,14 +807,28 @@ static void sdhci_prepare_data(struct sdhci_host *host,
> struct mmc_command *cmd)
> >         * is ADMA.
> >         */
> >        if (host->version >= SDHCI_SPEC_200) {
> > -               ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
> > -               ctrl &= ~SDHCI_CTRL_DMA_MASK;
> > -               if ((host->flags & SDHCI_REQ_USE_DMA) &&
> > -                       (host->flags & SDHCI_USE_ADMA))
> > -                       ctrl |= SDHCI_CTRL_ADMA32;
> > -               else
> > -                       ctrl |= SDHCI_CTRL_SDMA;
> > -               sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
> > +               if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD) {
> > +#define ESDHCI_PROCTL_DMAS_MASK                0x00000300
> > +#define ESDHCI_PROCTL_ADMA32           0x00000200
> > +#define ESDHCI_PROCTL_SDMA             0x00000000
> 
> Breaks the code flow / readability. Can be moved to top of the file ?
The defines are only used in the following section. Why it will break
the readability?
I can also see this kind of define in the file
...
#define SAMPLE_COUNT    5

static int sdhci_get_ro(struct mmc_host *mmc)
...

Any rule should follow?


[snip]
> > @@ -1162,6 +1189,17 @@ static void sdhci_set_power(struct sdhci_host *host,
> unsigned short power)
> >
> >        host->pwr = pwr;
> >
> > +       /* Now FSL ESDHC Controller has no Bus Power bit,
> > +        * and PROCTL[21] bit is for voltage selection */
> 
> Multiline comment style needed..
Will update.
please help to explain your previous comment.
Thanks.
Roy

RE: [PATCH 2/3] eSDHC: Fix errors when booting kernel with fsl esdhc

[Zang Roy-R61911]

> -----Original Message-----
> From: S, Venkatraman [mailto:svenkatr@ti.com]
> Sent: Tuesday, July 05, 2011 14:17 PM
> To: Zang Roy-R61911
> Cc: linux-mmc; linuxppc-dev; cbouatmailru; akpm; Xu Lei-B33228; Kumar Gal=
a
> Subject: Re: [PATCH 2/3] eSDHC: Fix errors when booting kernel with fsl e=
sdhc
>=20
> On Tue, Jul 5, 2011 at 9:49 AM, Roy Zang <tie-fei.zang@freescale.com> wro=
te:
> > From: Xu lei <B33228@freescale.com>
> >
> > When esdhc module was enabled in p5020, there were following errors:
> >
> > mmc0: Timeout waiting for hardware interrupt.
> > mmc0: error -110 whilst initialising SD card
> > mmc0: Unexpected interrupt 0x02000000.
> > mmc0: Timeout waiting for hardware interrupt.
> > mmc0: error -110 whilst initialising SD card
> > mmc0: Unexpected interrupt 0x02000000.
> >
> > It is because ESDHC controller has different bit setting for PROCTL
> > register, when kernel sets Power Control Register by method for standar=
d
> > SD Host Specification, it would overwritten FSL ESDHC PROCTL[DMAS];
> > when it set Host Control Registers[DMAS], it sets PROCTL[EMODE] and
> > PROCTL[D3CD]. These operations will set bad bits for PROCTL Register
> > on FSL ESDHC Controller and cause errors, so this patch will make esdhc
> > driver access FSL PROCTL Register according to block guide instead of
> > standard SD Host Specification.
> >
> > For some FSL chips, such as MPC8536/P2020, PROCTL[VOLT_SEL] and PROCTL[=
DMAS]
> > bits are reserved and even if they are set to wrong bits there is no er=
ror.
> > But considering that all FSL ESDHC Controller register map is not fully
> > compliant to standard SD Host Specification, we put the patch to all of
> > FSL ESDHC Controllers.
> >
> > Signed-off-by: Lei Xu <B33228@freescale.com>
> > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> > Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> > ---
> > =A0drivers/mmc/host/sdhci-of-core.c | =A0 =A03 ++
> > =A0drivers/mmc/host/sdhci.c =A0 =A0 =A0 =A0 | =A0 62 ++++++++++++++++++=
++++++++++++-----
> --
> > =A0include/linux/mmc/sdhci.h =A0 =A0 =A0 =A0| =A0 =A06 ++-
> > =A03 files changed, 57 insertions(+), 14 deletions(-)
[snip]

> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index 58d5436..77174e5 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -674,7 +674,7 @@ static void sdhci_set_transfer_irqs(struct sdhci_ho=
st
> *host)
> > =A0static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_c=
ommand
> *cmd)
> > =A0{
> > =A0 =A0 =A0 =A0u8 count;
> > - =A0 =A0 =A0 u8 ctrl;
> > + =A0 =A0 =A0 u32 ctrl;
> > =A0 =A0 =A0 =A0struct mmc_data *data =3D cmd->data;
> > =A0 =A0 =A0 =A0int ret;
> >
> > @@ -807,14 +807,28 @@ static void sdhci_prepare_data(struct sdhci_host =
*host,
> struct mmc_command *cmd)
> > =A0 =A0 =A0 =A0 * is ADMA.
> > =A0 =A0 =A0 =A0 */
> > =A0 =A0 =A0 =A0if (host->version >=3D SDHCI_SPEC_200) {
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl =3D sdhci_readb(host, SDHCI_HOST_CON=
TROL);
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl &=3D ~SDHCI_CTRL_DMA_MASK;
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if ((host->flags & SDHCI_REQ_USE_DMA) &&
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (host->flags & SDHCI_USE_=
ADMA))
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl |=3D SDHCI_CTRL_ADMA=
32;
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl |=3D SDHCI_CTRL_SDMA=
;
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 sdhci_writeb(host, ctrl, SDHCI_HOST_CONTR=
OL);
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (host->quirks & SDHCI_QUIRK_QORIQ_PROC=
TL_WEIRD) {
> > +#define ESDHCI_PROCTL_DMAS_MASK =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x00000=
300
> > +#define ESDHCI_PROCTL_ADMA32 =A0 =A0 =A0 =A0 =A0 0x00000200
> > +#define ESDHCI_PROCTL_SDMA =A0 =A0 =A0 =A0 =A0 =A0 0x00000000
>=20
> Breaks the code flow / readability. Can be moved to top of the file ?
The defines are only used in the following section. Why it will break
the readability?
I can also see this kind of define in the file
...
#define SAMPLE_COUNT    5

static int sdhci_get_ro(struct mmc_host *mmc)
...

Any rule should follow?


[snip]
> > @@ -1162,6 +1189,17 @@ static void sdhci_set_power(struct sdhci_host *h=
ost,
> unsigned short power)
> >
> > =A0 =A0 =A0 =A0host->pwr =3D pwr;
> >
> > + =A0 =A0 =A0 /* Now FSL ESDHC Controller has no Bus Power bit,
> > + =A0 =A0 =A0 =A0* and PROCTL[21] bit is for voltage selection */
>=20
> Multiline comment style needed..
Will update.
please help to explain your previous comment.
Thanks.
Roy

Re: [PATCH 2/3] eSDHC: Fix errors when booting kernel with fsl esdhc

[S, Venkatraman]

On Mon, Jul 18, 2011 at 11:31 AM, Zang Roy-R61911 <r61911@freescale.com> wrote:
>
>
>> -----Original Message-----
>> From: S, Venkatraman [mailto:svenkatr@ti.com]
>> Sent: Tuesday, July 05, 2011 14:17 PM
>> To: Zang Roy-R61911
>> Cc: linux-mmc; linuxppc-dev; cbouatmailru; akpm; Xu Lei-B33228; Kumar Gala
>> Subject: Re: [PATCH 2/3] eSDHC: Fix errors when booting kernel with fsl esdhc
>>
>> On Tue, Jul 5, 2011 at 9:49 AM, Roy Zang <tie-fei.zang@freescale.com> wrote:
>> > From: Xu lei <B33228@freescale.com>
>> >
>> > When esdhc module was enabled in p5020, there were following errors:
>> >
>> > mmc0: Timeout waiting for hardware interrupt.
>> > mmc0: error -110 whilst initialising SD card
>> > mmc0: Unexpected interrupt 0x02000000.
>> > mmc0: Timeout waiting for hardware interrupt.
>> > mmc0: error -110 whilst initialising SD card
>> > mmc0: Unexpected interrupt 0x02000000.
>> >
>> > It is because ESDHC controller has different bit setting for PROCTL
>> > register, when kernel sets Power Control Register by method for standard
>> > SD Host Specification, it would overwritten FSL ESDHC PROCTL[DMAS];
>> > when it set Host Control Registers[DMAS], it sets PROCTL[EMODE] and
>> > PROCTL[D3CD]. These operations will set bad bits for PROCTL Register
>> > on FSL ESDHC Controller and cause errors, so this patch will make esdhc
>> > driver access FSL PROCTL Register according to block guide instead of
>> > standard SD Host Specification.
>> >
>> > For some FSL chips, such as MPC8536/P2020, PROCTL[VOLT_SEL] and PROCTL[DMAS]
>> > bits are reserved and even if they are set to wrong bits there is no error.
>> > But considering that all FSL ESDHC Controller register map is not fully
>> > compliant to standard SD Host Specification, we put the patch to all of
>> > FSL ESDHC Controllers.
>> >
>> > Signed-off-by: Lei Xu <B33228@freescale.com>
>> > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
>> > Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>> > ---
>> >  drivers/mmc/host/sdhci-of-core.c |    3 ++
>> >  drivers/mmc/host/sdhci.c         |   62 ++++++++++++++++++++++++++++++-----
>> --
>> >  include/linux/mmc/sdhci.h        |    6 ++-
>> >  3 files changed, 57 insertions(+), 14 deletions(-)
> [snip]
>
>> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> > index 58d5436..77174e5 100644
>> > --- a/drivers/mmc/host/sdhci.c
>> > +++ b/drivers/mmc/host/sdhci.c
>> > @@ -674,7 +674,7 @@ static void sdhci_set_transfer_irqs(struct sdhci_host
>> *host)
>> >  static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command
>> *cmd)
>> >  {
>> >        u8 count;
>> > -       u8 ctrl;
>> > +       u32 ctrl;
>> >        struct mmc_data *data = cmd->data;
>> >        int ret;
>> >
>> > @@ -807,14 +807,28 @@ static void sdhci_prepare_data(struct sdhci_host *host,
>> struct mmc_command *cmd)
>> >         * is ADMA.
>> >         */
>> >        if (host->version >= SDHCI_SPEC_200) {
>> > -               ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
>> > -               ctrl &= ~SDHCI_CTRL_DMA_MASK;
>> > -               if ((host->flags & SDHCI_REQ_USE_DMA) &&
>> > -                       (host->flags & SDHCI_USE_ADMA))
>> > -                       ctrl |= SDHCI_CTRL_ADMA32;
>> > -               else
>> > -                       ctrl |= SDHCI_CTRL_SDMA;
>> > -               sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>> > +               if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD) {
>> > +#define ESDHCI_PROCTL_DMAS_MASK                0x00000300
>> > +#define ESDHCI_PROCTL_ADMA32           0x00000200
>> > +#define ESDHCI_PROCTL_SDMA             0x00000000
>>
>> Breaks the code flow / readability. Can be moved to top of the file ?
> The defines are only used in the following section. Why it will break
> the readability?
> I can also see this kind of define in the file
> ...
> #define SAMPLE_COUNT    5
>
> static int sdhci_get_ro(struct mmc_host *mmc)
> ...
>
> Any rule should follow?
>
>
> [snip]
>> > @@ -1162,6 +1189,17 @@ static void sdhci_set_power(struct sdhci_host *host,
>> unsigned short power)
>> >
>> >        host->pwr = pwr;
>> >
>> > +       /* Now FSL ESDHC Controller has no Bus Power bit,
>> > +        * and PROCTL[21] bit is for voltage selection */
>>
>> Multiline comment style needed..
> Will update.
> please help to explain your previous comment.
> Thanks.
> Roy

There aren't very hard rules on this. Simple #defines are good, as a
one off usage.
These bit mask fields are very verbose, and they tend to grow more
than a screenful.
The remaining bits will never be defined ?

RE: [PATCH 2/3] eSDHC: Fix errors when booting kernel with fsl esdhc

[Zang Roy-R61911]

> -----Original Message-----
> From: S, Venkatraman [mailto:svenkatr@ti.com]
> Sent: Tuesday, July 19, 2011 23:58 PM
> To: Zang Roy-R61911
> Cc: linux-mmc; linuxppc-dev; cbouatmailru; akpm; Xu Lei-B33228; Kumar Gala
> Subject: Re: [PATCH 2/3] eSDHC: Fix errors when booting kernel with fsl esdhc
> 
> On Mon, Jul 18, 2011 at 11:31 AM, Zang Roy-R61911 <r61911@freescale.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: S, Venkatraman [mailto:svenkatr@ti.com]
> >> Sent: Tuesday, July 05, 2011 14:17 PM
> >> To: Zang Roy-R61911
> >> Cc: linux-mmc; linuxppc-dev; cbouatmailru; akpm; Xu Lei-B33228; Kumar Gala
> >> Subject: Re: [PATCH 2/3] eSDHC: Fix errors when booting kernel with fsl esdhc
> >>
> >> On Tue, Jul 5, 2011 at 9:49 AM, Roy Zang <tie-fei.zang@freescale.com> wrote:
> >> > From: Xu lei <B33228@freescale.com>
> >> >
> >> > When esdhc module was enabled in p5020, there were following errors:
> >> >
> >> > mmc0: Timeout waiting for hardware interrupt.
> >> > mmc0: error -110 whilst initialising SD card
> >> > mmc0: Unexpected interrupt 0x02000000.
> >> > mmc0: Timeout waiting for hardware interrupt.
> >> > mmc0: error -110 whilst initialising SD card
> >> > mmc0: Unexpected interrupt 0x02000000.
> >> >
> >> > It is because ESDHC controller has different bit setting for PROCTL
> >> > register, when kernel sets Power Control Register by method for standard
> >> > SD Host Specification, it would overwritten FSL ESDHC PROCTL[DMAS];
> >> > when it set Host Control Registers[DMAS], it sets PROCTL[EMODE] and
> >> > PROCTL[D3CD]. These operations will set bad bits for PROCTL Register
> >> > on FSL ESDHC Controller and cause errors, so this patch will make esdhc
> >> > driver access FSL PROCTL Register according to block guide instead of
> >> > standard SD Host Specification.
> >> >
> >> > For some FSL chips, such as MPC8536/P2020, PROCTL[VOLT_SEL] and PROCTL[DMAS]
> >> > bits are reserved and even if they are set to wrong bits there is no error.
> >> > But considering that all FSL ESDHC Controller register map is not fully
> >> > compliant to standard SD Host Specification, we put the patch to all of
> >> > FSL ESDHC Controllers.
> >> >
> >> > Signed-off-by: Lei Xu <B33228@freescale.com>
> >> > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> >> > Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> >> > ---
> >> >  drivers/mmc/host/sdhci-of-core.c |    3 ++
> >> >  drivers/mmc/host/sdhci.c         |   62 ++++++++++++++++++++++++++++++----
> -
> >> --
> >> >  include/linux/mmc/sdhci.h        |    6 ++-
> >> >  3 files changed, 57 insertions(+), 14 deletions(-)
> > [snip]
> >
> >> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> >> > index 58d5436..77174e5 100644
> >> > --- a/drivers/mmc/host/sdhci.c
> >> > +++ b/drivers/mmc/host/sdhci.c
> >> > @@ -674,7 +674,7 @@ static void sdhci_set_transfer_irqs(struct sdhci_host
> >> *host)
> >> >  static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command
> >> *cmd)
> >> >  {
> >> >        u8 count;
> >> > -       u8 ctrl;
> >> > +       u32 ctrl;
> >> >        struct mmc_data *data = cmd->data;
> >> >        int ret;
> >> >
> >> > @@ -807,14 +807,28 @@ static void sdhci_prepare_data(struct sdhci_host
> *host,
> >> struct mmc_command *cmd)
> >> >         * is ADMA.
> >> >         */
> >> >        if (host->version >= SDHCI_SPEC_200) {
> >> > -               ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
> >> > -               ctrl &= ~SDHCI_CTRL_DMA_MASK;
> >> > -               if ((host->flags & SDHCI_REQ_USE_DMA) &&
> >> > -                       (host->flags & SDHCI_USE_ADMA))
> >> > -                       ctrl |= SDHCI_CTRL_ADMA32;
> >> > -               else
> >> > -                       ctrl |= SDHCI_CTRL_SDMA;
> >> > -               sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
> >> > +               if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD) {
> >> > +#define ESDHCI_PROCTL_DMAS_MASK                0x00000300
> >> > +#define ESDHCI_PROCTL_ADMA32           0x00000200
> >> > +#define ESDHCI_PROCTL_SDMA             0x00000000
> >>
> >> Breaks the code flow / readability. Can be moved to top of the file ?
> > The defines are only used in the following section. Why it will break
> > the readability?
> > I can also see this kind of define in the file
> > ...
> > #define SAMPLE_COUNT    5
> >
> > static int sdhci_get_ro(struct mmc_host *mmc)
> > ...
> >
> > Any rule should follow?
> >
> >
> > [snip]
> >> > @@ -1162,6 +1189,17 @@ static void sdhci_set_power(struct sdhci_host *host,
> >> unsigned short power)
> >> >
> >> >        host->pwr = pwr;
> >> >
> >> > +       /* Now FSL ESDHC Controller has no Bus Power bit,
> >> > +        * and PROCTL[21] bit is for voltage selection */
> >>
> >> Multiline comment style needed..
> > Will update.
> > please help to explain your previous comment.
> > Thanks.
> > Roy
> 
> There aren't very hard rules on this. Simple #defines are good, as a
> one off usage.
> These bit mask fields are very verbose, and they tend to grow more
> than a screenful.
> The remaining bits will never be defined ?
The mask fields and bits are only for some WERID defines. I do not think
they will grow more than a screen.
The remain bits will have little chance to be defined.
Thanks for the suggestion. I will update the comment style and post again.
Roy

RE: [PATCH 2/3] eSDHC: Fix errors when booting kernel with fsl esdhc

[Zang Roy-R61911]

> -----Original Message-----
> From: S, Venkatraman [mailto:svenkatr@ti.com]
> Sent: Tuesday, July 19, 2011 23:58 PM
> To: Zang Roy-R61911
> Cc: linux-mmc; linuxppc-dev; cbouatmailru; akpm; Xu Lei-B33228; Kumar Gal=
a
> Subject: Re: [PATCH 2/3] eSDHC: Fix errors when booting kernel with fsl e=
sdhc
>=20
> On Mon, Jul 18, 2011 at 11:31 AM, Zang Roy-R61911 <r61911@freescale.com> =
wrote:
> >
> >
> >> -----Original Message-----
> >> From: S, Venkatraman [mailto:svenkatr@ti.com]
> >> Sent: Tuesday, July 05, 2011 14:17 PM
> >> To: Zang Roy-R61911
> >> Cc: linux-mmc; linuxppc-dev; cbouatmailru; akpm; Xu Lei-B33228; Kumar =
Gala
> >> Subject: Re: [PATCH 2/3] eSDHC: Fix errors when booting kernel with fs=
l esdhc
> >>
> >> On Tue, Jul 5, 2011 at 9:49 AM, Roy Zang <tie-fei.zang@freescale.com> =
wrote:
> >> > From: Xu lei <B33228@freescale.com>
> >> >
> >> > When esdhc module was enabled in p5020, there were following errors:
> >> >
> >> > mmc0: Timeout waiting for hardware interrupt.
> >> > mmc0: error -110 whilst initialising SD card
> >> > mmc0: Unexpected interrupt 0x02000000.
> >> > mmc0: Timeout waiting for hardware interrupt.
> >> > mmc0: error -110 whilst initialising SD card
> >> > mmc0: Unexpected interrupt 0x02000000.
> >> >
> >> > It is because ESDHC controller has different bit setting for PROCTL
> >> > register, when kernel sets Power Control Register by method for stan=
dard
> >> > SD Host Specification, it would overwritten FSL ESDHC PROCTL[DMAS];
> >> > when it set Host Control Registers[DMAS], it sets PROCTL[EMODE] and
> >> > PROCTL[D3CD]. These operations will set bad bits for PROCTL Register
> >> > on FSL ESDHC Controller and cause errors, so this patch will make es=
dhc
> >> > driver access FSL PROCTL Register according to block guide instead o=
f
> >> > standard SD Host Specification.
> >> >
> >> > For some FSL chips, such as MPC8536/P2020, PROCTL[VOLT_SEL] and PROC=
TL[DMAS]
> >> > bits are reserved and even if they are set to wrong bits there is no=
 error.
> >> > But considering that all FSL ESDHC Controller register map is not fu=
lly
> >> > compliant to standard SD Host Specification, we put the patch to all=
 of
> >> > FSL ESDHC Controllers.
> >> >
> >> > Signed-off-by: Lei Xu <B33228@freescale.com>
> >> > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> >> > Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> >> > ---
> >> > =A0drivers/mmc/host/sdhci-of-core.c | =A0 =A03 ++
> >> > =A0drivers/mmc/host/sdhci.c =A0 =A0 =A0 =A0 | =A0 62 +++++++++++++++=
+++++++++++++++----
> -
> >> --
> >> > =A0include/linux/mmc/sdhci.h =A0 =A0 =A0 =A0| =A0 =A06 ++-
> >> > =A03 files changed, 57 insertions(+), 14 deletions(-)
> > [snip]
> >
> >> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> >> > index 58d5436..77174e5 100644
> >> > --- a/drivers/mmc/host/sdhci.c
> >> > +++ b/drivers/mmc/host/sdhci.c
> >> > @@ -674,7 +674,7 @@ static void sdhci_set_transfer_irqs(struct sdhci=
_host
> >> *host)
> >> > =A0static void sdhci_prepare_data(struct sdhci_host *host, struct mm=
c_command
> >> *cmd)
> >> > =A0{
> >> > =A0 =A0 =A0 =A0u8 count;
> >> > - =A0 =A0 =A0 u8 ctrl;
> >> > + =A0 =A0 =A0 u32 ctrl;
> >> > =A0 =A0 =A0 =A0struct mmc_data *data =3D cmd->data;
> >> > =A0 =A0 =A0 =A0int ret;
> >> >
> >> > @@ -807,14 +807,28 @@ static void sdhci_prepare_data(struct sdhci_ho=
st
> *host,
> >> struct mmc_command *cmd)
> >> > =A0 =A0 =A0 =A0 * is ADMA.
> >> > =A0 =A0 =A0 =A0 */
> >> > =A0 =A0 =A0 =A0if (host->version >=3D SDHCI_SPEC_200) {
> >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl =3D sdhci_readb(host, SDHCI_HOST_=
CONTROL);
> >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl &=3D ~SDHCI_CTRL_DMA_MASK;
> >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if ((host->flags & SDHCI_REQ_USE_DMA) =
&&
> >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (host->flags & SDHCI_U=
SE_ADMA))
> >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl |=3D SDHCI_CTRL_A=
DMA32;
> >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ctrl |=3D SDHCI_CTRL_S=
DMA;
> >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 sdhci_writeb(host, ctrl, SDHCI_HOST_CO=
NTROL);
> >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (host->quirks & SDHCI_QUIRK_QORIQ_P=
ROCTL_WEIRD) {
> >> > +#define ESDHCI_PROCTL_DMAS_MASK =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x00=
000300
> >> > +#define ESDHCI_PROCTL_ADMA32 =A0 =A0 =A0 =A0 =A0 0x00000200
> >> > +#define ESDHCI_PROCTL_SDMA =A0 =A0 =A0 =A0 =A0 =A0 0x00000000
> >>
> >> Breaks the code flow / readability. Can be moved to top of the file ?
> > The defines are only used in the following section. Why it will break
> > the readability?
> > I can also see this kind of define in the file
> > ...
> > #define SAMPLE_COUNT =A0 =A05
> >
> > static int sdhci_get_ro(struct mmc_host *mmc)
> > ...
> >
> > Any rule should follow?
> >
> >
> > [snip]
> >> > @@ -1162,6 +1189,17 @@ static void sdhci_set_power(struct sdhci_host=
 *host,
> >> unsigned short power)
> >> >
> >> > =A0 =A0 =A0 =A0host->pwr =3D pwr;
> >> >
> >> > + =A0 =A0 =A0 /* Now FSL ESDHC Controller has no Bus Power bit,
> >> > + =A0 =A0 =A0 =A0* and PROCTL[21] bit is for voltage selection */
> >>
> >> Multiline comment style needed..
> > Will update.
> > please help to explain your previous comment.
> > Thanks.
> > Roy
>=20
> There aren't very hard rules on this. Simple #defines are good, as a
> one off usage.
> These bit mask fields are very verbose, and they tend to grow more
> than a screenful.
> The remaining bits will never be defined ?
The mask fields and bits are only for some WERID defines. I do not think
they will grow more than a screen.
The remain bits will have little chance to be defined.
Thanks for the suggestion. I will update the comment style and post again.
Roy