[U-Boot] [PATCH v2 0/6] mmc: sdhci: some fixes for CONFIG_BLK migration and macro renaming
Changes in v2: - Adjust comment block
Masahiro Yamada (6): mmc: sdhci: move sdhci_reset() call to sdhci_init() mmc: sdhci: move error message to more relevant place mmc: sdhci: move broken voltage quirk handling to sdhci_setup_cfg() mmc: sdhci: move SDMA capability check to sdhci_setup_cfg() mmc: sdhci: drop CONFIG_ from CONFIG_SDHCI_CMD_DEFAULT_TIME mmc: sdhci: drop CONFIG_ from CONFIG_SDHCI_CMD_MAX_TIMEOUT
drivers/mmc/sdhci.c | 55 +++++++++++++++++++++++++---------------------------- 1 file changed, 26 insertions(+), 29 deletions(-)
If CONFIG_BLK is enabled, add_sdhci() is never called. So, sdhci_reset() is not called, either. This is a problem for my board as it needs the reset to start from a sane state.
Move the add_sdhci() call to sdhci_init(), which is visited by both of the with/without CONFIG_BLK cases.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
Changes in v2: None
drivers/mmc/sdhci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 7ddb549..91cc8b2 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -451,6 +451,8 @@ static int sdhci_init(struct mmc *mmc) { struct sdhci_host *host = mmc->priv;
+ sdhci_reset(host, SDHCI_RESET_ALL); + if ((host->quirks & SDHCI_QUIRK_32BIT_DMA_ADDR) && !aligned_buffer) { aligned_buffer = memalign(8, 512*1024); if (!aligned_buffer) { @@ -595,8 +597,6 @@ int add_sdhci(struct sdhci_host *host, u32 max_clk, u32 min_clk) if (host->quirks & SDHCI_QUIRK_BROKEN_VOLTAGE) host->cfg.voltages |= host->voltages;
- sdhci_reset(host, SDHCI_RESET_ALL); - host->mmc = mmc_create(&host->cfg, host); if (host->mmc == NULL) { printf("%s: mmc create fail!\n", __func__);
"Hardware doesn't specify base clock frequency" may not be only the error case of sdhci_setup_cfg(). It is better to print this where the corresponding error is triggered.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
Changes in v2: None
drivers/mmc/sdhci.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 91cc8b2..d886777 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -536,8 +536,11 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host, SDHCI_CLOCK_BASE_SHIFT; cfg->f_max *= 1000000; } - if (cfg->f_max == 0) + if (cfg->f_max == 0) { + printf("%s: Hardware doesn't specify base clock frequency\n", + __func__); return -EINVAL; + } if (min_clk) cfg->f_min = min_clk; else { @@ -577,6 +580,8 @@ int sdhci_bind(struct udevice *dev, struct mmc *mmc, struct mmc_config *cfg) #else int add_sdhci(struct sdhci_host *host, u32 max_clk, u32 min_clk) { + int ret; + #ifdef CONFIG_MMC_SDMA unsigned int caps;
@@ -588,11 +593,9 @@ int add_sdhci(struct sdhci_host *host, u32 max_clk, u32 min_clk) } #endif
- if (sdhci_setup_cfg(&host->cfg, host, max_clk, min_clk)) { - printf("%s: Hardware doesn't specify base clock frequency\n", - __func__); - return -EINVAL; - } + ret = sdhci_setup_cfg(&host->cfg, host, max_clk, min_clk); + if (ret) + return ret;
if (host->quirks & SDHCI_QUIRK_BROKEN_VOLTAGE) host->cfg.voltages |= host->voltages;
If CONFIG_BLK is enabled, add_sdhci() is never called. Move this quirk handling to sdhci_setup_cfg(), which is now the central place for hardware capability checks.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
Changes in v2: None
drivers/mmc/sdhci.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index d886777..c66151c 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -557,6 +557,9 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host, if (caps & SDHCI_CAN_VDD_180) cfg->voltages |= MMC_VDD_165_195;
+ if (host->quirks & SDHCI_QUIRK_BROKEN_VOLTAGE) + cfg->voltages |= host->voltages; + 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) @@ -597,9 +600,6 @@ int add_sdhci(struct sdhci_host *host, u32 max_clk, u32 min_clk) if (ret) return ret;
- if (host->quirks & SDHCI_QUIRK_BROKEN_VOLTAGE) - host->cfg.voltages |= host->voltages; - host->mmc = mmc_create(&host->cfg, host); if (host->mmc == NULL) { printf("%s: mmc create fail!\n", __func__);
If CONFIG_BLK is enabled, add_sdhci() is never called. Move this quirk handling to sdhci_setup_cfg(), which is now the central place for hardware capability checks.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
Changes in v2: None
drivers/mmc/sdhci.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index c66151c..da73653 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -519,6 +519,14 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host, u32 caps;
caps = sdhci_readl(host, SDHCI_CAPABILITIES); + +#ifdef CONFIG_MMC_SDMA + if (!(caps & SDHCI_CAN_DO_SDMA)) { + printf("%s: Your controller doesn't support SDMA!!\n", + __func__); + return -EINVAL; + } +#endif host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
cfg->name = host->name; @@ -585,17 +593,6 @@ int add_sdhci(struct sdhci_host *host, u32 max_clk, u32 min_clk) { int ret;
-#ifdef CONFIG_MMC_SDMA - unsigned int caps; - - caps = sdhci_readl(host, SDHCI_CAPABILITIES); - if (!(caps & SDHCI_CAN_DO_SDMA)) { - printf("%s: Your controller doesn't support SDMA!!\n", - __func__); - return -1; - } -#endif - ret = sdhci_setup_cfg(&host->cfg, host, max_clk, min_clk); if (ret) return ret;
This CONFIG is not configurable since it is not guarded by #ifndef. Nobody has complained about that, so there is no need to keep it as a CONFIG option.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
Changes in v2: None
drivers/mmc/sdhci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index da73653..d486082 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -127,7 +127,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data, #ifndef CONFIG_SDHCI_CMD_MAX_TIMEOUT #define CONFIG_SDHCI_CMD_MAX_TIMEOUT 3200 #endif -#define CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT 100 +#define SDHCI_CMD_DEFAULT_TIMEOUT 100 #define SDHCI_READ_STATUS_TIMEOUT 1000
#ifdef CONFIG_DM_MMC_OPS @@ -151,7 +151,7 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, unsigned start = get_timer(0);
/* Timeout unit - ms */ - static unsigned int cmd_timeout = CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT; + static unsigned int cmd_timeout = SDHCI_CMD_DEFAULT_TIMEOUT;
sdhci_writel(host, SDHCI_INT_ALL_MASK, SDHCI_INT_STATUS); mask = SDHCI_CMD_INHIBIT | SDHCI_DATA_INHIBIT;
No need for per-SoC adjustment for this parameter. It should be determined by the slowest hardware. Currently, no board overrides this CONFIG, so 3.2 sec is large enough. (If not, we can make it even larger.)
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
Changes in v2: - Adjust comment block
drivers/mmc/sdhci.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index d486082..504f2d2 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -121,12 +121,9 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data, * for card ready state. * Every time when card is busy after timeout then (last) timeout value will be * increased twice but only if it doesn't exceed global defined maximum. - * Each function call will use last timeout value. Max timeout can be redefined - * in board config file. + * Each function call will use last timeout value. */ -#ifndef CONFIG_SDHCI_CMD_MAX_TIMEOUT -#define CONFIG_SDHCI_CMD_MAX_TIMEOUT 3200 -#endif +#define SDHCI_CMD_MAX_TIMEOUT 3200 #define SDHCI_CMD_DEFAULT_TIMEOUT 100 #define SDHCI_READ_STATUS_TIMEOUT 1000
@@ -164,7 +161,7 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) { if (time >= cmd_timeout) { printf("%s: MMC: %d busy ", __func__, mmc_dev); - if (2 * cmd_timeout <= CONFIG_SDHCI_CMD_MAX_TIMEOUT) { + if (2 * cmd_timeout <= SDHCI_CMD_MAX_TIMEOUT) { cmd_timeout += cmd_timeout; printf("timeout increasing to: %u ms.\n", cmd_timeout);
Hi Masahiro,
On 08/25/2016 04:07 PM, Masahiro Yamada wrote:
Changes in v2:
- Adjust comment block
Masahiro Yamada (6): mmc: sdhci: move sdhci_reset() call to sdhci_init() mmc: sdhci: move error message to more relevant place mmc: sdhci: move broken voltage quirk handling to sdhci_setup_cfg() mmc: sdhci: move SDMA capability check to sdhci_setup_cfg() mmc: sdhci: drop CONFIG_ from CONFIG_SDHCI_CMD_DEFAULT_TIME mmc: sdhci: drop CONFIG_ from CONFIG_SDHCI_CMD_MAX_TIMEOUT
Applied on u-boot-mmc. Thanks!
Best Regards, Jaehoon Chung
drivers/mmc/sdhci.c | 55 +++++++++++++++++++++++++---------------------------- 1 file changed, 26 insertions(+), 29 deletions(-)
Hi Jaehoon,
2016-08-30 18:40 GMT+09:00 Jaehoon Chung jh80.chung@samsung.com:
Hi Masahiro,
On 08/25/2016 04:07 PM, Masahiro Yamada wrote:
Changes in v2:
- Adjust comment block
Masahiro Yamada (6): mmc: sdhci: move sdhci_reset() call to sdhci_init() mmc: sdhci: move error message to more relevant place mmc: sdhci: move broken voltage quirk handling to sdhci_setup_cfg() mmc: sdhci: move SDMA capability check to sdhci_setup_cfg() mmc: sdhci: drop CONFIG_ from CONFIG_SDHCI_CMD_DEFAULT_TIME mmc: sdhci: drop CONFIG_ from CONFIG_SDHCI_CMD_MAX_TIMEOUT
Applied on u-boot-mmc. Thanks!
Best Regards, Jaehoon Chung
Could you send a PR, please?
"mmc: sdhci: move sdhci_reset() call to sdhci_init()" is a bug-fix for me. I want it in the mainline.
Hi Masahiro
On 09/21/2016 12:59 PM, Masahiro Yamada wrote:
Hi Jaehoon,
2016-08-30 18:40 GMT+09:00 Jaehoon Chung jh80.chung@samsung.com:
Hi Masahiro,
On 08/25/2016 04:07 PM, Masahiro Yamada wrote:
Changes in v2:
- Adjust comment block
Masahiro Yamada (6): mmc: sdhci: move sdhci_reset() call to sdhci_init() mmc: sdhci: move error message to more relevant place mmc: sdhci: move broken voltage quirk handling to sdhci_setup_cfg() mmc: sdhci: move SDMA capability check to sdhci_setup_cfg() mmc: sdhci: drop CONFIG_ from CONFIG_SDHCI_CMD_DEFAULT_TIME mmc: sdhci: drop CONFIG_ from CONFIG_SDHCI_CMD_MAX_TIMEOUT
Applied on u-boot-mmc. Thanks!
Best Regards, Jaehoon Chung
Could you send a PR, please?
Sure, I will do that on toady!
Best Regards, Jaehoon Chung
"mmc: sdhci: move sdhci_reset() call to sdhci_init()" is a bug-fix for me. I want it in the mainline.
participants (2)
-
Jaehoon Chung -
Masahiro Yamada