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

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(-)

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;
/*

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 {

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 }

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 */

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?

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)

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);

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;

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;

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

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 */

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; }

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(-)

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(-)

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!
participants (3)
-
Jaehoon Chung
-
Kever Yang
-
Masahiro Yamada