[U-Boot] [PATCH 00/11] mmc: clean the sdhci host controller

[U-Boot] [PATCH 00/11] mmc: clean the sdhci host controller

[Jaehoon Chung]

It's cleaned the condes related to SDHCI host controller.
Move the callback function into sdhci_ops structure.
And removed the unnecessary things.

My next step should be more clearly than now.
- Support DM feature than now.
- Make driver model for controller that unused driver mode.

Jaehoon Chung (11):
  mmc: sdhci: disable the 8bit mode when host doesn't support it
  mmc: sdhci: add the get_cd callback function in sdhci_ops
  mmc: sdhci: remove the unused code about testing Card detect
  mmc: pic32_sdhci: move the code to pic32_sdhci.c
  mmc: sdhci: remove the SDHCI_QUIRK_NO_CD
  mmc: change the set_ios return type from void to int
  mmc: s5p_sdhci: add the s5p_set_clock function
  mmc: sdhci: move the callback function into sdhci_ops
  mmc: sdhci: use the bitops APIs in sdhci.h
  mmc: sdhci: remove the SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER
  mmc: sdhci: combine the Host controller v3.0 feature into one
    condition

 drivers/mmc/dw_mmc.c      |   5 +-
 drivers/mmc/pic32_sdhci.c |  15 +++++-
 drivers/mmc/s5p_sdhci.c   |  15 ++++--
 drivers/mmc/sdhci.c       |  59 ++++++---------------
 include/mmc.h             |   2 +-
 include/sdhci.h           | 131 +++++++++++++++++++++++-----------------------
 6 files changed, 111 insertions(+), 116 deletions(-)

-- 
2.10.2

[U-Boot] [PATCH 01/11] mmc: sdhci: disable the 8bit mode when host doesn't support it

[Jaehoon Chung]

Buswidth is depeneded on Hardware schematic.
Evne though host can support the 8bit buswidth, if hardware doesn't
support 8bit mode, it doesn't work fine.
So the buswidth mode selection leaves a matter in each SoC drivers.

On the contrary to this, hardware supports 8bit mode, but host doesn't
support it. then controller has to disable the MMC_MODE_8BIT.
(Host can check whether 8bit mode is supported or not, since V3.0)

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/sdhci.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index cbf5f56..853c268 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -594,14 +594,13 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host,
 
 	cfg->host_caps = MMC_MODE_HS | MMC_MODE_HS_52MHz | MMC_MODE_4BIT;
 	if (SDHCI_GET_VERSION(host) >= SDHCI_SPEC_300) {
-		if (caps & SDHCI_CAN_DO_8BIT)
-			cfg->host_caps |= MMC_MODE_8BIT;
+		if (!(caps & SDHCI_CAN_DO_8BIT))
+			cfg->host_caps &= ~MMC_MODE_8BIT;
 	}
 
 	if (host->host_caps)
 		cfg->host_caps |= host->host_caps;
 
-
 	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
 
 	/*
-- 
2.10.2

[U-Boot] [PATCH 02/11] mmc: sdhci: add the get_cd callback function in sdhci_ops

[Jaehoon Chung]

Some SoCs can have their own card dect scheme.
Then they may use this get_cd callback function after implementing init
in their drivers.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 include/sdhci.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/sdhci.h b/include/sdhci.h
index 144570f..0c0f48f 100644
--- a/include/sdhci.h
+++ b/include/sdhci.h
@@ -235,6 +235,7 @@ struct sdhci_ops {
 	void            (*write_w)(struct sdhci_host *host, u16 val, int reg);
 	void            (*write_b)(struct sdhci_host *host, u8 val, int reg);
 #endif
+	int		(*get_cd)(struct sdhci_host *host);
 };
 
 struct sdhci_host {
-- 
2.10.2

[U-Boot] [PATCH 03/11] mmc: sdhci: remove the unused code about testing Card detect

[Jaehoon Chung]

This code is dead code..There is no usage anywhere.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/sdhci.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 853c268..c512a4c 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -490,17 +490,6 @@ static int sdhci_init(struct mmc *mmc)
 		 * - set CD_TEST and clear CD_TEST_INS bit
 		 */
 		sdhci_writeb(host, SDHCI_CTRL_CD_TEST, SDHCI_HOST_CONTROL);
-#else
-		unsigned int status;
-
-		sdhci_writeb(host, SDHCI_CTRL_CD_TEST_INS | SDHCI_CTRL_CD_TEST,
-			SDHCI_HOST_CONTROL);
-
-		status = sdhci_readl(host, SDHCI_PRESENT_STATE);
-		while ((!(status & SDHCI_CARD_PRESENT)) ||
-		    (!(status & SDHCI_CARD_STATE_STABLE)) ||
-		    (!(status & SDHCI_CARD_DETECT_PIN_LEVEL)))
-			status = sdhci_readl(host, SDHCI_PRESENT_STATE);
 #endif
 	}
 
-- 
2.10.2

[U-Boot] [PATCH 04/11] mmc: pic32_sdhci: move the code to pic32_sdhci.c

[Jaehoon Chung]

