[PATCH V3 1/4] ARM: imx53_loco: add esdhc device support

[PATCH V3 1/4] ARM: imx53_loco: add esdhc device support

[Richard Zhu]

Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
 arch/arm/mach-mx5/Kconfig           |    1 +
 arch/arm/mach-mx5/board-mx53_loco.c |    2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig
index f065a0d..a72c833 100644
--- a/arch/arm/mach-mx5/Kconfig
+++ b/arch/arm/mach-mx5/Kconfig
@@ -162,6 +162,7 @@ config MACH_MX53_LOCO
 	select IMX_HAVE_PLATFORM_IMX2_WDT
 	select IMX_HAVE_PLATFORM_IMX_I2C
 	select IMX_HAVE_PLATFORM_IMX_UART
+	select IMX_HAVE_PLATFORM_SDHCI_ESDHC_IMX
 	help
 	  Include support for MX53 LOCO platform. This includes specific
 	  configurations for the board and its peripherals.
diff --git a/arch/arm/mach-mx5/board-mx53_loco.c b/arch/arm/mach-mx5/board-mx53_loco.c
index 160899e..0a18f8d 100644
--- a/arch/arm/mach-mx5/board-mx53_loco.c
+++ b/arch/arm/mach-mx5/board-mx53_loco.c
@@ -213,6 +213,8 @@ static void __init mx53_loco_board_init(void)
 	imx53_add_imx2_wdt(0, NULL);
 	imx53_add_imx_i2c(0, &mx53_loco_i2c_data);
 	imx53_add_imx_i2c(1, &mx53_loco_i2c_data);
+	imx53_add_sdhci_esdhc_imx(0, NULL);
+	imx53_add_sdhci_esdhc_imx(2, NULL);
 }
 
 static void __init mx53_loco_timer_init(void)
-- 
1.7.1

[PATCH V3 1/4] ARM: imx53_loco: add esdhc device support

[Richard Zhu]

Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
 arch/arm/mach-mx5/Kconfig           |    1 +
 arch/arm/mach-mx5/board-mx53_loco.c |    2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig
index f065a0d..a72c833 100644
--- a/arch/arm/mach-mx5/Kconfig
+++ b/arch/arm/mach-mx5/Kconfig
@@ -162,6 +162,7 @@ config MACH_MX53_LOCO
 	select IMX_HAVE_PLATFORM_IMX2_WDT
 	select IMX_HAVE_PLATFORM_IMX_I2C
 	select IMX_HAVE_PLATFORM_IMX_UART
+	select IMX_HAVE_PLATFORM_SDHCI_ESDHC_IMX
 	help
 	  Include support for MX53 LOCO platform. This includes specific
 	  configurations for the board and its peripherals.
diff --git a/arch/arm/mach-mx5/board-mx53_loco.c b/arch/arm/mach-mx5/board-mx53_loco.c
index 160899e..0a18f8d 100644
--- a/arch/arm/mach-mx5/board-mx53_loco.c
+++ b/arch/arm/mach-mx5/board-mx53_loco.c
@@ -213,6 +213,8 @@ static void __init mx53_loco_board_init(void)
 	imx53_add_imx2_wdt(0, NULL);
 	imx53_add_imx_i2c(0, &mx53_loco_i2c_data);
 	imx53_add_imx_i2c(1, &mx53_loco_i2c_data);
+	imx53_add_sdhci_esdhc_imx(0, NULL);
+	imx53_add_sdhci_esdhc_imx(2, NULL);
 }
 
 static void __init mx53_loco_timer_init(void)
-- 
1.7.1

[PATCH V3 2/4] ARM: imx51/53: add sdhc3/4 clock

[Richard Zhu]

Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
 arch/arm/mach-mx5/clock-mx51-mx53.c |  140 ++++++++++++++++++++++++++++++++++-
 arch/arm/mach-mx5/crm_regs.h        |    7 ++
 2 files changed, 146 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
index 8164b1d..652ace4 100644
--- a/arch/arm/mach-mx5/clock-mx51-mx53.c
+++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
@@ -42,6 +42,9 @@ static struct clk usboh3_clk;
 static struct clk emi_fast_clk;
 static struct clk ipu_clk;
 static struct clk mipi_hsc1_clk;
+static struct clk esdhc1_clk;
+static struct clk esdhc2_clk;
+static struct clk esdhc3_mx53_clk;
 
 #define MAX_DPLL_WAIT_TRIES	1000 /* 1000 * udelay(1) = 1ms */
 
@@ -1143,10 +1146,80 @@ CLK_GET_RATE(esdhc1, 1, ESDHC1_MSHC1)
 CLK_SET_PARENT(esdhc1, 1, ESDHC1_MSHC1)
 CLK_SET_RATE(esdhc1, 1, ESDHC1_MSHC1)
 
+/* mx51 specific */
 CLK_GET_RATE(esdhc2, 1, ESDHC2_MSHC2)
 CLK_SET_PARENT(esdhc2, 1, ESDHC2_MSHC2)
 CLK_SET_RATE(esdhc2, 1, ESDHC2_MSHC2)
 