This code is used for only pic32_sdhci controller.
To remove the "#ifdef", moves to pic32_sdhci.c.
And use the get_cd callback function.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/pic32_sdhci.c | 13 +++++++++++++
 drivers/mmc/sdhci.c       |  9 +++------
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/pic32_sdhci.c b/drivers/mmc/pic32_sdhci.c
index 2abf943..d8b6d8a 100644
--- a/drivers/mmc/pic32_sdhci.c
+++ b/drivers/mmc/pic32_sdhci.c
@@ -15,6 +15,18 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+static int pci32_sdhci_get_cd(struct sdhci_host)
+{
+	/* PIC32 SDHCI CD errata:
+	 * - set CD_TEST and clear CD_TEST_INS bit
+	 */
+	sdhci_writeb(host, SDHCI_CTRL_CD_TEST, SDHCI_HOST_CONTROL);
+}
+
+static const struct sdhci_ops pic32_sdhci_ops = {
+	.get_cd	= pci32_sdhci_get_cd,
+};
+
 static int pic32_sdhci_probe(struct udevice *dev)
 {
 	struct sdhci_host *host = dev_get_priv(dev);
@@ -33,6 +45,7 @@ static int pic32_sdhci_probe(struct udevice *dev)
 	host->quirks	= SDHCI_QUIRK_NO_HISPD_BIT | SDHCI_QUIRK_NO_CD;
 	host->bus_width	= fdtdec_get_int(gd->fdt_blob, dev->of_offset,
 					"bus-width", 4);
+	host->ops = &pci32_sdhci_ops;
 
 	ret = fdtdec_get_int_array(gd->fdt_blob, dev->of_offset,
 				   "clock-freq-min-max", f_min_max, 2);
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index c512a4c..aeac805 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -484,13 +484,10 @@ static int sdhci_init(struct mmc *mmc)
 
 	sdhci_set_power(host, fls(mmc->cfg->voltages) - 1);
 
+
 	if (host->quirks & SDHCI_QUIRK_NO_CD) {
-#if defined(CONFIG_PIC32_SDHCI)
-		/* PIC32 SDHCI CD errata:
-		 * - set CD_TEST and clear CD_TEST_INS bit
-		 */
-		sdhci_writeb(host, SDHCI_CTRL_CD_TEST, SDHCI_HOST_CONTROL);
-#endif
+		if (host->ops->get_cd)
+			host->ops->get_cd(host);
 	}
 
 	/* Enable only interrupts served by the SD controller */
-- 
2.10.2

[U-Boot] [PATCH 05/11] mmc: sdhci: remove the SDHCI_QUIRK_NO_CD

[Jaehoon Chung]

This quirk doesn't need anymore.
It's replaced to get_cd callback function.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/pic32_sdhci.c | 2 +-
 drivers/mmc/sdhci.c       | 7 ++-----
 include/sdhci.h           | 1 -
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/pic32_sdhci.c b/drivers/mmc/pic32_sdhci.c
index d8b6d8a..2f061f4 100644
--- a/drivers/mmc/pic32_sdhci.c
+++ b/drivers/mmc/pic32_sdhci.c
@@ -42,7 +42,7 @@ static int pic32_sdhci_probe(struct udevice *dev)
 
 	host->ioaddr	= ioremap(addr, size);
 	host->name	= dev->name;
-	host->quirks	= SDHCI_QUIRK_NO_HISPD_BIT | SDHCI_QUIRK_NO_CD;
+	host->quirks	= SDHCI_QUIRK_NO_HISPD_BIT;
 	host->bus_width	= fdtdec_get_int(gd->fdt_blob, dev->of_offset,
 					"bus-width", 4);
 	host->ops = &pci32_sdhci_ops;
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index aeac805..dadee15 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -484,11 +484,8 @@ static int sdhci_init(struct mmc *mmc)
 
 	sdhci_set_power(host, fls(mmc->cfg->voltages) - 1);
 
-
-	if (host->quirks & SDHCI_QUIRK_NO_CD) {
-		if (host->ops->get_cd)
-			host->ops->get_cd(host);
-	}
+	if (host->ops->get_cd)
+		host->ops->get_cd(host);
 
 	/* Enable only interrupts served by the SD controller */
 	sdhci_writel(host, SDHCI_INT_DATA_MASK | SDHCI_INT_CMD_MASK,
diff --git a/include/sdhci.h b/include/sdhci.h
index 0c0f48f..e4299d1 100644
--- a/include/sdhci.h
+++ b/include/sdhci.h
@@ -213,7 +213,6 @@
 #define SDHCI_QUIRK_BROKEN_R1B		(1 << 2)
 #define SDHCI_QUIRK_NO_HISPD_BIT	(1 << 3)
 #define SDHCI_QUIRK_BROKEN_VOLTAGE	(1 << 4)
-#define SDHCI_QUIRK_NO_CD		(1 << 5)
 #define SDHCI_QUIRK_WAIT_SEND_CMD	(1 << 6)
 #define SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER (1 << 7)
 #define SDHCI_QUIRK_USE_WIDE8		(1 << 8)
-- 
2.10.2

[U-Boot] [PATCH 06/11] mmc: change the set_ios return type from void to int

[Jaehoon Chung]

To maintain consistency, set_ios type of legacy mmc_ops changed to int.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/dw_mmc.c | 5 ++---
 drivers/mmc/sdhci.c  | 5 ++---
 include/mmc.h        | 2 +-
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index d6ac46c..700f764 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -388,7 +388,7 @@ static int dwmci_set_ios(struct udevice *dev)
 {
 	struct mmc *mmc = mmc_get_mmc_dev(dev);
 #else
-static void dwmci_set_ios(struct mmc *mmc)
+static int dwmci_set_ios(struct mmc *mmc)
 {
 #endif
 	struct dwmci_host *host = (struct dwmci_host *)mmc->priv;
@@ -421,9 +421,8 @@ static void dwmci_set_ios(struct mmc *mmc)
 
 	if (host->clksel)
 		host->clksel(host);
-#ifdef CONFIG_DM_MMC_OPS
+
 	return 0;
-#endif
 }
 
 static int dwmci_init(struct mmc *mmc)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index dadee15..9125e5d 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -424,7 +424,7 @@ static int sdhci_set_ios(struct udevice *dev)
 {
 	struct mmc *mmc = mmc_get_mmc_dev(dev);
 #else
-static void sdhci_set_ios(struct mmc *mmc)
+static int sdhci_set_ios(struct mmc *mmc)
 {
 #endif
 	u32 ctrl;
@@ -462,9 +462,8 @@ static void sdhci_set_ios(struct mmc *mmc)
 		ctrl &= ~SDHCI_CTRL_HISPD;
 
 	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
-#ifdef CONFIG_DM_MMC_OPS
+
 	return 0;
-#endif
 }
 
 static int sdhci_init(struct mmc *mmc)
diff --git a/include/mmc.h b/include/mmc.h
index 1720955..fad12d6 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -376,7 +376,7 @@ int mmc_getwp(struct mmc *mmc);
 struct mmc_ops {
 	int (*send_cmd)(struct mmc *mmc,
 			struct mmc_cmd *cmd, struct mmc_data *data);
-	void (*set_ios)(struct mmc *mmc);
+	int (*set_ios)(struct mmc *mmc);
 	int (*init)(struct mmc *mmc);
 	int (*getcd)(struct mmc *mmc);
 	int (*getwp)(struct mmc *mmc);
-- 
2.10.2

[U-Boot] [PATCH 07/11] mmc: s5p_sdhci: add the s5p_set_clock function

[Jaehoon Chung]

Add the s5p_set_clock function.
It's not good that "set_mmc_clk" is assigned directly.
In future, it should be changed to use the clock framework.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/s5p_sdhci.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c
index ac737e0..2a4cdc0 100644
--- a/drivers/mmc/s5p_sdhci.c
+++ b/drivers/mmc/s5p_sdhci.c
@@ -73,6 +73,12 @@ static void s5p_sdhci_set_control_reg(struct sdhci_host *host)
 	sdhci_writel(host, ctrl, SDHCI_CONTROL2);
 }
 
+static void s5p_set_clock(struct sdhci_host *host, u32 div)
+{
+	/* ToDo : Use the Clock Framework */
+	set_mmc_clk(host->index, div);
+}
+
 static int s5p_sdhci_core_init(struct sdhci_host *host)
 {
 	host->name = S5P_NAME;
@@ -83,7 +89,7 @@ static int s5p_sdhci_core_init(struct sdhci_host *host)
 	host->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
 
 	host->set_control_reg = &s5p_sdhci_set_control_reg;
-	host->set_clock = set_mmc_clk;
+	host->set_clock = &s5p_set_clock;
 
 	if (host->bus_width == 8)
 		host->host_caps |= MMC_MODE_8BIT;
-- 
2.10.2

[U-Boot] [PATCH 08/11] mmc: sdhci: move the callback function into sdhci_ops

[Jaehoon Chung]

callback function should be moved into sdhci_ops struct.
Other controller can use these ops for controlling clock or their own
specific register.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/s5p_sdhci.c |  9 ++++++---
 drivers/mmc/sdhci.c     |  8 ++++----
 include/sdhci.h         | 18 +++++++++---------
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c
index 2a4cdc0..1f1d2ed 100644
--- a/drivers/mmc/s5p_sdhci.c
+++ b/drivers/mmc/s5p_sdhci.c
@@ -79,6 +79,11 @@ static void s5p_set_clock(struct sdhci_host *host, u32 div)
 	set_mmc_clk(host->index, div);
 }
 
+static const struct sdhci_ops s5p_sdhci_ops = {
+	.set_clock	= &s5p_set_clock,
+	.set_control_reg = &s5p_sdhci_set_control_reg,
+};
+
 static int s5p_sdhci_core_init(struct sdhci_host *host)
 {
 	host->name = S5P_NAME;
@@ -87,9 +92,7 @@ static int s5p_sdhci_core_init(struct sdhci_host *host)
 		SDHCI_QUIRK_32BIT_DMA_ADDR |
 		SDHCI_QUIRK_WAIT_SEND_CMD | SDHCI_QUIRK_USE_WIDE8;
 	host->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
-
-	host->set_control_reg = &s5p_sdhci_set_control_reg;
-	host->set_clock = &s5p_set_clock;
+	host->ops = &s5p_sdhci_ops;
 
 	if (host->bus_width == 8)
 		host->host_caps |= MMC_MODE_8BIT;
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 9125e5d..6ce5e8f 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -359,8 +359,8 @@ static int sdhci_set_clock(struct mmc *mmc, unsigned int clock)
 		div >>= 1;
 	}
 
-	if (host->set_clock)
-		host->set_clock(host->index, div);
+	if (host->ops->set_clock)
+		host->ops->set_clock(host, div);
 
 	clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
 	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
@@ -430,8 +430,8 @@ static int sdhci_set_ios(struct mmc *mmc)
 	u32 ctrl;
 	struct sdhci_host *host = mmc->priv;
 
-	if (host->set_control_reg)
-		host->set_control_reg(host);
+	if (host->ops->set_control_reg)
+		host->ops->set_control_reg(host);
 
 	if (mmc->clock != host->clock)
 		sdhci_set_clock(mmc, mmc->clock);
diff --git a/include/sdhci.h b/include/sdhci.h
index e4299d1..abe4846 100644
--- a/include/sdhci.h
+++ b/include/sdhci.h
@@ -227,14 +227,16 @@ struct sdhci_host;
 #define SDHCI_DEFAULT_BOUNDARY_ARG	(7)
 struct sdhci_ops {
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
-	u32             (*read_l)(struct sdhci_host *host, int reg);
-	u16             (*read_w)(struct sdhci_host *host, int reg);
-	u8              (*read_b)(struct sdhci_host *host, int reg);
-	void            (*write_l)(struct sdhci_host *host, u32 val, int reg);
-	void            (*write_w)(struct sdhci_host *host, u16 val, int reg);
-	void            (*write_b)(struct sdhci_host *host, u8 val, int reg);
+	u32	(*read_l)(struct sdhci_host *host, int reg);
+	u16	(*read_w)(struct sdhci_host *host, int reg);
+	u8	(*read_b)(struct sdhci_host *host, int reg);
+	void	(*write_l)(struct sdhci_host *host, u32 val, int reg);
+	void	(*write_w)(struct sdhci_host *host, u16 val, int reg);
+	void	(*write_b)(struct sdhci_host *host, u8 val, int reg);
 #endif
-	int		(*get_cd)(struct sdhci_host *host);
+	int	(*get_cd)(struct sdhci_host *host);
+	void	(*set_control_reg)(struct sdhci_host *host);
+	void	(*set_clock)(struct sdhci_host *host, u32 div);
 };
 
 struct sdhci_host {
@@ -253,8 +255,6 @@ struct sdhci_host {
 	struct gpio_desc pwr_gpio;	/* Power GPIO */
 	struct gpio_desc cd_gpio;		/* Card Detect GPIO */
 
-	void (*set_control_reg)(struct sdhci_host *host);
-	void (*set_clock)(int dev_index, unsigned int div);
 	uint	voltages;
 
 	struct mmc_config cfg;
-- 
2.10.2

[U-Boot] [PATCH 09/11] mmc: sdhci: use the bitops APIs in sdhci.h

[Jaehoon Chung]

The using the bitops is too easy controlling than now.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 include/sdhci.h | 112 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 56 insertions(+), 56 deletions(-)

diff --git a/include/sdhci.h b/include/sdhci.h
index abe4846..9f6bbc8 100644
--- a/include/sdhci.h
+++ b/include/sdhci.h
@@ -28,11 +28,11 @@
 #define SDHCI_ARGUMENT		0x08
 
 #define SDHCI_TRANSFER_MODE	0x0C
-#define  SDHCI_TRNS_DMA		0x01
-#define  SDHCI_TRNS_BLK_CNT_EN	0x02
-#define  SDHCI_TRNS_ACMD12	0x04
-#define  SDHCI_TRNS_READ	0x10
-#define  SDHCI_TRNS_MULTI	0x20
+#define  SDHCI_TRNS_DMA		BIT(0)
+#define  SDHCI_TRNS_BLK_CNT_EN	BIT(1)
+#define  SDHCI_TRNS_ACMD12	BIT(2)
+#define  SDHCI_TRNS_READ	BIT(4)
+#define  SDHCI_TRNS_MULTI	BIT(5)
 
 #define SDHCI_COMMAND		0x0E
 #define  SDHCI_CMD_RESP_MASK	0x03
@@ -54,29 +54,29 @@
 #define SDHCI_BUFFER		0x20
 
 #define SDHCI_PRESENT_STATE	0x24
-#define  SDHCI_CMD_INHIBIT	0x00000001
-#define  SDHCI_DATA_INHIBIT	0x00000002
-#define  SDHCI_DOING_WRITE	0x00000100
-#define  SDHCI_DOING_READ	0x00000200
-#define  SDHCI_SPACE_AVAILABLE	0x00000400
-#define  SDHCI_DATA_AVAILABLE	0x00000800
-#define  SDHCI_CARD_PRESENT	0x00010000
-#define  SDHCI_CARD_STATE_STABLE	0x00020000
-#define  SDHCI_CARD_DETECT_PIN_LEVEL	0x00040000
-#define  SDHCI_WRITE_PROTECT	0x00080000
+#define  SDHCI_CMD_INHIBIT	BIT(0)
+#define  SDHCI_DATA_INHIBIT	BIT(1)
+#define  SDHCI_DOING_WRITE	BIT(8)
+#define  SDHCI_DOING_READ	BIT(9)
+#define  SDHCI_SPACE_AVAILABLE	BIT(10)
+#define  SDHCI_DATA_AVAILABLE	BIT(11)
+#define  SDHCI_CARD_PRESENT	BIT(16)
+#define  SDHCI_CARD_STATE_STABLE	BIT(17)
+#define  SDHCI_CARD_DETECT_PIN_LEVEL	BIT(18)
+#define  SDHCI_WRITE_PROTECT	BIT(19)
 
 #define SDHCI_HOST_CONTROL	0x28
-#define  SDHCI_CTRL_LED		0x01
-#define  SDHCI_CTRL_4BITBUS	0x02
-#define  SDHCI_CTRL_HISPD	0x04
+#define  SDHCI_CTRL_LED		BIT(0)
+#define  SDHCI_CTRL_4BITBUS	BIT(1)
+#define  SDHCI_CTRL_HISPD	BIT(2)
 #define  SDHCI_CTRL_DMA_MASK	0x18
 #define   SDHCI_CTRL_SDMA	0x00
 #define   SDHCI_CTRL_ADMA1	0x08
 #define   SDHCI_CTRL_ADMA32	0x10
 #define   SDHCI_CTRL_ADMA64	0x18
-#define  SDHCI_CTRL_8BITBUS	0x20
-#define  SDHCI_CTRL_CD_TEST_INS	0x40
-#define  SDHCI_CTRL_CD_TEST	0x80
+#define  SDHCI_CTRL_8BITBUS	BIT(5)
+#define  SDHCI_CTRL_CD_TEST_INS	BIT(6)
+#define  SDHCI_CTRL_CD_TEST	BIT(7)
 
 #define SDHCI_POWER_CONTROL	0x29
 #define  SDHCI_POWER_ON		0x01
@@ -87,9 +87,9 @@
 #define SDHCI_BLOCK_GAP_CONTROL	0x2A
 
 #define SDHCI_WAKE_UP_CONTROL	0x2B
-#define  SDHCI_WAKE_ON_INT	0x01
-#define  SDHCI_WAKE_ON_INSERT	0x02
-#define  SDHCI_WAKE_ON_REMOVE	0x04
+#define  SDHCI_WAKE_ON_INT	BIT(0)
+#define  SDHCI_WAKE_ON_INSERT	BIT(1)
+#define  SDHCI_WAKE_ON_REMOVE	BIT(2)
 
 #define SDHCI_CLOCK_CONTROL	0x2C
 #define  SDHCI_DIVIDER_SHIFT	8
@@ -97,10 +97,10 @@
 #define  SDHCI_DIV_MASK	0xFF
 #define  SDHCI_DIV_MASK_LEN	8
 #define  SDHCI_DIV_HI_MASK	0x300
-#define  SDHCI_PROG_CLOCK_MODE  0x0020
-#define  SDHCI_CLOCK_CARD_EN	0x0004
-#define  SDHCI_CLOCK_INT_STABLE	0x0002
-#define  SDHCI_CLOCK_INT_EN	0x0001
+#define  SDHCI_PROG_CLOCK_MODE  BIT(5)
+#define  SDHCI_CLOCK_CARD_EN	BIT(2)
+#define  SDHCI_CLOCK_INT_STABLE	BIT(1)
+#define  SDHCI_CLOCK_INT_EN	BIT(0)
 
 #define SDHCI_TIMEOUT_CONTROL	0x2E
 
@@ -112,25 +112,25 @@
 #define SDHCI_INT_STATUS	0x30
 #define SDHCI_INT_ENABLE	0x34
 #define SDHCI_SIGNAL_ENABLE	0x38
-#define  SDHCI_INT_RESPONSE	0x00000001
-#define  SDHCI_INT_DATA_END	0x00000002
-#define  SDHCI_INT_DMA_END	0x00000008
-#define  SDHCI_INT_SPACE_AVAIL	0x00000010
-#define  SDHCI_INT_DATA_AVAIL	0x00000020
-#define  SDHCI_INT_CARD_INSERT	0x00000040
-#define  SDHCI_INT_CARD_REMOVE	0x00000080
-#define  SDHCI_INT_CARD_INT	0x00000100
-#define  SDHCI_INT_ERROR	0x00008000
-#define  SDHCI_INT_TIMEOUT	0x00010000
-#define  SDHCI_INT_CRC		0x00020000
-#define  SDHCI_INT_END_BIT	0x00040000
-#define  SDHCI_INT_INDEX	0x00080000
-#define  SDHCI_INT_DATA_TIMEOUT	0x00100000
-#define  SDHCI_INT_DATA_CRC	0x00200000
-#define  SDHCI_INT_DATA_END_BIT	0x00400000
-#define  SDHCI_INT_BUS_POWER	0x00800000
-#define  SDHCI_INT_ACMD12ERR	0x01000000
-#define  SDHCI_INT_ADMA_ERROR	0x02000000
+#define  SDHCI_INT_RESPONSE	BIT(0)
+#define  SDHCI_INT_DATA_END	BIT(1)
+#define  SDHCI_INT_DMA_END	BIT(3)
+#define  SDHCI_INT_SPACE_AVAIL	BIT(4)
+#define  SDHCI_INT_DATA_AVAIL	BIT(5)
+#define  SDHCI_INT_CARD_INSERT	BIT(6)
+#define  SDHCI_INT_CARD_REMOVE	BIT(7)
+#define  SDHCI_INT_CARD_INT	BIT(8)
+#define  SDHCI_INT_ERROR	BIT(15)
+#define  SDHCI_INT_TIMEOUT	BIT(16)
+#define  SDHCI_INT_CRC		BIT(17)
+#define  SDHCI_INT_END_BIT	BIT(18)
+#define  SDHCI_INT_INDEX	BIT(19)
+#define  SDHCI_INT_DATA_TIMEOUT	BIT(20)
+#define  SDHCI_INT_DATA_CRC	BIT(21)
+#define  SDHCI_INT_DATA_END_BIT	BIT(22)
+#define  SDHCI_INT_BUS_POWER	BIT(23)
+#define  SDHCI_INT_ACMD12ERR	BIT(24)
+#define  SDHCI_INT_ADMA_ERROR	BIT(25)
 
 #define  SDHCI_INT_NORMAL_MASK	0x00007FFF
 #define  SDHCI_INT_ERROR_MASK	0xFFFF8000
@@ -156,15 +156,15 @@
 #define  SDHCI_CLOCK_BASE_SHIFT	8
 #define  SDHCI_MAX_BLOCK_MASK	0x00030000
 #define  SDHCI_MAX_BLOCK_SHIFT  16
-#define  SDHCI_CAN_DO_8BIT	0x00040000
-#define  SDHCI_CAN_DO_ADMA2	0x00080000
-#define  SDHCI_CAN_DO_ADMA1	0x00100000
-#define  SDHCI_CAN_DO_HISPD	0x00200000
-#define  SDHCI_CAN_DO_SDMA	0x00400000
-#define  SDHCI_CAN_VDD_330	0x01000000
-#define  SDHCI_CAN_VDD_300	0x02000000
-#define  SDHCI_CAN_VDD_180	0x04000000
-#define  SDHCI_CAN_64BIT	0x10000000
+#define  SDHCI_CAN_DO_8BIT	BIT(18)
+#define  SDHCI_CAN_DO_ADMA2	BIT(19)
+#define  SDHCI_CAN_DO_ADMA1	BIT(20)
+#define  SDHCI_CAN_DO_HISPD	BIT(21)
+#define  SDHCI_CAN_DO_SDMA	BIT(22)
+#define  SDHCI_CAN_VDD_330	BIT(24)
+#define  SDHCI_CAN_VDD_300	BIT(25)
+#define  SDHCI_CAN_VDD_180	BIT(26)
+#define  SDHCI_CAN_64BIT	BIT(28)
 
 #define SDHCI_CAPABILITIES_1	0x44
 #define  SDHCI_CLOCK_MUL_MASK	0x00FF0000
-- 
2.10.2

[U-Boot] [PATCH 10/11] mmc: sdhci: remove the SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER

[Jaehoon Chung]

Ther is no usage anywhere. It doesn't need to maintain this bit.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/sdhci.c | 3 ---
 include/sdhci.h     | 1 -
 2 files changed, 4 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 6ce5e8f..5d8969a 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -411,9 +411,6 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
 		return;
 	}
 
-	if (host->quirks & SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER)
-		sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);
-
 	pwr |= SDHCI_POWER_ON;
 
 	sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);
diff --git a/include/sdhci.h b/include/sdhci.h
index 9f6bbc8..7544b49 100644
--- a/include/sdhci.h
+++ b/include/sdhci.h
@@ -214,7 +214,6 @@
 #define SDHCI_QUIRK_NO_HISPD_BIT	(1 << 3)
 #define SDHCI_QUIRK_BROKEN_VOLTAGE	(1 << 4)
 #define SDHCI_QUIRK_WAIT_SEND_CMD	(1 << 6)
-#define SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER (1 << 7)
 #define SDHCI_QUIRK_USE_WIDE8		(1 << 8)
 
 /* to make gcc happy */
-- 
2.10.2

[U-Boot] [PATCH 11/11] mmc: sdhci: combine the Host controller v3.0 feature into one condition

[Jaehoon Chung]

It doesn't need to seperate the condition.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/sdhci.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 5d8969a..3a1f4f7 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -572,9 +572,16 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host,
 		cfg->voltages |= host->voltages;
 
 	cfg->host_caps = MMC_MODE_HS | MMC_MODE_HS_52MHz | MMC_MODE_4BIT;
+
+	/* Since Host Controller Version3.0 */
 	if (SDHCI_GET_VERSION(host) >= SDHCI_SPEC_300) {
 		if (!(caps & SDHCI_CAN_DO_8BIT))
 			cfg->host_caps &= ~MMC_MODE_8BIT;
+
+		/* Find out whether clock multiplier is supported */
+		caps_1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
+		host->clk_mul = (caps_1 & SDHCI_CLOCK_MUL_MASK) >>
+				SDHCI_CLOCK_MUL_SHIFT;
 	}
 
 	if (host->host_caps)
@@ -582,16 +589,6 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host,
 
 	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
 
-	/*
-	 * In case of Host Controller v3.00, find out whether clock
-	 * multiplier is supported.
-	 */
-	if (SDHCI_GET_VERSION(host) >= SDHCI_SPEC_300) {
-		caps_1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
-		host->clk_mul = (caps_1 & SDHCI_CLOCK_MUL_MASK) >>
-				SDHCI_CLOCK_MUL_SHIFT;
-	}
-
 	return 0;
 }
 
-- 
2.10.2

[U-Boot] [PATCH 00/11] mmc: clean the sdhci host controller

[Kever Yang]

Hi Jaehoon,


On 12/30/2016 02:30 PM, Jaehoon Chung wrote:
> It's cleaned the condes related to SDHCI host controller.
> Move the callback function into sdhci_ops structure.
> And removed the unnecessary things.
>
> My next step should be more clearly than now.
> - Support DM feature than now.
> - Make driver model for controller that unused driver mode.

Is it possible to add "enable ADMA" and "support HS200" for SDHCI to 
your TODO list?

Thanks,
- Kever
>
> Jaehoon Chung (11):
>    mmc: sdhci: disable the 8bit mode when host doesn't support it
>    mmc: sdhci: add the get_cd callback function in sdhci_ops
>    mmc: sdhci: remove the unused code about testing Card detect
>    mmc: pic32_sdhci: move the code to pic32_sdhci.c
>    mmc: sdhci: remove the SDHCI_QUIRK_NO_CD
>    mmc: change the set_ios return type from void to int
>    mmc: s5p_sdhci: add the s5p_set_clock function
>    mmc: sdhci: move the callback function into sdhci_ops
>    mmc: sdhci: use the bitops APIs in sdhci.h
>    mmc: sdhci: remove the SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER
>    mmc: sdhci: combine the Host controller v3.0 feature into one
>      condition
>
>   drivers/mmc/dw_mmc.c      |   5 +-
>   drivers/mmc/pic32_sdhci.c |  15 +++++-
>   drivers/mmc/s5p_sdhci.c   |  15 ++++--
>   drivers/mmc/sdhci.c       |  59 ++++++---------------
>   include/mmc.h             |   2 +-
>   include/sdhci.h           | 131 +++++++++++++++++++++++-----------------------
>   6 files changed, 111 insertions(+), 116 deletions(-)
>

[U-Boot] [PATCH 00/11] mmc: clean the sdhci host controller

[Jaehoon Chung]

Hi Kever,

On 12/30/2016 03:55 PM, Kever Yang wrote:
> Hi Jaehoon,
> 
> 
> On 12/30/2016 02:30 PM, Jaehoon Chung wrote:
>> It's cleaned the condes related to SDHCI host controller.
>> Move the callback function into sdhci_ops structure.
>> And removed the unnecessary things.
>>
>> My next step should be more clearly than now.
>> - Support DM feature than now.
>> - Make driver model for controller that unused driver mode.
> 
> Is it possible to add "enable ADMA" and "support HS200" for SDHCI to your TODO list?

Yes..And I already have the patch for HS200 on u-boot.
But i needs to check something..so i didn't send the patch at mailing.

Best Regards,
Jaehoon Chung

> 
> Thanks,
> - Kever
>>
>> Jaehoon Chung (11):
>>    mmc: sdhci: disable the 8bit mode when host doesn't support it
>>    mmc: sdhci: add the get_cd callback function in sdhci_ops
>>    mmc: sdhci: remove the unused code about testing Card detect
>>    mmc: pic32_sdhci: move the code to pic32_sdhci.c
>>    mmc: sdhci: remove the SDHCI_QUIRK_NO_CD
>>    mmc: change the set_ios return type from void to int
>>    mmc: s5p_sdhci: add the s5p_set_clock function
>>    mmc: sdhci: move the callback function into sdhci_ops
>>    mmc: sdhci: use the bitops APIs in sdhci.h
>>    mmc: sdhci: remove the SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER
>>    mmc: sdhci: combine the Host controller v3.0 feature into one
>>      condition
>>
>>   drivers/mmc/dw_mmc.c      |   5 +-
>>   drivers/mmc/pic32_sdhci.c |  15 +++++-
>>   drivers/mmc/s5p_sdhci.c   |  15 ++++--
>>   drivers/mmc/sdhci.c       |  59 ++++++---------------
>>   include/mmc.h             |   2 +-
>>   include/sdhci.h           | 131 +++++++++++++++++++++++-----------------------
>>   6 files changed, 111 insertions(+), 116 deletions(-)
>>
> 
> 
> 
> 
> 

[U-Boot] [PATCH 00/11] mmc: clean the sdhci host controller

[Jaehoon Chung]

On 12/30/2016 03:30 PM, Jaehoon Chung wrote:
> It's cleaned the condes related to SDHCI host controller.
> Move the callback function into sdhci_ops structure.
> And removed the unnecessary things.
> 
> My next step should be more clearly than now.
> - Support DM feature than now.
> - Make driver model for controller that unused driver mode.
> 
> Jaehoon Chung (11):
>   mmc: sdhci: disable the 8bit mode when host doesn't support it
>   mmc: sdhci: add the get_cd callback function in sdhci_ops
>   mmc: sdhci: remove the unused code about testing Card detect
>   mmc: pic32_sdhci: move the code to pic32_sdhci.c
>   mmc: sdhci: remove the SDHCI_QUIRK_NO_CD
>   mmc: change the set_ios return type from void to int
>   mmc: s5p_sdhci: add the s5p_set_clock function
>   mmc: sdhci: move the callback function into sdhci_ops
>   mmc: sdhci: use the bitops APIs in sdhci.h
>   mmc: sdhci: remove the SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER
>   mmc: sdhci: combine the Host controller v3.0 feature into one
>     condition
> 
>  drivers/mmc/dw_mmc.c      |   5 +-
>  drivers/mmc/pic32_sdhci.c |  15 +++++-
>  drivers/mmc/s5p_sdhci.c   |  15 ++++--
>  drivers/mmc/sdhci.c       |  59 ++++++---------------
>  include/mmc.h             |   2 +-
>  include/sdhci.h           | 131 +++++++++++++++++++++++-----------------------
>  6 files changed, 111 insertions(+), 116 deletions(-)

Applied on u-boot-mmc.

Thanks!

> 

[U-Boot] [PATCH 04/11] mmc: pic32_sdhci: move the code to pic32_sdhci.c

[Masahiro Yamada]

2016-12-30 15:30 GMT+09:00 Jaehoon Chung <jh80.chung@samsung.com>:
> This code is used for only pic32_sdhci controller.
> To remove the "#ifdef", moves to pic32_sdhci.c.
> And use the get_cd callback function.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/mmc/pic32_sdhci.c | 13 +++++++++++++
>  drivers/mmc/sdhci.c       |  9 +++------
>  2 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/pic32_sdhci.c b/drivers/mmc/pic32_sdhci.c
> index 2abf943..d8b6d8a 100644
> --- a/drivers/mmc/pic32_sdhci.c
> +++ b/drivers/mmc/pic32_sdhci.c
> @@ -15,6 +15,18 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> +static int pci32_sdhci_get_cd(struct sdhci_host)
> +{
> +       /* PIC32 SDHCI CD errata:
> +        * - set CD_TEST and clear CD_TEST_INS bit
> +        */
> +       sdhci_writeb(host, SDHCI_CTRL_CD_TEST, SDHCI_HOST_CONTROL);
> +}
> +
> +static const struct sdhci_ops pic32_sdhci_ops = {
> +       .get_cd = pci32_sdhci_get_cd,
> +};
> +
>  static int pic32_sdhci_probe(struct udevice *dev)
>  {
>         struct sdhci_host *host = dev_get_priv(dev);
> @@ -33,6 +45,7 @@ static int pic32_sdhci_probe(struct udevice *dev)
>         host->quirks    = SDHCI_QUIRK_NO_HISPD_BIT | SDHCI_QUIRK_NO_CD;
>         host->bus_width = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
>                                         "bus-width", 4);
> +       host->ops = &pci32_sdhci_ops;
>
>         ret = fdtdec_get_int_array(gd->fdt_blob, dev->of_offset,
>                                    "clock-freq-min-max", f_min_max, 2);
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index c512a4c..aeac805 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -484,13 +484,10 @@ static int sdhci_init(struct mmc *mmc)
>
>         sdhci_set_power(host, fls(mmc->cfg->voltages) - 1);
>
> +
>         if (host->quirks & SDHCI_QUIRK_NO_CD) {
> -#if defined(CONFIG_PIC32_SDHCI)
> -               /* PIC32 SDHCI CD errata:
> -                * - set CD_TEST and clear CD_TEST_INS bit
> -                */
> -               sdhci_writeb(host, SDHCI_CTRL_CD_TEST, SDHCI_HOST_CONTROL);
> -#endif
> +               if (host->ops->get_cd)
> +                       host->ops->get_cd(host);

Question:

The name get_cd implies something (status of something?)
is returned and actually its return type is "int".

But, the return value of host->ops->get_cd is ignored, here.

What should this function do?

If the return value is unneeded, how about changing it into void type?


-- 
Best Regards
Masahiro Yamada