+static int clk_esdhc3_set_parent(struct clk *clk, struct clk *parent)
+{
+	u32 reg;
+
+	reg = __raw_readl(MXC_CCM_CSCMR1);
+	if (parent == &esdhc1_clk)
+		reg &= ~MXC_CCM_CSCMR1_ESDHC3_CLK_SEL;
+	else if (parent == &esdhc2_clk)
+		reg |= MXC_CCM_CSCMR1_ESDHC3_CLK_SEL;
+	else
+		return -EINVAL;
+	__raw_writel(reg, MXC_CCM_CSCMR1);
+
+	return 0;
+}
+
+static int clk_esdhc4_set_parent(struct clk *clk, struct clk *parent)
+{
+	u32 reg;
+
+	reg = __raw_readl(MXC_CCM_CSCMR1);
+	if (parent == &esdhc1_clk)
+		reg &= ~MXC_CCM_CSCMR1_ESDHC4_CLK_SEL;
+	else if (parent == &esdhc2_clk)
+		reg |= MXC_CCM_CSCMR1_ESDHC4_CLK_SEL;
+	else
+		return -EINVAL;
+	__raw_writel(reg, MXC_CCM_CSCMR1);
+
+	return 0;
+}
+
+/* mx53 specific */
+static int clk_esdhc2_mx53_set_parent(struct clk *clk, struct clk *parent)
+{
+	u32 reg;
+
+	reg = __raw_readl(MXC_CCM_CSCMR1);
+	if (parent == &esdhc1_clk)
+		reg &= ~MXC_CCM_CSCMR1_ESDHC2_MSHC2_MX53_CLK_SEL;
+	else if (parent == &esdhc3_mx53_clk)
+		reg |= MXC_CCM_CSCMR1_ESDHC2_MSHC2_MX53_CLK_SEL;
+	else
+		return -EINVAL;
+	__raw_writel(reg, MXC_CCM_CSCMR1);
+
+	return 0;
+}
+
+CLK_GET_RATE(esdhc3_mx53, 1, ESDHC3_MX53)
+CLK_SET_PARENT(esdhc3_mx53, 1, ESDHC3_MX53)
+CLK_SET_RATE(esdhc3_mx53, 1, ESDHC3_MX53)
+
+static int clk_esdhc4_mx53_set_parent(struct clk *clk, struct clk *parent)
+{
+	u32 reg;
+
+	reg = __raw_readl(MXC_CCM_CSCMR1);
+	if (parent == &esdhc1_clk)
+		reg &= ~MXC_CCM_CSCMR1_ESDHC4_CLK_SEL;
+	else if (parent == &esdhc3_mx53_clk)
+		reg |= MXC_CCM_CSCMR1_ESDHC4_CLK_SEL;
+	else
+		return -EINVAL;
+	__raw_writel(reg, MXC_CCM_CSCMR1);
+
+	return 0;
+}
+
 #define DEFINE_CLOCK_FULL(name, i, er, es, gr, sr, e, d, p, s)		\
 	static struct clk name = {					\
 		.id		= i,					\
@@ -1251,9 +1324,62 @@ DEFINE_CLOCK_MAX(esdhc1_clk, 0, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG1_OFFSET,
 	clk_esdhc1, &pll2_sw_clk, &esdhc1_ipg_clk);
 DEFINE_CLOCK_FULL(esdhc2_ipg_clk, 1, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG2_OFFSET,
 	NULL,  NULL, _clk_max_enable, _clk_max_disable, &ipg_clk, NULL);
+DEFINE_CLOCK_FULL(esdhc3_ipg_clk, 2, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG4_OFFSET,
+	NULL,  NULL, _clk_max_enable, _clk_max_disable, &ipg_clk, NULL);
+DEFINE_CLOCK_FULL(esdhc4_ipg_clk, 3, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG6_OFFSET,
+	NULL,  NULL, _clk_max_enable, _clk_max_disable, &ipg_clk, NULL);
+
+/* mx51 specific */
 DEFINE_CLOCK_MAX(esdhc2_clk, 1, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG3_OFFSET,
 	clk_esdhc2, &pll2_sw_clk, &esdhc2_ipg_clk);
 
+static struct clk esdhc3_clk = {
+	.id = 2,
+	.parent = &esdhc1_clk,
+	.set_parent = clk_esdhc3_set_parent,
+	.enable_reg = MXC_CCM_CCGR3,
+	.enable_shift = MXC_CCM_CCGRx_CG5_OFFSET,
+	.enable  = _clk_max_enable,
+	.disable = _clk_max_disable,
+	.secondary = &esdhc3_ipg_clk,
+};
+static struct clk esdhc4_clk = {
+	.id = 3,
+	.parent = &esdhc1_clk,
+	.set_parent = clk_esdhc4_set_parent,
+	.enable_reg = MXC_CCM_CCGR3,
+	.enable_shift = MXC_CCM_CCGRx_CG7_OFFSET,
+	.enable  = _clk_max_enable,
+	.disable = _clk_max_disable,
+	.secondary = &esdhc4_ipg_clk,
+};
+
+/* mx53 specific */
+static struct clk esdhc2_mx53_clk = {
+	.id = 2,
+	.parent = &esdhc1_clk,
+	.set_parent = clk_esdhc2_mx53_set_parent,
+	.enable_reg = MXC_CCM_CCGR3,
+	.enable_shift = MXC_CCM_CCGRx_CG3_OFFSET,
+	.enable  = _clk_max_enable,
+	.disable = _clk_max_disable,
+	.secondary = &esdhc3_ipg_clk,
+};
+
+DEFINE_CLOCK_MAX(esdhc3_mx53_clk, 2, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG5_OFFSET,
+	clk_esdhc3_mx53, &pll2_sw_clk, &esdhc2_ipg_clk);
+
+static struct clk esdhc4_mx53_clk = {
+	.id = 3,
+	.parent = &esdhc1_clk,
+	.set_parent = clk_esdhc4_mx53_set_parent,
+	.enable_reg = MXC_CCM_CCGR3,
+	.enable_shift = MXC_CCM_CCGRx_CG7_OFFSET,
+	.enable  = _clk_max_enable,
+	.disable = _clk_max_disable,
+	.secondary = &esdhc4_ipg_clk,
+};
+
 DEFINE_CLOCK(mipi_esc_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG5_OFFSET, NULL, NULL, NULL, &pll2_sw_clk);
 DEFINE_CLOCK(mipi_hsc2_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG4_OFFSET, NULL, NULL, &mipi_esc_clk, &pll2_sw_clk);
 DEFINE_CLOCK(mipi_hsc1_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG3_OFFSET, NULL, NULL, &mipi_hsc2_clk, &pll2_sw_clk);
@@ -1312,6 +1438,8 @@ static struct clk_lookup mx51_lookups[] = {
 	_REGISTER_CLOCK("imx51-cspi.0", NULL, cspi_clk)
 	_REGISTER_CLOCK("sdhci-esdhc-imx.0", NULL, esdhc1_clk)
 	_REGISTER_CLOCK("sdhci-esdhc-imx.1", NULL, esdhc2_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx.2", NULL, esdhc3_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx.3", NULL, esdhc4_clk)
 	_REGISTER_CLOCK(NULL, "cpu_clk", cpu_clk)
 	_REGISTER_CLOCK(NULL, "iim_clk", iim_clk)
 	_REGISTER_CLOCK("imx2-wdt.0", NULL, dummy_clk)
@@ -1332,7 +1460,9 @@ static struct clk_lookup mx53_lookups[] = {
 	_REGISTER_CLOCK("imx-i2c.0", NULL, i2c1_clk)
 	_REGISTER_CLOCK("imx-i2c.1", NULL, i2c2_clk)
 	_REGISTER_CLOCK("sdhci-esdhc-imx.0", NULL, esdhc1_clk)
-	_REGISTER_CLOCK("sdhci-esdhc-imx.1", NULL, esdhc2_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx.1", NULL, esdhc2_mx53_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx.2", NULL, esdhc3_mx53_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx.3", NULL, esdhc4_mx53_clk)
 	_REGISTER_CLOCK("imx53-ecspi.0", NULL, ecspi1_clk)
 	_REGISTER_CLOCK("imx53-ecspi.1", NULL, ecspi2_clk)
 	_REGISTER_CLOCK("imx53-cspi.0", NULL, cspi_clk)
@@ -1425,6 +1555,14 @@ int __init mx53_clocks_init(unsigned long ckil, unsigned long osc,
 	mx53_revision();
 	clk_disable(&iim_clk);
 
+	/* Set SDHC parents to be PLL2 */
+	clk_set_parent(&esdhc1_clk, &pll2_sw_clk);
+	clk_set_parent(&esdhc3_mx53_clk, &pll2_sw_clk);
+
+	/* set SDHC root clock as 200MHZ*/
+	clk_set_rate(&esdhc1_clk, 200000000);
+	clk_set_rate(&esdhc3_mx53_clk, 200000000);
+
 	/* System timer */
 	mxc_timer_init(&gpt_clk, MX53_IO_ADDRESS(MX53_GPT1_BASE_ADDR),
 		MX53_INT_GPT);
diff --git a/arch/arm/mach-mx5/crm_regs.h b/arch/arm/mach-mx5/crm_regs.h
index b462c22..87c0c58 100644
--- a/arch/arm/mach-mx5/crm_regs.h
+++ b/arch/arm/mach-mx5/crm_regs.h
@@ -217,9 +217,12 @@
 #define MXC_CCM_CSCMR1_ESDHC1_MSHC1_CLK_SEL_OFFSET	(20)
 #define MXC_CCM_CSCMR1_ESDHC1_MSHC1_CLK_SEL_MASK	(0x3 << 20)
 #define MXC_CCM_CSCMR1_ESDHC3_CLK_SEL			(0x1 << 19)
+#define MXC_CCM_CSCMR1_ESDHC2_MSHC2_MX53_CLK_SEL	(0x1 << 19)
 #define MXC_CCM_CSCMR1_ESDHC4_CLK_SEL			(0x1 << 18)
 #define MXC_CCM_CSCMR1_ESDHC2_MSHC2_CLK_SEL_OFFSET	(16)
 #define MXC_CCM_CSCMR1_ESDHC2_MSHC2_CLK_SEL_MASK	(0x3 << 16)
+#define MXC_CCM_CSCMR1_ESDHC3_MX53_CLK_SEL_OFFSET	(16)
+#define MXC_CCM_CSCMR1_ESDHC3_MX53_CLK_SEL_MASK		(0x3 << 16)
 #define MXC_CCM_CSCMR1_SSI1_CLK_SEL_OFFSET		(14)
 #define MXC_CCM_CSCMR1_SSI1_CLK_SEL_MASK		(0x3 << 14)
 #define MXC_CCM_CSCMR1_SSI2_CLK_SEL_OFFSET		(12)
@@ -271,6 +274,10 @@
 #define MXC_CCM_CSCDR1_ESDHC2_MSHC2_CLK_PRED_MASK	(0x7 << 22)
 #define MXC_CCM_CSCDR1_ESDHC2_MSHC2_CLK_PODF_OFFSET	(19)
 #define MXC_CCM_CSCDR1_ESDHC2_MSHC2_CLK_PODF_MASK	(0x7 << 19)
+#define MXC_CCM_CSCDR1_ESDHC3_MX53_CLK_PRED_OFFSET	(22)
+#define MXC_CCM_CSCDR1_ESDHC3_MX53_CLK_PRED_MASK	(0x7 << 22)
+#define MXC_CCM_CSCDR1_ESDHC3_MX53_CLK_PODF_OFFSET	(19)
+#define MXC_CCM_CSCDR1_ESDHC3_MX53_CLK_PODF_MASK	(0x7 << 19)
 #define MXC_CCM_CSCDR1_ESDHC1_MSHC1_CLK_PRED_OFFSET	(16)
 #define MXC_CCM_CSCDR1_ESDHC1_MSHC1_CLK_PRED_MASK	(0x7 << 16)
 #define MXC_CCM_CSCDR1_PGC_CLK_PODF_OFFSET		(14)
-- 
1.7.1

[PATCH V3 2/4] ARM: imx51/53: add sdhc3/4 clock

[Richard Zhu]

Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
 arch/arm/mach-mx5/clock-mx51-mx53.c |  140 ++++++++++++++++++++++++++++++++++-
 arch/arm/mach-mx5/crm_regs.h        |    7 ++
 2 files changed, 146 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c
index 8164b1d..652ace4 100644
--- a/arch/arm/mach-mx5/clock-mx51-mx53.c
+++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
@@ -42,6 +42,9 @@ static struct clk usboh3_clk;
 static struct clk emi_fast_clk;
 static struct clk ipu_clk;
 static struct clk mipi_hsc1_clk;
+static struct clk esdhc1_clk;
+static struct clk esdhc2_clk;
+static struct clk esdhc3_mx53_clk;
 
 #define MAX_DPLL_WAIT_TRIES	1000 /* 1000 * udelay(1) = 1ms */
 
@@ -1143,10 +1146,80 @@ CLK_GET_RATE(esdhc1, 1, ESDHC1_MSHC1)
 CLK_SET_PARENT(esdhc1, 1, ESDHC1_MSHC1)
 CLK_SET_RATE(esdhc1, 1, ESDHC1_MSHC1)
 
+/* mx51 specific */
 CLK_GET_RATE(esdhc2, 1, ESDHC2_MSHC2)
 CLK_SET_PARENT(esdhc2, 1, ESDHC2_MSHC2)
 CLK_SET_RATE(esdhc2, 1, ESDHC2_MSHC2)
 
+static int clk_esdhc3_set_parent(struct clk *clk, struct clk *parent)
+{
+	u32 reg;
+
+	reg = __raw_readl(MXC_CCM_CSCMR1);
+	if (parent == &esdhc1_clk)
+		reg &= ~MXC_CCM_CSCMR1_ESDHC3_CLK_SEL;
+	else if (parent == &esdhc2_clk)
+		reg |= MXC_CCM_CSCMR1_ESDHC3_CLK_SEL;
+	else
+		return -EINVAL;
+	__raw_writel(reg, MXC_CCM_CSCMR1);
+
+	return 0;
+}
+
+static int clk_esdhc4_set_parent(struct clk *clk, struct clk *parent)
+{
+	u32 reg;
+
+	reg = __raw_readl(MXC_CCM_CSCMR1);
+	if (parent == &esdhc1_clk)
+		reg &= ~MXC_CCM_CSCMR1_ESDHC4_CLK_SEL;
+	else if (parent == &esdhc2_clk)
+		reg |= MXC_CCM_CSCMR1_ESDHC4_CLK_SEL;
+	else
+		return -EINVAL;
+	__raw_writel(reg, MXC_CCM_CSCMR1);
+
+	return 0;
+}
+
+/* mx53 specific */
+static int clk_esdhc2_mx53_set_parent(struct clk *clk, struct clk *parent)
+{
+	u32 reg;
+
+	reg = __raw_readl(MXC_CCM_CSCMR1);
+	if (parent == &esdhc1_clk)
+		reg &= ~MXC_CCM_CSCMR1_ESDHC2_MSHC2_MX53_CLK_SEL;
+	else if (parent == &esdhc3_mx53_clk)
+		reg |= MXC_CCM_CSCMR1_ESDHC2_MSHC2_MX53_CLK_SEL;
+	else
+		return -EINVAL;
+	__raw_writel(reg, MXC_CCM_CSCMR1);
+
+	return 0;
+}
+
+CLK_GET_RATE(esdhc3_mx53, 1, ESDHC3_MX53)
+CLK_SET_PARENT(esdhc3_mx53, 1, ESDHC3_MX53)
+CLK_SET_RATE(esdhc3_mx53, 1, ESDHC3_MX53)
+
+static int clk_esdhc4_mx53_set_parent(struct clk *clk, struct clk *parent)
+{
+	u32 reg;
+
+	reg = __raw_readl(MXC_CCM_CSCMR1);
+	if (parent == &esdhc1_clk)
+		reg &= ~MXC_CCM_CSCMR1_ESDHC4_CLK_SEL;
+	else if (parent == &esdhc3_mx53_clk)
+		reg |= MXC_CCM_CSCMR1_ESDHC4_CLK_SEL;
+	else
+		return -EINVAL;
+	__raw_writel(reg, MXC_CCM_CSCMR1);
+
+	return 0;
+}
+
 #define DEFINE_CLOCK_FULL(name, i, er, es, gr, sr, e, d, p, s)		\
 	static struct clk name = {					\
 		.id		= i,					\
@@ -1251,9 +1324,62 @@ DEFINE_CLOCK_MAX(esdhc1_clk, 0, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG1_OFFSET,
 	clk_esdhc1, &pll2_sw_clk, &esdhc1_ipg_clk);
 DEFINE_CLOCK_FULL(esdhc2_ipg_clk, 1, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG2_OFFSET,
 	NULL,  NULL, _clk_max_enable, _clk_max_disable, &ipg_clk, NULL);
+DEFINE_CLOCK_FULL(esdhc3_ipg_clk, 2, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG4_OFFSET,
+	NULL,  NULL, _clk_max_enable, _clk_max_disable, &ipg_clk, NULL);
+DEFINE_CLOCK_FULL(esdhc4_ipg_clk, 3, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG6_OFFSET,
+	NULL,  NULL, _clk_max_enable, _clk_max_disable, &ipg_clk, NULL);
+
+/* mx51 specific */
 DEFINE_CLOCK_MAX(esdhc2_clk, 1, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG3_OFFSET,
 	clk_esdhc2, &pll2_sw_clk, &esdhc2_ipg_clk);
 
+static struct clk esdhc3_clk = {
+	.id = 2,
+	.parent = &esdhc1_clk,
+	.set_parent = clk_esdhc3_set_parent,
+	.enable_reg = MXC_CCM_CCGR3,
+	.enable_shift = MXC_CCM_CCGRx_CG5_OFFSET,
+	.enable  = _clk_max_enable,
+	.disable = _clk_max_disable,
+	.secondary = &esdhc3_ipg_clk,
+};
+static struct clk esdhc4_clk = {
+	.id = 3,
+	.parent = &esdhc1_clk,
+	.set_parent = clk_esdhc4_set_parent,
+	.enable_reg = MXC_CCM_CCGR3,
+	.enable_shift = MXC_CCM_CCGRx_CG7_OFFSET,
+	.enable  = _clk_max_enable,
+	.disable = _clk_max_disable,
+	.secondary = &esdhc4_ipg_clk,
+};
+
+/* mx53 specific */
+static struct clk esdhc2_mx53_clk = {
+	.id = 2,
+	.parent = &esdhc1_clk,
+	.set_parent = clk_esdhc2_mx53_set_parent,
+	.enable_reg = MXC_CCM_CCGR3,
+	.enable_shift = MXC_CCM_CCGRx_CG3_OFFSET,
+	.enable  = _clk_max_enable,
+	.disable = _clk_max_disable,
+	.secondary = &esdhc3_ipg_clk,
+};
+
+DEFINE_CLOCK_MAX(esdhc3_mx53_clk, 2, MXC_CCM_CCGR3, MXC_CCM_CCGRx_CG5_OFFSET,
+	clk_esdhc3_mx53, &pll2_sw_clk, &esdhc2_ipg_clk);
+
+static struct clk esdhc4_mx53_clk = {
+	.id = 3,
+	.parent = &esdhc1_clk,
+	.set_parent = clk_esdhc4_mx53_set_parent,
+	.enable_reg = MXC_CCM_CCGR3,
+	.enable_shift = MXC_CCM_CCGRx_CG7_OFFSET,
+	.enable  = _clk_max_enable,
+	.disable = _clk_max_disable,
+	.secondary = &esdhc4_ipg_clk,
+};
+
 DEFINE_CLOCK(mipi_esc_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG5_OFFSET, NULL, NULL, NULL, &pll2_sw_clk);
 DEFINE_CLOCK(mipi_hsc2_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG4_OFFSET, NULL, NULL, &mipi_esc_clk, &pll2_sw_clk);
 DEFINE_CLOCK(mipi_hsc1_clk, 0, MXC_CCM_CCGR4, MXC_CCM_CCGRx_CG3_OFFSET, NULL, NULL, &mipi_hsc2_clk, &pll2_sw_clk);
@@ -1312,6 +1438,8 @@ static struct clk_lookup mx51_lookups[] = {
 	_REGISTER_CLOCK("imx51-cspi.0", NULL, cspi_clk)
 	_REGISTER_CLOCK("sdhci-esdhc-imx.0", NULL, esdhc1_clk)
 	_REGISTER_CLOCK("sdhci-esdhc-imx.1", NULL, esdhc2_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx.2", NULL, esdhc3_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx.3", NULL, esdhc4_clk)
 	_REGISTER_CLOCK(NULL, "cpu_clk", cpu_clk)
 	_REGISTER_CLOCK(NULL, "iim_clk", iim_clk)
 	_REGISTER_CLOCK("imx2-wdt.0", NULL, dummy_clk)
@@ -1332,7 +1460,9 @@ static struct clk_lookup mx53_lookups[] = {
 	_REGISTER_CLOCK("imx-i2c.0", NULL, i2c1_clk)
 	_REGISTER_CLOCK("imx-i2c.1", NULL, i2c2_clk)
 	_REGISTER_CLOCK("sdhci-esdhc-imx.0", NULL, esdhc1_clk)
-	_REGISTER_CLOCK("sdhci-esdhc-imx.1", NULL, esdhc2_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx.1", NULL, esdhc2_mx53_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx.2", NULL, esdhc3_mx53_clk)
+	_REGISTER_CLOCK("sdhci-esdhc-imx.3", NULL, esdhc4_mx53_clk)
 	_REGISTER_CLOCK("imx53-ecspi.0", NULL, ecspi1_clk)
 	_REGISTER_CLOCK("imx53-ecspi.1", NULL, ecspi2_clk)
 	_REGISTER_CLOCK("imx53-cspi.0", NULL, cspi_clk)
@@ -1425,6 +1555,14 @@ int __init mx53_clocks_init(unsigned long ckil, unsigned long osc,
 	mx53_revision();
 	clk_disable(&iim_clk);
 
+	/* Set SDHC parents to be PLL2 */
+	clk_set_parent(&esdhc1_clk, &pll2_sw_clk);
+	clk_set_parent(&esdhc3_mx53_clk, &pll2_sw_clk);
+
+	/* set SDHC root clock as 200MHZ*/
+	clk_set_rate(&esdhc1_clk, 200000000);
+	clk_set_rate(&esdhc3_mx53_clk, 200000000);
+
 	/* System timer */
 	mxc_timer_init(&gpt_clk, MX53_IO_ADDRESS(MX53_GPT1_BASE_ADDR),
 		MX53_INT_GPT);
diff --git a/arch/arm/mach-mx5/crm_regs.h b/arch/arm/mach-mx5/crm_regs.h
index b462c22..87c0c58 100644
--- a/arch/arm/mach-mx5/crm_regs.h
+++ b/arch/arm/mach-mx5/crm_regs.h
@@ -217,9 +217,12 @@
 #define MXC_CCM_CSCMR1_ESDHC1_MSHC1_CLK_SEL_OFFSET	(20)
 #define MXC_CCM_CSCMR1_ESDHC1_MSHC1_CLK_SEL_MASK	(0x3 << 20)
 #define MXC_CCM_CSCMR1_ESDHC3_CLK_SEL			(0x1 << 19)
+#define MXC_CCM_CSCMR1_ESDHC2_MSHC2_MX53_CLK_SEL	(0x1 << 19)
 #define MXC_CCM_CSCMR1_ESDHC4_CLK_SEL			(0x1 << 18)
 #define MXC_CCM_CSCMR1_ESDHC2_MSHC2_CLK_SEL_OFFSET	(16)
 #define MXC_CCM_CSCMR1_ESDHC2_MSHC2_CLK_SEL_MASK	(0x3 << 16)
+#define MXC_CCM_CSCMR1_ESDHC3_MX53_CLK_SEL_OFFSET	(16)
+#define MXC_CCM_CSCMR1_ESDHC3_MX53_CLK_SEL_MASK		(0x3 << 16)
 #define MXC_CCM_CSCMR1_SSI1_CLK_SEL_OFFSET		(14)
 #define MXC_CCM_CSCMR1_SSI1_CLK_SEL_MASK		(0x3 << 14)
 #define MXC_CCM_CSCMR1_SSI2_CLK_SEL_OFFSET		(12)
@@ -271,6 +274,10 @@
 #define MXC_CCM_CSCDR1_ESDHC2_MSHC2_CLK_PRED_MASK	(0x7 << 22)
 #define MXC_CCM_CSCDR1_ESDHC2_MSHC2_CLK_PODF_OFFSET	(19)
 #define MXC_CCM_CSCDR1_ESDHC2_MSHC2_CLK_PODF_MASK	(0x7 << 19)
+#define MXC_CCM_CSCDR1_ESDHC3_MX53_CLK_PRED_OFFSET	(22)
+#define MXC_CCM_CSCDR1_ESDHC3_MX53_CLK_PRED_MASK	(0x7 << 22)
+#define MXC_CCM_CSCDR1_ESDHC3_MX53_CLK_PODF_OFFSET	(19)
+#define MXC_CCM_CSCDR1_ESDHC3_MX53_CLK_PODF_MASK	(0x7 << 19)
 #define MXC_CCM_CSCDR1_ESDHC1_MSHC1_CLK_PRED_OFFSET	(16)
 #define MXC_CCM_CSCDR1_ESDHC1_MSHC1_CLK_PRED_MASK	(0x7 << 16)
 #define MXC_CCM_CSCDR1_PGC_CLK_PODF_OFFSET		(14)
-- 
1.7.1

[PATCH V3 3/4] mmc: sdhci-esdhc: remove SDHCI_QUIRK_NO_CARD_NO_RESET from ESDHC_DEFAULT_QUIRKS

[Richard Zhu]

sdhci-esdhc-imx does not need SDHCI_QUIRK_NO_CARD_NO_RESET. Make it OF-specific.

Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>
---
 drivers/mmc/host/sdhci-esdhc.h    |    3 +--
 drivers/mmc/host/sdhci-of-esdhc.c |    2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
index afaf1bc..303cde0 100644
--- a/drivers/mmc/host/sdhci-esdhc.h
+++ b/drivers/mmc/host/sdhci-esdhc.h
@@ -24,8 +24,7 @@
 				SDHCI_QUIRK_NONSTANDARD_CLOCK | \
 				SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | \
 				SDHCI_QUIRK_PIO_NEEDS_DELAY | \
-				SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET | \
-				SDHCI_QUIRK_NO_CARD_NO_RESET)
+				SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET)
 
 #define ESDHC_SYSTEM_CONTROL	0x2c
 #define ESDHC_CLOCK_MASK	0x0000fff0
diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index fcd0e1f..6337607 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -73,7 +73,7 @@ static unsigned int esdhc_of_get_min_clock(struct sdhci_host *host)
 }
 
 struct sdhci_of_data sdhci_esdhc = {
-	.quirks = ESDHC_DEFAULT_QUIRKS,
+	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_NO_CARD_NO_RESET,
 	.ops = {
 		.read_l = sdhci_be32bs_readl,
 		.read_w = esdhc_readw,
-- 
1.7.1

[PATCH V3 3/4] mmc: sdhci-esdhc: remove SDHCI_QUIRK_NO_CARD_NO_RESET from ESDHC_DEFAULT_QUIRKS

[Richard Zhu]

sdhci-esdhc-imx does not need SDHCI_QUIRK_NO_CARD_NO_RESET. Make it OF-specific.

Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>
---
 drivers/mmc/host/sdhci-esdhc.h    |    3 +--
 drivers/mmc/host/sdhci-of-esdhc.c |    2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
index afaf1bc..303cde0 100644
--- a/drivers/mmc/host/sdhci-esdhc.h
+++ b/drivers/mmc/host/sdhci-esdhc.h
@@ -24,8 +24,7 @@
 				SDHCI_QUIRK_NONSTANDARD_CLOCK | \
 				SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | \
 				SDHCI_QUIRK_PIO_NEEDS_DELAY | \
-				SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET | \
-				SDHCI_QUIRK_NO_CARD_NO_RESET)
+				SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET)
 
 #define ESDHC_SYSTEM_CONTROL	0x2c
 #define ESDHC_CLOCK_MASK	0x0000fff0
diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index fcd0e1f..6337607 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -73,7 +73,7 @@ static unsigned int esdhc_of_get_min_clock(struct sdhci_host *host)
 }
 
 struct sdhci_of_data sdhci_esdhc = {
-	.quirks = ESDHC_DEFAULT_QUIRKS,
+	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_NO_CARD_NO_RESET,
 	.ops = {
 		.read_l = sdhci_be32bs_readl,
 		.read_w = esdhc_readw,
-- 
1.7.1

[PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53

[Richard Zhu]

Fix the NO INT in the Multi-BLK IO in SD/MMC, and
Multi-BLK read in SDIO

The CMDTYPE of the CMD register(offset 0xE) should be set to
"11" when the STOP CMD12 is issued on imx53 to abort one
open ended multi-blk IO. Otherwise one the TC INT wouldn't
be generated.

In exact block transfer, the controller doesn't complete the
operations automatically as required at the end of the
transfer and remains on hold if the abort command is not sent.
As a result, the TC flag is not asserted and SW  received timeout
exeception. set bit1 of Vendor Spec registor to fix it

Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>
Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c |   77 ++++++++++++++++++++++++++++++++++-
 drivers/mmc/host/sdhci-pltfm.h     |    2 +-
 2 files changed, 75 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 9b82910..32af7c4 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -15,13 +15,41 @@
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/clk.h>
+#include <linux/slab.h>
 #include <linux/mmc/host.h>
 #include <linux/mmc/sdhci-pltfm.h>
+#include <linux/mmc/mmc.h>
+#include <linux/mmc/sdio.h>
 #include <mach/hardware.h>
 #include "sdhci.h"
 #include "sdhci-pltfm.h"
 #include "sdhci-esdhc.h"
 
+/* Abort type definition in the command register  */
+#define  SDHCI_CMD_ABORTCMD	0xC0
+/* VENDOR SPEC register */
+#define SDHCI_VENDOR_SPEC	0xC0
+
+/*
+ * The CMDTYPE of the CMD register(offset 0xE) should be set to
+ * "11" when the STOP CMD12 is issued on imx53 to abort one
+ * open ended multi-blk IO. Otherwise one the TC INT wouldn't
+ * be generated.
+ * In exact block transfer, the controller doesn't complete the
+ * operations automatically as required at the end of the
+ * transfer and remains on hold if the abort command is not sent.
+ * As a result, the TC flag is not asserted and SW  received timeout
+ * exeception. Bit1 of Vendor Spec registor is used to fix it.
+ */
+#define IMX_MULTIBLK_NO_INT		(1 << 0)
+
+struct pltfm_imx_data {
+	int flags;
+	u32 mod_val;
+};
+
+static struct sdhci_ops sdhci_esdhc_ops;
+
 static inline void esdhc_clrset_le(struct sdhci_host *host, u32 mask, u32 val, int reg)
 {
 	void __iomem *base = host->ioaddr + (reg & ~0x3);
@@ -38,20 +66,51 @@ static u16 esdhc_readw_le(struct sdhci_host *host, int reg)
 	return readw(host->ioaddr + reg);
 }
 
+static void esdhc_writel_le(struct sdhci_host *host, u32 val, int reg)
+{
+	switch (reg) {
+	case SDHCI_INT_STATUS:
+		if (val & SDHCI_INT_DATA_END) {
+			u32 v;
+			v = readl(host->ioaddr + SDHCI_VENDOR_SPEC);
+			if (v & 0x2) {
+				v &= ~0x2;
+				writel(v, host->ioaddr + SDHCI_VENDOR_SPEC);
+			}
+		}
+		break;
+	}
+	writel(val, host->ioaddr + reg);
+}
+
 static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data =
+				(struct pltfm_imx_data *)pltfm_host->priv;
 
 	switch (reg) {
 	case SDHCI_TRANSFER_MODE:
+		if ((host->cmd->opcode == SD_IO_RW_EXTENDED)
+				&& (host->cmd->data->blocks > 1)
+				&& (host->cmd->data->flags & MMC_DATA_READ)
+				&& (imx_data->flags & IMX_MULTIBLK_NO_INT)) {
+			u32 v;
+			v = readl(host->ioaddr + SDHCI_VENDOR_SPEC);
+			v |= 0x2;
+			writel(v, host->ioaddr + SDHCI_VENDOR_SPEC);
+		}
 		/*
 		 * Postpone this write, we must do it together with a
 		 * command write that is down below.
 		 */
-		pltfm_host->scratchpad = val;
+		imx_data->mod_val = val;
 		return;
 	case SDHCI_COMMAND:
-		writel(val << 16 | pltfm_host->scratchpad,
+		if ((host->cmd->opcode == MMC_STOP_TRANSMISSION)
+			&& (imx_data->flags & IMX_MULTIBLK_NO_INT))
+			val |= SDHCI_CMD_ABORTCMD;
+		writel(val << 16 | imx_data->mod_val,
 			host->ioaddr + SDHCI_TRANSFER_MODE);
 		return;
 	case SDHCI_BLOCK_SIZE:
@@ -104,6 +163,10 @@ static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pd
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct clk *clk;
+	struct pltfm_imx_data *imx_data;
+
+	imx_data = kzalloc(sizeof(struct pltfm_imx_data), GFP_KERNEL);
+	pltfm_host->priv = (void *)imx_data;
 
 	clk = clk_get(mmc_dev(host->mmc), NULL);
 	if (IS_ERR(clk)) {
@@ -113,22 +176,30 @@ static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pd
 	clk_enable(clk);
 	pltfm_host->clk = clk;
 
-	if (cpu_is_mx35() || cpu_is_mx51())
+	if (!cpu_is_mx25())
 		host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
 	/* Fix errata ENGcm07207 which is present on i.MX25 and i.MX35 */
 	if (cpu_is_mx25() || cpu_is_mx35())
 		host->quirks |= SDHCI_QUIRK_NO_MULTIBLOCK;
 
+	if (!(cpu_is_mx25() || cpu_is_mx35() || cpu_is_mx51())) {
+		imx_data->flags |= IMX_MULTIBLK_NO_INT;
+		sdhci_esdhc_ops.write_l = esdhc_writel_le;
+	}
+
 	return 0;
 }
 
 static void esdhc_pltfm_exit(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data =
+				(struct pltfm_imx_data *)pltfm_host->priv;
 
 	clk_disable(pltfm_host->clk);
 	clk_put(pltfm_host->clk);
+	kfree(imx_data);
 }
 
 static struct sdhci_ops sdhci_esdhc_ops = {
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index ea2e44d..2b37016 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -17,7 +17,7 @@
 
 struct sdhci_pltfm_host {
 	struct clk *clk;
-	u32 scratchpad; /* to handle quirks across io-accessor calls */
+	void *priv; /* to handle quirks across io-accessor calls */
 };
 
 extern struct sdhci_pltfm_data sdhci_cns3xxx_pdata;
-- 
1.7.1

[PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53

[Richard Zhu]

Fix the NO INT in the Multi-BLK IO in SD/MMC, and
Multi-BLK read in SDIO

The CMDTYPE of the CMD register(offset 0xE) should be set to
"11" when the STOP CMD12 is issued on imx53 to abort one
open ended multi-blk IO. Otherwise one the TC INT wouldn't
be generated.

In exact block transfer, the controller doesn't complete the
operations automatically as required at the end of the
transfer and remains on hold if the abort command is not sent.
As a result, the TC flag is not asserted and SW  received timeout
exeception. set bit1 of Vendor Spec registor to fix it

Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>
Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c |   77 ++++++++++++++++++++++++++++++++++-
 drivers/mmc/host/sdhci-pltfm.h     |    2 +-
 2 files changed, 75 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 9b82910..32af7c4 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -15,13 +15,41 @@
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/clk.h>
+#include <linux/slab.h>
 #include <linux/mmc/host.h>
 #include <linux/mmc/sdhci-pltfm.h>
+#include <linux/mmc/mmc.h>
+#include <linux/mmc/sdio.h>
 #include <mach/hardware.h>
 #include "sdhci.h"
 #include "sdhci-pltfm.h"
 #include "sdhci-esdhc.h"
 
+/* Abort type definition in the command register  */
+#define  SDHCI_CMD_ABORTCMD	0xC0
+/* VENDOR SPEC register */
+#define SDHCI_VENDOR_SPEC	0xC0
+
+/*
+ * The CMDTYPE of the CMD register(offset 0xE) should be set to
+ * "11" when the STOP CMD12 is issued on imx53 to abort one
+ * open ended multi-blk IO. Otherwise one the TC INT wouldn't
+ * be generated.
+ * In exact block transfer, the controller doesn't complete the
+ * operations automatically as required at the end of the
+ * transfer and remains on hold if the abort command is not sent.
+ * As a result, the TC flag is not asserted and SW  received timeout
+ * exeception. Bit1 of Vendor Spec registor is used to fix it.
+ */
+#define IMX_MULTIBLK_NO_INT		(1 << 0)
+
+struct pltfm_imx_data {
+	int flags;
+	u32 mod_val;
+};
+
+static struct sdhci_ops sdhci_esdhc_ops;
+
 static inline void esdhc_clrset_le(struct sdhci_host *host, u32 mask, u32 val, int reg)
 {
 	void __iomem *base = host->ioaddr + (reg & ~0x3);
@@ -38,20 +66,51 @@ static u16 esdhc_readw_le(struct sdhci_host *host, int reg)
 	return readw(host->ioaddr + reg);
 }
 
+static void esdhc_writel_le(struct sdhci_host *host, u32 val, int reg)
+{
+	switch (reg) {
+	case SDHCI_INT_STATUS:
+		if (val & SDHCI_INT_DATA_END) {
+			u32 v;
+			v = readl(host->ioaddr + SDHCI_VENDOR_SPEC);
+			if (v & 0x2) {
+				v &= ~0x2;
+				writel(v, host->ioaddr + SDHCI_VENDOR_SPEC);
+			}
+		}
+		break;
+	}
+	writel(val, host->ioaddr + reg);
+}
+
 static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data =
+				(struct pltfm_imx_data *)pltfm_host->priv;
 
 	switch (reg) {
 	case SDHCI_TRANSFER_MODE:
+		if ((host->cmd->opcode == SD_IO_RW_EXTENDED)
+				&& (host->cmd->data->blocks > 1)
+				&& (host->cmd->data->flags & MMC_DATA_READ)
+				&& (imx_data->flags & IMX_MULTIBLK_NO_INT)) {
+			u32 v;
+			v = readl(host->ioaddr + SDHCI_VENDOR_SPEC);
+			v |= 0x2;
+			writel(v, host->ioaddr + SDHCI_VENDOR_SPEC);
+		}
 		/*
 		 * Postpone this write, we must do it together with a
 		 * command write that is down below.
 		 */
-		pltfm_host->scratchpad = val;
+		imx_data->mod_val = val;
 		return;
 	case SDHCI_COMMAND:
-		writel(val << 16 | pltfm_host->scratchpad,
+		if ((host->cmd->opcode == MMC_STOP_TRANSMISSION)
+			&& (imx_data->flags & IMX_MULTIBLK_NO_INT))
+			val |= SDHCI_CMD_ABORTCMD;
+		writel(val << 16 | imx_data->mod_val,
 			host->ioaddr + SDHCI_TRANSFER_MODE);
 		return;
 	case SDHCI_BLOCK_SIZE:
@@ -104,6 +163,10 @@ static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pd
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct clk *clk;
+	struct pltfm_imx_data *imx_data;
+
+	imx_data = kzalloc(sizeof(struct pltfm_imx_data), GFP_KERNEL);
+	pltfm_host->priv = (void *)imx_data;
 
 	clk = clk_get(mmc_dev(host->mmc), NULL);
 	if (IS_ERR(clk)) {
@@ -113,22 +176,30 @@ static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pd
 	clk_enable(clk);
 	pltfm_host->clk = clk;
 
-	if (cpu_is_mx35() || cpu_is_mx51())
+	if (!cpu_is_mx25())
 		host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
 	/* Fix errata ENGcm07207 which is present on i.MX25 and i.MX35 */
 	if (cpu_is_mx25() || cpu_is_mx35())
 		host->quirks |= SDHCI_QUIRK_NO_MULTIBLOCK;
 
+	if (!(cpu_is_mx25() || cpu_is_mx35() || cpu_is_mx51())) {
+		imx_data->flags |= IMX_MULTIBLK_NO_INT;
+		sdhci_esdhc_ops.write_l = esdhc_writel_le;
+	}
+
 	return 0;
 }
 
 static void esdhc_pltfm_exit(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data =
+				(struct pltfm_imx_data *)pltfm_host->priv;
 
 	clk_disable(pltfm_host->clk);
 	clk_put(pltfm_host->clk);
+	kfree(imx_data);
 }
 
 static struct sdhci_ops sdhci_esdhc_ops = {
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index ea2e44d..2b37016 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -17,7 +17,7 @@
 
 struct sdhci_pltfm_host {
 	struct clk *clk;
-	u32 scratchpad; /* to handle quirks across io-accessor calls */
+	void *priv; /* to handle quirks across io-accessor calls */
 };
 
 extern struct sdhci_pltfm_data sdhci_cns3xxx_pdata;
-- 
1.7.1

RE: [PATCH V3 1/4] ARM: imx53_loco: add esdhc device support

[Zhu Richard-R65037]

Cc the patchs to linaro guys.

Best Regards
Richard Zhu


> -----Original Message-----
> From: Zhu Richard-R65037
> Sent: Monday, February 28, 2011 7:32 PM
> To: linux-arm-kernel@lists.infradead.org
> Cc: kernel@pengutronix.de; linux-mmc@vger.kernel.org; cjb@laptop.org;
> avorontsov@ru.mvista.com; eric@eukrea.com; w.sang@pengutronix.de;
> linuxzsc@gmail.com; Zhao Richard-B20223; Freescale@lists.linaro.org;
> eric.miao@linaro.org; Zhu Richard-R65037
> Subject: [PATCH V3 1/4] ARM: imx53_loco: add esdhc device support
>
> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
> ---
>  arch/arm/mach-mx5/Kconfig           |    1 +
>  arch/arm/mach-mx5/board-mx53_loco.c |    2 ++
>  2 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig index
> f065a0d..a72c833 100644
> --- a/arch/arm/mach-mx5/Kconfig
> +++ b/arch/arm/mach-mx5/Kconfig
> @@ -162,6 +162,7 @@ config MACH_MX53_LOCO
>       select IMX_HAVE_PLATFORM_IMX2_WDT
>       select IMX_HAVE_PLATFORM_IMX_I2C
>       select IMX_HAVE_PLATFORM_IMX_UART
> +     select IMX_HAVE_PLATFORM_SDHCI_ESDHC_IMX
>       help
>         Include support for MX53 LOCO platform. This includes specific
>         configurations for the board and its peripherals.
> diff --git a/arch/arm/mach-mx5/board-mx53_loco.c b/arch/arm/mach-
> mx5/board-mx53_loco.c
> index 160899e..0a18f8d 100644
> --- a/arch/arm/mach-mx5/board-mx53_loco.c
> +++ b/arch/arm/mach-mx5/board-mx53_loco.c
> @@ -213,6 +213,8 @@ static void __init mx53_loco_board_init(void)
>       imx53_add_imx2_wdt(0, NULL);
>       imx53_add_imx_i2c(0, &mx53_loco_i2c_data);
>       imx53_add_imx_i2c(1, &mx53_loco_i2c_data);
> +     imx53_add_sdhci_esdhc_imx(0, NULL);
> +     imx53_add_sdhci_esdhc_imx(2, NULL);
>  }
>
>  static void __init mx53_loco_timer_init(void)
> --
> 1.7.1

[PATCH V3 1/4] ARM: imx53_loco: add esdhc device support

[Zhu Richard-R65037]

Cc the patchs to linaro guys.

Best Regards
Richard Zhu


> -----Original Message-----
> From: Zhu Richard-R65037
> Sent: Monday, February 28, 2011 7:32 PM
> To: linux-arm-kernel at lists.infradead.org
> Cc: kernel at pengutronix.de; linux-mmc at vger.kernel.org; cjb at laptop.org;
> avorontsov at ru.mvista.com; eric at eukrea.com; w.sang at pengutronix.de;
> linuxzsc at gmail.com; Zhao Richard-B20223; Freescale at lists.linaro.org;
> eric.miao at linaro.org; Zhu Richard-R65037
> Subject: [PATCH V3 1/4] ARM: imx53_loco: add esdhc device support
>
> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
> ---
>  arch/arm/mach-mx5/Kconfig           |    1 +
>  arch/arm/mach-mx5/board-mx53_loco.c |    2 ++
>  2 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig index
> f065a0d..a72c833 100644
> --- a/arch/arm/mach-mx5/Kconfig
> +++ b/arch/arm/mach-mx5/Kconfig
> @@ -162,6 +162,7 @@ config MACH_MX53_LOCO
>       select IMX_HAVE_PLATFORM_IMX2_WDT
>       select IMX_HAVE_PLATFORM_IMX_I2C
>       select IMX_HAVE_PLATFORM_IMX_UART
> +     select IMX_HAVE_PLATFORM_SDHCI_ESDHC_IMX
>       help
>         Include support for MX53 LOCO platform. This includes specific
>         configurations for the board and its peripherals.
> diff --git a/arch/arm/mach-mx5/board-mx53_loco.c b/arch/arm/mach-
> mx5/board-mx53_loco.c
> index 160899e..0a18f8d 100644
> --- a/arch/arm/mach-mx5/board-mx53_loco.c
> +++ b/arch/arm/mach-mx5/board-mx53_loco.c
> @@ -213,6 +213,8 @@ static void __init mx53_loco_board_init(void)
>       imx53_add_imx2_wdt(0, NULL);
>       imx53_add_imx_i2c(0, &mx53_loco_i2c_data);
>       imx53_add_imx_i2c(1, &mx53_loco_i2c_data);
> +     imx53_add_sdhci_esdhc_imx(0, NULL);
> +     imx53_add_sdhci_esdhc_imx(2, NULL);
>  }
>
>  static void __init mx53_loco_timer_init(void)
> --
> 1.7.1

Re: [PATCH V3 1/4] ARM: imx53_loco: add esdhc device support

[Wolfram Sang]

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

On Mon, Feb 28, 2011 at 07:32:02PM +0800, Richard Zhu wrote:
> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>

Acked-by: Wolfram Sang <w.sang@pengutronix.de>

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

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

[PATCH V3 1/4] ARM: imx53_loco: add esdhc device support

[Wolfram Sang]

On Mon, Feb 28, 2011 at 07:32:02PM +0800, Richard Zhu wrote:
> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>

Acked-by: Wolfram Sang <w.sang@pengutronix.de>

-- 
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/20110228/deaf223d/attachment.sig>

Re: [PATCH V3 3/4] mmc: sdhci-esdhc: remove SDHCI_QUIRK_NO_CARD_NO_RESET from ESDHC_DEFAULT_QUIRKS

[Wolfram Sang]

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

On Mon, Feb 28, 2011 at 07:32:04PM +0800, Richard Zhu wrote:
> sdhci-esdhc-imx does not need SDHCI_QUIRK_NO_CARD_NO_RESET. Make it OF-specific.
> 
> Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>

Seems to work on mx35 at least:

Tested-by: Wolfram Sang <w.sang@pengutronix.de>

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

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

[PATCH V3 3/4] mmc: sdhci-esdhc: remove SDHCI_QUIRK_NO_CARD_NO_RESET from ESDHC_DEFAULT_QUIRKS

[Wolfram Sang]

On Mon, Feb 28, 2011 at 07:32:04PM +0800, Richard Zhu wrote:
> sdhci-esdhc-imx does not need SDHCI_QUIRK_NO_CARD_NO_RESET. Make it OF-specific.
> 
> Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>

Seems to work on mx35 at least:

Tested-by: Wolfram Sang <w.sang@pengutronix.de>

-- 
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/20110228/3b4ad884/attachment.sig>

Re: [PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53

[Wolfram Sang]

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

On Mon, Feb 28, 2011 at 07:32:05PM +0800, Richard Zhu wrote:
> Fix the NO INT in the Multi-BLK IO in SD/MMC, and
> Multi-BLK read in SDIO
> 
> The CMDTYPE of the CMD register(offset 0xE) should be set to
> "11" when the STOP CMD12 is issued on imx53 to abort one
> open ended multi-blk IO. Otherwise one the TC INT wouldn't
> be generated.
> 
> In exact block transfer, the controller doesn't complete the
> operations automatically as required at the end of the
> transfer and remains on hold if the abort command is not sent.
> As a result, the TC flag is not asserted and SW  received timeout
> exeception. set bit1 of Vendor Spec registor to fix it
> 
> Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>
> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c |   77 ++++++++++++++++++++++++++++++++++-
>  drivers/mmc/host/sdhci-pltfm.h     |    2 +-
>  2 files changed, 75 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 9b82910..32af7c4 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -15,13 +15,41 @@
>  #include <linux/delay.h>
>  #include <linux/err.h>
>  #include <linux/clk.h>
> +#include <linux/slab.h>
>  #include <linux/mmc/host.h>
>  #include <linux/mmc/sdhci-pltfm.h>
> +#include <linux/mmc/mmc.h>
> +#include <linux/mmc/sdio.h>
>  #include <mach/hardware.h>
>  #include "sdhci.h"
>  #include "sdhci-pltfm.h"
>  #include "sdhci-esdhc.h"
>  
> +/* Abort type definition in the command register  */
> +#define  SDHCI_CMD_ABORTCMD	0xC0

Won't that belong into sd.h (unless I misunderstood your last mail)?

> +/* VENDOR SPEC register */
> +#define SDHCI_VENDOR_SPEC	0xC0
> +
> +/*
> + * The CMDTYPE of the CMD register(offset 0xE) should be set to

Check spaces.

> + * "11" when the STOP CMD12 is issued on imx53 to abort one
> + * open ended multi-blk IO. Otherwise one the TC INT wouldn't
> + * be generated.
> + * In exact block transfer, the controller doesn't complete the
> + * operations automatically as required at the end of the
> + * transfer and remains on hold if the abort command is not sent.
> + * As a result, the TC flag is not asserted and SW  received timeout
> + * exeception. Bit1 of Vendor Spec registor is used to fix it.
> + */

This is a better description, thanks. But what does the bit technically
do? Can't we keep it enabled all the time? Can't we use it to fix CMD12,
too?

> +#define IMX_MULTIBLK_NO_INT		(1 << 0)
> +
> +struct pltfm_imx_data {
> +	int flags;
> +	u32 mod_val;
> +};

Hmm, to me, just using cpu_is_mx53() is more readable than introducing
another layer of flags/quirks.

> +
> +static struct sdhci_ops sdhci_esdhc_ops;
> +

Move them to the front. But I did this already, so no worries :) I will
ping Chris to merge my series, so we will have something better to
develop on.


>  static inline void esdhc_clrset_le(struct sdhci_host *host, u32 mask, u32 val, int reg)
>  {
>  	void __iomem *base = host->ioaddr + (reg & ~0x3);
> @@ -38,20 +66,51 @@ static u16 esdhc_readw_le(struct sdhci_host *host, int reg)
>  	return readw(host->ioaddr + reg);
>  }
>  
> +static void esdhc_writel_le(struct sdhci_host *host, u32 val, int reg)
> +{
> +	switch (reg) {
> +	case SDHCI_INT_STATUS:
> +		if (val & SDHCI_INT_DATA_END) {
> +			u32 v;
> +			v = readl(host->ioaddr + SDHCI_VENDOR_SPEC);
> +			if (v & 0x2) {
> +				v &= ~0x2;

Forgot to mention this before. Please use a define instead of hardocded
values.

I skipped the rest because we probably need to answer the questions
above first and to get a stable base with out conflicts.

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

[PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53

[Wolfram Sang]

On Mon, Feb 28, 2011 at 07:32:05PM +0800, Richard Zhu wrote:
> Fix the NO INT in the Multi-BLK IO in SD/MMC, and
> Multi-BLK read in SDIO
> 
> The CMDTYPE of the CMD register(offset 0xE) should be set to
> "11" when the STOP CMD12 is issued on imx53 to abort one
> open ended multi-blk IO. Otherwise one the TC INT wouldn't
> be generated.
> 
> In exact block transfer, the controller doesn't complete the
> operations automatically as required at the end of the
> transfer and remains on hold if the abort command is not sent.
> As a result, the TC flag is not asserted and SW  received timeout
> exeception. set bit1 of Vendor Spec registor to fix it
> 
> Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>
> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c |   77 ++++++++++++++++++++++++++++++++++-
>  drivers/mmc/host/sdhci-pltfm.h     |    2 +-
>  2 files changed, 75 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 9b82910..32af7c4 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -15,13 +15,41 @@
>  #include <linux/delay.h>
>  #include <linux/err.h>
>  #include <linux/clk.h>
> +#include <linux/slab.h>
>  #include <linux/mmc/host.h>
>  #include <linux/mmc/sdhci-pltfm.h>
> +#include <linux/mmc/mmc.h>
> +#include <linux/mmc/sdio.h>
>  #include <mach/hardware.h>
>  #include "sdhci.h"
>  #include "sdhci-pltfm.h"
>  #include "sdhci-esdhc.h"
>  
> +/* Abort type definition in the command register  */
> +#define  SDHCI_CMD_ABORTCMD	0xC0

Won't that belong into sd.h (unless I misunderstood your last mail)?

> +/* VENDOR SPEC register */
> +#define SDHCI_VENDOR_SPEC	0xC0
> +
> +/*
> + * The CMDTYPE of the CMD register(offset 0xE) should be set to

Check spaces.

> + * "11" when the STOP CMD12 is issued on imx53 to abort one
> + * open ended multi-blk IO. Otherwise one the TC INT wouldn't
> + * be generated.
> + * In exact block transfer, the controller doesn't complete the
> + * operations automatically as required at the end of the
> + * transfer and remains on hold if the abort command is not sent.
> + * As a result, the TC flag is not asserted and SW  received timeout
> + * exeception. Bit1 of Vendor Spec registor is used to fix it.
> + */

This is a better description, thanks. But what does the bit technically
do? Can't we keep it enabled all the time? Can't we use it to fix CMD12,
too?

> +#define IMX_MULTIBLK_NO_INT		(1 << 0)
> +
> +struct pltfm_imx_data {
> +	int flags;
> +	u32 mod_val;
> +};

Hmm, to me, just using cpu_is_mx53() is more readable than introducing
another layer of flags/quirks.

> +
> +static struct sdhci_ops sdhci_esdhc_ops;
> +

Move them to the front. But I did this already, so no worries :) I will
ping Chris to merge my series, so we will have something better to
develop on.


>  static inline void esdhc_clrset_le(struct sdhci_host *host, u32 mask, u32 val, int reg)
>  {
>  	void __iomem *base = host->ioaddr + (reg & ~0x3);
> @@ -38,20 +66,51 @@ static u16 esdhc_readw_le(struct sdhci_host *host, int reg)
>  	return readw(host->ioaddr + reg);
>  }
>  
> +static void esdhc_writel_le(struct sdhci_host *host, u32 val, int reg)
> +{
> +	switch (reg) {
> +	case SDHCI_INT_STATUS:
> +		if (val & SDHCI_INT_DATA_END) {
> +			u32 v;
> +			v = readl(host->ioaddr + SDHCI_VENDOR_SPEC);
> +			if (v & 0x2) {
> +				v &= ~0x2;

Forgot to mention this before. Please use a define instead of hardocded
values.

I skipped the rest because we probably need to answer the questions
above first and to get a stable base with out conflicts.

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/20110228/09241a4c/attachment.sig>

RE: [PATCH V3 3/4] mmc: sdhci-esdhc: remove SDHCI_QUIRK_NO_CARD_NO_RESET from ESDHC_DEFAULT_QUIRKS

[Zhu Richard-R65037]

Hi Wolfram:
Thanks for you to make tests on your mx35 platform.

Best Regards
Richard Zhu


> -----Original Message-----
> From: Wolfram Sang [mailto:w.sang@pengutronix.de]
> Sent: Monday, February 28, 2011 10:21 PM
> To: Zhu Richard-R65037
> Cc: linux-arm-kernel@lists.infradead.org; kernel@pengutronix.de; linux-
> mmc@vger.kernel.org; cjb@laptop.org; avorontsov@ru.mvista.com;
> eric@eukrea.com; linuxzsc@gmail.com; Zhao Richard-B20223;
> Freescale@lists.linaro.org; eric.miao@linaro.org
> Subject: Re: [PATCH V3 3/4] mmc: sdhci-esdhc: remove
> SDHCI_QUIRK_NO_CARD_NO_RESET from ESDHC_DEFAULT_QUIRKS
>
> On Mon, Feb 28, 2011 at 07:32:04PM +0800, Richard Zhu wrote:
> > sdhci-esdhc-imx does not need SDHCI_QUIRK_NO_CARD_NO_RESET. Make it OF-
> specific.
> >
> > Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>
>
> Seems to work on mx35 at least:
>
> Tested-by: Wolfram Sang <w.sang@pengutronix.de>
>
> --
> Pengutronix e.K.                           | Wolfram Sang
> |
> Industrial Linux Solutions                 | http://www.pengutronix.de/
> |

[PATCH V3 3/4] mmc: sdhci-esdhc: remove SDHCI_QUIRK_NO_CARD_NO_RESET from ESDHC_DEFAULT_QUIRKS

[Zhu Richard-R65037]

Hi Wolfram:
Thanks for you to make tests on your mx35 platform.

Best Regards
Richard Zhu


> -----Original Message-----
> From: Wolfram Sang [mailto:w.sang at pengutronix.de]
> Sent: Monday, February 28, 2011 10:21 PM
> To: Zhu Richard-R65037
> Cc: linux-arm-kernel at lists.infradead.org; kernel at pengutronix.de; linux-
> mmc at vger.kernel.org; cjb at laptop.org; avorontsov at ru.mvista.com;
> eric at eukrea.com; linuxzsc at gmail.com; Zhao Richard-B20223;
> Freescale at lists.linaro.org; eric.miao at linaro.org
> Subject: Re: [PATCH V3 3/4] mmc: sdhci-esdhc: remove
> SDHCI_QUIRK_NO_CARD_NO_RESET from ESDHC_DEFAULT_QUIRKS
>
> On Mon, Feb 28, 2011 at 07:32:04PM +0800, Richard Zhu wrote:
> > sdhci-esdhc-imx does not need SDHCI_QUIRK_NO_CARD_NO_RESET. Make it OF-
> specific.
> >
> > Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>
>
> Seems to work on mx35 at least:
>
> Tested-by: Wolfram Sang <w.sang@pengutronix.de>
>
> --
> Pengutronix e.K.                           | Wolfram Sang
> |
> Industrial Linux Solutions                 | http://www.pengutronix.de/
> |

RE: [PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53

[Zhu Richard-R65037]

Hi Wolfram:
Thanks a lot for your review-comments firstly. :)

Best Regards
Richard Zhu


> -----Original Message-----
> From: Wolfram Sang [mailto:w.sang@pengutronix.de]
> Sent: Monday, February 28, 2011 10:48 PM
> To: Zhu Richard-R65037
> Cc: linux-arm-kernel@lists.infradead.org; kernel@pengutronix.de; linux-
> mmc@vger.kernel.org; cjb@laptop.org; avorontsov@ru.mvista.com;
> eric@eukrea.com; linuxzsc@gmail.com; Zhao Richard-B20223;
> Freescale@lists.linaro.org; eric.miao@linaro.org
> Subject: Re: [PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53
>
> On Mon, Feb 28, 2011 at 07:32:05PM +0800, Richard Zhu wrote:
> > Fix the NO INT in the Multi-BLK IO in SD/MMC, and Multi-BLK read in
> > SDIO
> >
> > The CMDTYPE of the CMD register(offset 0xE) should be set to "11" when
> > the STOP CMD12 is issued on imx53 to abort one open ended multi-blk
> > IO. Otherwise one the TC INT wouldn't be generated.
> >
> > In exact block transfer, the controller doesn't complete the
> > operations automatically as required at the end of the transfer and
> > remains on hold if the abort command is not sent.
> > As a result, the TC flag is not asserted and SW  received timeout
> > exeception. set bit1 of Vendor Spec registor to fix it
> >
> > Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>
> > Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
> > ---
> >  drivers/mmc/host/sdhci-esdhc-imx.c |   77
> ++++++++++++++++++++++++++++++++++-
> >  drivers/mmc/host/sdhci-pltfm.h     |    2 +-
> >  2 files changed, 75 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> > b/drivers/mmc/host/sdhci-esdhc-imx.c
> > index 9b82910..32af7c4 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -15,13 +15,41 @@
> >  #include <linux/delay.h>
> >  #include <linux/err.h>
> >  #include <linux/clk.h>
> > +#include <linux/slab.h>
> >  #include <linux/mmc/host.h>
> >  #include <linux/mmc/sdhci-pltfm.h>
> > +#include <linux/mmc/mmc.h>
> > +#include <linux/mmc/sdio.h>
> >  #include <mach/hardware.h>
> >  #include "sdhci.h"
> >  #include "sdhci-pltfm.h"
> >  #include "sdhci-esdhc.h"
> >
> > +/* Abort type definition in the command register  */
> > +#define  SDHCI_CMD_ABORTCMD        0xC0
>
> Won't that belong into sd.h (unless I misunderstood your last mail)?
This is the bit definitions of the ABORTCMD CMD-TYPE on the bit6~7 of CMD register.
Here is the definition of the CMD register derived from SDHC spec. FYI.
D15   D14 D13  D08     D07 D06       D05                  D04                          D03                       D02         D01 D00
Rsvd  Command  Index   Command Type  Data Present Select  Command Index Check Enable   Command CRC Check Enable  Rsvd        Response Type Select
>
> > +/* VENDOR SPEC register */
> > +#define SDHCI_VENDOR_SPEC  0xC0
> > +
> > +/*
> > + * The CMDTYPE of the CMD register(offset 0xE) should be set to
>
> Check spaces.
I used the <kernel_dir>/./scripts/checkpatch.pl script to check the patches, and didn't find that there are issues about the spaces.
Can you tell me what's kinds of spaces issue should be fixed?
Thanks in advanced. :)

>
> > + * "11" when the STOP CMD12 is issued on imx53 to abort one
> > + * open ended multi-blk IO. Otherwise one the TC INT wouldn't
> > + * be generated.
> > + * In exact block transfer, the controller doesn't complete the
> > + * operations automatically as required at the end of the
> > + * transfer and remains on hold if the abort command is not sent.
> > + * As a result, the TC flag is not asserted and SW  received timeout
> > + * exeception. Bit1 of Vendor Spec registor is used to fix it.
> > + */
>
> This is a better description, thanks. But what does the bit technically
> do? Can't we keep it enabled all the time? Can't we use it to fix CMD12,
> too?
>

No, we can't keep it enabled all the time.
This bit should be set to '1'/clear to '0' at the begin/end of the transfer.
Unfortunately, We can't use it to fix CMD12 issue either, this bit is only used to fix SDIO Multi-BLK NO INT case.
IC guy insist that the CMD12 case is not a bug refer to the SD HOST controller spec, the bit7-6 should be
Set to 11b when the abort CMD is issued.

> > +#define IMX_MULTIBLK_NO_INT                (1 << 0)
> > +
> > +struct pltfm_imx_data {
> > +   int flags;
> > +   u32 mod_val;
> > +};
>
> Hmm, to me, just using cpu_is_mx53() is more readable than introducing
> another layer of flags/quirks.
Hi Wolfram:
I discussed it with Richard Zhao before sending out these V3 patches.
As we know that there is not only mx53 has this issue, maybe some following SOCs have this issue too.
So we make a decision that we introduce another flags/quirks to declare it for all those SOCs that required this
mechanism in the end.
>
> > +
> > +static struct sdhci_ops sdhci_esdhc_ops;
> > +
>
> Move them to the front. But I did this already, so no worries :) I will
> ping Chris to merge my series, so we will have something better to
> develop on.
>
Thanks.:)
>
> >  static inline void esdhc_clrset_le(struct sdhci_host *host, u32 mask,
> > u32 val, int reg)  {
> >     void __iomem *base = host->ioaddr + (reg & ~0x3); @@ -38,20 +66,51
> > @@ static u16 esdhc_readw_le(struct sdhci_host *host, int reg)
> >     return readw(host->ioaddr + reg);
> >  }
> >
> > +static void esdhc_writel_le(struct sdhci_host *host, u32 val, int
> > +reg) {
> > +   switch (reg) {
> > +   case SDHCI_INT_STATUS:
> > +           if (val & SDHCI_INT_DATA_END) {
> > +                   u32 v;
> > +                   v = readl(host->ioaddr + SDHCI_VENDOR_SPEC);
> > +                   if (v & 0x2) {
> > +                           v &= ~0x2;
>
> Forgot to mention this before. Please use a define instead of hardocded
> values.
Ok, Accepted.
>
> I skipped the rest because we probably need to answer the questions above
> first and to get a stable base with out conflicts.
>
> Regards,
>
>    Wolfram
>
> --
> Pengutronix e.K.                           | Wolfram Sang
> |
> Industrial Linux Solutions                 | http://www.pengutronix.de/
> |

[PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53

[Zhu Richard-R65037]

Hi Wolfram:
Thanks a lot for your review-comments firstly. :)

Best Regards
Richard Zhu


> -----Original Message-----
> From: Wolfram Sang [mailto:w.sang at pengutronix.de]
> Sent: Monday, February 28, 2011 10:48 PM
> To: Zhu Richard-R65037
> Cc: linux-arm-kernel at lists.infradead.org; kernel at pengutronix.de; linux-
> mmc at vger.kernel.org; cjb at laptop.org; avorontsov at ru.mvista.com;
> eric at eukrea.com; linuxzsc at gmail.com; Zhao Richard-B20223;
> Freescale at lists.linaro.org; eric.miao at linaro.org
> Subject: Re: [PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53
>
> On Mon, Feb 28, 2011 at 07:32:05PM +0800, Richard Zhu wrote:
> > Fix the NO INT in the Multi-BLK IO in SD/MMC, and Multi-BLK read in
> > SDIO
> >
> > The CMDTYPE of the CMD register(offset 0xE) should be set to "11" when
> > the STOP CMD12 is issued on imx53 to abort one open ended multi-blk
> > IO. Otherwise one the TC INT wouldn't be generated.
> >
> > In exact block transfer, the controller doesn't complete the
> > operations automatically as required at the end of the transfer and
> > remains on hold if the abort command is not sent.
> > As a result, the TC flag is not asserted and SW  received timeout
> > exeception. set bit1 of Vendor Spec registor to fix it
> >
> > Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>
> > Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
> > ---
> >  drivers/mmc/host/sdhci-esdhc-imx.c |   77
> ++++++++++++++++++++++++++++++++++-
> >  drivers/mmc/host/sdhci-pltfm.h     |    2 +-
> >  2 files changed, 75 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> > b/drivers/mmc/host/sdhci-esdhc-imx.c
> > index 9b82910..32af7c4 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -15,13 +15,41 @@
> >  #include <linux/delay.h>
> >  #include <linux/err.h>
> >  #include <linux/clk.h>
> > +#include <linux/slab.h>
> >  #include <linux/mmc/host.h>
> >  #include <linux/mmc/sdhci-pltfm.h>
> > +#include <linux/mmc/mmc.h>
> > +#include <linux/mmc/sdio.h>
> >  #include <mach/hardware.h>
> >  #include "sdhci.h"
> >  #include "sdhci-pltfm.h"
> >  #include "sdhci-esdhc.h"
> >
> > +/* Abort type definition in the command register  */
> > +#define  SDHCI_CMD_ABORTCMD        0xC0
>
> Won't that belong into sd.h (unless I misunderstood your last mail)?
This is the bit definitions of the ABORTCMD CMD-TYPE on the bit6~7 of CMD register.
Here is the definition of the CMD register derived from SDHC spec. FYI.
D15   D14 D13  D08     D07 D06       D05                  D04                          D03                       D02         D01 D00
Rsvd  Command  Index   Command Type  Data Present Select  Command Index Check Enable   Command CRC Check Enable  Rsvd        Response Type Select
>
> > +/* VENDOR SPEC register */
> > +#define SDHCI_VENDOR_SPEC  0xC0
> > +
> > +/*
> > + * The CMDTYPE of the CMD register(offset 0xE) should be set to
>
> Check spaces.
I used the <kernel_dir>/./scripts/checkpatch.pl script to check the patches, and didn't find that there are issues about the spaces.
Can you tell me what's kinds of spaces issue should be fixed?
Thanks in advanced. :)

>
> > + * "11" when the STOP CMD12 is issued on imx53 to abort one
> > + * open ended multi-blk IO. Otherwise one the TC INT wouldn't
> > + * be generated.
> > + * In exact block transfer, the controller doesn't complete the
> > + * operations automatically as required at the end of the
> > + * transfer and remains on hold if the abort command is not sent.
> > + * As a result, the TC flag is not asserted and SW  received timeout
> > + * exeception. Bit1 of Vendor Spec registor is used to fix it.
> > + */
>
> This is a better description, thanks. But what does the bit technically
> do? Can't we keep it enabled all the time? Can't we use it to fix CMD12,
> too?
>

No, we can't keep it enabled all the time.
This bit should be set to '1'/clear to '0' at the begin/end of the transfer.
Unfortunately, We can't use it to fix CMD12 issue either, this bit is only used to fix SDIO Multi-BLK NO INT case.
IC guy insist that the CMD12 case is not a bug refer to the SD HOST controller spec, the bit7-6 should be
Set to 11b when the abort CMD is issued.

> > +#define IMX_MULTIBLK_NO_INT                (1 << 0)
> > +
> > +struct pltfm_imx_data {
> > +   int flags;
> > +   u32 mod_val;
> > +};
>
> Hmm, to me, just using cpu_is_mx53() is more readable than introducing
> another layer of flags/quirks.
Hi Wolfram:
I discussed it with Richard Zhao before sending out these V3 patches.
As we know that there is not only mx53 has this issue, maybe some following SOCs have this issue too.
So we make a decision that we introduce another flags/quirks to declare it for all those SOCs that required this
mechanism in the end.
>
> > +
> > +static struct sdhci_ops sdhci_esdhc_ops;
> > +
>
> Move them to the front. But I did this already, so no worries :) I will
> ping Chris to merge my series, so we will have something better to
> develop on.
>
Thanks.:)
>
> >  static inline void esdhc_clrset_le(struct sdhci_host *host, u32 mask,
> > u32 val, int reg)  {
> >     void __iomem *base = host->ioaddr + (reg & ~0x3); @@ -38,20 +66,51
> > @@ static u16 esdhc_readw_le(struct sdhci_host *host, int reg)
> >     return readw(host->ioaddr + reg);
> >  }
> >
> > +static void esdhc_writel_le(struct sdhci_host *host, u32 val, int
> > +reg) {
> > +   switch (reg) {
> > +   case SDHCI_INT_STATUS:
> > +           if (val & SDHCI_INT_DATA_END) {
> > +                   u32 v;
> > +                   v = readl(host->ioaddr + SDHCI_VENDOR_SPEC);
> > +                   if (v & 0x2) {
> > +                           v &= ~0x2;
>
> Forgot to mention this before. Please use a define instead of hardocded
> values.
Ok, Accepted.
>
> I skipped the rest because we probably need to answer the questions above
> first and to get a stable base with out conflicts.
>
> Regards,
>
>    Wolfram
>
> --
> Pengutronix e.K.                           | Wolfram Sang
> |
> Industrial Linux Solutions                 | http://www.pengutronix.de/
> |

Re: [PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53

[Eric Miao]

>> Hmm, to me, just using cpu_is_mx53() is more readable than introducing
>> another layer of flags/quirks.
> Hi Wolfram:
> I discussed it with Richard Zhao before sending out these V3 patches.
> As we know that there is not only mx53 has this issue, maybe some following SOCs have this issue too.
> So we make a decision that we introduce another flags/quirks to declare it for all those SOCs that required this
> mechanism in the end.

I agree with Richard here that a flag/quirk will be more generic than
cpu_is_*(). Otherwise we may end up one day growing a big list of
these cpu_is_*().

One other point to consider is the SoC stepping, as this might be
fixed in later steppings, and keeping cpu_is_mx53() or
cpu_is_mx53_stepping_b2_or_above() doesn't look very good in
the driver code itself.

[PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53

[Eric Miao]

>> Hmm, to me, just using cpu_is_mx53() is more readable than introducing
>> another layer of flags/quirks.
> Hi Wolfram:
> I discussed it with Richard Zhao before sending out these V3 patches.
> As we know that there is not only mx53 has this issue, maybe some following SOCs have this issue too.
> So we make a decision that we introduce another flags/quirks to declare it for all those SOCs that required this
> mechanism in the end.

I agree with Richard here that a flag/quirk will be more generic than
cpu_is_*(). Otherwise we may end up one day growing a big list of
these cpu_is_*().

One other point to consider is the SoC stepping, as this might be
fixed in later steppings, and keeping cpu_is_mx53() or
cpu_is_mx53_stepping_b2_or_above() doesn't look very good in
the driver code itself.

Re: [PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53

[Wolfram Sang]

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

Hi Richard,

> Thanks a lot for your review-comments firstly. :)

You are welcome :)

> > > +/* Abort type definition in the command register  */
> > > +#define  SDHCI_CMD_ABORTCMD        0xC0
> >
> > Won't that belong into sd.h (unless I misunderstood your last mail)?
> This is the bit definitions of the ABORTCMD CMD-TYPE on the bit6~7 of CMD register.
> Here is the definition of the CMD register derived from SDHC spec. FYI.
> D15   D14 D13  D08     D07 D06       D05                  D04                          D03                       D02         D01 D00
> Rsvd  Command  Index   Command Type  Data Present Select  Command Index Check Enable   Command CRC Check Enable  Rsvd        Response Type Select

Ack, I found that, too. Exactly because it is in the standard, I thought
this should rather go into sd.h than sdhci-esdhc-imx.c. Would be a
seperate patch, though.

> >
> > > +/* VENDOR SPEC register */
> > > +#define SDHCI_VENDOR_SPEC  0xC0
> > > +
> > > +/*
> > > + * The CMDTYPE of the CMD register(offset 0xE) should be set to
> >
> > Check spaces.
> I used the <kernel_dir>/./scripts/checkpatch.pl script to check the patches, and didn't find that there are issues about the spaces.
> Can you tell me what's kinds of spaces issue should be fixed?

Space before opening brace -> "register (offset ..."

> No, we can't keep it enabled all the time.
> This bit should be set to '1'/clear to '0' at the begin/end of the transfer.
> Unfortunately, We can't use it to fix CMD12 issue either, this bit is only used to fix SDIO Multi-BLK NO INT case.

Ok, thanks for checking.

> IC guy insist that the CMD12 case is not a bug refer to the SD HOST controller spec, the bit7-6 should be
> Set to 11b when the abort CMD is issued.

That's a flaw in the core then? Need to investigate that.

> > Hmm, to me, just using cpu_is_mx53() is more readable than introducing
> > another layer of flags/quirks.
> Hi Wolfram:
> I discussed it with Richard Zhao before sending out these V3 patches.
> As we know that there is not only mx53 has this issue, maybe some following SOCs have this issue too.
> So we make a decision that we introduce another flags/quirks to declare it for all those SOCs that required this
> mechanism in the end.

Seems I am outnumbered on this matter, so OK. I just got a bit afraid
of that approach seeing it didn't scale very well with sdhci.c.

> > > +
> > > +static struct sdhci_ops sdhci_esdhc_ops;
> > > +
> >
> > Move them to the front. But I did this already, so no worries :) I will
> > ping Chris to merge my series, so we will have something better to
> > develop on.
> >
> Thanks.:)

He pulled the changes now, so please rebase your patches against
mmc-next. There is already a write_le-function now, but this should be
not too hard, hopefully.

Keep in mind that you don't need to cast void*.

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

[PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53

[Wolfram Sang]

Hi Richard,

> Thanks a lot for your review-comments firstly. :)

You are welcome :)

> > > +/* Abort type definition in the command register  */
> > > +#define  SDHCI_CMD_ABORTCMD        0xC0
> >
> > Won't that belong into sd.h (unless I misunderstood your last mail)?
> This is the bit definitions of the ABORTCMD CMD-TYPE on the bit6~7 of CMD register.
> Here is the definition of the CMD register derived from SDHC spec. FYI.
> D15   D14 D13  D08     D07 D06       D05                  D04                          D03                       D02         D01 D00
> Rsvd  Command  Index   Command Type  Data Present Select  Command Index Check Enable   Command CRC Check Enable  Rsvd        Response Type Select

Ack, I found that, too. Exactly because it is in the standard, I thought
this should rather go into sd.h than sdhci-esdhc-imx.c. Would be a
seperate patch, though.

> >
> > > +/* VENDOR SPEC register */
> > > +#define SDHCI_VENDOR_SPEC  0xC0
> > > +
> > > +/*
> > > + * The CMDTYPE of the CMD register(offset 0xE) should be set to
> >
> > Check spaces.
> I used the <kernel_dir>/./scripts/checkpatch.pl script to check the patches, and didn't find that there are issues about the spaces.
> Can you tell me what's kinds of spaces issue should be fixed?

Space before opening brace -> "register (offset ..."

> No, we can't keep it enabled all the time.
> This bit should be set to '1'/clear to '0' at the begin/end of the transfer.
> Unfortunately, We can't use it to fix CMD12 issue either, this bit is only used to fix SDIO Multi-BLK NO INT case.

Ok, thanks for checking.

> IC guy insist that the CMD12 case is not a bug refer to the SD HOST controller spec, the bit7-6 should be
> Set to 11b when the abort CMD is issued.

That's a flaw in the core then? Need to investigate that.

> > Hmm, to me, just using cpu_is_mx53() is more readable than introducing
> > another layer of flags/quirks.
> Hi Wolfram:
> I discussed it with Richard Zhao before sending out these V3 patches.
> As we know that there is not only mx53 has this issue, maybe some following SOCs have this issue too.
> So we make a decision that we introduce another flags/quirks to declare it for all those SOCs that required this
> mechanism in the end.

Seems I am outnumbered on this matter, so OK. I just got a bit afraid
of that approach seeing it didn't scale very well with sdhci.c.

> > > +
> > > +static struct sdhci_ops sdhci_esdhc_ops;
> > > +
> >
> > Move them to the front. But I did this already, so no worries :) I will
> > ping Chris to merge my series, so we will have something better to
> > develop on.
> >
> Thanks.:)

He pulled the changes now, so please rebase your patches against
mmc-next. There is already a write_le-function now, but this should be
not too hard, hopefully.

Keep in mind that you don't need to cast void*.

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/20110301/acca923e/attachment.sig>

RE: [PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53

[Zhu Richard-R65037]

Hi Wolfram:

Best Regards
Richard Zhu


> -----Original Message-----
> From: Wolfram Sang [mailto:w.sang@pengutronix.de]
> Sent: Tuesday, March 01, 2011 7:31 PM
> To: Zhu Richard-R65037
> Cc: linux-arm-kernel@lists.infradead.org; kernel@pengutronix.de; linux-
> mmc@vger.kernel.org; cjb@laptop.org; avorontsov@ru.mvista.com;
> eric@eukrea.com; linuxzsc@gmail.com; Zhao Richard-B20223;
> Freescale@lists.linaro.org; eric.miao@linaro.org
> Subject: Re: [PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53
>
> Hi Richard,
>
> > Thanks a lot for your review-comments firstly. :)
>
> You are welcome :)
>
> > > > +/* Abort type definition in the command register  */
> > > > +#define  SDHCI_CMD_ABORTCMD        0xC0
> > >
> > > Won't that belong into sd.h (unless I misunderstood your last mail)?
> > This is the bit definitions of the ABORTCMD CMD-TYPE on the bit6~7 of
> CMD register.
> > Here is the definition of the CMD register derived from SDHC spec. FYI.
> > D15   D14 D13  D08     D07 D06       D05                  D04
> D03                       D02         D01 D00
> > Rsvd  Command  Index   Command Type  Data Present Select  Command Index
> Check Enable   Command CRC Check Enable  Rsvd        Response Type Select
>
> Ack, I found that, too. Exactly because it is in the standard, I thought
> this should rather go into sd.h than sdhci-esdhc-imx.c. Would be a
> seperate patch, though.
>
Do you means the sdhci.h file?
I find that the sd.h file doesn't contain the bit definitions stuff of SD HOST CONTROLLER registers.
The sdhci.h file has them, such as:
#define SDHCI_COMMAND           0x0E
#define  SDHCI_CMD_RESP_MASK    0x03
#define  SDHCI_CMD_CRC          0x08
#define  SDHCI_CMD_INDEX        0x10
#define  SDHCI_CMD_DATA         0x20

> > >
> > > > +/* VENDOR SPEC register */
> > > > +#define SDHCI_VENDOR_SPEC  0xC0
> > > > +
> > > > +/*
> > > > + * The CMDTYPE of the CMD register(offset 0xE) should be set to
> > >
> > > Check spaces.
> > I used the <kernel_dir>/./scripts/checkpatch.pl script to check the
> patches, and didn't find that there are issues about the spaces.
> > Can you tell me what's kinds of spaces issue should be fixed?
>
> Space before opening brace -> "register (offset ..."

Accepted.

>
> > No, we can't keep it enabled all the time.
> > This bit should be set to '1'/clear to '0' at the begin/end of the
> transfer.
> > Unfortunately, We can't use it to fix CMD12 issue either, this bit is
> only used to fix SDIO Multi-BLK NO INT case.
>
> Ok, thanks for checking.
>
You are welcome. :)

> > IC guy insist that the CMD12 case is not a bug refer to the SD HOST
> > controller spec, the bit7-6 should be Set to 11b when the abort CMD is
> issued.
>
> That's a flaw in the core then? Need to investigate that.
>
> > > Hmm, to me, just using cpu_is_mx53() is more readable than
> > > introducing another layer of flags/quirks.
> > Hi Wolfram:
> > I discussed it with Richard Zhao before sending out these V3 patches.
> > As we know that there is not only mx53 has this issue, maybe some
> following SOCs have this issue too.
> > So we make a decision that we introduce another flags/quirks to
> > declare it for all those SOCs that required this mechanism in the end.
>
> Seems I am outnumbered on this matter, so OK. I just got a bit afraid of
> that approach seeing it didn't scale very well with sdhci.c.
>
Thanks.
> > > > +
> > > > +static struct sdhci_ops sdhci_esdhc_ops;
> > > > +
> > >
> > > Move them to the front. But I did this already, so no worries :) I
> > > will ping Chris to merge my series, so we will have something better
> > > to develop on.
> > >
> > Thanks.:)
>
> He pulled the changes now, so please rebase your patches against mmc-next.
> There is already a write_le-function now, but this should be not too hard,
> hopefully.
>
> Keep in mind that you don't need to cast void*.
Accepted.
>
> Regards,
>
>    Wolfram
>
> --
> Pengutronix e.K.                           | Wolfram Sang
> |
> Industrial Linux Solutions                 | http://www.pengutronix.de/
> |

[PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53

[Zhu Richard-R65037]

Hi Wolfram:

Best Regards
Richard Zhu


> -----Original Message-----
> From: Wolfram Sang [mailto:w.sang at pengutronix.de]
> Sent: Tuesday, March 01, 2011 7:31 PM
> To: Zhu Richard-R65037
> Cc: linux-arm-kernel at lists.infradead.org; kernel at pengutronix.de; linux-
> mmc at vger.kernel.org; cjb at laptop.org; avorontsov at ru.mvista.com;
> eric at eukrea.com; linuxzsc at gmail.com; Zhao Richard-B20223;
> Freescale at lists.linaro.org; eric.miao at linaro.org
> Subject: Re: [PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53
>
> Hi Richard,
>
> > Thanks a lot for your review-comments firstly. :)
>
> You are welcome :)
>
> > > > +/* Abort type definition in the command register  */
> > > > +#define  SDHCI_CMD_ABORTCMD        0xC0
> > >
> > > Won't that belong into sd.h (unless I misunderstood your last mail)?
> > This is the bit definitions of the ABORTCMD CMD-TYPE on the bit6~7 of
> CMD register.
> > Here is the definition of the CMD register derived from SDHC spec. FYI.
> > D15   D14 D13  D08     D07 D06       D05                  D04
> D03                       D02         D01 D00
> > Rsvd  Command  Index   Command Type  Data Present Select  Command Index
> Check Enable   Command CRC Check Enable  Rsvd        Response Type Select
>
> Ack, I found that, too. Exactly because it is in the standard, I thought
> this should rather go into sd.h than sdhci-esdhc-imx.c. Would be a
> seperate patch, though.
>
Do you means the sdhci.h file?
I find that the sd.h file doesn't contain the bit definitions stuff of SD HOST CONTROLLER registers.
The sdhci.h file has them, such as:
#define SDHCI_COMMAND           0x0E
#define  SDHCI_CMD_RESP_MASK    0x03
#define  SDHCI_CMD_CRC          0x08
#define  SDHCI_CMD_INDEX        0x10
#define  SDHCI_CMD_DATA         0x20

> > >
> > > > +/* VENDOR SPEC register */
> > > > +#define SDHCI_VENDOR_SPEC  0xC0
> > > > +
> > > > +/*
> > > > + * The CMDTYPE of the CMD register(offset 0xE) should be set to
> > >
> > > Check spaces.
> > I used the <kernel_dir>/./scripts/checkpatch.pl script to check the
> patches, and didn't find that there are issues about the spaces.
> > Can you tell me what's kinds of spaces issue should be fixed?
>
> Space before opening brace -> "register (offset ..."

Accepted.

>
> > No, we can't keep it enabled all the time.
> > This bit should be set to '1'/clear to '0' at the begin/end of the
> transfer.
> > Unfortunately, We can't use it to fix CMD12 issue either, this bit is
> only used to fix SDIO Multi-BLK NO INT case.
>
> Ok, thanks for checking.
>
You are welcome. :)

> > IC guy insist that the CMD12 case is not a bug refer to the SD HOST
> > controller spec, the bit7-6 should be Set to 11b when the abort CMD is
> issued.
>
> That's a flaw in the core then? Need to investigate that.
>
> > > Hmm, to me, just using cpu_is_mx53() is more readable than
> > > introducing another layer of flags/quirks.
> > Hi Wolfram:
> > I discussed it with Richard Zhao before sending out these V3 patches.
> > As we know that there is not only mx53 has this issue, maybe some
> following SOCs have this issue too.
> > So we make a decision that we introduce another flags/quirks to
> > declare it for all those SOCs that required this mechanism in the end.
>
> Seems I am outnumbered on this matter, so OK. I just got a bit afraid of
> that approach seeing it didn't scale very well with sdhci.c.
>
Thanks.
> > > > +
> > > > +static struct sdhci_ops sdhci_esdhc_ops;
> > > > +
> > >
> > > Move them to the front. But I did this already, so no worries :) I
> > > will ping Chris to merge my series, so we will have something better
> > > to develop on.
> > >
> > Thanks.:)
>
> He pulled the changes now, so please rebase your patches against mmc-next.
> There is already a write_le-function now, but this should be not too hard,
> hopefully.
>
> Keep in mind that you don't need to cast void*.
Accepted.
>
> Regards,
>
>    Wolfram
>
> --
> Pengutronix e.K.                           | Wolfram Sang
> |
> Industrial Linux Solutions                 | http://www.pengutronix.de/
> |

Re: [PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53

[Wolfram Sang]

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


> > > > > +/* Abort type definition in the command register  */
> > > > > +#define  SDHCI_CMD_ABORTCMD        0xC0
> > > >
> > > > Won't that belong into sd.h (unless I misunderstood your last mail)?
> > > This is the bit definitions of the ABORTCMD CMD-TYPE on the bit6~7 of
> > CMD register.
> > > Here is the definition of the CMD register derived from SDHC spec. FYI.
> > > D15   D14 D13  D08     D07 D06       D05                  D04
> > D03                       D02         D01 D00
> > > Rsvd  Command  Index   Command Type  Data Present Select  Command Index
> > Check Enable   Command CRC Check Enable  Rsvd        Response Type Select
> >
> > Ack, I found that, too. Exactly because it is in the standard, I thought
> > this should rather go into sd.h than sdhci-esdhc-imx.c. Would be a
> > seperate patch, though.
> >
> Do you means the sdhci.h file?
> I find that the sd.h file doesn't contain the bit definitions stuff of SD HOST CONTROLLER registers.
> The sdhci.h file has them, such as:
> #define SDHCI_COMMAND           0x0E
> #define  SDHCI_CMD_RESP_MASK    0x03
> #define  SDHCI_CMD_CRC          0x08
> #define  SDHCI_CMD_INDEX        0x10
> #define  SDHCI_CMD_DATA         0x20

My idea was that it belongs more to the command definitions, thus sd.h.
But as long as there is nothing to be solved at core-level sdhci.h might
do for now as well.

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

[PATCH V3 4/4] mmc: sdhci-esdhc: enable esdhc on imx53

[Wolfram Sang]

> > > > > +/* Abort type definition in the command register  */
> > > > > +#define  SDHCI_CMD_ABORTCMD        0xC0
> > > >
> > > > Won't that belong into sd.h (unless I misunderstood your last mail)?
> > > This is the bit definitions of the ABORTCMD CMD-TYPE on the bit6~7 of
> > CMD register.
> > > Here is the definition of the CMD register derived from SDHC spec. FYI.
> > > D15   D14 D13  D08     D07 D06       D05                  D04
> > D03                       D02         D01 D00
> > > Rsvd  Command  Index   Command Type  Data Present Select  Command Index
> > Check Enable   Command CRC Check Enable  Rsvd        Response Type Select
> >
> > Ack, I found that, too. Exactly because it is in the standard, I thought
> > this should rather go into sd.h than sdhci-esdhc-imx.c. Would be a
> > seperate patch, though.
> >
> Do you means the sdhci.h file?
> I find that the sd.h file doesn't contain the bit definitions stuff of SD HOST CONTROLLER registers.
> The sdhci.h file has them, such as:
> #define SDHCI_COMMAND           0x0E
> #define  SDHCI_CMD_RESP_MASK    0x03
> #define  SDHCI_CMD_CRC          0x08
> #define  SDHCI_CMD_INDEX        0x10
> #define  SDHCI_CMD_DATA         0x20

My idea was that it belongs more to the command definitions, thus sd.h.
But as long as there is nothing to be solved at core-level sdhci.h might
do for now as well.

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/20110303/7ac90df5/attachment.sig>

Re: [PATCH V3 1/4] ARM: imx53_loco: add esdhc device support

[Wolfram Sang]

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


> >> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
> >> ---
> >>  arch/arm/mach-mx5/Kconfig           |    1 +
> >>  arch/arm/mach-mx5/board-mx53_loco.c |    2 ++
> >>  2 files changed, 3 insertions(+), 0 deletions(-)
> >
> > I think this patch should be the last one of the series.
> Yes, but I don't think it matter much.

It will if you bisect and end up here without the clock patch, no?

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

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

[PATCH V3 1/4] ARM: imx53_loco: add esdhc device support

[Wolfram Sang]

> >> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
> >> ---
> >> ?arch/arm/mach-mx5/Kconfig ? ? ? ? ? | ? ?1 +
> >> ?arch/arm/mach-mx5/board-mx53_loco.c | ? ?2 ++
> >> ?2 files changed, 3 insertions(+), 0 deletions(-)
> >
> > I think this patch should be the last one of the series.
> Yes, but I don't think it matter much.

It will if you bisect and end up here without the clock patch, no?

-- 
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/20110228/caf7af73/attachment.sig>

Re: [PATCH V3 1/4] ARM: imx53_loco: add esdhc device support

[Richard Zhao]

On Mon, Feb 28, 2011 at 8:43 PM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Richard,
>
> On Mon, Feb 28, 2011 at 8:25 AM, Richard Zhu
> <Hong-Xing.Zhu@freescale.com> wrote:
>> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
>> ---
>>  arch/arm/mach-mx5/Kconfig           |    1 +
>>  arch/arm/mach-mx5/board-mx53_loco.c |    2 ++
>>  2 files changed, 3 insertions(+), 0 deletions(-)
>
> I think this patch should be the last one of the series.
Yes, but I don't think it matter much.

Thanks
Richard
>
> Regards,
>
> Fabio Estevam
>

[PATCH V3 1/4] ARM: imx53_loco: add esdhc device support

[Richard Zhao]

On Mon, Feb 28, 2011 at 8:43 PM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Richard,
>
> On Mon, Feb 28, 2011 at 8:25 AM, Richard Zhu
> <Hong-Xing.Zhu@freescale.com> wrote:
>> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
>> ---
>> ?arch/arm/mach-mx5/Kconfig ? ? ? ? ? | ? ?1 +
>> ?arch/arm/mach-mx5/board-mx53_loco.c | ? ?2 ++
>> ?2 files changed, 3 insertions(+), 0 deletions(-)
>
> I think this patch should be the last one of the series.
Yes, but I don't think it matter much.

Thanks
Richard
>
> Regards,
>
> Fabio Estevam
>

Re: [PATCH V3 1/4] ARM: imx53_loco: add esdhc device support

[Fabio Estevam]

Hi Richard,

On Mon, Feb 28, 2011 at 8:25 AM, Richard Zhu
<Hong-Xing.Zhu@freescale.com> wrote:
> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
> ---
>  arch/arm/mach-mx5/Kconfig           |    1 +
>  arch/arm/mach-mx5/board-mx53_loco.c |    2 ++
>  2 files changed, 3 insertions(+), 0 deletions(-)

I think this patch should be the last one of the series.

Regards,

Fabio Estevam

[PATCH V3 1/4] ARM: imx53_loco: add esdhc device support

[Fabio Estevam]

Hi Richard,

On Mon, Feb 28, 2011 at 8:25 AM, Richard Zhu
<Hong-Xing.Zhu@freescale.com> wrote:
> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
> ---
> ?arch/arm/mach-mx5/Kconfig ? ? ? ? ? | ? ?1 +
> ?arch/arm/mach-mx5/board-mx53_loco.c | ? ?2 ++
> ?2 files changed, 3 insertions(+), 0 deletions(-)

I think this patch should be the last one of the series.

Regards,

Fabio Estevam

[PATCH V3 1/4] ARM: imx53_loco: add esdhc device support

[Richard Zhu]

Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
 arch/arm/mach-mx5/Kconfig           |    1 +
 arch/arm/mach-mx5/board-mx53_loco.c |    2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig
index f065a0d..a72c833 100644
--- a/arch/arm/mach-mx5/Kconfig
+++ b/arch/arm/mach-mx5/Kconfig
@@ -162,6 +162,7 @@ config MACH_MX53_LOCO
 	select IMX_HAVE_PLATFORM_IMX2_WDT
 	select IMX_HAVE_PLATFORM_IMX_I2C
 	select IMX_HAVE_PLATFORM_IMX_UART
+	select IMX_HAVE_PLATFORM_SDHCI_ESDHC_IMX
 	help
 	  Include support for MX53 LOCO platform. This includes specific
 	  configurations for the board and its peripherals.
diff --git a/arch/arm/mach-mx5/board-mx53_loco.c b/arch/arm/mach-mx5/board-mx53_loco.c
index 160899e..0a18f8d 100644
--- a/arch/arm/mach-mx5/board-mx53_loco.c
+++ b/arch/arm/mach-mx5/board-mx53_loco.c
@@ -213,6 +213,8 @@ static void __init mx53_loco_board_init(void)
 	imx53_add_imx2_wdt(0, NULL);
 	imx53_add_imx_i2c(0, &mx53_loco_i2c_data);
 	imx53_add_imx_i2c(1, &mx53_loco_i2c_data);
+	imx53_add_sdhci_esdhc_imx(0, NULL);
+	imx53_add_sdhci_esdhc_imx(2, NULL);
 }
 
 static void __init mx53_loco_timer_init(void)
-- 
1.7.1

[PATCH V3 1/4] ARM: imx53_loco: add esdhc device support

[Richard Zhu]

Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
 arch/arm/mach-mx5/Kconfig           |    1 +
 arch/arm/mach-mx5/board-mx53_loco.c |    2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig
index f065a0d..a72c833 100644
--- a/arch/arm/mach-mx5/Kconfig
+++ b/arch/arm/mach-mx5/Kconfig
@@ -162,6 +162,7 @@ config MACH_MX53_LOCO
 	select IMX_HAVE_PLATFORM_IMX2_WDT
 	select IMX_HAVE_PLATFORM_IMX_I2C
 	select IMX_HAVE_PLATFORM_IMX_UART
+	select IMX_HAVE_PLATFORM_SDHCI_ESDHC_IMX
 	help
 	  Include support for MX53 LOCO platform. This includes specific
 	  configurations for the board and its peripherals.
diff --git a/arch/arm/mach-mx5/board-mx53_loco.c b/arch/arm/mach-mx5/board-mx53_loco.c
index 160899e..0a18f8d 100644
--- a/arch/arm/mach-mx5/board-mx53_loco.c
+++ b/arch/arm/mach-mx5/board-mx53_loco.c
@@ -213,6 +213,8 @@ static void __init mx53_loco_board_init(void)
 	imx53_add_imx2_wdt(0, NULL);
 	imx53_add_imx_i2c(0, &mx53_loco_i2c_data);
 	imx53_add_imx_i2c(1, &mx53_loco_i2c_data);
+	imx53_add_sdhci_esdhc_imx(0, NULL);
+	imx53_add_sdhci_esdhc_imx(2, NULL);
 }
 
 static void __init mx53_loco_timer_init(void)
-- 
1.7.1