Re: [PATCH V2 15/15] mmc: core: Don't print reset warning if reset is not supported
Hi Adrian and Arend, 2015-02-09 10:47 GMT+01:00 Adrian Hunter adrian.hun...@intel.com: On 09/02/15 11:33, Arend van Spriel wrote: On 01/29/15 10:00, Adrian Hunter wrote: Check the error code for EOPNOTSUPP and do not print reset warning in that case. Signed-off-by: Adrian Hunteradrian.hun...@intel.com --- drivers/mmc/core/core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 392a150..d439bf0 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2368,7 +2368,8 @@ int mmc_hw_reset(struct mmc_host *host) ret = host-bus_ops-reset(host); mmc_bus_put(host); -pr_warn(%s: tried to reset card\n, mmc_hostname(host)); +if (ret != -EOPNOTSUPP) +pr_warn(%s: tried to reset card\n, mmc_hostname(host)); Guess you don't want this message either if ret is zero. Thanks for your comment. I assumed the original author wants to see the message whenever reset is attempted, which is OK by me because it is on the recovery path i.e. hopefully rare. The EOPNOTSUPP case is consistent with the code further up which returns EOPNOTSUPP when there is no -reset() callback. Speaking as the original author, yes, this was the idea. Thank you for this patch. BR, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 0/3] mmc: core: hw_reset changes
2015-01-13 9:19 GMT+01:00 Ulf Hansson ulf.hans...@linaro.org: On 12 January 2015 at 16:11, Ulf Hansson ulf.hans...@linaro.org wrote: On 12 January 2015 at 15:38, Johan Rudholm johan.rudh...@axis.com wrote: Make the mmc_hw_reset routine more generic, by putting the (e)MMC- specific stuff in the new bus_ops-reset in mmc.c. Add a bus_ops-reset for SD cards, allowing them to be restarted when errors occur. For MMC cards, always check if the reset was sucessful. This allows us to remove the mmc_hw_reset_check() interface. As I don't have an eMMC device myself, much less one with a reset line, I'd be very happy if someone could help me test the code with an eMMC? v7: - Rename bus_ops-hw_reset to reset - Move the send_status check to mmc.c - Put the changes concerning mmc_hw_reset_check in a separate patch v6: - Always perform the mmc_send_status reset check, which allows us to have only one interface to the card reset functions - Because of this, add the bus_ops-hw_reset to sd.c instead of falling back to a power_cycle v5: - Move the mmc_test-specific code to mmc_test.c - Fall back to a power_cycle if the bus_ops-hw_reset is missing - Because of this, skip the bus_ops-hw_reset in sd.c v4: - Rebase onto next v3: - Keep mmc_can_reset - Always set bus_mode = MMC_BUSMODE_PUSHPULL in mmc_set_initial_state() v2: - Call the new bus_ops member hw_reset instead of power_reset - Create mmc_set_initial_state and call it from mmc_mmc_hw_reset instead of mmc_power_up - Keep mmc_hw_reset naming Johan Rudholm (3): mmc: core: always check status after reset mmc: core: refactor the hw_reset routines mmc: sd: add reset bus_ops callback drivers/mmc/card/mmc_test.c | 18 + drivers/mmc/core/core.c | 59 +++--- drivers/mmc/core/core.h |1 + drivers/mmc/core/mmc.c | 41 + drivers/mmc/core/sd.c |7 + include/linux/mmc/core.h|1 - 6 files changed, 66 insertions(+), 61 deletions(-) -- 1.7.2.5 Thanks! Applied for next. Patch 1 caused compiler error and thus breaking bisectability. I decided to fix it up myself this time, but future wise please make sure each patch builds separately. Very sorry about that, won't happen again. Thanks! //Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 3/3] mmc: sd: add reset bus_ops callback
Enable power cycle and re-initialization of SD cards via the reset bus_ops. Power cycling a buggy SD card sometimes helps it get back on track. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/sd.c |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 29fccdc..36d5333 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -1197,6 +1197,12 @@ static int mmc_sd_power_restore(struct mmc_host *host) return ret; } +static int mmc_sd_reset(struct mmc_host *host) +{ + mmc_power_cycle(host, host-card-ocr); + return mmc_sd_power_restore(host); +} + static const struct mmc_bus_ops mmc_sd_ops = { .remove = mmc_sd_remove, .detect = mmc_sd_detect, @@ -1207,6 +1213,7 @@ static const struct mmc_bus_ops mmc_sd_ops = { .power_restore = mmc_sd_power_restore, .alive = mmc_sd_alive, .shutdown = mmc_sd_suspend, + .reset = mmc_sd_reset, }; /* -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 0/3] mmc: core: hw_reset changes
Make the mmc_hw_reset routine more generic, by putting the (e)MMC- specific stuff in the new bus_ops-reset in mmc.c. Add a bus_ops-reset for SD cards, allowing them to be restarted when errors occur. For MMC cards, always check if the reset was sucessful. This allows us to remove the mmc_hw_reset_check() interface. As I don't have an eMMC device myself, much less one with a reset line, I'd be very happy if someone could help me test the code with an eMMC? v7: - Rename bus_ops-hw_reset to reset - Move the send_status check to mmc.c - Put the changes concerning mmc_hw_reset_check in a separate patch v6: - Always perform the mmc_send_status reset check, which allows us to have only one interface to the card reset functions - Because of this, add the bus_ops-hw_reset to sd.c instead of falling back to a power_cycle v5: - Move the mmc_test-specific code to mmc_test.c - Fall back to a power_cycle if the bus_ops-hw_reset is missing - Because of this, skip the bus_ops-hw_reset in sd.c v4: - Rebase onto next v3: - Keep mmc_can_reset - Always set bus_mode = MMC_BUSMODE_PUSHPULL in mmc_set_initial_state() v2: - Call the new bus_ops member hw_reset instead of power_reset - Create mmc_set_initial_state and call it from mmc_mmc_hw_reset instead of mmc_power_up - Keep mmc_hw_reset naming Johan Rudholm (3): mmc: core: always check status after reset mmc: core: refactor the hw_reset routines mmc: sd: add reset bus_ops callback drivers/mmc/card/mmc_test.c | 18 + drivers/mmc/core/core.c | 59 +++--- drivers/mmc/core/core.h |1 + drivers/mmc/core/mmc.c | 41 + drivers/mmc/core/sd.c |7 + include/linux/mmc/core.h|1 - 6 files changed, 66 insertions(+), 61 deletions(-) -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/2] mmc: core: refactor the hw_reset routines
2015-01-12 13:55 GMT+01:00 Ulf Hansson ulf.hans...@linaro.org: On 9 January 2015 at 12:08, Johan Rudholm johan.rudh...@axis.com wrote: Move the (e)MMC specific hw_reset code from core.c into mmc.c Call the code from the new bus_ops member hw_reset. This also allows for adding a SD card specific reset procedure. Always check if the card is alive after a successful reset. This allows us to remove mmc_hw_reset_check(), leaving mmc_hw_reset() as the only card reset interface. This also informs any callers, for instance the block layer, if a reset was sucessful or not. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/card/mmc_test.c | 18 --- drivers/mmc/core/core.c | 70 +++ drivers/mmc/core/core.h |1 + drivers/mmc/core/mmc.c | 32 +++ include/linux/mmc/core.h|1 - 5 files changed, 64 insertions(+), 58 deletions(-) diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c index 0a7430f..7dac469 100644 --- a/drivers/mmc/card/mmc_test.c +++ b/drivers/mmc/card/mmc_test.c @@ -2342,20 +2342,16 @@ static int mmc_test_hw_reset(struct mmc_test_card *test) struct mmc_host *host = card-host; int err; - err = mmc_hw_reset_check(host); + if (!mmc_card_mmc(card) || !mmc_can_reset(card)) + return RESULT_UNSUP_CARD; + + err = mmc_hw_reset(host); if (!err) return RESULT_OK; + else if (err == -EOPNOTSUPP) + return RESULT_UNSUP_HOST; - if (err == -ENOSYS) - return RESULT_FAIL; - - if (err != -EOPNOTSUPP) - return err; - - if (!mmc_can_reset(card)) - return RESULT_UNSUP_CARD; - - return RESULT_UNSUP_HOST; + return RESULT_FAIL; } static const struct mmc_test_case mmc_test_cases[] = { diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index d3bfbdf..8598287 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2273,67 +2273,45 @@ static void mmc_hw_reset_for_init(struct mmc_host *host) mmc_host_clk_release(host); } -int mmc_can_reset(struct mmc_card *card) -{ - u8 rst_n_function; - - if (!mmc_card_mmc(card)) - return 0; - rst_n_function = card-ext_csd.rst_n_function; - if ((rst_n_function EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED) - return 0; - return 1; -} -EXPORT_SYMBOL(mmc_can_reset); - -static int mmc_do_hw_reset(struct mmc_host *host, int check) +int mmc_hw_reset(struct mmc_host *host) { - struct mmc_card *card = host-card; - - if (!(host-caps MMC_CAP_HW_RESET) || !host-ops-hw_reset) - return -EOPNOTSUPP; + int ret = 0; + int card_alive = 0; + u32 status; - if (!card) + if (!host-card) return -EINVAL; - if (!mmc_can_reset(card)) - return -EOPNOTSUPP; - - mmc_host_clk_hold(host); - mmc_set_clock(host, host-f_init); + mmc_bus_get(host); + if (!host-bus_ops || host-bus_dead || !host-bus_ops-hw_reset) { + ret = -EOPNOTSUPP; + goto out; + } - host-ops-hw_reset(host); + ret = host-bus_ops-hw_reset(host); + if (ret) + goto out; /* If the reset has happened, then a status command will fail */ - if (check) { - u32 status; + if (!mmc_send_status(host-card, status)) + card_alive = 1; - if (!mmc_send_status(card, status)) { - mmc_host_clk_release(host); - return -ENOSYS; - } - } + pr_warn(%s: tried to reset card (%s)\n, mmc_hostname(host), + card_alive ? failed : ok); /* Set initial state and call mmc_set_ios */ mmc_set_initial_state(host); + ret = host-bus_ops-power_restore(host); - mmc_host_clk_release(host); - - return host-bus_ops-power_restore(host); I would like you to move the mmc_send_status() and onwards code in this function, into the mmc specific -hw_reset() callback. In that way, mmc/sd/sdio can separately decide if the mmc_send_status() is needed and we also can remove two users (sd/mmc) of the -power_restore() callback. Good idea, this makes the core.c code cleaner. I've decided to put the removal of mmc_hw_reset_check() into a separate patch, to make it easier to see what's happened. -} - -int mmc_hw_reset(struct mmc_host *host) -{ - return mmc_do_hw_reset(host, 0); +out: + mmc_bus_put(host); + if (card_alive) + return -ENOSYS; + else + return ret; } EXPORT_SYMBOL(mmc_hw_reset); -int mmc_hw_reset_check(struct mmc_host *host) -{ - return
[PATCH v7 2/3] mmc: core: refactor the hw_reset routines
Move the (e)MMC specific hw_reset code from core.c into mmc.c. Call the code from the new bus_ops member reset. This also allows for adding a SD card specific reset procedure. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/core.c | 45 ++--- drivers/mmc/core/core.h |1 + drivers/mmc/core/mmc.c | 41 + 3 files changed, 52 insertions(+), 35 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 9959997e..2cdb06e 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2273,50 +2273,25 @@ static void mmc_hw_reset_for_init(struct mmc_host *host) mmc_host_clk_release(host); } -int mmc_can_reset(struct mmc_card *card) +int mmc_hw_reset(struct mmc_host *host) { - u8 rst_n_function; - - if (!mmc_card_mmc(card)) - return 0; - rst_n_function = card-ext_csd.rst_n_function; - if ((rst_n_function EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED) - return 0; - return 1; -} -EXPORT_SYMBOL(mmc_can_reset); - -static int mmc_hw_reset(struct mmc_host *host) -{ - struct mmc_card *card = host-card; - u32 status; - - if (!(host-caps MMC_CAP_HW_RESET) || !host-ops-hw_reset) - return -EOPNOTSUPP; + int ret; - if (!card) + if (!host-card) return -EINVAL; - if (!mmc_can_reset(card)) + mmc_bus_get(host); + if (!host-bus_ops || host-bus_dead || !host-bus_ops-reset) { + mmc_bus_put(host); return -EOPNOTSUPP; - - mmc_host_clk_hold(host); - mmc_set_clock(host, host-f_init); - - host-ops-hw_reset(host); - - /* If the reset has happened, then a status command will fail */ - if (!mmc_send_status(card, status)) { - mmc_host_clk_release(host); - return -ENOSYS; } - /* Set initial state and call mmc_set_ios */ - mmc_set_initial_state(host); + ret = host-bus_ops-reset(host); + mmc_bus_put(host); - mmc_host_clk_release(host); + pr_warn(%s: tried to reset card\n, mmc_hostname(host)); - return host-bus_ops-power_restore(host); + return ret; } EXPORT_SYMBOL(mmc_hw_reset); diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index b528c0e..a0bccbc 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -27,6 +27,7 @@ struct mmc_bus_ops { int (*power_restore)(struct mmc_host *); int (*alive)(struct mmc_host *); int (*shutdown)(struct mmc_host *); + int (*reset)(struct mmc_host *); }; void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops); diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 0b8ec87..d5d576a 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1795,6 +1795,46 @@ static int mmc_power_restore(struct mmc_host *host) return ret; } +int mmc_can_reset(struct mmc_card *card) +{ + u8 rst_n_function; + + rst_n_function = card-ext_csd.rst_n_function; + if ((rst_n_function EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED) + return 0; + return 1; +} +EXPORT_SYMBOL(mmc_can_reset); + +static int mmc_reset(struct mmc_host *host) +{ + struct mmc_card *card = host-card; + u32 status; + + if (!(host-caps MMC_CAP_HW_RESET) || !host-ops-hw_reset) + return -EOPNOTSUPP; + + if (!mmc_can_reset(card)) + return -EOPNOTSUPP; + + mmc_host_clk_hold(host); + mmc_set_clock(host, host-f_init); + + host-ops-hw_reset(host); + + /* If the reset has happened, then a status command will fail */ + if (!mmc_send_status(card, status)) { + mmc_host_clk_release(host); + return -ENOSYS; + } + + /* Set initial state and call mmc_set_ios */ + mmc_set_initial_state(host); + mmc_host_clk_release(host); + + return mmc_power_restore(host); +} + static const struct mmc_bus_ops mmc_ops = { .remove = mmc_remove, .detect = mmc_detect, @@ -1805,6 +1845,7 @@ static const struct mmc_bus_ops mmc_ops = { .power_restore = mmc_power_restore, .alive = mmc_alive, .shutdown = mmc_shutdown, + .reset = mmc_reset, }; /* -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 2/2] mmc: sd: add hw_reset callback
Enable power cycle and re-initialization of SD cards via the hw_reset bus_ops. Power cycling a buggy SD card sometimes helps it get back on track. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/sd.c |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 29fccdc..1228ef7 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -1197,6 +1197,12 @@ static int mmc_sd_power_restore(struct mmc_host *host) return ret; } +static int mmc_sd_hw_reset(struct mmc_host *host) +{ + mmc_power_cycle(host, host-card-ocr); + return 0; +} + static const struct mmc_bus_ops mmc_sd_ops = { .remove = mmc_sd_remove, .detect = mmc_sd_detect, @@ -1207,6 +1213,7 @@ static const struct mmc_bus_ops mmc_sd_ops = { .power_restore = mmc_sd_power_restore, .alive = mmc_sd_alive, .shutdown = mmc_sd_suspend, + .hw_reset = mmc_sd_hw_reset, }; /* -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 0/2] mmc: core: hw_reset changes
Make the mmc_hw_reset routine more generic, by putting the (e)MMC- specific stuff in the new bus_ops-hw_reset in mmc.c. Add a bus_ops-hw_reset for SD cards, allowing them to be restarted when errors occur. Always check if the reset was sucessful and propagate this to the caller. As I don't have an eMMC device myself, much less one with a reset line, I'd be very happy if someone could help me test the code with an eMMC? v6: - Always perform the mmc_send_status reset check, which allows us to have only one interface to the card reset functions - Because of this, add the bus_ops-hw_reset to sd.c instead of falling back to a power_cycle v5: - Move the mmc_test-specific code to mmc_test.c - Fall back to a power_cycle if the bus_ops-hw_reset is missing - Because of this, skip the bus_ops-hw_reset in sd.c v4: - Rebase onto next v3: - Keep mmc_can_reset - Always set bus_mode = MMC_BUSMODE_PUSHPULL in mmc_set_initial_state() v2: - Call the new bus_ops member hw_reset instead of power_reset - Create mmc_set_initial_state and call it from mmc_mmc_hw_reset instead of mmc_power_up - Keep mmc_hw_reset naming Johan Rudholm (2): mmc: core: refactor the hw_reset routines mmc: sd: add hw_reset callback drivers/mmc/card/mmc_test.c | 18 --- drivers/mmc/core/core.c | 70 +++ drivers/mmc/core/core.h |1 + drivers/mmc/core/mmc.c | 32 +++ drivers/mmc/core/sd.c |7 include/linux/mmc/core.h|1 - 6 files changed, 71 insertions(+), 58 deletions(-) -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] mmc: core: turn hw_reset into a bus_ops
Hi Ulf, 2014-12-01 11:50 GMT+01:00 Ulf Hansson ulf.hans...@linaro.org: On 28 November 2014 at 16:54, Johan Rudholm johan.rudh...@axis.com wrote: 2014-11-27 10:50 GMT+01:00 Ulf Hansson ulf.hans...@linaro.org: On 27 November 2014 at 10:05, Johan Rudholm johan.rudh...@axis.com wrote: Hi Ulf, 2014-11-25 14:48 GMT+01:00 Ulf Hansson ulf.hans...@linaro.org: On 24 November 2014 at 12:06, Johan Rudholm johan.rudh...@axis.com wrote: Move the (e)MMC specific hw_reset code from core.c into mmc.c and call it from the new bus_ops member hw_reset. This also lets us add code for resetting SD cards as well. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/core.c | 56 +++--- drivers/mmc/core/core.h |5 drivers/mmc/core/mmc.c | 40 + 3 files changed, 63 insertions(+), 38 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 9584bff..492b3e5 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2245,67 +2245,47 @@ static void mmc_hw_reset_for_init(struct mmc_host *host) mmc_host_clk_release(host); } -int mmc_can_reset(struct mmc_card *card) -{ - u8 rst_n_function; - - if (!mmc_card_mmc(card)) - return 0; - rst_n_function = card-ext_csd.rst_n_function; - if ((rst_n_function EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED) - return 0; - return 1; -} -EXPORT_SYMBOL(mmc_can_reset); Isn't the mmc_can_reset() function used from mmc_test.c? Yes. Maybe we should move this stuff to mmc_test instead. We could also move the code that checks if the reset worked or not to mmc_test, since this is the only place where the check is performed. - +/* Reset card in a bus-specific way */ static int mmc_do_hw_reset(struct mmc_host *host, int check) { - struct mmc_card *card = host-card; - - if (!(host-caps MMC_CAP_HW_RESET) || !host-ops-hw_reset) - return -EOPNOTSUPP; + int ret; - if (!card) + if (!host-card) return -EINVAL; - if (!mmc_can_reset(card)) You need a mmc_bus_get() here before accessing the host-bus_ops callbacks. Well, if you would executed this code with the host claimed and from the mmc block layer, you can be sure the bus_ops to exist. Now, since this is an exported function, I think you need to be more safe to also do the mmc_bus_get|put(). Got it, thanks. + if (!host-bus_ops || !host-bus_ops-hw_reset || + host-bus_ops-hw_reset(host, MMC_HW_RESET_TEST)) return -EOPNOTSUPP; - mmc_host_clk_hold(host); - mmc_set_clock(host, host-f_init); + ret = host-bus_ops-hw_reset(host, check); What's going on here? You are invoking the -hw_reset() callback twice. That seems odd. - host-ops-hw_reset(host); - - /* If the reset has happened, then a status command will fail */ - if (check) { - u32 status; - - if (!mmc_send_status(card, status)) { - mmc_host_clk_release(host); - return -ENOSYS; - } - } - - /* Set initial state and call mmc_set_ios */ - mmc_set_initial_state(host); + if (check == MMC_HW_RESET_TEST_CARD) + return ret; - mmc_host_clk_release(host); + pr_warn(%s: tried to reset card (status %d)\n, + mmc_hostname(host), ret); return host-bus_ops-power_restore(host); } int mmc_hw_reset(struct mmc_host *host) { - return mmc_do_hw_reset(host, 0); + return mmc_do_hw_reset(host, MMC_HW_RESET_RESET); } EXPORT_SYMBOL(mmc_hw_reset); int mmc_hw_reset_check(struct mmc_host *host) { - return mmc_do_hw_reset(host, 1); + return mmc_do_hw_reset(host, MMC_HW_RESET_CHECK); } EXPORT_SYMBOL(mmc_hw_reset_check); +int mmc_can_reset(struct mmc_card *card) +{ + return mmc_do_hw_reset(card-host, MMC_HW_RESET_TEST_CARD); +} +EXPORT_SYMBOL(mmc_can_reset); + static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq) { host-f_init = freq; diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index d76597c..f6e0a52 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -27,6 +27,11 @@ struct mmc_bus_ops { int (*power_restore)(struct mmc_host *); int (*alive)(struct mmc_host *); int (*shutdown)(struct mmc_host *); + int (*hw_reset)(struct mmc_host *, int); +#define MMC_HW_RESET_RESET 0 +#define MMC_HW_RESET_TEST 1 +#define MMC_HW_RESET_CHECK 2 +#define MMC_HW_RESET_TEST_CARD 3 Urgh. Is there a way to remove all this? I just don't like all these options. In fact, I would prefer to have none of them. If we move the test and check
[PATCH v6 1/2] mmc: core: refactor the hw_reset routines
Move the (e)MMC specific hw_reset code from core.c into mmc.c Call the code from the new bus_ops member hw_reset. This also allows for adding a SD card specific reset procedure. Always check if the card is alive after a successful reset. This allows us to remove mmc_hw_reset_check(), leaving mmc_hw_reset() as the only card reset interface. This also informs any callers, for instance the block layer, if a reset was sucessful or not. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/card/mmc_test.c | 18 --- drivers/mmc/core/core.c | 70 +++ drivers/mmc/core/core.h |1 + drivers/mmc/core/mmc.c | 32 +++ include/linux/mmc/core.h|1 - 5 files changed, 64 insertions(+), 58 deletions(-) diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c index 0a7430f..7dac469 100644 --- a/drivers/mmc/card/mmc_test.c +++ b/drivers/mmc/card/mmc_test.c @@ -2342,20 +2342,16 @@ static int mmc_test_hw_reset(struct mmc_test_card *test) struct mmc_host *host = card-host; int err; - err = mmc_hw_reset_check(host); + if (!mmc_card_mmc(card) || !mmc_can_reset(card)) + return RESULT_UNSUP_CARD; + + err = mmc_hw_reset(host); if (!err) return RESULT_OK; + else if (err == -EOPNOTSUPP) + return RESULT_UNSUP_HOST; - if (err == -ENOSYS) - return RESULT_FAIL; - - if (err != -EOPNOTSUPP) - return err; - - if (!mmc_can_reset(card)) - return RESULT_UNSUP_CARD; - - return RESULT_UNSUP_HOST; + return RESULT_FAIL; } static const struct mmc_test_case mmc_test_cases[] = { diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index d3bfbdf..8598287 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2273,67 +2273,45 @@ static void mmc_hw_reset_for_init(struct mmc_host *host) mmc_host_clk_release(host); } -int mmc_can_reset(struct mmc_card *card) -{ - u8 rst_n_function; - - if (!mmc_card_mmc(card)) - return 0; - rst_n_function = card-ext_csd.rst_n_function; - if ((rst_n_function EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED) - return 0; - return 1; -} -EXPORT_SYMBOL(mmc_can_reset); - -static int mmc_do_hw_reset(struct mmc_host *host, int check) +int mmc_hw_reset(struct mmc_host *host) { - struct mmc_card *card = host-card; - - if (!(host-caps MMC_CAP_HW_RESET) || !host-ops-hw_reset) - return -EOPNOTSUPP; + int ret = 0; + int card_alive = 0; + u32 status; - if (!card) + if (!host-card) return -EINVAL; - if (!mmc_can_reset(card)) - return -EOPNOTSUPP; - - mmc_host_clk_hold(host); - mmc_set_clock(host, host-f_init); + mmc_bus_get(host); + if (!host-bus_ops || host-bus_dead || !host-bus_ops-hw_reset) { + ret = -EOPNOTSUPP; + goto out; + } - host-ops-hw_reset(host); + ret = host-bus_ops-hw_reset(host); + if (ret) + goto out; /* If the reset has happened, then a status command will fail */ - if (check) { - u32 status; + if (!mmc_send_status(host-card, status)) + card_alive = 1; - if (!mmc_send_status(card, status)) { - mmc_host_clk_release(host); - return -ENOSYS; - } - } + pr_warn(%s: tried to reset card (%s)\n, mmc_hostname(host), + card_alive ? failed : ok); /* Set initial state and call mmc_set_ios */ mmc_set_initial_state(host); + ret = host-bus_ops-power_restore(host); - mmc_host_clk_release(host); - - return host-bus_ops-power_restore(host); -} - -int mmc_hw_reset(struct mmc_host *host) -{ - return mmc_do_hw_reset(host, 0); +out: + mmc_bus_put(host); + if (card_alive) + return -ENOSYS; + else + return ret; } EXPORT_SYMBOL(mmc_hw_reset); -int mmc_hw_reset_check(struct mmc_host *host) -{ - return mmc_do_hw_reset(host, 1); -} -EXPORT_SYMBOL(mmc_hw_reset_check); - static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq) { host-f_init = freq; diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index b528c0e..4d1ee8a 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -27,6 +27,7 @@ struct mmc_bus_ops { int (*power_restore)(struct mmc_host *); int (*alive)(struct mmc_host *); int (*shutdown)(struct mmc_host *); + int (*hw_reset)(struct mmc_host *); }; void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops); diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 0b8ec87..f64a53e 100644 --- a/drivers
Re: [PATCH v4 1/2] mmc: core: turn hw_reset into a bus_ops
2014-11-27 10:50 GMT+01:00 Ulf Hansson ulf.hans...@linaro.org: On 27 November 2014 at 10:05, Johan Rudholm johan.rudh...@axis.com wrote: Hi Ulf, 2014-11-25 14:48 GMT+01:00 Ulf Hansson ulf.hans...@linaro.org: On 24 November 2014 at 12:06, Johan Rudholm johan.rudh...@axis.com wrote: Move the (e)MMC specific hw_reset code from core.c into mmc.c and call it from the new bus_ops member hw_reset. This also lets us add code for resetting SD cards as well. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/core.c | 56 +++--- drivers/mmc/core/core.h |5 drivers/mmc/core/mmc.c | 40 + 3 files changed, 63 insertions(+), 38 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 9584bff..492b3e5 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2245,67 +2245,47 @@ static void mmc_hw_reset_for_init(struct mmc_host *host) mmc_host_clk_release(host); } -int mmc_can_reset(struct mmc_card *card) -{ - u8 rst_n_function; - - if (!mmc_card_mmc(card)) - return 0; - rst_n_function = card-ext_csd.rst_n_function; - if ((rst_n_function EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED) - return 0; - return 1; -} -EXPORT_SYMBOL(mmc_can_reset); Isn't the mmc_can_reset() function used from mmc_test.c? Yes. Maybe we should move this stuff to mmc_test instead. We could also move the code that checks if the reset worked or not to mmc_test, since this is the only place where the check is performed. - +/* Reset card in a bus-specific way */ static int mmc_do_hw_reset(struct mmc_host *host, int check) { - struct mmc_card *card = host-card; - - if (!(host-caps MMC_CAP_HW_RESET) || !host-ops-hw_reset) - return -EOPNOTSUPP; + int ret; - if (!card) + if (!host-card) return -EINVAL; - if (!mmc_can_reset(card)) You need a mmc_bus_get() here before accessing the host-bus_ops callbacks. Well, if you would executed this code with the host claimed and from the mmc block layer, you can be sure the bus_ops to exist. Now, since this is an exported function, I think you need to be more safe to also do the mmc_bus_get|put(). Got it, thanks. + if (!host-bus_ops || !host-bus_ops-hw_reset || + host-bus_ops-hw_reset(host, MMC_HW_RESET_TEST)) return -EOPNOTSUPP; - mmc_host_clk_hold(host); - mmc_set_clock(host, host-f_init); + ret = host-bus_ops-hw_reset(host, check); What's going on here? You are invoking the -hw_reset() callback twice. That seems odd. - host-ops-hw_reset(host); - - /* If the reset has happened, then a status command will fail */ - if (check) { - u32 status; - - if (!mmc_send_status(card, status)) { - mmc_host_clk_release(host); - return -ENOSYS; - } - } - - /* Set initial state and call mmc_set_ios */ - mmc_set_initial_state(host); + if (check == MMC_HW_RESET_TEST_CARD) + return ret; - mmc_host_clk_release(host); + pr_warn(%s: tried to reset card (status %d)\n, + mmc_hostname(host), ret); return host-bus_ops-power_restore(host); } int mmc_hw_reset(struct mmc_host *host) { - return mmc_do_hw_reset(host, 0); + return mmc_do_hw_reset(host, MMC_HW_RESET_RESET); } EXPORT_SYMBOL(mmc_hw_reset); int mmc_hw_reset_check(struct mmc_host *host) { - return mmc_do_hw_reset(host, 1); + return mmc_do_hw_reset(host, MMC_HW_RESET_CHECK); } EXPORT_SYMBOL(mmc_hw_reset_check); +int mmc_can_reset(struct mmc_card *card) +{ + return mmc_do_hw_reset(card-host, MMC_HW_RESET_TEST_CARD); +} +EXPORT_SYMBOL(mmc_can_reset); + static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq) { host-f_init = freq; diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index d76597c..f6e0a52 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -27,6 +27,11 @@ struct mmc_bus_ops { int (*power_restore)(struct mmc_host *); int (*alive)(struct mmc_host *); int (*shutdown)(struct mmc_host *); + int (*hw_reset)(struct mmc_host *, int); +#define MMC_HW_RESET_RESET 0 +#define MMC_HW_RESET_TEST 1 +#define MMC_HW_RESET_CHECK 2 +#define MMC_HW_RESET_TEST_CARD 3 Urgh. Is there a way to remove all this? I just don't like all these options. In fact, I would prefer to have none of them. If we move the test and check functionality to mmc_test, I think we can avoid these. What do you think of this approach? I like that approach. In principle I think we should have only one
[PATCH v5 0/2] mmc: core: hw_reset changes
Make the mmc_hw_reset routines more generic, by putting the (e)MMC- specific stuff in the new bus_ops-hw_reset. Simplify them by putting the functionality required by the mmc_test framework in mmc_test.c As I don't have an eMMC device myself, much less one with a reset line, I'd be very happy if someone could help me test the code with an eMMC? v5: - Move the mmc_test-specific code to mmc_test.c - Fall back to a power_cycle if the bus_ops-hw_reset is missing - Because of this, skip the bus_ops-hw_reset in sd.c v4: - Rebase onto next v3: - Keep mmc_can_reset - Always set bus_mode = MMC_BUSMODE_PUSHPULL in mmc_set_initial_state() v2: - Call the new bus_ops member hw_reset instead of power_reset - Create mmc_set_initial_state and call it from mmc_mmc_hw_reset instead of mmc_power_up - Keep mmc_hw_reset naming Johan Rudholm (2): mmc: core: refactor the hw_reset routines mmc: core: let hw_reset default to power_cycle drivers/mmc/card/mmc_test.c | 24 +- drivers/mmc/core/core.c | 70 -- drivers/mmc/core/core.h |2 +- drivers/mmc/core/mmc.c | 23 ++ drivers/mmc/core/mmc_ops.c |1 + drivers/mmc/core/mmc_ops.h |1 - include/linux/mmc/core.h|5 ++- 7 files changed, 70 insertions(+), 56 deletions(-) -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 2/2] mmc: core: let hw_reset default to power_cycle
If bus_ops-hw_reset is missing, try to power cycle the card instead. This will allow SD cards to be power cycled and re-initialized as well. Power cycling a buggy SD card sometimes helps it get back on track. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/core.c | 18 +++--- 1 files changed, 11 insertions(+), 7 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 7a17cd2..45b367d 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2254,19 +2254,23 @@ static int mmc_do_hw_reset(struct mmc_host *host, int do_init) return -EINVAL; mmc_bus_get(host); - if (!host-bus_ops || host-bus_dead || !host-bus_ops-hw_reset) { + if (!host-bus_ops || host-bus_dead) { ret = -EINVAL; goto out; } - ret = host-bus_ops-hw_reset(host); - if (ret) - goto out; + if (!host-bus_ops-hw_reset) { + mmc_power_cycle(host, host-card-ocr); + } else { + ret = host-bus_ops-hw_reset(host); + if (ret) + goto out; + if (do_init) + mmc_set_initial_state(host); + } - if (do_init) { - mmc_set_initial_state(host); + if (do_init) ret = host-bus_ops-power_restore(host); - } pr_warn(%s: tried to reset card\n, mmc_hostname(host)); out: -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 1/2] mmc: core: refactor the hw_reset routines
Split the (e)MMC specific hw_reset code from core.c into mmc.c and mmc_test.c. Call the code from the new bus_ops member hw_reset. Export mmc_set_initial_state and mmc_send_status so that they may be called from mmc_test. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/card/mmc_test.c | 24 ++-- drivers/mmc/core/core.c | 66 +++ drivers/mmc/core/core.h |2 +- drivers/mmc/core/mmc.c | 23 +++ drivers/mmc/core/mmc_ops.c |1 + drivers/mmc/core/mmc_ops.h |1 - include/linux/mmc/core.h|5 ++- 7 files changed, 66 insertions(+), 56 deletions(-) diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c index 0a7430f..ca5267e 100644 --- a/drivers/mmc/card/mmc_test.c +++ b/drivers/mmc/card/mmc_test.c @@ -2342,20 +2342,26 @@ static int mmc_test_hw_reset(struct mmc_test_card *test) struct mmc_host *host = card-host; int err; - err = mmc_hw_reset_check(host); - if (!err) - return RESULT_OK; + if (!mmc_card_mmc(card)) + return RESULT_UNSUP_CARD; - if (err == -ENOSYS) - return RESULT_FAIL; + err = mmc_hw_reset_noinit(host); + if (!err) { + u32 status; - if (err != -EOPNOTSUPP) - return err; + if (!mmc_send_status(card, status)) + return RESULT_FAIL; + mmc_set_initial_state(host); + mmc_power_restore_host(host); + return RESULT_OK; + } - if (!mmc_can_reset(card)) + if (err == -ENOSYS) + return RESULT_UNSUP_HOST; + else if (err == -EOPNOTSUPP) return RESULT_UNSUP_CARD; - return RESULT_UNSUP_HOST; + return RESULT_FAIL; } static const struct mmc_test_case mmc_test_cases[] = { diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 9584bff..7a17cd2 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1114,6 +1114,7 @@ void mmc_set_initial_state(struct mmc_host *host) mmc_set_ios(host); } +EXPORT_SYMBOL_GPL(mmc_set_initial_state); /** * mmc_vdd_to_ocrbitnum - Convert a voltage to the OCR bit number @@ -2245,66 +2246,45 @@ static void mmc_hw_reset_for_init(struct mmc_host *host) mmc_host_clk_release(host); } -int mmc_can_reset(struct mmc_card *card) +static int mmc_do_hw_reset(struct mmc_host *host, int do_init) { - u8 rst_n_function; - - if (!mmc_card_mmc(card)) - return 0; - rst_n_function = card-ext_csd.rst_n_function; - if ((rst_n_function EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED) - return 0; - return 1; -} -EXPORT_SYMBOL(mmc_can_reset); - -static int mmc_do_hw_reset(struct mmc_host *host, int check) -{ - struct mmc_card *card = host-card; - - if (!(host-caps MMC_CAP_HW_RESET) || !host-ops-hw_reset) - return -EOPNOTSUPP; + int ret = 0; - if (!card) + if (!host-card) return -EINVAL; - if (!mmc_can_reset(card)) - return -EOPNOTSUPP; - - mmc_host_clk_hold(host); - mmc_set_clock(host, host-f_init); - - host-ops-hw_reset(host); - - /* If the reset has happened, then a status command will fail */ - if (check) { - u32 status; - - if (!mmc_send_status(card, status)) { - mmc_host_clk_release(host); - return -ENOSYS; - } + mmc_bus_get(host); + if (!host-bus_ops || host-bus_dead || !host-bus_ops-hw_reset) { + ret = -EINVAL; + goto out; } - /* Set initial state and call mmc_set_ios */ - mmc_set_initial_state(host); + ret = host-bus_ops-hw_reset(host); + if (ret) + goto out; - mmc_host_clk_release(host); + if (do_init) { + mmc_set_initial_state(host); + ret = host-bus_ops-power_restore(host); + } - return host-bus_ops-power_restore(host); + pr_warn(%s: tried to reset card\n, mmc_hostname(host)); +out: + mmc_bus_put(host); + return ret; } int mmc_hw_reset(struct mmc_host *host) { - return mmc_do_hw_reset(host, 0); + return mmc_do_hw_reset(host, 1); } EXPORT_SYMBOL(mmc_hw_reset); -int mmc_hw_reset_check(struct mmc_host *host) +int mmc_hw_reset_noinit(struct mmc_host *host) { - return mmc_do_hw_reset(host, 1); + return mmc_do_hw_reset(host, 0); } -EXPORT_SYMBOL(mmc_hw_reset_check); +EXPORT_SYMBOL(mmc_hw_reset_noinit); static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq) { diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index d76597c..4112356 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -27,6 +27,7 @@ struct mmc_bus_ops { int (*power_restore
Re: [PATCH v4 1/2] mmc: core: turn hw_reset into a bus_ops
Hi Ulf, 2014-11-25 14:48 GMT+01:00 Ulf Hansson ulf.hans...@linaro.org: On 24 November 2014 at 12:06, Johan Rudholm johan.rudh...@axis.com wrote: Move the (e)MMC specific hw_reset code from core.c into mmc.c and call it from the new bus_ops member hw_reset. This also lets us add code for resetting SD cards as well. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/core.c | 56 +++--- drivers/mmc/core/core.h |5 drivers/mmc/core/mmc.c | 40 + 3 files changed, 63 insertions(+), 38 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 9584bff..492b3e5 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2245,67 +2245,47 @@ static void mmc_hw_reset_for_init(struct mmc_host *host) mmc_host_clk_release(host); } -int mmc_can_reset(struct mmc_card *card) -{ - u8 rst_n_function; - - if (!mmc_card_mmc(card)) - return 0; - rst_n_function = card-ext_csd.rst_n_function; - if ((rst_n_function EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED) - return 0; - return 1; -} -EXPORT_SYMBOL(mmc_can_reset); Isn't the mmc_can_reset() function used from mmc_test.c? Yes. Maybe we should move this stuff to mmc_test instead. We could also move the code that checks if the reset worked or not to mmc_test, since this is the only place where the check is performed. - +/* Reset card in a bus-specific way */ static int mmc_do_hw_reset(struct mmc_host *host, int check) { - struct mmc_card *card = host-card; - - if (!(host-caps MMC_CAP_HW_RESET) || !host-ops-hw_reset) - return -EOPNOTSUPP; + int ret; - if (!card) + if (!host-card) return -EINVAL; - if (!mmc_can_reset(card)) You need a mmc_bus_get() here before accessing the host-bus_ops callbacks. Well, if you would executed this code with the host claimed and from the mmc block layer, you can be sure the bus_ops to exist. Now, since this is an exported function, I think you need to be more safe to also do the mmc_bus_get|put(). Got it, thanks. + if (!host-bus_ops || !host-bus_ops-hw_reset || + host-bus_ops-hw_reset(host, MMC_HW_RESET_TEST)) return -EOPNOTSUPP; - mmc_host_clk_hold(host); - mmc_set_clock(host, host-f_init); + ret = host-bus_ops-hw_reset(host, check); What's going on here? You are invoking the -hw_reset() callback twice. That seems odd. - host-ops-hw_reset(host); - - /* If the reset has happened, then a status command will fail */ - if (check) { - u32 status; - - if (!mmc_send_status(card, status)) { - mmc_host_clk_release(host); - return -ENOSYS; - } - } - - /* Set initial state and call mmc_set_ios */ - mmc_set_initial_state(host); + if (check == MMC_HW_RESET_TEST_CARD) + return ret; - mmc_host_clk_release(host); + pr_warn(%s: tried to reset card (status %d)\n, + mmc_hostname(host), ret); return host-bus_ops-power_restore(host); } int mmc_hw_reset(struct mmc_host *host) { - return mmc_do_hw_reset(host, 0); + return mmc_do_hw_reset(host, MMC_HW_RESET_RESET); } EXPORT_SYMBOL(mmc_hw_reset); int mmc_hw_reset_check(struct mmc_host *host) { - return mmc_do_hw_reset(host, 1); + return mmc_do_hw_reset(host, MMC_HW_RESET_CHECK); } EXPORT_SYMBOL(mmc_hw_reset_check); +int mmc_can_reset(struct mmc_card *card) +{ + return mmc_do_hw_reset(card-host, MMC_HW_RESET_TEST_CARD); +} +EXPORT_SYMBOL(mmc_can_reset); + static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq) { host-f_init = freq; diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index d76597c..f6e0a52 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -27,6 +27,11 @@ struct mmc_bus_ops { int (*power_restore)(struct mmc_host *); int (*alive)(struct mmc_host *); int (*shutdown)(struct mmc_host *); + int (*hw_reset)(struct mmc_host *, int); +#define MMC_HW_RESET_RESET 0 +#define MMC_HW_RESET_TEST 1 +#define MMC_HW_RESET_CHECK 2 +#define MMC_HW_RESET_TEST_CARD 3 Urgh. Is there a way to remove all this? I just don't like all these options. In fact, I would prefer to have none of them. If we move the test and check functionality to mmc_test, I think we can avoid these. What do you think of this approach? //Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/2] mmc: sd: add hw_reset callback
Enable power cycle and re-initialization of SD cards via the hw_reset bus_ops. Power cycling a buggy SD card sometimes helps it get back on track. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/sd.c | 13 + 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index d90a6de..a7fa124 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -1180,6 +1180,18 @@ static int mmc_sd_runtime_resume(struct mmc_host *host) return 0; } +static int mmc_sd_hw_reset(struct mmc_host *host, int test) +{ + struct mmc_card *card = host-card; + + if (test == MMC_HW_RESET_TEST || test == MMC_HW_RESET_TEST_CARD) + return 0; + + mmc_power_cycle(host, card-ocr); + + return 0; +} + static int mmc_sd_power_restore(struct mmc_host *host) { int ret; @@ -1201,6 +1213,7 @@ static const struct mmc_bus_ops mmc_sd_ops = { .power_restore = mmc_sd_power_restore, .alive = mmc_sd_alive, .shutdown = mmc_sd_suspend, + .hw_reset = mmc_sd_hw_reset, }; /* -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/2] mmc: core: turn hw_reset into a bus_ops
Move the (e)MMC specific hw_reset code from core.c into mmc.c and call it from the new bus_ops member hw_reset. This also lets us add code for resetting SD cards as well. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/core.c | 56 +++--- drivers/mmc/core/core.h |5 drivers/mmc/core/mmc.c | 40 + 3 files changed, 63 insertions(+), 38 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 9584bff..492b3e5 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2245,67 +2245,47 @@ static void mmc_hw_reset_for_init(struct mmc_host *host) mmc_host_clk_release(host); } -int mmc_can_reset(struct mmc_card *card) -{ - u8 rst_n_function; - - if (!mmc_card_mmc(card)) - return 0; - rst_n_function = card-ext_csd.rst_n_function; - if ((rst_n_function EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED) - return 0; - return 1; -} -EXPORT_SYMBOL(mmc_can_reset); - +/* Reset card in a bus-specific way */ static int mmc_do_hw_reset(struct mmc_host *host, int check) { - struct mmc_card *card = host-card; - - if (!(host-caps MMC_CAP_HW_RESET) || !host-ops-hw_reset) - return -EOPNOTSUPP; + int ret; - if (!card) + if (!host-card) return -EINVAL; - if (!mmc_can_reset(card)) + if (!host-bus_ops || !host-bus_ops-hw_reset || + host-bus_ops-hw_reset(host, MMC_HW_RESET_TEST)) return -EOPNOTSUPP; - mmc_host_clk_hold(host); - mmc_set_clock(host, host-f_init); + ret = host-bus_ops-hw_reset(host, check); - host-ops-hw_reset(host); - - /* If the reset has happened, then a status command will fail */ - if (check) { - u32 status; - - if (!mmc_send_status(card, status)) { - mmc_host_clk_release(host); - return -ENOSYS; - } - } - - /* Set initial state and call mmc_set_ios */ - mmc_set_initial_state(host); + if (check == MMC_HW_RESET_TEST_CARD) + return ret; - mmc_host_clk_release(host); + pr_warn(%s: tried to reset card (status %d)\n, + mmc_hostname(host), ret); return host-bus_ops-power_restore(host); } int mmc_hw_reset(struct mmc_host *host) { - return mmc_do_hw_reset(host, 0); + return mmc_do_hw_reset(host, MMC_HW_RESET_RESET); } EXPORT_SYMBOL(mmc_hw_reset); int mmc_hw_reset_check(struct mmc_host *host) { - return mmc_do_hw_reset(host, 1); + return mmc_do_hw_reset(host, MMC_HW_RESET_CHECK); } EXPORT_SYMBOL(mmc_hw_reset_check); +int mmc_can_reset(struct mmc_card *card) +{ + return mmc_do_hw_reset(card-host, MMC_HW_RESET_TEST_CARD); +} +EXPORT_SYMBOL(mmc_can_reset); + static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq) { host-f_init = freq; diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index d76597c..f6e0a52 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -27,6 +27,11 @@ struct mmc_bus_ops { int (*power_restore)(struct mmc_host *); int (*alive)(struct mmc_host *); int (*shutdown)(struct mmc_host *); + int (*hw_reset)(struct mmc_host *, int); +#define MMC_HW_RESET_RESET 0 +#define MMC_HW_RESET_TEST 1 +#define MMC_HW_RESET_CHECK 2 +#define MMC_HW_RESET_TEST_CARD 3 }; void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops); diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 02ad792..7ffa3f2 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1821,6 +1821,45 @@ static int mmc_power_restore(struct mmc_host *host) return ret; } +static int mmc_mmc_hw_reset(struct mmc_host *host, int test) +{ + struct mmc_card *card = host-card; + u8 rst_n_function; + + rst_n_function = card-ext_csd.rst_n_function; + if ((rst_n_function EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED) + return -EOPNOTSUPP; + + if (test == MMC_HW_RESET_TEST_CARD) + return 0; + + if (!(host-caps MMC_CAP_HW_RESET) || !host-ops-hw_reset) + return -EOPNOTSUPP; + + if (test == MMC_HW_RESET_TEST) + return 0; + + mmc_host_clk_hold(host); + mmc_set_clock(host, host-f_init); + + host-ops-hw_reset(host); + + if (test == MMC_HW_RESET_CHECK) { + u32 status; + + if (!mmc_send_status(card, status)) { + mmc_host_clk_release(host); + return -ENOSYS; + } + } + + mmc_set_initial_state(host); + + mmc_host_clk_release(host); + + return 0; +} + static const struct mmc_bus_ops mmc_ops
[PATCH v4 0/2] mmc: core: hw_reset changes
Make the mmc_hw_reset routines more generic, so we can easily add a power cycle of SD cards as well. Also simplify the (e)MMC specific parts of the reset code. As I don't have an eMMC device myself, much less one with a reset line, I'd be very happy if someone could help me test the code with an eMMC? v4: - Rebase onto next v3: - Keep mmc_can_reset - Always set bus_mode = MMC_BUSMODE_PUSHPULL in mmc_set_initial_state() v2: - Call the new bus_ops member hw_reset instead of power_reset - Create mmc_set_initial_state and call it from mmc_mmc_hw_reset instead of mmc_power_up - Keep mmc_hw_reset naming Johan Rudholm (2): mmc: core: turn hw_reset into a bus_ops mmc: sd: add hw_reset callback drivers/mmc/core/core.c | 56 +++--- drivers/mmc/core/core.h |5 drivers/mmc/core/mmc.c | 40 + drivers/mmc/core/sd.c | 13 +++ 4 files changed, 76 insertions(+), 38 deletions(-) -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] mmc: core: hold SD Clock before CMD11 during Signal
Hi, 2014-11-21 1:51 GMT+01:00 Vincent Yang vincent.yang.fuji...@gmail.com: Voltage Switch Procedure This patch is to fix an issue found on mb86s7x platforms. [symptom] There are some UHS-1 SD memory cards sometimes cannot be detected correctly, e.g., Transcend 600x SDXC 64GB UHS-1 memory card. During Signal Voltage Switch Procedure, failure to switch is indicated by the card holding DAT[3:0] low. [analysis] According to SD Host Controller Simplified Specification Version 3.00 chapter 3.6.1, the Signal Voltage Switch Procedure should be: (1) Check S18A; (2) Issue CMD11; (3) Check CMD 11 response; (4) Stop providing SD clock; (5) Check DAT[3:0] should be b; (6) Set 1.8V Signal Enable; (7) Wait 5ms; (8) Check 1.8V Signal Enable; (9) Provide SD Clock; (10) Wait 1ms; (11) Check DAT[3:0] should be b; (12) error handling With CONFIG_MMC_CLKGATE=y, sometimes there is one more gating/un-gating SD clock between (2) and (3). In this case, some UHS-1 SD cards will hold DAT[3:0] b at (11) and thus fails Signal Voltage Switch Procedure. [solution] By mmc_host_clk_hold() before CMD11, the additional gating/un-gating SD clock between (2) and (3) can be prevented and thus no failure at (11). It has been verified with many UHS-1 SD cards on mb86s7x platforms and works correctly. Signed-off-by: Vincent Yang vincent.y...@tw.fujitsu.com Reviewed-by: Johan Rudholm jrudh...@gmail.com BR, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] mmc: core: group default initial state
Hi Ulf, 2014-11-06 10:38 GMT+01:00 Ulf Hansson ulf.hans...@linaro.org: [...] Earlier we used always used MMC_BUSMODE_PUSHPULL, now it will depend on spi. The JEDEC spec says that in device identification mode the bus should be open-drain and not push-pull. So this way should be compliant to the spec, right? Unfortunately, I will require help in testing this on a proper eMMC. For MMC yes, but you will break SD/SDIO in this patch. The default bus mode at initialization needs to be MMC_BUSMODE_PUSHPULL, since that's required by the SD/SDIO spec. The MMC cards are being initialized after SD and SDIO, which means a switch to MMC_BUSMODE_OPENDRAIN is done in mmc_attach_mmc(). Additionally, to handle resume of MMC cards, mmc_init_card() also do a switch to MMC_BUSMODE_OPENDRAIN. I can't find anything in the JEDEC or the SD spec about bus mode in SPI mode though. However, this behavior is not changed by this patch, so we should be safe. SPI mode has been removed from the eMMC spec. The SD spec doesn't have open drain mode but do support SPI mode. That tells me that we should keep using MMC_BUSMODE_PUSHPULL for SPI mode. Thank you for explaining this. So I think we should be OK by always choosing MMC_BUSMODE_PUSHPULL, because for MMC cards mmc_init_card is called from mmc_power_restore which is called after the hw_reset. Will post a v3 of this patch. //Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/3] mmc: sd: add hw_reset callback
Enable power cycle and re-initialization of SD cards via the hw_reset bus_ops. Power cycling a buggy SD card sometimes helps it get back on track. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/sd.c | 13 + 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index d90a6de..a7fa124 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -1180,6 +1180,18 @@ static int mmc_sd_runtime_resume(struct mmc_host *host) return 0; } +static int mmc_sd_hw_reset(struct mmc_host *host, int test) +{ + struct mmc_card *card = host-card; + + if (test == MMC_HW_RESET_TEST || test == MMC_HW_RESET_TEST_CARD) + return 0; + + mmc_power_cycle(host, card-ocr); + + return 0; +} + static int mmc_sd_power_restore(struct mmc_host *host) { int ret; @@ -1201,6 +1213,7 @@ static const struct mmc_bus_ops mmc_sd_ops = { .power_restore = mmc_sd_power_restore, .alive = mmc_sd_alive, .shutdown = mmc_sd_suspend, + .hw_reset = mmc_sd_hw_reset, }; /* -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/3] mmc: core: turn hw_reset into a bus_ops
Move the (e)MMC specific hw_reset code from core.c into mmc.c and call it from the new bus_ops member hw_reset. This also lets us add code for resetting SD cards as well. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/core.c | 56 +++--- drivers/mmc/core/core.h |5 drivers/mmc/core/mmc.c | 40 + 3 files changed, 63 insertions(+), 38 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 5bda29b..63ce146 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2242,67 +2242,47 @@ static void mmc_hw_reset_for_init(struct mmc_host *host) mmc_host_clk_release(host); } -int mmc_can_reset(struct mmc_card *card) -{ - u8 rst_n_function; - - if (!mmc_card_mmc(card)) - return 0; - rst_n_function = card-ext_csd.rst_n_function; - if ((rst_n_function EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED) - return 0; - return 1; -} -EXPORT_SYMBOL(mmc_can_reset); - +/* Reset card in a bus-specific way */ static int mmc_do_hw_reset(struct mmc_host *host, int check) { - struct mmc_card *card = host-card; - - if (!(host-caps MMC_CAP_HW_RESET) || !host-ops-hw_reset) - return -EOPNOTSUPP; + int ret; - if (!card) + if (!host-card) return -EINVAL; - if (!mmc_can_reset(card)) + if (!host-bus_ops || !host-bus_ops-hw_reset || + host-bus_ops-hw_reset(host, MMC_HW_RESET_TEST)) return -EOPNOTSUPP; - mmc_host_clk_hold(host); - mmc_set_clock(host, host-f_init); + ret = host-bus_ops-hw_reset(host, check); - host-ops-hw_reset(host); - - /* If the reset has happened, then a status command will fail */ - if (check) { - u32 status; - - if (!mmc_send_status(card, status)) { - mmc_host_clk_release(host); - return -ENOSYS; - } - } - - /* Set initial state and call mmc_set_ios */ - mmc_set_initial_state(host); + if (check == MMC_HW_RESET_TEST_CARD) + return ret; - mmc_host_clk_release(host); + pr_warn(%s: tried to reset card (status %d)\n, + mmc_hostname(host), ret); return host-bus_ops-power_restore(host); } int mmc_hw_reset(struct mmc_host *host) { - return mmc_do_hw_reset(host, 0); + return mmc_do_hw_reset(host, MMC_HW_RESET_RESET); } EXPORT_SYMBOL(mmc_hw_reset); int mmc_hw_reset_check(struct mmc_host *host) { - return mmc_do_hw_reset(host, 1); + return mmc_do_hw_reset(host, MMC_HW_RESET_CHECK); } EXPORT_SYMBOL(mmc_hw_reset_check); +int mmc_can_reset(struct mmc_card *card) +{ + return mmc_do_hw_reset(card-host, MMC_HW_RESET_TEST_CARD); +} +EXPORT_SYMBOL(mmc_can_reset); + static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq) { host-f_init = freq; diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index d76597c..f6e0a52 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -27,6 +27,11 @@ struct mmc_bus_ops { int (*power_restore)(struct mmc_host *); int (*alive)(struct mmc_host *); int (*shutdown)(struct mmc_host *); + int (*hw_reset)(struct mmc_host *, int); +#define MMC_HW_RESET_RESET 0 +#define MMC_HW_RESET_TEST 1 +#define MMC_HW_RESET_CHECK 2 +#define MMC_HW_RESET_TEST_CARD 3 }; void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops); diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 02ad792..7ffa3f2 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1821,6 +1821,45 @@ static int mmc_power_restore(struct mmc_host *host) return ret; } +static int mmc_mmc_hw_reset(struct mmc_host *host, int test) +{ + struct mmc_card *card = host-card; + u8 rst_n_function; + + rst_n_function = card-ext_csd.rst_n_function; + if ((rst_n_function EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED) + return -EOPNOTSUPP; + + if (test == MMC_HW_RESET_TEST_CARD) + return 0; + + if (!(host-caps MMC_CAP_HW_RESET) || !host-ops-hw_reset) + return -EOPNOTSUPP; + + if (test == MMC_HW_RESET_TEST) + return 0; + + mmc_host_clk_hold(host); + mmc_set_clock(host, host-f_init); + + host-ops-hw_reset(host); + + if (test == MMC_HW_RESET_CHECK) { + u32 status; + + if (!mmc_send_status(card, status)) { + mmc_host_clk_release(host); + return -ENOSYS; + } + } + + mmc_set_initial_state(host); + + mmc_host_clk_release(host); + + return 0; +} + static const struct mmc_bus_ops mmc_ops
[PATCH v3 0/3] mmc: core: hw_reset changes
Make the mmc_hw_reset routines more generic, so we can easily add a power cycle of SD cards as well. Also simplify the (e)MMC specific parts of the reset code. As I don't have an eMMC device myself, much less one with a reset line, I'd be very happy if someone could help me test the code with an eMMC? Please have a look at the handling of bus_mode in mmc_set_initial_state(), we set it to MMC_BUSMODE_PUSHPULL also when powering off a device. If this is not OK, we'll just have to leave bus_mode out of it. v3: - Keep mmc_can_reset - Always set bus_mode = MMC_BUSMODE_PUSHPULL in mmc_set_initial_state() v2: - Call the new bus_ops member hw_reset instead of power_reset - Create mmc_set_initial_state and call it from mmc_mmc_hw_reset instead of mmc_power_up - Keep mmc_hw_reset naming Johan Rudholm (3): mmc: core: consistent handling of initial values mmc: core: turn hw_reset into a bus_ops mmc: sd: add hw_reset callback drivers/mmc/core/core.c | 99 ++- drivers/mmc/core/core.h |6 +++ drivers/mmc/core/mmc.c | 40 +++ drivers/mmc/core/sd.c | 13 ++ 4 files changed, 97 insertions(+), 61 deletions(-) -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/3] mmc: core: consistent handling of initial values
mmc_do_hw_reset(), mmc_power_up() and mmc_power_off() all set similar initial values for bus_mode, bus_width, chip_select and timing. Let's make this handling simpler and more consistent by sticking them together in a common function. This will introduce small changes in behavior in the following places: mmc_power_off(): For SPI hosts, explicitly set bus_mode = MMC_BUSMODE_PUSHPULL and chip_select = MMC_CS_HIGH, before we left them as they were. For non-SPI hosts, set bus_mode = MMC_BUSMODE_PUSHPULL instead of MMC_BUSMODE_OPENDRAIN as before. These two changes should not be a problem since the device will be powered off anyway. mmc_do_hw_reset(): Always set bus_mode = MMC_BUSMODE_PUSHPULL, as required by SD/SDIO cards. MMC cards require MMC_BUSMODE_OPENDRAIN, but this is taken care of by mmc_init_card() and mmc_attach_mmc(). Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/core.c | 47 ++- drivers/mmc/core/core.h |1 + 2 files changed, 23 insertions(+), 25 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index a32bea2..5bda29b 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1099,6 +1099,22 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned int width) mmc_host_clk_release(host); } +/* + * Set initial state after a power cycle or a hw_reset. + */ +void mmc_set_initial_state(struct mmc_host *host) +{ + if (mmc_host_is_spi(host)) + host-ios.chip_select = MMC_CS_HIGH; + else + host-ios.chip_select = MMC_CS_DONTCARE; + host-ios.bus_mode = MMC_BUSMODE_PUSHPULL; + host-ios.bus_width = MMC_BUS_WIDTH_1; + host-ios.timing = MMC_TIMING_LEGACY; + + mmc_set_ios(host); +} + /** * mmc_vdd_to_ocrbitnum - Convert a voltage to the OCR bit number * @vdd: voltage (mV) @@ -1537,15 +1553,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr) mmc_host_clk_hold(host); host-ios.vdd = fls(ocr) - 1; - if (mmc_host_is_spi(host)) - host-ios.chip_select = MMC_CS_HIGH; - else - host-ios.chip_select = MMC_CS_DONTCARE; - host-ios.bus_mode = MMC_BUSMODE_PUSHPULL; host-ios.power_mode = MMC_POWER_UP; - host-ios.bus_width = MMC_BUS_WIDTH_1; - host-ios.timing = MMC_TIMING_LEGACY; - mmc_set_ios(host); + /* Set initial state and call mmc_set_ios */ + mmc_set_initial_state(host); /* Try to set signal voltage to 3.3V but fall back to 1.8v or 1.2v */ if (__mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330) == 0) @@ -1585,14 +1595,9 @@ void mmc_power_off(struct mmc_host *host) host-ios.clock = 0; host-ios.vdd = 0; - if (!mmc_host_is_spi(host)) { - host-ios.bus_mode = MMC_BUSMODE_OPENDRAIN; - host-ios.chip_select = MMC_CS_DONTCARE; - } host-ios.power_mode = MMC_POWER_OFF; - host-ios.bus_width = MMC_BUS_WIDTH_1; - host-ios.timing = MMC_TIMING_LEGACY; - mmc_set_ios(host); + /* Set initial state and call mmc_set_ios */ + mmc_set_initial_state(host); /* * Some configurations, such as the 802.11 SDIO card in the OLPC @@ -2278,16 +2283,8 @@ static int mmc_do_hw_reset(struct mmc_host *host, int check) } } - if (mmc_host_is_spi(host)) { - host-ios.chip_select = MMC_CS_HIGH; - host-ios.bus_mode = MMC_BUSMODE_PUSHPULL; - } else { - host-ios.chip_select = MMC_CS_DONTCARE; - host-ios.bus_mode = MMC_BUSMODE_OPENDRAIN; - } - host-ios.bus_width = MMC_BUS_WIDTH_1; - host-ios.timing = MMC_TIMING_LEGACY; - mmc_set_ios(host); + /* Set initial state and call mmc_set_ios */ + mmc_set_initial_state(host); mmc_host_clk_release(host); diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index 443a584..d76597c 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -49,6 +49,7 @@ void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type); void mmc_power_up(struct mmc_host *host, u32 ocr); void mmc_power_off(struct mmc_host *host); void mmc_power_cycle(struct mmc_host *host, u32 ocr); +void mmc_set_initial_state(struct mmc_host *host); static inline void mmc_delay(unsigned int ms) { -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] mmc: core: group default initial state
Hi Ulf, 2014-11-05 11:13 GMT+01:00 Ulf Hansson ulf.hans...@linaro.org: On 4 November 2014 16:07, Johan Rudholm johan.rudh...@axis.com wrote: mmc_do_hw_reset, mmc_power_up and mmc_power_off all set the same group of initial values, simplify by sticking them together. That's not entirely true. This patch will actually change the behaviour for the ios.bus_mode, which isn't exactly what the commit message are telling us. See more below. I still think this patch is interesting, since apparently the value of ios.bus_mode hasn't been handled consistently. In principle I like the approach of this patch, but we need to make sure it's working and following the specs. Thank you for pointing this out. I've double-checked the JEDEC 5.01 spec regarding the bus mode in the context of this initial state, see below for comments. Kind regards Uffe Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/core.c | 49 +++ drivers/mmc/core/core.h |1 + 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 5d215ee..2c39d26 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1088,6 +1088,24 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned int width) mmc_host_clk_release(host); } +/* + * Set initial state after a power cycle or a hw_reset. + */ +void mmc_set_initial_state(struct mmc_host *host) +{ + if (mmc_host_is_spi(host)) { + host-ios.chip_select = MMC_CS_HIGH; + host-ios.bus_mode = MMC_BUSMODE_PUSHPULL; + } else { + host-ios.chip_select = MMC_CS_DONTCARE; + host-ios.bus_mode = MMC_BUSMODE_OPENDRAIN; + } + host-ios.bus_width = MMC_BUS_WIDTH_1; + host-ios.timing = MMC_TIMING_LEGACY; + + mmc_set_ios(host); +} + /** * mmc_vdd_to_ocrbitnum - Convert a voltage to the OCR bit number * @vdd: voltage (mV) @@ -1534,15 +1552,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr) mmc_host_clk_hold(host); host-ios.vdd = fls(ocr) - 1; - if (mmc_host_is_spi(host)) - host-ios.chip_select = MMC_CS_HIGH; - else - host-ios.chip_select = MMC_CS_DONTCARE; - host-ios.bus_mode = MMC_BUSMODE_PUSHPULL; Earlier we used always used MMC_BUSMODE_PUSHPULL, now it will depend on spi. The JEDEC spec says that in device identification mode the bus should be open-drain and not push-pull. So this way should be compliant to the spec, right? Unfortunately, I will require help in testing this on a proper eMMC. I can't find anything in the JEDEC or the SD spec about bus mode in SPI mode though. However, this behavior is not changed by this patch, so we should be safe. host-ios.power_mode = MMC_POWER_UP; - host-ios.bus_width = MMC_BUS_WIDTH_1; - host-ios.timing = MMC_TIMING_LEGACY; - mmc_set_ios(host); + /* Set initial state and call mmc_set_ios */ + mmc_set_initial_state(host); /* Try to set signal voltage to 3.3V but fall back to 1.8v or 1.2v */ if (__mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330) == 0) @@ -1582,14 +1594,9 @@ void mmc_power_off(struct mmc_host *host) host-ios.clock = 0; host-ios.vdd = 0; - if (!mmc_host_is_spi(host)) { - host-ios.bus_mode = MMC_BUSMODE_OPENDRAIN; - host-ios.chip_select = MMC_CS_DONTCARE; So in here another change in behaviour, right!? Ok, so the behavior for SPI is changed in this case, now we explicitly set the bus+cs modes and before we left them as they were. Does this matter though, since we're powering off the device and the bus+cs modes will be set on the subsequent mmc_power_up? - } host-ios.power_mode = MMC_POWER_OFF; - host-ios.bus_width = MMC_BUS_WIDTH_1; - host-ios.timing = MMC_TIMING_LEGACY; - mmc_set_ios(host); + /* Set initial state and call mmc_set_ios */ + mmc_set_initial_state(host); /* * Some configurations, such as the 802.11 SDIO card in the OLPC @@ -2273,16 +2280,8 @@ static int mmc_do_hw_reset(struct mmc_host *host, int check) } } - if (mmc_host_is_spi(host)) { - host-ios.chip_select = MMC_CS_HIGH; - host-ios.bus_mode = MMC_BUSMODE_PUSHPULL; - } else { - host-ios.chip_select = MMC_CS_DONTCARE; - host-ios.bus_mode = MMC_BUSMODE_OPENDRAIN; - } - host-ios.bus_width = MMC_BUS_WIDTH_1; - host-ios.timing = MMC_TIMING_LEGACY; - mmc_set_ios(host); + /* Set initial state and call mmc_set_ios */ + mmc_set_initial_state(host); mmc_host_clk_release(host); diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index 443a584..d76597c 100644 --- a/drivers/mmc
Re: [PATCH v2 3/4] mmc: core: turn hw_reset into a bus_ops
Hi Adrian, 2014-11-05 13:10 GMT+01:00 Adrian Hunter adrian.hun...@intel.com: On 04/11/14 17:07, Johan Rudholm wrote: Move the (e)MMC specific hw_reset code from core.c into mmc.c and call it from the new bus_ops member hw_reset. This also lets us add code for reseting SD cards as well. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/card/mmc_test.c |3 -- drivers/mmc/core/core.c | 48 -- drivers/mmc/core/core.h |4 +++ drivers/mmc/core/mmc.c | 37 + include/linux/mmc/core.h|1 - 5 files changed, 50 insertions(+), 43 deletions(-) diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c index 0c0fc52..2cd3385 100644 --- a/drivers/mmc/card/mmc_test.c +++ b/drivers/mmc/card/mmc_test.c @@ -2349,9 +2349,6 @@ static int mmc_test_hw_reset(struct mmc_test_card *test) if (err != -EOPNOTSUPP) return err; - if (!mmc_can_reset(card)) - return RESULT_UNSUP_CARD; - I would like to retain the ability for mmc_test to distinguish RESULT_UNSUP_CARD from RESULT_UNSUP_HOST Ok, coming up in v3, thanks. //Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/4] mmc: sd: add hw_reset callback
Enable power cycle and re-initialization of SD cards via the hw_reset bus_ops. Power cycling a buggy SD card sometimes helps it get back on track. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/sd.c | 13 + 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 0c44510..383320e 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -1177,6 +1177,18 @@ static int mmc_sd_runtime_resume(struct mmc_host *host) return 0; } +static int mmc_sd_hw_reset(struct mmc_host *host, int test) +{ + struct mmc_card *card = host-card; + + if (test == MMC_HW_RESET_TEST) + return 0; + + mmc_power_cycle(host, card-ocr); + + return 0; +} + static int mmc_sd_power_restore(struct mmc_host *host) { int ret; @@ -1198,6 +1210,7 @@ static const struct mmc_bus_ops mmc_sd_ops = { .power_restore = mmc_sd_power_restore, .alive = mmc_sd_alive, .shutdown = mmc_sd_suspend, + .hw_reset = mmc_sd_hw_reset, }; /* -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/4] mmc: core: turn hw_reset into a bus_ops
Move the (e)MMC specific hw_reset code from core.c into mmc.c and call it from the new bus_ops member hw_reset. This also lets us add code for reseting SD cards as well. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/card/mmc_test.c |3 -- drivers/mmc/core/core.c | 48 -- drivers/mmc/core/core.h |4 +++ drivers/mmc/core/mmc.c | 37 + include/linux/mmc/core.h|1 - 5 files changed, 50 insertions(+), 43 deletions(-) diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c index 0c0fc52..2cd3385 100644 --- a/drivers/mmc/card/mmc_test.c +++ b/drivers/mmc/card/mmc_test.c @@ -2349,9 +2349,6 @@ static int mmc_test_hw_reset(struct mmc_test_card *test) if (err != -EOPNOTSUPP) return err; - if (!mmc_can_reset(card)) - return RESULT_UNSUP_CARD; - return RESULT_UNSUP_HOST; } diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 2c39d26..b35f205 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2239,64 +2239,34 @@ static void mmc_hw_reset_for_init(struct mmc_host *host) mmc_host_clk_release(host); } -int mmc_can_reset(struct mmc_card *card) -{ - u8 rst_n_function; - - if (!mmc_card_mmc(card)) - return 0; - rst_n_function = card-ext_csd.rst_n_function; - if ((rst_n_function EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED) - return 0; - return 1; -} -EXPORT_SYMBOL(mmc_can_reset); - +/* Reset card in a bus-specific way */ static int mmc_do_hw_reset(struct mmc_host *host, int check) { - struct mmc_card *card = host-card; - - if (!(host-caps MMC_CAP_HW_RESET) || !host-ops-hw_reset) - return -EOPNOTSUPP; + int ret; - if (!card) + if (!host-card) return -EINVAL; - if (!mmc_can_reset(card)) + if (!host-bus_ops || !host-bus_ops-hw_reset || + host-bus_ops-hw_reset(host, MMC_HW_RESET_TEST)) return -EOPNOTSUPP; - mmc_host_clk_hold(host); - mmc_set_clock(host, host-f_init); - - host-ops-hw_reset(host); - - /* If the reset has happened, then a status command will fail */ - if (check) { - u32 status; - - if (!mmc_send_status(card, status)) { - mmc_host_clk_release(host); - return -ENOSYS; - } - } - - /* Set initial state and call mmc_set_ios */ - mmc_set_initial_state(host); + ret = host-bus_ops-hw_reset(host, check); - mmc_host_clk_release(host); + pr_warn(%s: tried to reset card (%d)\n, mmc_hostname(host), ret); return host-bus_ops-power_restore(host); } int mmc_hw_reset(struct mmc_host *host) { - return mmc_do_hw_reset(host, 0); + return mmc_do_hw_reset(host, MMC_HW_RESET_RESET); } EXPORT_SYMBOL(mmc_hw_reset); int mmc_hw_reset_check(struct mmc_host *host) { - return mmc_do_hw_reset(host, 1); + return mmc_do_hw_reset(host, MMC_HW_RESET_CHECK); } EXPORT_SYMBOL(mmc_hw_reset_check); diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index d76597c..918bbe8 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -27,6 +27,10 @@ struct mmc_bus_ops { int (*power_restore)(struct mmc_host *); int (*alive)(struct mmc_host *); int (*shutdown)(struct mmc_host *); + int (*hw_reset)(struct mmc_host *, int); +#define MMC_HW_RESET_RESET 0 +#define MMC_HW_RESET_TEST 1 +#define MMC_HW_RESET_CHECK 2 }; void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops); diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 793c6f7..55e1a97 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1773,6 +1773,42 @@ static int mmc_power_restore(struct mmc_host *host) return ret; } +static int mmc_mmc_hw_reset(struct mmc_host *host, int test) +{ + struct mmc_card *card = host-card; + u8 rst_n_function; + + if (!(host-caps MMC_CAP_HW_RESET) || !host-ops-hw_reset) + return -EOPNOTSUPP; + + rst_n_function = card-ext_csd.rst_n_function; + if ((rst_n_function EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED) + return -EOPNOTSUPP; + + if (test == MMC_HW_RESET_TEST) + return 0; + + mmc_host_clk_hold(host); + mmc_set_clock(host, host-f_init); + + host-ops-hw_reset(host); + + if (test == MMC_HW_RESET_CHECK) { + u32 status; + + if (!mmc_send_status(card, status)) { + mmc_host_clk_release(host); + return -ENOSYS; + } + } + + mmc_set_initial_state(host); + + mmc_host_clk_release(host); + + return 0; +} + static const struct
[PATCH v2 1/4] mmc: core: use mmc_send_status to check hw_reset
Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/core.c | 10 ++ 1 files changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 7dc0c85..5d215ee 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2265,15 +2265,9 @@ static int mmc_do_hw_reset(struct mmc_host *host, int check) /* If the reset has happened, then a status command will fail */ if (check) { - struct mmc_command cmd = {0}; - int err; + u32 status; - cmd.opcode = MMC_SEND_STATUS; - if (!mmc_host_is_spi(card-host)) - cmd.arg = card-rca 16; - cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC; - err = mmc_wait_for_cmd(card-host, cmd, 0); - if (!err) { + if (!mmc_send_status(card, status)) { mmc_host_clk_release(host); return -ENOSYS; } -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/4] mmc: core: hw_reset changes
Make the mmc_hw_reset routines more generic, so we can easily add a power cycle of SD cards as well. Also simplify the (e)MMC specific parts of the reset code. As I don't have an eMMC device myself, much less one with a reset line, I'd be very happy if someone could help me test the code with an eMMC? v2: - Call the new bus_ops member hw_reset instead of power_reset - Create mmc_set_initial_state and call it from mmc_mmc_hw_reset instead of mmc_power_up - Keep mmc_hw_reset naming Johan Rudholm (4): mmc: core: use mmc_send_status to check hw_reset mmc: core: group default initial state mmc: core: turn hw_reset into a bus_ops mmc: sd: add hw_reset callback drivers/mmc/card/mmc_test.c |3 - drivers/mmc/core/core.c | 99 +- drivers/mmc/core/core.h |5 ++ drivers/mmc/core/mmc.c | 37 drivers/mmc/core/sd.c | 13 ++ include/linux/mmc/core.h|1 - 6 files changed, 86 insertions(+), 72 deletions(-) -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/4] mmc: core: group default initial state
mmc_do_hw_reset, mmc_power_up and mmc_power_off all set the same group of initial values, simplify by sticking them together. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/core.c | 49 +++ drivers/mmc/core/core.h |1 + 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 5d215ee..2c39d26 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1088,6 +1088,24 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned int width) mmc_host_clk_release(host); } +/* + * Set initial state after a power cycle or a hw_reset. + */ +void mmc_set_initial_state(struct mmc_host *host) +{ + if (mmc_host_is_spi(host)) { + host-ios.chip_select = MMC_CS_HIGH; + host-ios.bus_mode = MMC_BUSMODE_PUSHPULL; + } else { + host-ios.chip_select = MMC_CS_DONTCARE; + host-ios.bus_mode = MMC_BUSMODE_OPENDRAIN; + } + host-ios.bus_width = MMC_BUS_WIDTH_1; + host-ios.timing = MMC_TIMING_LEGACY; + + mmc_set_ios(host); +} + /** * mmc_vdd_to_ocrbitnum - Convert a voltage to the OCR bit number * @vdd: voltage (mV) @@ -1534,15 +1552,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr) mmc_host_clk_hold(host); host-ios.vdd = fls(ocr) - 1; - if (mmc_host_is_spi(host)) - host-ios.chip_select = MMC_CS_HIGH; - else - host-ios.chip_select = MMC_CS_DONTCARE; - host-ios.bus_mode = MMC_BUSMODE_PUSHPULL; host-ios.power_mode = MMC_POWER_UP; - host-ios.bus_width = MMC_BUS_WIDTH_1; - host-ios.timing = MMC_TIMING_LEGACY; - mmc_set_ios(host); + /* Set initial state and call mmc_set_ios */ + mmc_set_initial_state(host); /* Try to set signal voltage to 3.3V but fall back to 1.8v or 1.2v */ if (__mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330) == 0) @@ -1582,14 +1594,9 @@ void mmc_power_off(struct mmc_host *host) host-ios.clock = 0; host-ios.vdd = 0; - if (!mmc_host_is_spi(host)) { - host-ios.bus_mode = MMC_BUSMODE_OPENDRAIN; - host-ios.chip_select = MMC_CS_DONTCARE; - } host-ios.power_mode = MMC_POWER_OFF; - host-ios.bus_width = MMC_BUS_WIDTH_1; - host-ios.timing = MMC_TIMING_LEGACY; - mmc_set_ios(host); + /* Set initial state and call mmc_set_ios */ + mmc_set_initial_state(host); /* * Some configurations, such as the 802.11 SDIO card in the OLPC @@ -2273,16 +2280,8 @@ static int mmc_do_hw_reset(struct mmc_host *host, int check) } } - if (mmc_host_is_spi(host)) { - host-ios.chip_select = MMC_CS_HIGH; - host-ios.bus_mode = MMC_BUSMODE_PUSHPULL; - } else { - host-ios.chip_select = MMC_CS_DONTCARE; - host-ios.bus_mode = MMC_BUSMODE_OPENDRAIN; - } - host-ios.bus_width = MMC_BUS_WIDTH_1; - host-ios.timing = MMC_TIMING_LEGACY; - mmc_set_ios(host); + /* Set initial state and call mmc_set_ios */ + mmc_set_initial_state(host); mmc_host_clk_release(host); diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index 443a584..d76597c 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -49,6 +49,7 @@ void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type); void mmc_power_up(struct mmc_host *host, u32 ocr); void mmc_power_off(struct mmc_host *host); void mmc_power_cycle(struct mmc_host *host, u32 ocr); +void mmc_set_initial_state(struct mmc_host *host); static inline void mmc_delay(unsigned int ms) { -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] mmc: core: use mmc_power_up in hw_reset
2014-11-03 10:21 GMT+01:00 Adrian Hunter adrian.hun...@intel.com: On 24/10/14 15:46, Johan Rudholm wrote: The steps performed in mmc_do_hw_reset for eMMC:s after the hardware reset are very similar to those performed by mmc_power_up, so do a call They perform different functions. mmc_power_up() does nothing if the card is powered up. To share the common code you need to create a function for the common functionality which is setting the initial state e.g. int mmc_set_initial_state(struct mmc_host *host) { ... } And call that from mmc_do_hw_reset and mmc_power_up. Thank you for the comment. What do you think of doing a complete mmc_power_cycle after having performed a (successful) reset? In this way we can simplify the SD card reset and we don't have to break out the common code from mmc_power_up. Naming discussion apart, something like this: diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 486cda8..941f631 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2236,12 +2236,15 @@ static void mmc_hw_reset_for_init(struct mmc_host *host) static int mmc_do_reset(struct mmc_host *host, int check) { int ret; + struct mmc_card *card = host-card; if (!host-bus_ops || !host-bus_ops-power_reset || host-bus_ops-power_reset(host, MMC_POWER_RESET_TEST)) return -EOPNOTSUPP; ret = host-bus_ops-power_reset(host, check); + if (!ret card) + mmc_power_cycle(host, card-ocr); pr_warning(%s: tried to reset card (%d)\n, mmc_hostname(host), ret); diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index ac5192c..900133a 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1794,8 +1794,6 @@ static int mmc_power_reset(struct mmc_host *host, int test) } } - mmc_power_up(host, card-ocr); - mmc_host_clk_release(host); return 0; diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 9ad46b2..d5b8ee7 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -1187,8 +1187,6 @@ static int mmc_sd_power_reset(struct mmc_host *host, int test) if (test == MMC_POWER_RESET_TEST) return 0; - mmc_power_cycle(host, card-ocr); - return 0; } //Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] mmc: core: make hw_reset generic
2014-11-03 10:20 GMT+01:00 Adrian Hunter adrian.hun...@intel.com: On 24/10/14 15:46, Johan Rudholm wrote: Move the (e)MMC specific hw_reset code from core.c into mmc.c and call it from the new bus_ops member power_reset. This also lets us add code power_reset is not a good name because it does not necessarily have anything to do with power. I am not sure why you don't stick with hw_reset. for reseting SD cards as well. Rename the mmc_hw_reset* functions into mmc_reset*, since what they now actually do depends on the device type (and it may be something else than doing a hw_reset). I don't follow your reasoning about the rename. Cycling the power is a kind of hardware reset isn't it. Since we finalize the reset by calling bus_ops-power_restore, I was looking for something symmetrical. Ulf mentioned perhaps extending bus_ops-power_save, but I couldn't find a reasonable way of doing this. Do you propose simply calling it bus_ops-hw_reset ? //Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] mmc: core: hw_reset changes
Make the mmc_hw_reset routines more generic, so we can easily add a power cycle of SD cards as well. Also simplify the (e)MMC specific parts of the reset code. As I don't have an eMMC device myself, much less one with a reset line, I'd be very happy if someone could help me test the code with an eMMC? Johan Rudholm (4): mmc: core: use mmc_send_status to check hw_reset mmc: core: use mmc_power_up in hw_reset mmc: core: make hw_reset generic mmc: sd: add power_reset callback drivers/mmc/card/block.c|2 +- drivers/mmc/card/mmc_test.c |5 +-- drivers/mmc/core/core.c | 73 --- drivers/mmc/core/core.h |4 ++ drivers/mmc/core/mmc.c | 40 +++ drivers/mmc/core/sd.c | 16 + include/linux/mmc/core.h|5 +-- 7 files changed, 77 insertions(+), 68 deletions(-) -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] mmc: core: use mmc_power_up in hw_reset
The steps performed in mmc_do_hw_reset for eMMC:s after the hardware reset are very similar to those performed by mmc_power_up, so do a call to this function instead. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/core.c | 11 +-- 1 files changed, 1 insertions(+), 10 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 5d215ee..d56e222 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2273,16 +2273,7 @@ static int mmc_do_hw_reset(struct mmc_host *host, int check) } } - if (mmc_host_is_spi(host)) { - host-ios.chip_select = MMC_CS_HIGH; - host-ios.bus_mode = MMC_BUSMODE_PUSHPULL; - } else { - host-ios.chip_select = MMC_CS_DONTCARE; - host-ios.bus_mode = MMC_BUSMODE_OPENDRAIN; - } - host-ios.bus_width = MMC_BUS_WIDTH_1; - host-ios.timing = MMC_TIMING_LEGACY; - mmc_set_ios(host); + mmc_power_up(host, card-ocr); mmc_host_clk_release(host); -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] mmc: core: make hw_reset generic
Move the (e)MMC specific hw_reset code from core.c into mmc.c and call it from the new bus_ops member power_reset. This also lets us add code for reseting SD cards as well. Rename the mmc_hw_reset* functions into mmc_reset*, since what they now actually do depends on the device type (and it may be something else than doing a hw_reset). Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/card/block.c|2 +- drivers/mmc/card/mmc_test.c |5 +--- drivers/mmc/core/core.c | 58 +- drivers/mmc/core/core.h |4 +++ drivers/mmc/core/mmc.c | 40 + include/linux/mmc/core.h|5 +-- 6 files changed, 61 insertions(+), 53 deletions(-) diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c index 452782b..8c4fa46 100644 --- a/drivers/mmc/card/block.c +++ b/drivers/mmc/card/block.c @@ -1001,7 +1001,7 @@ static int mmc_blk_reset(struct mmc_blk_data *md, struct mmc_host *host, return -EEXIST; md-reset_done |= type; - err = mmc_hw_reset(host); + err = mmc_reset(host); /* Ensure we switch back to the correct partition */ if (err != -EOPNOTSUPP) { struct mmc_blk_data *main_md = mmc_get_drvdata(host-card); diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c index 0c0fc52..599c683 100644 --- a/drivers/mmc/card/mmc_test.c +++ b/drivers/mmc/card/mmc_test.c @@ -2339,7 +2339,7 @@ static int mmc_test_hw_reset(struct mmc_test_card *test) struct mmc_host *host = card-host; int err; - err = mmc_hw_reset_check(host); + err = mmc_reset_check(host); if (!err) return RESULT_OK; @@ -2349,9 +2349,6 @@ static int mmc_test_hw_reset(struct mmc_test_card *test) if (err != -EOPNOTSUPP) return err; - if (!mmc_can_reset(card)) - return RESULT_UNSUP_CARD; - return RESULT_UNSUP_HOST; } diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index d56e222..486cda8 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2232,65 +2232,33 @@ static void mmc_hw_reset_for_init(struct mmc_host *host) mmc_host_clk_release(host); } -int mmc_can_reset(struct mmc_card *card) +/* Reset card in a bus-specific way */ +static int mmc_do_reset(struct mmc_host *host, int check) { - u8 rst_n_function; - - if (!mmc_card_mmc(card)) - return 0; - rst_n_function = card-ext_csd.rst_n_function; - if ((rst_n_function EXT_CSD_RST_N_EN_MASK) != EXT_CSD_RST_N_ENABLED) - return 0; - return 1; -} -EXPORT_SYMBOL(mmc_can_reset); - -static int mmc_do_hw_reset(struct mmc_host *host, int check) -{ - struct mmc_card *card = host-card; - - if (!(host-caps MMC_CAP_HW_RESET) || !host-ops-hw_reset) - return -EOPNOTSUPP; - - if (!card) - return -EINVAL; + int ret; - if (!mmc_can_reset(card)) + if (!host-bus_ops || !host-bus_ops-power_reset || + host-bus_ops-power_reset(host, MMC_POWER_RESET_TEST)) return -EOPNOTSUPP; - mmc_host_clk_hold(host); - mmc_set_clock(host, host-f_init); - - host-ops-hw_reset(host); - - /* If the reset has happened, then a status command will fail */ - if (check) { - u32 status; - - if (!mmc_send_status(card, status)) { - mmc_host_clk_release(host); - return -ENOSYS; - } - } - - mmc_power_up(host, card-ocr); + ret = host-bus_ops-power_reset(host, check); - mmc_host_clk_release(host); + pr_warning(%s: tried to reset card (%d)\n, mmc_hostname(host), ret); return host-bus_ops-power_restore(host); } -int mmc_hw_reset(struct mmc_host *host) +int mmc_reset(struct mmc_host *host) { - return mmc_do_hw_reset(host, 0); + return mmc_do_reset(host, MMC_POWER_RESET_RESET); } -EXPORT_SYMBOL(mmc_hw_reset); +EXPORT_SYMBOL(mmc_reset); -int mmc_hw_reset_check(struct mmc_host *host) +int mmc_reset_check(struct mmc_host *host) { - return mmc_do_hw_reset(host, 1); + return mmc_do_reset(host, MMC_POWER_RESET_CHECK); } -EXPORT_SYMBOL(mmc_hw_reset_check); +EXPORT_SYMBOL(mmc_reset_check); static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq) { diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index 443a584..6a0420b 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -25,6 +25,10 @@ struct mmc_bus_ops { int (*runtime_resume)(struct mmc_host *); int (*power_save)(struct mmc_host *); int (*power_restore)(struct mmc_host *); + int (*power_reset)(struct mmc_host *, int); +#define MMC_POWER_RESET_RESET 0 +#define MMC_POWER_RESET_TEST 1 +#define MMC_POWER_RESET_CHECK 2 int (*alive)(struct mmc_host
[PATCH 4/4] mmc: sd: add power_reset callback
Enable power cycle and re-initialization of SD cards via the mmc_reset function. Power cycling a buggy SD card sometimes helps it get back on track. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/sd.c | 16 1 files changed, 16 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 0c44510..9ad46b2 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -1177,6 +1177,21 @@ static int mmc_sd_runtime_resume(struct mmc_host *host) return 0; } +static int mmc_sd_power_reset(struct mmc_host *host, int test) +{ + struct mmc_card *card = host-card; + + if (!card) + return -EINVAL; + + if (test == MMC_POWER_RESET_TEST) + return 0; + + mmc_power_cycle(host, card-ocr); + + return 0; +} + static int mmc_sd_power_restore(struct mmc_host *host) { int ret; @@ -1196,6 +1211,7 @@ static const struct mmc_bus_ops mmc_sd_ops = { .suspend = mmc_sd_suspend, .resume = mmc_sd_resume, .power_restore = mmc_sd_power_restore, + .power_reset = mmc_sd_power_reset, .alive = mmc_sd_alive, .shutdown = mmc_sd_suspend, }; -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] mmc: core: use mmc_send_status to check hw_reset
Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/core.c | 10 ++ 1 files changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 7dc0c85..5d215ee 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2265,15 +2265,9 @@ static int mmc_do_hw_reset(struct mmc_host *host, int check) /* If the reset has happened, then a status command will fail */ if (check) { - struct mmc_command cmd = {0}; - int err; + u32 status; - cmd.opcode = MMC_SEND_STATUS; - if (!mmc_host_is_spi(card-host)) - cmd.arg = card-rca 16; - cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC; - err = mmc_wait_for_cmd(card-host, cmd, 0); - if (!err) { + if (!mmc_send_status(card, status)) { mmc_host_clk_release(host); return -ENOSYS; } -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mmc: block: change stop errors to info
Stop command errors are not fatal to the transfer since we make sure that the card returns to the transfer state and check the card status. Change an unnecessary error to an info. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/card/block.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c index 452782b..fea0376 100644 --- a/drivers/mmc/card/block.c +++ b/drivers/mmc/card/block.c @@ -977,7 +977,7 @@ static int mmc_blk_cmd_recovery(struct mmc_card *card, struct request *req, return ERR_CONTINUE; /* Now for stop errors. These aren't fatal to the transfer. */ - pr_err(%s: error %d sending stop command, original cmd response %#x, card status %#x\n, + pr_info(%s: error %d sending stop command, original cmd response %#x, card status %#x\n, req-rq_disk-disk_name, brq-stop.error, brq-cmd.resp[0], status); -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: SD-card endurance, wear and crappiness
2014-09-09 11:11 GMT+02:00 Arnd Bergmann a...@arndb.de: On Tuesday 09 September 2014 10:54:51 Johan Rudholm wrote: 2014-09-03 16:24 GMT+02:00 Johan Rudholm jrudh...@gmail.com: Hi all, as you know, NAND flash can be programmed a limited number of times before it reaches end of life, the number of times varies with the NAND technology used, among other things. As far as I can tell from the simplified SD-spec, there is no way of asking the card about how many program/erase cycles it can handle, or how many p/e cycles are left before reaching EOL. Right? I think that is correct. So, if one should want to give the user some kind of early warning that it's time to change SD-cards, is there a way? Also, when a card has reached EOL, is there a way of telling this condition apart from all other error conditions that may arise? As you know, depending on the quality of the card and controller, read timeouts, write timeouts, lockups etc may occur but can usually be fixed with a power cycle. I'm thinking of collecting simple statistics from for instance card/block.c and exposing it via an ioctl or sysfs. The statistics can be gathered and processed by some user space process which can determine if the user needs to be alerted. The statistics can be, for instance: * Writes/reads that timeout, but succeed after a retry * Writes/reads that timeout and never succeeds * Different kinds of errors in the card status * Anything else? Perhaps it's not possible to detect worn out cards this way, but at least it could point out and warn about crappy cards? Any thoughts about this? Have you tried if this works? In my experience, the worn-out cards I have either just fail completely, or they return incorrect data, but I have not looked at this side of the problem much. Do you have cards that sometimes time out but always still return correct data on retry? I have noticed that some cards time out on a multi block read, but then succeeds when single block reads are attempted. I've also experimented with retrying the multi block read instead of falling back to single block reads, and some cards succeed after a number ( 10) of retries. However, these cards have not been close to being worn out. I have almost never seen an SD-card that's died because of wear, at least not under controlled circumstances. So, thanks for sharing your experiences! Maybe the bottom line will be that there is no guaranteed way of detecting that a card is nearing EOL. //Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: SD-card endurance, wear and crappiness
Hi Ulf, Arnd, do you guys have any thoughts on this? Kind regards, Johan 2014-09-03 16:24 GMT+02:00 Johan Rudholm jrudh...@gmail.com: Hi all, as you know, NAND flash can be programmed a limited number of times before it reaches end of life, the number of times varies with the NAND technology used, among other things. As far as I can tell from the simplified SD-spec, there is no way of asking the card about how many program/erase cycles it can handle, or how many p/e cycles are left before reaching EOL. Right? So, if one should want to give the user some kind of early warning that it's time to change SD-cards, is there a way? Also, when a card has reached EOL, is there a way of telling this condition apart from all other error conditions that may arise? As you know, depending on the quality of the card and controller, read timeouts, write timeouts, lockups etc may occur but can usually be fixed with a power cycle. I'm thinking of collecting simple statistics from for instance card/block.c and exposing it via an ioctl or sysfs. The statistics can be gathered and processed by some user space process which can determine if the user needs to be alerted. The statistics can be, for instance: * Writes/reads that timeout, but succeed after a retry * Writes/reads that timeout and never succeeds * Different kinds of errors in the card status * Anything else? Perhaps it's not possible to detect worn out cards this way, but at least it could point out and warn about crappy cards? Any thoughts about this? Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
SD-card endurance, wear and crappiness
Hi all, as you know, NAND flash can be programmed a limited number of times before it reaches end of life, the number of times varies with the NAND technology used, among other things. As far as I can tell from the simplified SD-spec, there is no way of asking the card about how many program/erase cycles it can handle, or how many p/e cycles are left before reaching EOL. Right? So, if one should want to give the user some kind of early warning that it's time to change SD-cards, is there a way? Also, when a card has reached EOL, is there a way of telling this condition apart from all other error conditions that may arise? As you know, depending on the quality of the card and controller, read timeouts, write timeouts, lockups etc may occur but can usually be fixed with a power cycle. I'm thinking of collecting simple statistics from for instance card/block.c and exposing it via an ioctl or sysfs. The statistics can be gathered and processed by some user space process which can determine if the user needs to be alerted. The statistics can be, for instance: * Writes/reads that timeout, but succeed after a retry * Writes/reads that timeout and never succeeds * Different kinds of errors in the card status * Anything else? Perhaps it's not possible to detect worn out cards this way, but at least it could point out and warn about crappy cards? Any thoughts about this? Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Question for emmc ioctl sync issue
Hi Wan, 2014-05-19 9:55 GMT+02:00 Wan ZongShun mcuos@gmail.com: Hi all, I have a question for mmc_blk_ioctl_cmd. I did not see any sync code in this function like mutex, semphere.., why? The ioctl will do partition switch as I know, so If more partitions call the ioctl from user layer at the same time, how to sync with them? I guess because locking here could result in deadlocks when the device is accessed from for instance the block driver. There is no guarantee that any commands you send to the card via consecutive calls to the ioctl interface are not interlaced by other commands sent via the block driver or other ioctl calls. Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mmc: sd: warn if card stays busy during init
The initialization of some SD-cards fails because the card never leaves the busy state. Aid trouble shooting by indicating this in the kernel log. Signed-off-by: Johan Rudholm joha...@axis.com --- drivers/mmc/core/sd_ops.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c index 274ef00..48d0c93 100644 --- a/drivers/mmc/core/sd_ops.c +++ b/drivers/mmc/core/sd_ops.c @@ -184,6 +184,9 @@ int mmc_send_app_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr) mmc_delay(10); } + if (!i) + pr_err(%s: card never left busy state\n, mmc_hostname(host)); + if (rocr !mmc_host_is_spi(host)) *rocr = cmd.resp[0]; -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] mmc: mmci: Add support for UHS cards
Hi, 2013/5/15 Ulf Hansson ulf.hans...@stericsson.com: From: Ulf Hansson ulf.hans...@linaro.org UHS cards in mode SDR12|25 operates at 1.8V I/O voltage. This patchset adds support to handle the corresponding voltage switch from the default 2.7-3.6 I/O voltage to 1.8V. The first patch, Fixup regulator handling for vqmmc has recently been sent as a separate patch, since it is a pure bugfix. Nevertheless this patchset depends on this patch, thus I decided to fold it here as well. Host capabilities for SDR12 and SDR25 needs to be set to enable support, but that will be handled outside this patchset. Ulf Hansson (4): mmc: mmci: Fixup regulator handling for vqmmc mmc: mmci: Support signal voltage switch for UHS cards mmc: mmci: Cache MMCIDATACTRL register mmc: mmci: Add card_busy function to improve UHS card support drivers/mmc/host/mmci.c | 94 +++ drivers/mmc/host/mmci.h |4 ++ 2 files changed, 90 insertions(+), 8 deletions(-) Looks good! Please add my Reviewed-by: Johan Rudholm jrudh...@gmail.com to this patch set. Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mmc: core: Fix select power class after resume
Hi, 2013/4/23 Fredrik Soderstedt fredrik.soderst...@stericsson.com: Use the saved values in card-ext_csd when selecting power class. By doing this the power class will be selected even if mmc_init_card is called with oldcard != NULL, which is the case after a suspend/resume. Today ext_csd is NULL if mmc_init_card is called with oldcard != NULL and power class will not be selected. According to the eMMC specification the POWER_CLASS value is reset after power failure, H/W reset assertion and any CMD0 reset. CC: Girish K S girish.shivananja...@linaro.org Signed-off-by: Fredrik Soderstedt fredrik.soderst...@stericsson.com Looks good to me! Please add my Reviewed-by: Johan Rudholm jrudh...@gmail.com Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: core: Fix select power class after resume
Hi, 2013/4/22 Fredrik Soderstedt fredrik.soderst...@stericsson.com: Use the saved values in card-ext_csd when selecting power class. By doing this the power class will be selected even if mmc_init_card is called with oldcard != NULL, which is the case after a suspend/resume. Today ext_csd is NULL if mmc_init_card is called with oldcard != NULL and power class will not be selected. According to the eMMC specification the POWER_CLASS value is reset after power failure, H/W reset assertion and any CMD0 reset. CC: Girish K S girish.shivananja...@linaro.org Signed-off-by: Fredrik Soderstedt fredrik.soderst...@stericsson.com --- snip @@ -720,13 +750,13 @@ static int mmc_select_powerclass(struct mmc_card *card, case MMC_VDD_34_35: case MMC_VDD_35_36: if (host-ios.clock = 2600) - index = EXT_CSD_PWR_CL_26_360; + pwrclass_val = card-ext_csd.raw_pwr_cl_26_360; else if (host-ios.clock = 5200) - index = (bus_width = EXT_CSD_BUS_WIDTH_8) ? - EXT_CSD_PWR_CL_52_360 : - EXT_CSD_PWR_CL_DDR_52_360; + pwrclass_val = (bus_width = EXT_CSD_BUS_WIDTH_8) ? + card-ext_csd.raw_pwr_cl_52_360 : + card-ext_csd.raw_pwr_cl_52_360; I believe you should have _ddr here, right? Otherwise it looks good. Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: core: Only execute tuning for SDR50 and SDR104
Hi, 2013/4/17 Fredrik Soderstedt fredrik.soderst...@stericsson.com: Only execute tuning for sd and sdio devices that are using SDR50 or SDR104. Make sure clock is hold during tuning for sdio devices. Signed-off-by: Fredrik Soderstedt fredrik.soderst...@stericsson.com Acked-by: Johan Rudholm jrudh...@gmail.com //Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: block: fix the host's claim-release in special request
Hi Seungwon, 2013/3/14 Seungwon Jeon tgih@samsung.com: For normal request mmc_blk_issue_rq is called twice with asynchronous transfer(cur and prev). Host's claim and release can be done in each mmc_blk_issue_rq. However, Special request is currently excluded in asynchronous transfer. After special request is finished, if there is no new request, mmc_release_host won't be called in mmc_blk_issue_rq. The problem is founded during mmc_suspend. [c0541124] (__schedule+0x0/0x78c) from [c05419e8] (schedule+0x38/0x78) [c05419b0] (schedule+0x0/0x78) from [c03a843c] (__mmc_claim_host+0xac/0x1b4) [c03a8390] (__mmc_claim_host+0x0/0x1b4) from [c03ac98c] (mmc_suspend+0x28/0x9c) [c03ac964] (mmc_suspend+0x0/0x9c) from [c03aad24] (mmc_suspend_host+0xb4/0x194) ... Reported-by: Johan Rudholm jrudh...@gmail.com Signed-off-by: Seungwon Jeon tgih@samsung.com Thank you for the quick response and fix! This fixes the problem. So please include my Tested-by: Johan Rudholm johan.rudh...@stericsson.com Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Trouble suspending with mmc: queue: exclude asynchronous transfer for special request
Hi Seungwon, we've just backported mmc-related patches from 3.9rc1 to our internal tree, and I'm seeing an issue with: 369d321 mmc: queue: exclude asynchronous transfer for special request When the device is going into suspend, it seems that mmcqd still has claimed the host and so the 20s DPM device timeout happens, and we get a kernel BUG: [ 42.049865] PM: Syncing filesystems ... done. [ 42.058044] Freezing user space processes ... (elapsed 0.03 seconds) done. [ 42.098937] Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done. [ 42.118988] Suspending console(s) (use no_console_suspend to debug) [ 42.129455] alarmtimer alarmtimer: Next rtc wakeup 2000.01.04 - 19:48:10 [ 42.155517] regulator regulator.24: VEXTSUPPLY3-disable (bank, reg, mask, value): 0x04, 0x08, 0x30, 0x00 [ 54.185241] DPM device timeout: sdi2 (mmci-pl18x) [ 54.185241] dpm suspend stack: [ 54.185302] [c071276c] (__schedule+0x310/0x624) from [c0712bd0] (schedule+0x40/0x80) [ 54.185302] [c0712bd0] (schedule+0x40/0x80) from [c0488774] (__mmc_claim_host+0xa4/0x1a8) [ 54.185333] [c0488774] (__mmc_claim_host+0xa4/0x1a8) from [c048dd68] (mmc_suspend+0x34/0x134) [ 54.185363] [c048dd68] (mmc_suspend+0x34/0x134) from [c048b688] (mmc_suspend_host.part.20+0x98/0x1b0) [ 54.185363] [c048b688] (mmc_suspend_host.part.20+0x98/0x1b0) from [c048b7fc] (mmc_suspend_host+0x5c/0x70) [ 54.185394] [c048b7fc] (mmc_suspend_host+0x5c/0x70) from [c049cce8] (mmci_suspend+0x28/0x64) [ 54.185424] [c049cce8] (mmci_suspend+0x28/0x64) from [c0315d54] (amba_pm_suspend+0x3c/0x68) [ 54.185455] [c0315d54] (amba_pm_suspend+0x3c/0x68) from [c0022b2c] (ux500_pd_amba_pm_suspend+0x3c/0x50) [ 54.185455] [c0022b2c] (ux500_pd_amba_pm_suspend+0x3c/0x50) from [c0374a8c] (dpm_run_callback+0x54/0x8c) [ 54.185485] [c0374a8c] (dpm_run_callback+0x54/0x8c) from [c0375c54] (__device_suspend+0x174/0x3f0) [ 54.185516] [c0375c54] (__device_suspend+0x174/0x3f0) from [c03768a4] (dpm_suspend+0x60/0x21c) [ 54.185516] [c03768a4] (dpm_suspend+0x60/0x21c) from [c0376bd4] (dpm_suspend_start+0x68/0x70) [ 54.185546] [c0376bd4] (dpm_suspend_start+0x68/0x70) from [c007d424] (suspend_devices_and_enter+0x70/0x20c) [ 54.185546] [c007d424] (suspend_devices_and_enter+0x70/0x20c) from [c007d66c] (enter_state+0xac/0x184) [ 54.185577] [c007d66c] (enter_state+0xac/0x184) from [c007d768] (pm_suspend+0x24/0x80) [ 54.185607] [c007d768] (pm_suspend+0x24/0x80) from [c007e774] (suspend.part.2+0x44/0x174) [ 54.185607] [c007e774] (suspend.part.2+0x44/0x174) from [c007ea04] (suspend+0x44/0x50) [ 54.185638] [c007ea04] (suspend+0x44/0x50) from [c0058280] (process_one_work+0x138/0x4b0) [ 54.185638] [c0058280] (process_one_work+0x138/0x4b0) from [c0058748] (worker_thread+0x150/0x37c) [ 54.185668] [c0058748] (worker_thread+0x150/0x37c) from [c005d768] (kthread+0x98/0xa4) [ 54.185699] [c005d768] (kthread+0x98/0xa4) from [c00106a8] (kernel_thread_exit+0x0/0x8) Do you have any idea what causes this? Please disregard the amba-stuff, this is our power domain. Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/2] mmc: core: enhance card removal judgement for slow removal
Hi, 2013/2/28 Ulf Hansson ulf.hans...@linaro.org: On 28 February 2013 08:29, Kevin Liu kl...@marvell.com wrote: Function _mmc_detect_card_removed will be called to know whether the card is still present when host-bus_ops-detect is called. In current code, the return value of this function generally only depend on the result of sending cmd13 to card, which may not safe for card with detection support like slot gpio detection. Because the communication status between host and card may out of sync with the detect status if remove the card slowly or hands shake during the process. The direct reason is the async between card detect switch and card/slot pad contaction in hardware, which is defined by spec. The spec define card insert/remove sequence as below (both standard size SD card and MicroSD card have the same sequence): Part 1 Standard Size SD Card Mechanical Addendum Ver4.00 Final, Appendix C: Card Detection Switch (Take normally open type as example) a)SD card insertion sequence: The card detection switch should be turned on after all SD card contact pads are connected to the host connector contact pads. b)SD removal sequence: The card detection switch should be turned off when the SD card is just going to be removed and before any SD card contact pad is disconnected from the host connector contact pad. Below is the sequence when this issue occur (Take slot gpio detection as example and remove the card slowly during the process): 1. gpio level changed and card detect interrupt triggered. 2. mmc_rescan was launched. 3. the card pads were still contacted with the slot pads because of slow removal. So _mmc_detect_card_removed and mmc_rescan think card was still present (cmd13 succeed). 4. card pads were discontacted from the card slot pads. So the card was actually removed finally but the card removal event has been missed by system. The interval length between step 1 and step 4 depends on the card removal speed. If it's longer than the detect work schedule delay which is 200ms, this issue will likely happen. This patch add the card detect status check in function _mmc_detect_card_removed if cmd13 check succeed and host-ops-get_cd provided. If get_cd detect no card present then schedule another detect work 200ms later. Signed-off-by: Kevin Liu kl...@marvell.com --- drivers/mmc/core/core.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index b8c3d41..d9ac96c 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2290,6 +2290,19 @@ int _mmc_detect_card_removed(struct mmc_host *host) return 1; ret = host-bus_ops-alive(host); + /* +* Card detect status and alive check may out of sync if card is +* removed slowly when card detect switch changed while card/slot +* pads still contacted in hardware.(refer to SD Card Mechanical +* Addendum, Appendix C: Card Detection Switch) So reschedule a +* detect work 200ms later for this case. +*/ + if (!ret host-ops-get_cd !host-ops-get_cd(host)) { + mmc_detect_change(host, msecs_to_jiffies(200)); + pr_debug(%s: card remove too slowly\n, + mmc_hostname(host)); + } + if (ret) { mmc_card_set_removed(host-card); pr_debug(%s: card remove detected\n, mmc_hostname(host)); -- 1.7.9.5 Great work Kevin and thanks for convincing me around this patch! :-) I also intend to remove the MMC_CAP2_DETECT_ON_ERR code, I don't see a need for it anymore. Acked-by: Ulf Hansson ulf.hans...@linaro.org and Tested-by: Johan Rudholm johan.rudh...@stericsson.com //Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mmc: host: sdhci: Fix parameter of sdhci_do_start_signal_voltage_switch()
Hi, 2013/2/14 Fabio Estevam fabio.este...@freescale.com: Commit 3714f4315354 (mmc: sdhci: update signal voltage switch code) changed the type of the second parameter of sdhci_do_start_signal_voltage_switch(), from struct mmc_ios *ios to int signal_voltage which causes the following build warning: drivers/mmc/host/sdhci.c:2044:2: warning: initialization from incompatible pointer type [enabled by default] drivers/mmc/host/sdhci.c:2044:2: warning: (near initialization for 'sdhci_ops.start_signal_voltage_switch') [enabled by default] Use the previous type so that it matches the start_signal_voltage_switch() definition from host.h. Signed-off-by: Fabio Estevam fabio.este...@freescale.com Reviewed-by: Johan Rudholm johan.rudh...@stericsson.com Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/5] mmc: core: Signal voltage switch procedure for UHS mode
Hi Chris, 2013/2/11 Chris Ball c...@laptop.org: Hi Johan, On Mon, Jan 28 2013, Johan Rudholm wrote: This patch series attempts to make the 1.8V signal voltage switch required for UHS mode work according to the SD specification. Thanks, pushed to mmc-next for 3.9. Cool! Just so we won't forget and break something, have you considered taking in the following patch too? From fc6069b79203f64e7efa8dd0bb295bebcd22f710 Mon Sep 17 00:00:00 2001 From: Kevin Liu kl...@marvell.com Date: Fri, 14 Dec 2012 16:44:10 +0800 Subject: [PATCH v3 3/3] mmc: sdhci: update signal voltage switch code otherwise I think that SDHCI may break. Kevin, please correct me if I'm wrong? Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 2/5] mmc: core: Add mmc_power_cycle
Add mmc_power_cycle which can be used to power cycle for instance SD-cards. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com Acked-by: Ulf Hansson ulf.hans...@linaro.org Tested-by: Wei WANG wei_w...@realsil.com.cn --- drivers/mmc/core/core.c |8 drivers/mmc/core/core.h |1 + 2 files changed, 9 insertions(+) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 8b3a122..304c97c 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1470,6 +1470,14 @@ void mmc_power_off(struct mmc_host *host) mmc_host_clk_release(host); } +void mmc_power_cycle(struct mmc_host *host) +{ + mmc_power_off(host); + /* Wait at least 1 ms according to SD spec */ + mmc_delay(1); + mmc_power_up(host); +} + /* * Cleanup when the last reference to the bus operator is dropped. */ diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index 0272b32..9007de7 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -45,6 +45,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, void mmc_set_timing(struct mmc_host *host, unsigned int timing); void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type); void mmc_power_off(struct mmc_host *host); +void mmc_power_cycle(struct mmc_host *host); static inline void mmc_delay(unsigned int ms) { -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 0/5] mmc: core: Signal voltage switch procedure for UHS mode
This patch series attempts to make the 1.8V signal voltage switch required for UHS mode work according to the SD specification. These patches have been tested with a couple of UHS SD cards, one of which sometimes requires up to five power cycles before it accepts the signal voltage switch. The patches have also been tested with various other SD cards, as well as one SDIO WLAN chip (cw1200) to check for regressions. The patches have also been tested with CONFIG_MMC_CLKGATE. As usual, I'd be very grateful if someone could help me test this patch with an UHS SDIO card and perhaps also a combo card (which does seem to be rare these days?)? This patch series is based on previous RFC/patch: [RFC/PATCH v2] mmc: core: Fixup signal voltage switch and more recently on Kevin Liu's patches: [PATCH v2 1/3] mmc: core: enhance the code for signal voltage setting [PATCH v2 2/3] mmc: core: cycle power the card in voltage switch rather than mmc_sd_get_cid Changelog: v5 - v6 - Rebase - Adapt sd.c to use mmc_host_uhs introduced by mmc: sdio: Fix SDIO 3.0 UHS-I initialization sequence v4 - v5 - Reset signal_voltage in struct ios if switch fails v3 - v4 - Added mmc: core: Break out start_signal_voltage_switch, which divides up mmc_set_signal_voltage to avoid a recursive call from mmc_power_up. This removed the third argument (cmd11) of the function - Minor bug fixes v2 - v3 - Minor bug fixes v1 - v2 - Move mmc_power_cycle to the mmc_set_signal_voltage function - Check card_busy before switching voltages - Return -EPERM if signal voltage switch with cmd11 is requested, but no start_signal_voltage_switch is defined. - If we're switching back to 3.3 V, just do the switch without checking card_busy etc v2 - This patch series - Removed the extra argument to the card_busy host_ops function - Added mmc_power_cycle - Some clarifying comments v1 - v2 - Removed reset of signal voltage in mmc_sd_get_cid, since mmc: core: reset signal voltage on power up previous two patches - v1: - Keep calls to mmc_host_clk_hold / mmc_host_clk_release - Add retry-loop / power cycle in sdio.c - Fall back to 3.3 V if the switch repeatedly fails - Add an extra argument to the card_busy host_ops function, which can be used to signal polling use of the function Johan Rudholm (5): mmc: sd: Simplify by using mmc_host_uhs mmc: core: Add mmc_power_cycle mmc: core: Add card_busy to host_ops mmc: core: Break out start_signal_voltage_switch mmc: core: Fixup signal voltage switch drivers/mmc/core/core.c | 114 +++--- drivers/mmc/core/core.h |5 +- drivers/mmc/core/mmc.c |8 ++-- drivers/mmc/core/sd.c| 27 +++ drivers/mmc/core/sdio.c | 23 -- include/linux/mmc/host.h |3 ++ 6 files changed, 145 insertions(+), 35 deletions(-) -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 3/5] mmc: core: Add card_busy to host_ops
This host_ops member is used to test if the card is signaling busy by pulling dat[0:3] low. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com Acked-by: Ulf Hansson ulf.hans...@linaro.org Tested-by: Wei WANG wei_w...@realsil.com.cn --- include/linux/mmc/host.h |3 +++ 1 file changed, 3 insertions(+) diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 523d570..0373b0a 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -131,6 +131,9 @@ struct mmc_host_ops { int (*start_signal_voltage_switch)(struct mmc_host *host, struct mmc_ios *ios); + /* Check if the card is pulling dat[0:3] low */ + int (*card_busy)(struct mmc_host *host); + /* The tuning command opcode value is different for SD and eMMC cards */ int (*execute_tuning)(struct mmc_host *host, u32 opcode); void(*enable_preset_value)(struct mmc_host *host, bool enable); -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 5/5] mmc: core: Fixup signal voltage switch
When switching SD and SDIO cards from 3.3V to 1.8V signal levels, the clock should be gated for 5 ms during the step. After enabling the clock, the host should wait for at least 1 ms before checking for failure. Failure by the card to switch is indicated by dat[0:3] being pulled low. The host should check for this condition and power-cycle the card if failure is indicated. Add a retry mechanism for the SDIO case. If the voltage switch fails repeatedly, give up and continue the initialization using the original voltage. This patch places a couple of requirements on the host driver: 1) mmc_set_ios with ios.clock = 0 must gate the clock 2) mmc_power_off must actually cut the power to the card 3) The card_busy host_ops member must be implemented if these requirements are not fulfilled, the 1.8V signal voltage switch will still be attempted but may not be successful. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com Signed-off-by: Kevin Liu kl...@marvell.com Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Tested-by: Wei WANG wei_w...@realsil.com.cn --- drivers/mmc/core/core.c | 83 +-- drivers/mmc/core/sd.c | 21 +--- drivers/mmc/core/sdio.c | 20 ++-- 3 files changed, 107 insertions(+), 17 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 3823351..2aee757 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1340,6 +1340,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage) { struct mmc_command cmd = {0}; int err = 0; + u32 clock; BUG_ON(!host); @@ -1347,20 +1348,82 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage) * Send CMD11 only if the request is to switch the card to * 1.8V signalling. */ - if (signal_voltage != MMC_SIGNAL_VOLTAGE_330) { - cmd.opcode = SD_SWITCH_VOLTAGE; - cmd.arg = 0; - cmd.flags = MMC_RSP_R1 | MMC_CMD_AC; + if (signal_voltage == MMC_SIGNAL_VOLTAGE_330) + return __mmc_set_signal_voltage(host, signal_voltage); - err = mmc_wait_for_cmd(host, cmd, 0); - if (err) - return err; + /* +* If we cannot switch voltages, return failure so the caller +* can continue without UHS mode +*/ + if (!host-ops-start_signal_voltage_switch) + return -EPERM; + if (!host-ops-card_busy) + pr_warning(%s: cannot verify signal voltage switch\n, + mmc_hostname(host)); + + cmd.opcode = SD_SWITCH_VOLTAGE; + cmd.arg = 0; + cmd.flags = MMC_RSP_R1 | MMC_CMD_AC; + + err = mmc_wait_for_cmd(host, cmd, 0); + if (err) + return err; - if (!mmc_host_is_spi(host) (cmd.resp[0] R1_ERROR)) - return -EIO; + if (!mmc_host_is_spi(host) (cmd.resp[0] R1_ERROR)) + return -EIO; + + mmc_host_clk_hold(host); + /* +* The card should drive cmd and dat[0:3] low immediately +* after the response of cmd11, but wait 1 ms to be sure +*/ + mmc_delay(1); + if (host-ops-card_busy !host-ops-card_busy(host)) { + err = -EAGAIN; + goto power_cycle; } + /* +* During a signal voltage level switch, the clock must be gated +* for 5 ms according to the SD spec +*/ + clock = host-ios.clock; + host-ios.clock = 0; + mmc_set_ios(host); - return __mmc_set_signal_voltage(host, signal_voltage); + if (__mmc_set_signal_voltage(host, signal_voltage)) { + /* +* Voltages may not have been switched, but we've already +* sent CMD11, so a power cycle is required anyway +*/ + err = -EAGAIN; + goto power_cycle; + } + + /* Keep clock gated for at least 5 ms */ + mmc_delay(5); + host-ios.clock = clock; + mmc_set_ios(host); + + /* Wait for at least 1 ms according to spec */ + mmc_delay(1); + + /* +* Failure to switch is indicated by the card holding +* dat[0:3] low +*/ + if (host-ops-card_busy host-ops-card_busy(host)) + err = -EAGAIN; + +power_cycle: + if (err) { + pr_debug(%s: Signal voltage switch failed, + power cycling card\n, mmc_hostname(host)); + mmc_power_cycle(host); + } + + mmc_host_clk_release(host); + + return err; } /* diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 9a59fcd..03134b1 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -712,6 +712,14 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr) { int err; u32 max_current
[PATCH v6 1/5] mmc: sd: Simplify by using mmc_host_uhs
Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com --- drivers/mmc/core/sd.c |6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 74972c2..9373639 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -444,8 +444,7 @@ static void sd_update_bus_speed_mode(struct mmc_card *card) * If the host doesn't support any of the UHS-I modes, fallback on * default speed. */ - if (!(card-host-caps (MMC_CAP_UHS_SDR12 | MMC_CAP_UHS_SDR25 | - MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_SDR104 | MMC_CAP_UHS_DDR50))) { + if (!mmc_host_uhs(card-host)) { card-sd_bus_speed = 0; return; } @@ -736,8 +735,7 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr) * If the host supports one of UHS-I modes, request the card * to switch to 1.8V signaling level. */ - if (host-caps (MMC_CAP_UHS_SDR12 | MMC_CAP_UHS_SDR25 | - MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_SDR104 | MMC_CAP_UHS_DDR50)) + if (mmc_host_uhs(host)) ocr |= SD_OCR_S18R; /* -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 4/5] mmc: core: Break out start_signal_voltage_switch
Allow callers to access the start_signal_voltage_switch host_ops member without going through any cmd11 logic. This is mostly a preparation for the following signal voltage switch patch. Also, reset ios.signal_voltage to its original value if start_signal_voltage_switch fails. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com Acked-by: Ulf Hansson ulf.hans...@linaro.org Tested-by: Wei WANG wei_w...@realsil.com.cn --- drivers/mmc/core/core.c | 35 +++ drivers/mmc/core/core.h |4 ++-- drivers/mmc/core/mmc.c |8 drivers/mmc/core/sd.c |2 +- drivers/mmc/core/sdio.c |3 +-- 5 files changed, 31 insertions(+), 21 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 304c97c..3823351 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1317,7 +1317,26 @@ u32 mmc_select_voltage(struct mmc_host *host, u32 ocr) return ocr; } -int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11) +int __mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage) +{ + int err = 0; + int old_signal_voltage = host-ios.signal_voltage; + + host-ios.signal_voltage = signal_voltage; + if (host-ops-start_signal_voltage_switch) { + mmc_host_clk_hold(host); + err = host-ops-start_signal_voltage_switch(host, host-ios); + mmc_host_clk_release(host); + } + + if (err) + host-ios.signal_voltage = old_signal_voltage; + + return err; + +} + +int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage) { struct mmc_command cmd = {0}; int err = 0; @@ -1328,7 +1347,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11 * Send CMD11 only if the request is to switch the card to * 1.8V signalling. */ - if ((signal_voltage != MMC_SIGNAL_VOLTAGE_330) cmd11) { + if (signal_voltage != MMC_SIGNAL_VOLTAGE_330) { cmd.opcode = SD_SWITCH_VOLTAGE; cmd.arg = 0; cmd.flags = MMC_RSP_R1 | MMC_CMD_AC; @@ -1341,15 +1360,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11 return -EIO; } - host-ios.signal_voltage = signal_voltage; - - if (host-ops-start_signal_voltage_switch) { - mmc_host_clk_hold(host); - err = host-ops-start_signal_voltage_switch(host, host-ios); - mmc_host_clk_release(host); - } - - return err; + return __mmc_set_signal_voltage(host, signal_voltage); } /* @@ -1412,7 +1423,7 @@ static void mmc_power_up(struct mmc_host *host) mmc_set_ios(host); /* Set signal voltage to 3.3V */ - mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330, false); + __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330); /* * This delay should be sufficient to allow the power supply diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index 9007de7..b9f18a2 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -40,8 +40,8 @@ void mmc_set_ungated(struct mmc_host *host); void mmc_set_bus_mode(struct mmc_host *host, unsigned int mode); void mmc_set_bus_width(struct mmc_host *host, unsigned int width); u32 mmc_select_voltage(struct mmc_host *host, u32 ocr); -int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, - bool cmd11); +int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage); +int __mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage); void mmc_set_timing(struct mmc_host *host, unsigned int timing); void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type); void mmc_power_off(struct mmc_host *host); diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index e6e3911..bd86771 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -769,11 +769,11 @@ static int mmc_select_hs200(struct mmc_card *card) if (card-ext_csd.card_type EXT_CSD_CARD_TYPE_SDR_1_2V host-caps2 MMC_CAP2_HS200_1_2V_SDR) - err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_120, 0); + err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_120); if (err card-ext_csd.card_type EXT_CSD_CARD_TYPE_SDR_1_8V host-caps2 MMC_CAP2_HS200_1_8V_SDR) - err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180, 0); + err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180); /* If fails try again during next card power cycle */ if (err) @@ -1221,8 +1221,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, * WARNING: eMMC rules are NOT the same as SD DDR */ if (ddr
Re: [PATCH v5 0/4] mmc: core: Signal voltage switch procedure for UHS mode
Hi, 2013/1/8 Ulf Hansson ulf.hans...@linaro.org: On 8 January 2013 15:05, Johan Rudholm johan.rudh...@stericsson.com wrote: This patch series attempts to make the 1.8V signal voltage switch required for UHS mode work according to the SD specification. These patches have been tested with a couple of UHS SD cards, one of which sometimes requires up to five power cycles before it accepts the signal voltage switch. The patches have also been tested with various other SD cards, as well as one SDIO WLAN chip (cw1200) to check for regressions. The patches have also been tested with CONFIG_MMC_CLKGATE. As usual, I'd be very grateful if someone could help me test this patch with an UHS SDIO card and perhaps also a combo card (which does seem to be rare these days?)? This patch series is based on previous RFC/patch: [RFC/PATCH v2] mmc: core: Fixup signal voltage switch and more recently on Kevin Liu's patches: [PATCH v2 1/3] mmc: core: enhance the code for signal voltage setting [PATCH v2 2/3] mmc: core: cycle power the card in voltage switch rather than mmc_sd_get_cid Changelog: v4 - v5 - Reset signal_voltage in struct ios if switch fails v3 - v4 - Added mmc: core: Break out start_signal_voltage_switch, which divides up mmc_set_signal_voltage to avoid a recursive call from mmc_power_up. This removed the third argument (cmd11) of the function - Minor bug fixes v2 - v3 - Minor bug fixes v1 - v2 - Move mmc_power_cycle to the mmc_set_signal_voltage function - Check card_busy before switching voltages - Return -EPERM if signal voltage switch with cmd11 is requested, but no start_signal_voltage_switch is defined. - If we're switching back to 3.3 V, just do the switch without checking card_busy etc v2 - This patch series - Removed the extra argument to the card_busy host_ops function - Added mmc_power_cycle - Some clarifying comments v1 - v2 - Removed reset of signal voltage in mmc_sd_get_cid, since mmc: core: reset signal voltage on power up previous two patches - v1: - Keep calls to mmc_host_clk_hold / mmc_host_clk_release - Add retry-loop / power cycle in sdio.c - Fall back to 3.3 V if the switch repeatedly fails - Add an extra argument to the card_busy host_ops function, which can be used to signal polling use of the function Johan Rudholm (4): mmc: core: Add mmc_power_cycle mmc: core: Add card_busy to host_ops mmc: core: Break out start_signal_voltage_switch mmc: core: Fixup signal voltage switch drivers/mmc/core/core.c | 114 +++--- drivers/mmc/core/core.h |5 +- drivers/mmc/core/mmc.c |8 ++-- drivers/mmc/core/sd.c| 23 +++--- drivers/mmc/core/sdio.c | 23 -- include/linux/mmc/host.h |3 ++ 6 files changed, 144 insertions(+), 32 deletions(-) -- 1.7.10 For those patches in this patchset that does not already have my signoff-by; Acked-by: Ulf Hansson ulf.hans...@linaro.org Also, since Wei very kindly agreed to test the patch set with the rtsx driver: Tested-by: Wei WANG wei_w...@realsil.com.cn Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/4] mmc: core: Break out start_signal_voltage_switch
Hi Kevin and Ulf, 2013/1/8 Ulf Hansson ulf.hans...@linaro.org: On 8 January 2013 01:56, Kevin Liu keyuan@gmail.com wrote: 2013/1/8 Ulf Hansson ulf.hans...@linaro.org: On 7 January 2013 16:10, Johan Rudholm johan.rudh...@stericsson.com wrote: Allow callers to access the start_signal_voltage_switch host_ops member without going through any cmd11 logic. This is mostly a preparation for the following signal voltage switch patch. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com --- drivers/mmc/core/core.c | 31 +++ drivers/mmc/core/core.h |4 ++-- drivers/mmc/core/mmc.c |8 drivers/mmc/core/sd.c |2 +- drivers/mmc/core/sdio.c |3 +-- 5 files changed, 27 insertions(+), 21 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 285f064..90f8e11 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1219,7 +1219,22 @@ u32 mmc_select_voltage(struct mmc_host *host, u32 ocr) return ocr; } -int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11) +int __mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage) +{ + int err = 0; + + host-ios.signal_voltage = signal_voltage; + if (host-ops-start_signal_voltage_switch) { + mmc_host_clk_hold(host); + err = host-ops-start_signal_voltage_switch(host, host-ios); Would it not make sense to restore the host-ios.signal_voltage value if this goes wrong? Why need to transfer ios to the voltage switch function which only need the voltage value. Why not just transfer the voltage value to the voltage switch function? If return ok then set host-ios.signal_voltage = signal_voltage otherwise do nothing. I did this in my previous patch. I like the idea! Although, if the host-ops-start_signal_voltage_switch function pointer is missing, we should fall back to set the new voltage value anyway I think. How about we keep the definition of start_signal_voltage_switch for now and maybe send it as a separate patch later? This change is not really functionally related to this patch series. I agree that reseting the value of ios.signal_voltage to the original value on failure is good, as Kevin initially suggested. I'll take care of this in the next version! Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 2/4] mmc: core: Add card_busy to host_ops
This host_ops member is used to test if the card is signaling busy by pulling dat[0:3] low. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com --- include/linux/mmc/host.h |3 +++ 1 file changed, 3 insertions(+) diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 61a10c1..3b56ef2 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -131,6 +131,9 @@ struct mmc_host_ops { int (*start_signal_voltage_switch)(struct mmc_host *host, struct mmc_ios *ios); + /* Check if the card is pulling dat[0:3] low */ + int (*card_busy)(struct mmc_host *host); + /* The tuning command opcode value is different for SD and eMMC cards */ int (*execute_tuning)(struct mmc_host *host, u32 opcode); void(*enable_preset_value)(struct mmc_host *host, bool enable); -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 3/4] mmc: core: Break out start_signal_voltage_switch
Allow callers to access the start_signal_voltage_switch host_ops member without going through any cmd11 logic. This is mostly a preparation for the following signal voltage switch patch. Also, reset ios.signal_voltage to its original value if start_signal_voltage_switch fails. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com --- drivers/mmc/core/core.c | 35 +++ drivers/mmc/core/core.h |4 ++-- drivers/mmc/core/mmc.c |8 drivers/mmc/core/sd.c |2 +- drivers/mmc/core/sdio.c |3 +-- 5 files changed, 31 insertions(+), 21 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 285f064..03db641 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1219,7 +1219,26 @@ u32 mmc_select_voltage(struct mmc_host *host, u32 ocr) return ocr; } -int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11) +int __mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage) +{ + int err = 0; + int old_signal_voltage = host-ios.signal_voltage; + + host-ios.signal_voltage = signal_voltage; + if (host-ops-start_signal_voltage_switch) { + mmc_host_clk_hold(host); + err = host-ops-start_signal_voltage_switch(host, host-ios); + mmc_host_clk_release(host); + } + + if (err) + host-ios.signal_voltage = old_signal_voltage; + + return err; + +} + +int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage) { struct mmc_command cmd = {0}; int err = 0; @@ -1230,7 +1249,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11 * Send CMD11 only if the request is to switch the card to * 1.8V signalling. */ - if ((signal_voltage != MMC_SIGNAL_VOLTAGE_330) cmd11) { + if (signal_voltage != MMC_SIGNAL_VOLTAGE_330) { cmd.opcode = SD_SWITCH_VOLTAGE; cmd.arg = 0; cmd.flags = MMC_RSP_R1 | MMC_CMD_AC; @@ -1243,15 +1262,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11 return -EIO; } - host-ios.signal_voltage = signal_voltage; - - if (host-ops-start_signal_voltage_switch) { - mmc_host_clk_hold(host); - err = host-ops-start_signal_voltage_switch(host, host-ios); - mmc_host_clk_release(host); - } - - return err; + return __mmc_set_signal_voltage(host, signal_voltage); } /* @@ -1314,7 +1325,7 @@ static void mmc_power_up(struct mmc_host *host) mmc_set_ios(host); /* Set signal voltage to 3.3V */ - mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330, false); + __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330); /* * This delay should be sufficient to allow the power supply diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index 4a90067..f2439d5 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -40,8 +40,8 @@ void mmc_set_ungated(struct mmc_host *host); void mmc_set_bus_mode(struct mmc_host *host, unsigned int mode); void mmc_set_bus_width(struct mmc_host *host, unsigned int width); u32 mmc_select_voltage(struct mmc_host *host, u32 ocr); -int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, - bool cmd11); +int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage); +int __mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage); void mmc_set_timing(struct mmc_host *host, unsigned int timing); void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type); void mmc_power_off(struct mmc_host *host); diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index e6e3911..bd86771 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -769,11 +769,11 @@ static int mmc_select_hs200(struct mmc_card *card) if (card-ext_csd.card_type EXT_CSD_CARD_TYPE_SDR_1_2V host-caps2 MMC_CAP2_HS200_1_2V_SDR) - err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_120, 0); + err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_120); if (err card-ext_csd.card_type EXT_CSD_CARD_TYPE_SDR_1_8V host-caps2 MMC_CAP2_HS200_1_8V_SDR) - err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180, 0); + err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180); /* If fails try again during next card power cycle */ if (err) @@ -1221,8 +1221,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, * WARNING: eMMC rules are NOT the same as SD DDR */ if (ddr == MMC_1_2V_DDR_MODE) { - err = mmc_set_signal_voltage(host
[PATCH v5 1/4] mmc: core: Add mmc_power_cycle
Add mmc_power_cycle which can be used to power cycle for instance SD-cards. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com --- drivers/mmc/core/core.c |8 drivers/mmc/core/core.h |1 + 2 files changed, 9 insertions(+) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index aaed768..285f064 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1372,6 +1372,14 @@ void mmc_power_off(struct mmc_host *host) mmc_host_clk_release(host); } +void mmc_power_cycle(struct mmc_host *host) +{ + mmc_power_off(host); + /* Wait at least 1 ms according to SD spec */ + mmc_delay(1); + mmc_power_up(host); +} + /* * Cleanup when the last reference to the bus operator is dropped. */ diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index 3bdafbc..4a90067 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -45,6 +45,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, void mmc_set_timing(struct mmc_host *host, unsigned int timing); void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type); void mmc_power_off(struct mmc_host *host); +void mmc_power_cycle(struct mmc_host *host); static inline void mmc_delay(unsigned int ms) { -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 4/4] mmc: core: Fixup signal voltage switch
When switching SD and SDIO cards from 3.3V to 1.8V signal levels, the clock should be gated for 5 ms during the step. After enabling the clock, the host should wait for at least 1 ms before checking for failure. Failure by the card to switch is indicated by dat[0:3] being pulled low. The host should check for this condition and power-cycle the card if failure is indicated. Add a retry mechanism for the SDIO case. If the voltage switch fails repeatedly, give up and continue the initialization using the original voltage. This patch places a couple of requirements on the host driver: 1) mmc_set_ios with ios.clock = 0 must gate the clock 2) mmc_power_off must actually cut the power to the card 3) The card_busy host_ops member must be implemented if these requirements are not fulfilled, the 1.8V signal voltage switch will still be attempted but may not be successful. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com Signed-off-by: Kevin Liu kl...@marvell.com Signed-off-by: Ulf Hansson ulf.hans...@linaro.org --- drivers/mmc/core/core.c | 83 +-- drivers/mmc/core/sd.c | 21 +--- drivers/mmc/core/sdio.c | 20 ++-- 3 files changed, 107 insertions(+), 17 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 03db641..bf10cf4 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1242,6 +1242,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage) { struct mmc_command cmd = {0}; int err = 0; + u32 clock; BUG_ON(!host); @@ -1249,20 +1250,82 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage) * Send CMD11 only if the request is to switch the card to * 1.8V signalling. */ - if (signal_voltage != MMC_SIGNAL_VOLTAGE_330) { - cmd.opcode = SD_SWITCH_VOLTAGE; - cmd.arg = 0; - cmd.flags = MMC_RSP_R1 | MMC_CMD_AC; + if (signal_voltage == MMC_SIGNAL_VOLTAGE_330) + return __mmc_set_signal_voltage(host, signal_voltage); - err = mmc_wait_for_cmd(host, cmd, 0); - if (err) - return err; + /* +* If we cannot switch voltages, return failure so the caller +* can continue without UHS mode +*/ + if (!host-ops-start_signal_voltage_switch) + return -EPERM; + if (!host-ops-card_busy) + pr_warning(%s: cannot verify signal voltage switch\n, + mmc_hostname(host)); + + cmd.opcode = SD_SWITCH_VOLTAGE; + cmd.arg = 0; + cmd.flags = MMC_RSP_R1 | MMC_CMD_AC; + + err = mmc_wait_for_cmd(host, cmd, 0); + if (err) + return err; - if (!mmc_host_is_spi(host) (cmd.resp[0] R1_ERROR)) - return -EIO; + if (!mmc_host_is_spi(host) (cmd.resp[0] R1_ERROR)) + return -EIO; + + mmc_host_clk_hold(host); + /* +* The card should drive cmd and dat[0:3] low immediately +* after the response of cmd11, but wait 1 ms to be sure +*/ + mmc_delay(1); + if (host-ops-card_busy !host-ops-card_busy(host)) { + err = -EAGAIN; + goto power_cycle; } + /* +* During a signal voltage level switch, the clock must be gated +* for 5 ms according to the SD spec +*/ + clock = host-ios.clock; + host-ios.clock = 0; + mmc_set_ios(host); - return __mmc_set_signal_voltage(host, signal_voltage); + if (__mmc_set_signal_voltage(host, signal_voltage)) { + /* +* Voltages may not have been switched, but we've already +* sent CMD11, so a power cycle is required anyway +*/ + err = -EAGAIN; + goto power_cycle; + } + + /* Keep clock gated for at least 5 ms */ + mmc_delay(5); + host-ios.clock = clock; + mmc_set_ios(host); + + /* Wait for at least 1 ms according to spec */ + mmc_delay(1); + + /* +* Failure to switch is indicated by the card holding +* dat[0:3] low +*/ + if (host-ops-card_busy host-ops-card_busy(host)) + err = -EAGAIN; + +power_cycle: + if (err) { + pr_debug(%s: Signal voltage switch failed, + power cycling card\n, mmc_hostname(host)); + mmc_power_cycle(host); + } + + mmc_host_clk_release(host); + + return err; } /* diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 0cc1b26..f5729b0 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -713,6 +713,14 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr) { int err; u32 max_current; + int retries = 10; + +try_again
[PATCH v5 0/4] mmc: core: Signal voltage switch procedure for UHS mode
This patch series attempts to make the 1.8V signal voltage switch required for UHS mode work according to the SD specification. These patches have been tested with a couple of UHS SD cards, one of which sometimes requires up to five power cycles before it accepts the signal voltage switch. The patches have also been tested with various other SD cards, as well as one SDIO WLAN chip (cw1200) to check for regressions. The patches have also been tested with CONFIG_MMC_CLKGATE. As usual, I'd be very grateful if someone could help me test this patch with an UHS SDIO card and perhaps also a combo card (which does seem to be rare these days?)? This patch series is based on previous RFC/patch: [RFC/PATCH v2] mmc: core: Fixup signal voltage switch and more recently on Kevin Liu's patches: [PATCH v2 1/3] mmc: core: enhance the code for signal voltage setting [PATCH v2 2/3] mmc: core: cycle power the card in voltage switch rather than mmc_sd_get_cid Changelog: v4 - v5 - Reset signal_voltage in struct ios if switch fails v3 - v4 - Added mmc: core: Break out start_signal_voltage_switch, which divides up mmc_set_signal_voltage to avoid a recursive call from mmc_power_up. This removed the third argument (cmd11) of the function - Minor bug fixes v2 - v3 - Minor bug fixes v1 - v2 - Move mmc_power_cycle to the mmc_set_signal_voltage function - Check card_busy before switching voltages - Return -EPERM if signal voltage switch with cmd11 is requested, but no start_signal_voltage_switch is defined. - If we're switching back to 3.3 V, just do the switch without checking card_busy etc v2 - This patch series - Removed the extra argument to the card_busy host_ops function - Added mmc_power_cycle - Some clarifying comments v1 - v2 - Removed reset of signal voltage in mmc_sd_get_cid, since mmc: core: reset signal voltage on power up previous two patches - v1: - Keep calls to mmc_host_clk_hold / mmc_host_clk_release - Add retry-loop / power cycle in sdio.c - Fall back to 3.3 V if the switch repeatedly fails - Add an extra argument to the card_busy host_ops function, which can be used to signal polling use of the function Johan Rudholm (4): mmc: core: Add mmc_power_cycle mmc: core: Add card_busy to host_ops mmc: core: Break out start_signal_voltage_switch mmc: core: Fixup signal voltage switch drivers/mmc/core/core.c | 114 +++--- drivers/mmc/core/core.h |5 +- drivers/mmc/core/mmc.c |8 ++-- drivers/mmc/core/sd.c| 23 +++--- drivers/mmc/core/sdio.c | 23 -- include/linux/mmc/host.h |3 ++ 6 files changed, 144 insertions(+), 32 deletions(-) -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/3] mmc: core: Fixup signal voltage switch
Hi, 2013/1/3 Johan Rudholm johan.rudh...@stericsson.com: snip + err = host-ops-start_signal_voltage_switch(host, host-ios); + if (err) { + /* +* Voltages may not been switched, but we've already +* sent CMD11, so a power cycle is required anyway +*/ + err = -EAGAIN; + goto power_cycle; + } else + yikes, sorry, this else is hanging loose, will be fixed in the next version. Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/4] mmc: core: Add card_busy to host_ops
This host_ops member is used to test if the card is signaling busy by pulling dat[0:3] low. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com --- include/linux/mmc/host.h |3 +++ 1 file changed, 3 insertions(+) diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 61a10c1..3b56ef2 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -131,6 +131,9 @@ struct mmc_host_ops { int (*start_signal_voltage_switch)(struct mmc_host *host, struct mmc_ios *ios); + /* Check if the card is pulling dat[0:3] low */ + int (*card_busy)(struct mmc_host *host); + /* The tuning command opcode value is different for SD and eMMC cards */ int (*execute_tuning)(struct mmc_host *host, u32 opcode); void(*enable_preset_value)(struct mmc_host *host, bool enable); -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/4] mmc: core: Add mmc_power_cycle
Add mmc_power_cycle which can be used to power cycle for instance SD-cards. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com --- drivers/mmc/core/core.c |8 drivers/mmc/core/core.h |1 + 2 files changed, 9 insertions(+) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index aaed768..285f064 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1372,6 +1372,14 @@ void mmc_power_off(struct mmc_host *host) mmc_host_clk_release(host); } +void mmc_power_cycle(struct mmc_host *host) +{ + mmc_power_off(host); + /* Wait at least 1 ms according to SD spec */ + mmc_delay(1); + mmc_power_up(host); +} + /* * Cleanup when the last reference to the bus operator is dropped. */ diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index 3bdafbc..4a90067 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -45,6 +45,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, void mmc_set_timing(struct mmc_host *host, unsigned int timing); void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type); void mmc_power_off(struct mmc_host *host); +void mmc_power_cycle(struct mmc_host *host); static inline void mmc_delay(unsigned int ms) { -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 4/4] mmc: core: Fixup signal voltage switch
When switching SD and SDIO cards from 3.3V to 1.8V signal levels, the clock should be gated for 5 ms during the step. After enabling the clock, the host should wait for at least 1 ms before checking for failure. Failure by the card to switch is indicated by dat[0:3] being pulled low. The host should check for this condition and power-cycle the card if failure is indicated. Add a retry mechanism for the SDIO case. If the voltage switch fails repeatedly, give up and continue the initialization using the original voltage. This patch places a couple of requirements on the host driver: 1) mmc_set_ios with ios.clock = 0 must gate the clock 2) mmc_power_off must actually cut the power to the card 3) The card_busy host_ops member must be implemented if these requirements are not fulfilled, the 1.8V signal voltage switch will still be attempted but may not be successful. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com Signed-off-by: Kevin Liu kl...@marvell.com Signed-off-by: Ulf Hansson ulf.hans...@linaro.org --- drivers/mmc/core/core.c | 83 +-- drivers/mmc/core/sd.c | 21 +--- drivers/mmc/core/sdio.c | 20 ++-- 3 files changed, 107 insertions(+), 17 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 90f8e11..0894a2f 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1238,6 +1238,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage) { struct mmc_command cmd = {0}; int err = 0; + u32 clock; BUG_ON(!host); @@ -1245,20 +1246,82 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage) * Send CMD11 only if the request is to switch the card to * 1.8V signalling. */ - if (signal_voltage != MMC_SIGNAL_VOLTAGE_330) { - cmd.opcode = SD_SWITCH_VOLTAGE; - cmd.arg = 0; - cmd.flags = MMC_RSP_R1 | MMC_CMD_AC; + if (signal_voltage == MMC_SIGNAL_VOLTAGE_330) + return __mmc_set_signal_voltage(host, signal_voltage); - err = mmc_wait_for_cmd(host, cmd, 0); - if (err) - return err; + /* +* If we cannot switch voltages, return failure so the caller +* can continue without UHS mode +*/ + if (!host-ops-start_signal_voltage_switch) + return -EPERM; + if (!host-ops-card_busy) + pr_warning(%s: cannot verify signal voltage switch\n, + mmc_hostname(host)); + + cmd.opcode = SD_SWITCH_VOLTAGE; + cmd.arg = 0; + cmd.flags = MMC_RSP_R1 | MMC_CMD_AC; + + err = mmc_wait_for_cmd(host, cmd, 0); + if (err) + return err; - if (!mmc_host_is_spi(host) (cmd.resp[0] R1_ERROR)) - return -EIO; + if (!mmc_host_is_spi(host) (cmd.resp[0] R1_ERROR)) + return -EIO; + + mmc_host_clk_hold(host); + /* +* The card should drive cmd and dat[0:3] low immediately +* after the response of cmd11, but wait 1 ms to be sure +*/ + mmc_delay(1); + if (host-ops-card_busy !host-ops-card_busy(host)) { + err = -EAGAIN; + goto power_cycle; } + /* +* During a signal voltage level switch, the clock must be gated +* for 5 ms according to the SD spec +*/ + clock = host-ios.clock; + host-ios.clock = 0; + mmc_set_ios(host); - return __mmc_set_signal_voltage(host, signal_voltage); + if (__mmc_set_signal_voltage(host, signal_voltage)) { + /* +* Voltages may not have been switched, but we've already +* sent CMD11, so a power cycle is required anyway +*/ + err = -EAGAIN; + goto power_cycle; + } + + /* Keep clock gated for at least 5 ms */ + mmc_delay(5); + host-ios.clock = clock; + mmc_set_ios(host); + + /* Wait for at least 1 ms according to spec */ + mmc_delay(1); + + /* +* Failure to switch is indicated by the card holding +* dat[0:3] low +*/ + if (host-ops-card_busy host-ops-card_busy(host)) + err = -EAGAIN; + +power_cycle: + if (err) { + pr_debug(%s: Signal voltage switch failed, + power cycling card\n, mmc_hostname(host)); + mmc_power_cycle(host); + } + + mmc_host_clk_release(host); + + return err; } /* diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 0cc1b26..f5729b0 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -713,6 +713,14 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr) { int err; u32 max_current; + int retries = 10; + +try_again
[PATCH v4 0/4] mmc: core: Signal voltage switch procedure for UHS mode
This patch series attempts to make the 1.8V signal voltage switch required for UHS mode work according to the SD specification. These patches have been tested with a couple of UHS SD cards, one of which sometimes requires up to five power cycles before it accepts the signal voltage switch. The patches have also been tested with various other SD cards, as well as one SDIO WLAN chip (cw1200) to check for regressions. The patches have also been tested with CONFIG_MMC_CLKGATE. As usual, I'd be very grateful if someone could help me test this patch with an UHS SDIO card and perhaps also a combo card (which does seem to be rare these days?)? This patch series is based on previous RFC/patch: [RFC/PATCH v2] mmc: core: Fixup signal voltage switch and more recently on Kevin Liu's patches: [PATCH v2 1/3] mmc: core: enhance the code for signal voltage setting [PATCH v2 2/3] mmc: core: cycle power the card in voltage switch rather than mmc_sd_get_cid Changelog: v3 - v4 - Added mmc: core: Break out start_signal_voltage_switch, which divides up mmc_set_signal_voltage to avoid a recursive call from mmc_power_up. This removed the third argument (cmd11) of the function - Minor bug fixes v2 - v3 - Minor bug fixes v1 - v2 - Move mmc_power_cycle to the mmc_set_signal_voltage function - Check card_busy before switching voltages - Return -EPERM if signal voltage switch with cmd11 is requested, but no start_signal_voltage_switch is defined. - If we're switching back to 3.3 V, just do the switch without checking card_busy etc v2 - This patch series - Removed the extra argument to the card_busy host_ops function - Added mmc_power_cycle - Some clarifying comments v1 - v2 - Removed reset of signal voltage in mmc_sd_get_cid, since mmc: core: reset signal voltage on power up previous two patches - v1: - Keep calls to mmc_host_clk_hold / mmc_host_clk_release - Add retry-loop / power cycle in sdio.c - Fall back to 3.3 V if the switch repeatedly fails - Add an extra argument to the card_busy host_ops function, which can be used to signal polling use of the function Johan Rudholm (4): mmc: core: Add mmc_power_cycle mmc: core: Add card_busy to host_ops mmc: core: Break out start_signal_voltage_switch mmc: core: Fixup signal voltage switch drivers/mmc/core/core.c | 110 +++--- drivers/mmc/core/core.h |5 ++- drivers/mmc/core/mmc.c |8 ++-- drivers/mmc/core/sd.c| 23 +++--- drivers/mmc/core/sdio.c | 23 -- include/linux/mmc/host.h |3 ++ 6 files changed, 140 insertions(+), 32 deletions(-) -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 3/4] mmc: core: Break out start_signal_voltage_switch
Allow callers to access the start_signal_voltage_switch host_ops member without going through any cmd11 logic. This is mostly a preparation for the following signal voltage switch patch. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com --- drivers/mmc/core/core.c | 31 +++ drivers/mmc/core/core.h |4 ++-- drivers/mmc/core/mmc.c |8 drivers/mmc/core/sd.c |2 +- drivers/mmc/core/sdio.c |3 +-- 5 files changed, 27 insertions(+), 21 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 285f064..90f8e11 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1219,7 +1219,22 @@ u32 mmc_select_voltage(struct mmc_host *host, u32 ocr) return ocr; } -int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11) +int __mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage) +{ + int err = 0; + + host-ios.signal_voltage = signal_voltage; + if (host-ops-start_signal_voltage_switch) { + mmc_host_clk_hold(host); + err = host-ops-start_signal_voltage_switch(host, host-ios); + mmc_host_clk_release(host); + } + + return err; + +} + +int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage) { struct mmc_command cmd = {0}; int err = 0; @@ -1230,7 +1245,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11 * Send CMD11 only if the request is to switch the card to * 1.8V signalling. */ - if ((signal_voltage != MMC_SIGNAL_VOLTAGE_330) cmd11) { + if (signal_voltage != MMC_SIGNAL_VOLTAGE_330) { cmd.opcode = SD_SWITCH_VOLTAGE; cmd.arg = 0; cmd.flags = MMC_RSP_R1 | MMC_CMD_AC; @@ -1243,15 +1258,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11 return -EIO; } - host-ios.signal_voltage = signal_voltage; - - if (host-ops-start_signal_voltage_switch) { - mmc_host_clk_hold(host); - err = host-ops-start_signal_voltage_switch(host, host-ios); - mmc_host_clk_release(host); - } - - return err; + return __mmc_set_signal_voltage(host, signal_voltage); } /* @@ -1314,7 +1321,7 @@ static void mmc_power_up(struct mmc_host *host) mmc_set_ios(host); /* Set signal voltage to 3.3V */ - mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330, false); + __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330); /* * This delay should be sufficient to allow the power supply diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index 4a90067..f2439d5 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -40,8 +40,8 @@ void mmc_set_ungated(struct mmc_host *host); void mmc_set_bus_mode(struct mmc_host *host, unsigned int mode); void mmc_set_bus_width(struct mmc_host *host, unsigned int width); u32 mmc_select_voltage(struct mmc_host *host, u32 ocr); -int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, - bool cmd11); +int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage); +int __mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage); void mmc_set_timing(struct mmc_host *host, unsigned int timing); void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type); void mmc_power_off(struct mmc_host *host); diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index e6e3911..bd86771 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -769,11 +769,11 @@ static int mmc_select_hs200(struct mmc_card *card) if (card-ext_csd.card_type EXT_CSD_CARD_TYPE_SDR_1_2V host-caps2 MMC_CAP2_HS200_1_2V_SDR) - err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_120, 0); + err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_120); if (err card-ext_csd.card_type EXT_CSD_CARD_TYPE_SDR_1_8V host-caps2 MMC_CAP2_HS200_1_8V_SDR) - err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180, 0); + err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180); /* If fails try again during next card power cycle */ if (err) @@ -1221,8 +1221,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, * WARNING: eMMC rules are NOT the same as SD DDR */ if (ddr == MMC_1_2V_DDR_MODE) { - err = mmc_set_signal_voltage(host, - MMC_SIGNAL_VOLTAGE_120, 0); + err = __mmc_set_signal_voltage(host, + MMC_SIGNAL_VOLTAGE_120); if (err
[PATCH v3 0/3] mmc: core: Signal voltage switch procedure for UHS mode
This patch series attempts to make the 1.8V signal voltage switch required for UHS mode work according to the SD specification. These patches have been tested with a couple of UHS SD cards, one of which sometimes requires up to five power cycles before it accepts the signal voltage switch. The patches have also been tested with various other SD cards, as well as one SDIO WLAN chip (cw1200) to check for regressions. The patches have also been tested with CONFIG_MMC_CLKGATE. As usual, I'd be very grateful if someone could help me test this patch with an UHS SDIO card and perhaps also a combo card (which does seem to be rare these days?)? This patch series is based on previous RFC/patch: [RFC/PATCH v2] mmc: core: Fixup signal voltage switch and more recently on Kevin Liu's patches: [PATCH v2 1/3] mmc: core: enhance the code for signal voltage setting [PATCH v2 2/3] mmc: core: cycle power the card in voltage switch rather than mmc_sd_get_cid Changelog: v2 - v3 - Minor bug fixes v1 - v2 - Move mmc_power_cycle to the mmc_set_signal_voltage function - Check card_busy before switching voltages - Return -EPERM if signal voltage switch with cmd11 is requested, but no start_signal_voltage_switch is defined. - If we're switching back to 3.3 V, just do the switch without checking card_busy etc v2 - This patch series - Removed the extra argument to the card_busy host_ops function - Added mmc_power_cycle - Some clarifying comments v1 - v2 - Removed reset of signal voltage in mmc_sd_get_cid, since mmc: core: reset signal voltage on power up previous two patches - v1: - Keep calls to mmc_host_clk_hold / mmc_host_clk_release - Add retry-loop / power cycle in sdio.c - Fall back to 3.3 V if the switch repeatedly fails - Add an extra argument to the card_busy host_ops function, which can be used to signal polling use of the function Johan Rudholm (3): mmc: core: Add mmc_power_cycle mmc: core: Add card_busy to host_ops mmc: core: Fixup signal voltage switch drivers/mmc/core/core.c | 80 +++--- drivers/mmc/core/core.h |1 + drivers/mmc/core/sd.c| 21 +--- drivers/mmc/core/sdio.c | 20 ++-- include/linux/mmc/host.h |3 ++ 5 files changed, 114 insertions(+), 11 deletions(-) -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/3] mmc: core: Fixup signal voltage switch
When switching SD and SDIO cards from 3.3V to 1.8V signal levels, the clock should be gated for 5 ms during the step. After enabling the clock, the host should wait for at least 1 ms before checking for failure. Failure by the card to switch is indicated by dat[0:3] being pulled low. The host should check for this condition and power-cycle the card if failure is indicated. Add a retry mechanism for the SDIO case. If the voltage switch fails repeatedly, give up and continue the initialization using the original voltage. This patch places a couple of requirements on the host driver: 1) mmc_set_ios with ios.clock = 0 must gate the clock 2) mmc_power_off must actually cut the power to the card 3) The card_busy host_ops member must be implemented if these requirements are not fulfilled, the 1.8V signal voltage switch will still be attempted but may not be successful. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com Signed-off-by: Kevin Liu kl...@marvell.com Signed-off-by: Ulf Hansson ulf.hans...@linaro.org --- drivers/mmc/core/core.c | 72 --- drivers/mmc/core/sd.c | 21 ++ drivers/mmc/core/sdio.c | 20 +++-- 3 files changed, 102 insertions(+), 11 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 285f064..e1fd9c5 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1223,6 +1223,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11 { struct mmc_command cmd = {0}; int err = 0; + u32 clock; BUG_ON(!host); @@ -1231,6 +1232,16 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11 * 1.8V signalling. */ if ((signal_voltage != MMC_SIGNAL_VOLTAGE_330) cmd11) { + /* +* If we cannot switch voltages, return failure so the caller +* can continue without UHS mode +*/ + if (!host-ops-start_signal_voltage_switch) + return -EPERM; + if (!host-ops-card_busy) + pr_warning(%s: cannot verify signal voltage switch\n, + mmc_hostname(host)); + cmd.opcode = SD_SWITCH_VOLTAGE; cmd.arg = 0; cmd.flags = MMC_RSP_R1 | MMC_CMD_AC; @@ -1241,16 +1252,69 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11 if (!mmc_host_is_spi(host) (cmd.resp[0] R1_ERROR)) return -EIO; - } - host-ios.signal_voltage = signal_voltage; + mmc_host_clk_hold(host); + /* +* The card should drive cmd and dat[0:3] low immediately +* after the response of cmd11, but wait 1 ms to be sure +*/ + mmc_delay(1); + if (host-ops-card_busy !host-ops-card_busy(host)) { + err = -EAGAIN; + goto power_cycle; + } + /* +* During a signal voltage level switch, the clock must be gated +* for 5 ms according to the SD spec +*/ + clock = host-ios.clock; + host-ios.clock = 0; + mmc_set_ios(host); + + host-ios.signal_voltage = signal_voltage; + err = host-ops-start_signal_voltage_switch(host, host-ios); + if (err) { + /* +* Voltages may not been switched, but we've already +* sent CMD11, so a power cycle is required anyway +*/ + err = -EAGAIN; + goto power_cycle; + } else + + /* Keep clock gated for at least 5 ms */ + mmc_delay(5); + host-ios.clock = clock; + mmc_set_ios(host); + + /* Wait for at least 1 ms according to spec */ + mmc_delay(1); - if (host-ops-start_signal_voltage_switch) { + /* +* Failure to switch is indicated by the card holding +* dat[0:3] low +*/ + if (host-ops-card_busy host-ops-card_busy(host)) + err = -EAGAIN; + } else if (host-ops-start_signal_voltage_switch) { mmc_host_clk_hold(host); + host-ios.signal_voltage = signal_voltage; err = host-ops-start_signal_voltage_switch(host, host-ios); - mmc_host_clk_release(host); + } else { + return 0; } + +power_cycle: + if (err cmd11) { + pr_debug(%s: Signal voltage switch failed, + power cycling card\n, mmc_hostname(host)); + /* Note that mmc_power_off calls
[PATCH v3 2/3] mmc: core: Add card_busy to host_ops
This host_ops member is used to test if the card is signaling busy by pulling dat[0:3] low. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com --- include/linux/mmc/host.h |3 +++ 1 file changed, 3 insertions(+) diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 61a10c1..3b56ef2 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -131,6 +131,9 @@ struct mmc_host_ops { int (*start_signal_voltage_switch)(struct mmc_host *host, struct mmc_ios *ios); + /* Check if the card is pulling dat[0:3] low */ + int (*card_busy)(struct mmc_host *host); + /* The tuning command opcode value is different for SD and eMMC cards */ int (*execute_tuning)(struct mmc_host *host, u32 opcode); void(*enable_preset_value)(struct mmc_host *host, bool enable); -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/3] mmc: core: Add mmc_power_cycle
Add mmc_power_cycle which can be used to power cycle for instance SD-cards. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com --- drivers/mmc/core/core.c |8 drivers/mmc/core/core.h |1 + 2 files changed, 9 insertions(+) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index aaed768..285f064 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1372,6 +1372,14 @@ void mmc_power_off(struct mmc_host *host) mmc_host_clk_release(host); } +void mmc_power_cycle(struct mmc_host *host) +{ + mmc_power_off(host); + /* Wait at least 1 ms according to SD spec */ + mmc_delay(1); + mmc_power_up(host); +} + /* * Cleanup when the last reference to the bus operator is dropped. */ diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index 3bdafbc..4a90067 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -45,6 +45,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, void mmc_set_timing(struct mmc_host *host, unsigned int timing); void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type); void mmc_power_off(struct mmc_host *host); +void mmc_power_cycle(struct mmc_host *host); static inline void mmc_delay(unsigned int ms) { -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] mmc: core: Fixup signal voltage switch
Hi Kevin, 2012/12/18 Kevin Liu keyuan@gmail.com: 2012/12/18 Johan Rudholm johan.rudh...@stericsson.com: ... + host-ios.signal_voltage = signal_voltage; err = host-ops-start_signal_voltage_switch(host, host-ios); - mmc_host_clk_release(host); + if (err) { + host-ios.clock = clock; + mmc_set_ios(host); Since will power cycle and init card again so no need to set clock here? This is true! We don't need to re-set the clock here. + mmc_host_clk_release(host); mmc_host_clk_hold and mmc_host_clk_release seems not in pairs? Also true, thanks. + return err; } I still prefer to update host-ios.signal_voltage after voltage switch succeed. And current code use host-ios as parameter while only the voltage value will be used in start_signal_voltage_switch. how do you think? I believe Ulf had some arguments against this, I don't remember exactly what they were. Maybe Ulf can elaborate a bit? Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] mmc: core: Fixup signal voltage switch
When switching SD and SDIO cards from 3.3V to 1.8V signal levels, the clock should be gated for 5 ms during the step. After enabling the clock, the host should wait for at least 1 ms before checking for failure. Failure by the card to switch is indicated by dat[0:3] being pulled low. The host should check for this condition and power-cycle the card if failure is indicated. Add a retry mechanism for the SDIO case. If the voltage switch fails repeatedly, give up and continue the initialization using the original voltage. This patch places a couple of requirements on the host driver: 1) mmc_set_ios with ios.clock = 0 must gate the clock 2) mmc_power_off must actually cut the power to the card 3) The card_busy host_ops member must be implemented if these requirements are not fulfilled, the 1.8V signal voltage switch will still be attempted but may not be successful. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com Signed-off-by: Kevin Liu kl...@marvell.com Signed-off-by: Ulf Hansson ulf.hans...@linaro.org --- drivers/mmc/core/core.c | 75 +++ drivers/mmc/core/sd.c | 21 + drivers/mmc/core/sdio.c | 20 +++-- 3 files changed, 104 insertions(+), 12 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 285f064..03a42db 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1223,6 +1223,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11 { struct mmc_command cmd = {0}; int err = 0; + u32 clock; BUG_ON(!host); @@ -1231,6 +1232,16 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11 * 1.8V signalling. */ if ((signal_voltage != MMC_SIGNAL_VOLTAGE_330) cmd11) { + /* +* If we cannot switch voltages, return failure so the caller +* can continue without UHS mode +*/ + if (!host-ops-start_signal_voltage_switch) + return -EPERM; + if (!host-ops-card_busy) + pr_warning(%s: cannot verify signal voltage switch\n, + mmc_hostname(host)); + cmd.opcode = SD_SWITCH_VOLTAGE; cmd.arg = 0; cmd.flags = MMC_RSP_R1 | MMC_CMD_AC; @@ -1241,16 +1252,70 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11 if (!mmc_host_is_spi(host) (cmd.resp[0] R1_ERROR)) return -EIO; - } - - host-ios.signal_voltage = signal_voltage; - if (host-ops-start_signal_voltage_switch) { mmc_host_clk_hold(host); + /* +* The card should drive cmd and dat[0:3] low immediately +* after the response of cmd11, but wait 1 ms to be sure +*/ + mmc_delay(1); + if (host-ops-card_busy !host-ops-card_busy(host)) { + err = -EAGAIN; + goto power_cycle; + } + /* +* During a signal voltage level switch, the clock must be gated +* for 5 ms according to the SD spec +*/ + clock = host-ios.clock; + host-ios.clock = 0; + mmc_set_ios(host); + + host-ios.signal_voltage = signal_voltage; err = host-ops-start_signal_voltage_switch(host, host-ios); - mmc_host_clk_release(host); + if (err) { + host-ios.clock = clock; + mmc_set_ios(host); + /* +* Voltages may not been switched, but we've already +* sent CMD11, so a power cycle is required anyway +*/ + err = -EAGAIN; + goto power_cycle; + } else + + /* Keep clock gated for at least 5 ms */ + mmc_delay(5); + host-ios.clock = clock; + mmc_set_ios(host); + + /* Wait for at least 1 ms according to spec */ + mmc_delay(1); + + /* +* Failure to switch is indicated by the card holding +* dat[0:3] low +*/ + if (host-ops-card_busy host-ops-card_busy(host)) + err = -EAGAIN; + } else if (host-ops-start_signal_voltage_switch) { + host-ios.signal_voltage = signal_voltage; + err = host-ops-start_signal_voltage_switch(host, host-ios); + } else { + return 0; } + +power_cycle: + if (err cmd11) { + pr_debug(%s: Signal voltage switch failed, + power cycling card\n, mmc_hostname(host
[PATCH v2 2/3] mmc: core: Add card_busy to host_ops
This host_ops member is used to test if the card is signaling busy by pulling dat[0:3] low. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com --- include/linux/mmc/host.h |3 +++ 1 file changed, 3 insertions(+) diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 23df21e..e2ed8c5 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -131,6 +131,9 @@ struct mmc_host_ops { int (*start_signal_voltage_switch)(struct mmc_host *host, struct mmc_ios *ios); + /* Check if the card is pulling dat[0:3] low */ + int (*card_busy)(struct mmc_host *host); + /* The tuning command opcode value is different for SD and eMMC cards */ int (*execute_tuning)(struct mmc_host *host, u32 opcode); void(*enable_preset_value)(struct mmc_host *host, bool enable); -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/3] mmc: core: Add mmc_power_cycle
Add mmc_power_cycle which can be used to power cycle for instance SD-cards. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com --- drivers/mmc/core/core.c |8 drivers/mmc/core/core.h |1 + 2 files changed, 9 insertions(+) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index aaed768..285f064 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1372,6 +1372,14 @@ void mmc_power_off(struct mmc_host *host) mmc_host_clk_release(host); } +void mmc_power_cycle(struct mmc_host *host) +{ + mmc_power_off(host); + /* Wait at least 1 ms according to SD spec */ + mmc_delay(1); + mmc_power_up(host); +} + /* * Cleanup when the last reference to the bus operator is dropped. */ diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index 3bdafbc..4a90067 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -45,6 +45,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, void mmc_set_timing(struct mmc_host *host, unsigned int timing); void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type); void mmc_power_off(struct mmc_host *host); +void mmc_power_cycle(struct mmc_host *host); static inline void mmc_delay(unsigned int ms) { -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/3] mmc: core: Signal voltage switch procedure for UHS mode
This patch series attempts to make the 1.8V signal voltage switch required for UHS mode work according to the SD specification. These patches have been tested with a couple of UHS SD cards, one of which sometimes requires up to five power cycles before it accepts the signal voltage switch. The patches have also been tested with various other SD cards, as well as one SDIO WLAN chip (cw1200) to check for regressions. The patches have also been tested with CONFIG_MMC_CLKGATE. As usual, I'd be very grateful if someone could help me test this patch with an UHS SDIO card and perhaps also a combo card (which does seem to be rare these days?)? This patch series is based on previous RFC/patch: [RFC/PATCH v2] mmc: core: Fixup signal voltage switch and more recently on Kevin Liu's patches: [PATCH v2 1/3] mmc: core: enhance the code for signal voltage setting [PATCH v2 2/3] mmc: core: cycle power the card in voltage switch rather than mmc_sd_get_cid Changelog: v1 - v2 - Move mmc_power_cycle to the mmc_set_signal_voltage function - Check card_busy before switching voltages - Return -EPERM if signal voltage switch with cmd11 is requested, but no start_signal_voltage_switch is defined. - If we're switching back to 3.3 V, just do the switch without checking card_busy etc v2 - This patch series - Removed the extra argument to the card_busy host_ops function - Added mmc_power_cycle - Some clarifying comments v1 - v2 - Removed reset of signal voltage in mmc_sd_get_cid, since mmc: core: reset signal voltage on power up previous two patches - v1: - Keep calls to mmc_host_clk_hold / mmc_host_clk_release - Add retry-loop / power cycle in sdio.c - Fall back to 3.3 V if the switch repeatedly fails - Add an extra argument to the card_busy host_ops function, which can be used to signal polling use of the function Johan Rudholm (3): mmc: core: Add mmc_power_cycle mmc: core: Add card_busy to host_ops mmc: core: Fixup signal voltage switch drivers/mmc/core/core.c | 83 +++--- drivers/mmc/core/core.h |1 + drivers/mmc/core/sd.c| 21 +--- drivers/mmc/core/sdio.c | 20 +-- include/linux/mmc/host.h |3 ++ 5 files changed, 116 insertions(+), 12 deletions(-) -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Signal voltage switch procedure for UHS mode
Hi Jackey, no, the cw1200 chip is a SDIO 2.0 device. Kind regards, Johan 2012/12/14 Shen, Jackey jackey.s...@amd.com: Hi Johan, Is the SDIO WLAN chip (cw1200), you mentioned, SDIO 3.0 device with SDIO interface? Thanks, Jackey -Original Message- From: linux-mmc-ow...@vger.kernel.org [mailto:linux-mmc-ow...@vger.kernel.org] On Behalf Of Johan Rudholm Sent: Saturday, December 08, 2012 12:20 AM To: linux-mmc@vger.kernel.org; Chris Ball Cc: Per Forlin; Ulf Hansson; Fredrik Soderstedt; Kevin Liu; Philip Rakity; Daniel Drake; Aaron; Subhash Jadavani; Johan Rudholm Subject: [PATCH 0/3] Signal voltage switch procedure for UHS mode This patch series attempts to make the 1.8V signal voltage switch required for UHS mode work according to the SD specification. These patches have been tested with a couple of UHS SD cards, one of which sometimes requires up to five power cycles before it accepts the signal voltage switch. The patches have also been tested with various other SD cards, as well as one SDIO WLAN chip (cw1200) to check for regressions. The patches have also been tested with CONFIG_MMC_CLKGATE. As usual, I'd be very grateful if someone could help me test this patch with an UHS SDIO card and perhaps also a combo card (which does seem to be rare these days?)? This patch series is based on previous RFC/patch: [RFC/PATCH v2] mmc: core: Fixup signal voltage switch Changelog: v2 - This patch series - Removed the extra argument to the card_busy host_ops function - Added mmc_power_cycle - Some clarifying comments v1 - v2 - Removed reset of signal voltage in mmc_sd_get_cid, since mmc: core: reset signal voltage on power up previous two patches - v1: - Keep calls to mmc_host_clk_hold / mmc_host_clk_release - Add retry-loop / power cycle in sdio.c - Fall back to 3.3 V if the switch repeatedly fails - Add an extra argument to the card_busy host_ops function, which can be used to signal polling use of the function Johan Rudholm (3): mmc: core: Add mmc_power_cycle mmc: core: Add card_busy to host_ops mmc: core: Fixup signal voltage switch drivers/mmc/core/core.c | 43 +++ drivers/mmc/core/core.h |1 + drivers/mmc/core/sd.c| 26 +- drivers/mmc/core/sdio.c | 25 +++-- include/linux/mmc/host.h |3 +++ 5 files changed, 91 insertions(+), 7 deletions(-) -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] mmc: core: Fixup signal voltage switch
Hi Subhash, 2012/12/11 Subhash Jadavani subha...@codeaurora.org: On 12/10/2012 1:51 PM, Johan Rudholm wrote: 2012/12/8 Subhash Jadavani subha...@codeaurora.org: On 12/7/2012 9:49 PM, Johan Rudholm wrote: When switching SD and SDIO cards from 3.3V to 1.8V signal levels, the clock should be gated for 5 ms during the step. After enabling the clock, the host should wait for at least 1 ms before checking for failure. Failure by the card to switch is indicated by dat[0:3] being pulled low. The host should check for this condition and power-cycle the card if failure is indicated. Add a retry mechanism for the SDIO case. If the voltage switch fails repeatedly, give up and continue the initialization using the original voltage. This patch places a couple of requirements on the host driver: 1) mmc_set_ios with ios.clock = 0 must gate the clock 2) mmc_power_off must actually cut the power to the card if these requirements are not fulfilled, the 1.8V signal voltage switch may not be successful. snip err = host-ops-start_signal_voltage_switch(host, host-ios); Shouldn't you fix all the existing host drivers who have already implemented start_signal_voltage_switch host ops? If you don't change them as part of this patch then i afraid UHS functionality would be broken on those platforms. Also, it's not just changing the start_signal_voltage_switch hot op implementation, you may also need to add card_busy() host op implementation for those drivers. This is true, and I did actually make an RFC for the sdhci driver ([RFC] mmc: sdhci: Let core handle UHS switch failure: https://patchwork.kernel.org/patch/1517211/). Daniel Drake tried this code for a problem related to the 1.8V switch (his board could actually not perform the switch to 1.8V even though this capability was announced), and I think this pointed out two areas for further investigation before a proper patch for the sdhci driver can be created: 1) the sdhci driver may not gate the clock when setting ios.clock = 0 and calling mmc_set_ios 2) mmc_power_off may not cut power to the card maybe 2) was only for Daniel's board, I'm not sure, but this needs to be investigated further anyway. Since I don't have any sdhci hardware with UHS support, this must either be done by some other kind soul or it will have to wait until I get the hardware. sdhci is the only driver I'm aware of that's got (mainlined) support for start_signal_voltage_switch, are you thinking of any other driver? You may look at only in tree drivers who have implemented start_singnal_voltage_switch() ops. Our msm_sdcc* driver has also implemented it but it's not in tree driver so i may need to take care of that later once we are synced to kernel version which will have your patch series. Ok, good. Yes, SDHCi seeems to be only one in tree driver implemented the start_singnal_voltage_switch() ops. So you might be good fixing the same. It seems the UHS support in the SDHCI driver is a complex issue, judging from Daniel's experience described above, so I think the best way forward is to get these patches in and then take care of the SDHCI driver (perhaps based on my previous RFC), unless any of the SDHCI guys thinks differently? Naturally, I will be happy to assist in fixing the SDHCI driver. Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] mmc: core: Fixup signal voltage switch
Hi Kevin, 2012/12/14 Kevin Liu keyuan@gmail.com: Just sent a patchset for sdhci based on your patch. Please help to review. Tested with both UHS and non-UHS sd card on sdhci-pxav3 platform. Great! I'll get on it right away. Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] mmc: sdhci: update signal voltage switch code
Hi Kevin, 2012/12/14 Kevin Liu kl...@marvell.com: The protocal related code is moved to core stack. So update the host driver accordingly. Signed-off-by: Kevin Liu kl...@marvell.com --- drivers/mmc/host/sdhci.c | 188 +++--- 1 files changed, 77 insertions(+), 111 deletions(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 6f0bfc0..d19b2aa 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c ... +static int sdhci_card_busy(struct mmc_host *mmc) +{ + struct sdhci_host *host = mmc_priv(mmc); + u32 present_state; + + /* Check whether DAT[3:0] is */ + present_state = sdhci_readl(host, SDHCI_PRESENT_STATE); + + return !(present_state SDHCI_DATA_LVL_MASK); +} + I'm not that familiar with SDHCI, but don't you need some calls to sdhci_runtime_pm_get / put here? Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] mmc: Signal voltage switch procedure for UHS mode (sdhci)
Hi Kevin and Ulf, 2012/12/14 Kevin Liu keyuan@gmail.com: 2012/12/14 Ulf Hansson ulf.hans...@linaro.org: Hi Kevin, Thanks for looking into this for SDHCI. On 14 December 2012 11:53, Kevin Liu kl...@marvell.com wrote: This patchset is based on Johan's below patch series submitted recently: mmc: core: Add mmc_power_cycle mmc: core: Add card_busy to host_ops mmc: core: Fixup signal voltage switch This patchset mainly update sdhci.c to co-work with Johan's above patches. The first two patches are some update for the core stack with Johan's patch. The third patch add the card_busy function and change the voltage_switch function in sdhci driver to work with the new code. Kevin Liu (3): mmc: core: enhance the code for signal voltage setting mmc: core: cycle power the card in voltage switch rather than mmc_sd_get_cid Regarding you patches on the core layer. I think it would make sense to instead of patching on top of Johan patchset, if needed, it is better that you put review comment directly on Johans patches. Would that be possible for you? Sure, it's better to integrate related change into one patch. I can update my patch, then Johan can just integrate my patch if he agree. Or Johan change his patch directly if needed. I'll try to have an updated patchset by Monday next week which integrates your changes and adds your Signed-off-by if that's OK with you. Thank you for testing this stuff! Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] mmc: core: Fixup signal voltage switch
Hi Subhash, 2012/12/8 Subhash Jadavani subha...@codeaurora.org: On 12/7/2012 9:49 PM, Johan Rudholm wrote: When switching SD and SDIO cards from 3.3V to 1.8V signal levels, the clock should be gated for 5 ms during the step. After enabling the clock, the host should wait for at least 1 ms before checking for failure. Failure by the card to switch is indicated by dat[0:3] being pulled low. The host should check for this condition and power-cycle the card if failure is indicated. Add a retry mechanism for the SDIO case. If the voltage switch fails repeatedly, give up and continue the initialization using the original voltage. This patch places a couple of requirements on the host driver: 1) mmc_set_ios with ios.clock = 0 must gate the clock 2) mmc_power_off must actually cut the power to the card if these requirements are not fulfilled, the 1.8V signal voltage switch may not be successful. snip err = host-ops-start_signal_voltage_switch(host, host-ios); Shouldn't you fix all the existing host drivers who have already implemented start_signal_voltage_switch host ops? If you don't change them as part of this patch then i afraid UHS functionality would be broken on those platforms. Also, it's not just changing the start_signal_voltage_switch hot op implementation, you may also need to add card_busy() host op implementation for those drivers. This is true, and I did actually make an RFC for the sdhci driver ([RFC] mmc: sdhci: Let core handle UHS switch failure: https://patchwork.kernel.org/patch/1517211/). Daniel Drake tried this code for a problem related to the 1.8V switch (his board could actually not perform the switch to 1.8V even though this capability was announced), and I think this pointed out two areas for further investigation before a proper patch for the sdhci driver can be created: 1) the sdhci driver may not gate the clock when setting ios.clock = 0 and calling mmc_set_ios 2) mmc_power_off may not cut power to the card maybe 2) was only for Daniel's board, I'm not sure, but this needs to be investigated further anyway. Since I don't have any sdhci hardware with UHS support, this must either be done by some other kind soul or it will have to wait until I get the hardware. sdhci is the only driver I'm aware of that's got (mainlined) support for start_signal_voltage_switch, are you thinking of any other driver? I'm developing for the MMCI driver, but the UHS support code is still pending mainlining. Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 2/3] mmc: sdio: Fix SDIO 3.0 UHS-I initialization sequence
Hi, 2012/12/4 Subhash Jadavani subha...@codeaurora.org: From: Sujit Reddy Thumma sthu...@codeaurora.org According to UHS-I initialization sequence for SDIO 3.0 cards, the host must set bit[24] (S18R) of OCR register during OCR handshake to know whether the SDIO card is capable of doing 1.8V I/O. Reviewed-By: Johan Rudholm johan.rudh...@stericsson.com -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] mmc: core: Add mmc_power_cycle
Add mmc_power_cycle which can be used to power cycle for instance SD-cards. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com --- drivers/mmc/core/core.c |8 drivers/mmc/core/core.h |1 + 2 files changed, 9 insertions(+) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index aaed768..285f064 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1372,6 +1372,14 @@ void mmc_power_off(struct mmc_host *host) mmc_host_clk_release(host); } +void mmc_power_cycle(struct mmc_host *host) +{ + mmc_power_off(host); + /* Wait at least 1 ms according to SD spec */ + mmc_delay(1); + mmc_power_up(host); +} + /* * Cleanup when the last reference to the bus operator is dropped. */ diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index 3bdafbc..4a90067 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -45,6 +45,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, void mmc_set_timing(struct mmc_host *host, unsigned int timing); void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type); void mmc_power_off(struct mmc_host *host); +void mmc_power_cycle(struct mmc_host *host); static inline void mmc_delay(unsigned int ms) { -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: UHS-I voltage switching on OLPC XO-1.75
Hi! 2012/11/18 Daniel Drake d...@laptop.org: On Fri, Nov 2, 2012 at 8:35 AM, Johan Rudholm jrudh...@gmail.com wrote: Good question. I’d guess that mmc_power_off/up does not work as expected here, that the card is not at all power cycled. Before going further on the find a way to quirk it route, there is something else we could look into. According to our hardware engineers, the external SD card power has been always-on until now. It is actually controlled by our embedded controller, separate from the CPU. In a test firmware, I can now control the SD card power via our OLPC EC interface, I call into that from mmc_power_up and mmc_power_down. And, with your hacky patch to make the voltage switch failure detection work, this fixes it: it tries 10 times at 1.8v then falls back to 3.3 successfully. No more problems with the power cycle. Ah, great! Then we know what the issue was at least. Then I guess that this code did not work with your driver: host-ios.clock = 0; mmc_set_ios(host); so with my original patch ([RFC/PATCH,v2] mmc: core: Fixup signal voltage switch), the clock was actually never stopped during the signal voltage switch? I guess this will need some further investigation also. So we have the option of fixing it that way: if we can fix the voltage switch failure detection, we could implement a custom vmmc regulator driver that uses our EC interface to enable and disable the SD power appropriately, solving our ability to power cycle. Being able to power cycle the SD-card might also come in handy in other situations, perhaps if a poor SD-card misbehaves in some way? SD-cards have no reset cord like eMMCs, so being able to power cycle the card feels like a good thing. On the other hand, we may have a good basis to add a quirk, triggered by the device tree, for when the hardware physically does not have 1.8v capabilities. This also seems proper, if we know we can't get 1.8V, then we shouldn't even try for it. In this way the detection will also be faster (no 10 retries). I'm also curious if there is a 3rd option. It seems like in the case of our SoC, the SoC design mandates that the SD card power is separate from the SDHCI interface - requiring either a GPIO or some other mechanism (e.g. OLPC EC) to be able to control it. I'm wondering if this is the same for all sdhci-pxa devices. And the same for all sdhci devices? Maybe the SDHCI specs would help here, but I guess they aren't public. If this is the case, the driver could have another heuristic: if there is no vmmc regulator, there is no way of cutting the card power, therefore we could avoid even trying 1.8v on the basis that we know we can't recover if things go wrong. So the driver could for instance drop the MMC_CAP_UHS_DDR50 cap if there is no way to power cycle the card? I think that sounds reasonable and according to spec, although also a little bit hard since there probably are cards out there that never require a power cycle to perform a proper voltage switch? Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
Hi, 2012/11/5 Huang Changming-R66093 r66...@freescale.com: 2012/11/2 Huang Changming-R66093 r66...@freescale.com: Hi, Johan, When quicks SDHCI_QUIRK_BROKEN_CARD_DETECTION is set, the driver will poll the card status with the command CMD13 periodically, then many interrupts will be generated, which affect the performance. Ok, just to see if I understand the scenario correctly: SDHCI_QUIRK_BROKEN_CARD_DETECTION causes the MMC_CAP_NEEDS_POLL cap to be set, which will cause mmc_rescan to be run at a one second interval. mmc_rescan calls bus_ops-detect (mmc_sd_detect) and this in turn calls _mmc_detect_card_removed, which will do the bus_ops-alive and send CMD13. So in this case, one CMD13 is sent per second, right? Is this the cause of the performance issue? Yes, You are right. Ok, it's a bit hard to understand how this can lead to a noticeable performance impact, but OK. :) A thought: if the host hardware does have a GPIO card detect pin hooked up, would it not be possible to make this pin generate an IRQ whenever a card is inserted or removed? This is what we do in the MMCI driver, for instance (mmci_cd_irq). Our silicones has this pin to do card detect, but some boards don't generate the interrupt when card is inserted or removed. So I have to use the poll mode. Yes, some cases to detect GPIO can't be trusted, so I only just implement this callback in eSDHC-of driver. that is to say, just when the platform support it, this callback can be implement, if not, continue to send the command CMD13. I'm just wondering how this will affect our system, where we use the cd GPIO to generate detect jobs on card insert/remove, but where the cd pin is not 100% synchronized with the electrical connection of the power and cmd line of the SD-card. So if I remember correctly, the cd pin may report that the card has been removed, but there is still an electric connection and the card is alive... I don't see this on our board, when card is inserted or removed, the register field can reflect this state correctly. We see a difference on our boards with this patch, but no functional change (actually a removed card is detected earlier now), so from my perspective: Reviewed-By: Johan Rudholm johan.rudh...@stericsson.com //Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
Hi Jerry, 2012/11/2 Huang Changming-R66093 r66...@freescale.com: Hi, Johan, When quicks SDHCI_QUIRK_BROKEN_CARD_DETECTION is set, the driver will poll the card status with the command CMD13 periodically, then many interrupts will be generated, which affect the performance. Ok, just to see if I understand the scenario correctly: SDHCI_QUIRK_BROKEN_CARD_DETECTION causes the MMC_CAP_NEEDS_POLL cap to be set, which will cause mmc_rescan to be run at a one second interval. mmc_rescan calls bus_ops-detect (mmc_sd_detect) and this in turn calls _mmc_detect_card_removed, which will do the bus_ops-alive and send CMD13. So in this case, one CMD13 is sent per second, right? Is this the cause of the performance issue? A thought: if the host hardware does have a GPIO card detect pin hooked up, would it not be possible to make this pin generate an IRQ whenever a card is inserted or removed? This is what we do in the MMCI driver, for instance (mmci_cd_irq). Yes, some cases to detect GPIO can't be trusted, so I only just implement this callback in eSDHC-of driver. that is to say, just when the platform support it, this callback can be implement, if not, continue to send the command CMD13. I'm just wondering how this will affect our system, where we use the cd GPIO to generate detect jobs on card insert/remove, but where the cd pin is not 100% synchronized with the electrical connection of the power and cmd line of the SD-card. So if I remember correctly, the cd pin may report that the card has been removed, but there is still an electric connection and the card is alive... Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: UHS-I voltage switching on OLPC XO-1.75
Hi Daniel, 2012/10/30 Daniel Drake d...@laptop.org: Hi, Following on from the recent thread [PATCH 2/2] mmc: core: Power cycle card on voltage switch fail I have tried to get a better grip on the problems being faced. Test setup: OLPC XO-1.75 laptop, based on Marvell MMP2 ARM SoC (includes sdhci-pxa interface) 32GB Sandisk Ultra SD card being inserted into external SD slot The SoC apparently has support for 1.8V, but we physically do not have the right power lines wired on the motherboard, so we need to detect the 1.8V failure and go back to 3.3V mode. An initial question, would it be possible to solve this by disabling the UHS support in the host cap, or through a quirk? Maybe this kind of solution is not applicable in this scenario? I wonder this because I guess it will be very hard to know how a card will react if we first tell it that we're going to switch voltages and then don't, since this is out of spec. I tracked down what causes this regression. In order to get the failure detection working again, I have to make the following change in mmc_set_signal_voltage(): if (cmd11) { host-ios.clock = 0; - mmc_set_ios(host); + //mmc_set_ios(host); } (i.e. don't mess with ios before asking sdhci to do the voltage switch) and at the top of sdhci_do_1_8v_signal_voltage_switch: + /* Stop SDCLK */ + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); + clk = ~SDHCI_CLOCK_CARD_EN; + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); (i.e. add the equivalent clock disabling code here) Ok, so this suggests that the SDHCI driver does not stop the clock when we set ios.clock = 0 and call mmc_set_ios? With that change, I now get: mmc2: Signal voltage switch failed, power cycling card (retries = 10) mmc2: error -110 whilst initialising SD card The -110 error comes from mmc_send_app_op_cond(), but its worth noting that mmc_send_if_cond() (called immediately above) silently failed with -110 too. Neither of these failed the first time around before 1.8V was tried. Full patch that I'm testing with: http://dev.laptop.org/~dsd/20121030/mmc.patch So, now the question is: why is the card not responding to if_cond/app_op_cond after being powered down and up again? Is the power down/up sequence OK? Good question. I’d guess that mmc_power_off/up does not work as expected here, that the card is not at all power cycled. But it seems that the power cycle code in sdhci_do_start_signal_voltage_switch works? What if we export a couple of debug functions from sdhci.c which allow us to power cycle and control the clock just like sdhci does, and call these functions from core.c and sd.c? If this works (and the failure is detected properly by the core layer, and 10 retries are made etc), then we know that the problems most likely depend on how mmc_set_ios and mmc_power_off/up work with sdhci. I've attached a patch suggesting this, if you'd like to give it a try (warning: the patch has not been tested). The patch does not do anything about pm_runtime, but perhaps we don't need to worry about this here. I tried inserting the mmc_power_off, mmc_power_up, mmc_send_if_cond sequence into various points in the codepaths being discussed. Measuring success as the card responding to mmc_send_if_cond(0x30080) without error (0x30080 is a known good value from when before 1.8V is tried), I find that this reset sequence works just fine up until the point when CMD11 is run. CMD11 is sent and returns successfully without error, but from this point, running the reset sequence will fail (send_if_cond will fail with -110). Is this kind of test valid? Does this suggest that something is wrong with the host controller hardware? My assumption is that whatever state is modified by CMD11 should be erased here, and of course the hope is that mmc_power_off and mmc_power_up will do a full power cycle anyway. But whatever is happening, it looks like the effects of CMD11 are persisting past the reset sequence and are breaking later communication. I think that the power cycle has simply failed here, I find it hard to believe that CMD11 has persistent effects :) If mmc_power_off/up actually works, then perhaps the 1ms udelay is too small for this case? Kind regards, Johan 0001-Clock-and-power-control.patch Description: Binary data
Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
Hi Jerry, 2012/10/30 r66...@freescale.com: From: Jerry Huang chang-ming.hu...@freescale.com In order to check whether the card has been removed, the function mmc_send_status() will send command CMD13 to card and ask the card to send its status register to sdhc driver, which will generate many interrupts repeatedly and make the system performance bad. Therefore, add callback function get_cd() to check whether the card has been removed when the driver has this callback function. If the card is present, 1 will return, if the card is absent, 0 will return. If the controller will not support this feature, -ENOSYS will return. In what cases is the performance affected by this? I believe this function is called only on some errors and on detect? Also, we've seen cases where the card detect GPIO cannot be trusted to tell wether the card is present or not, for instance in the corner case when an SD-card is removed very slowly... Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: core: debugfs: Add signal_voltage to ios dump
Hi, 2012/10/26 Philip Rakity prak...@nvidia.com: On Oct 26, 2012, at 11:31 AM, Johan Rudholm johan.rudh...@stericsson.com wrote: + default: + str = invalid; would is unknown be a better str ? Maybe... Actually, I just followed the standard in the function, all other strings default to invalid when the definition is missing. So if I change this, then I should also change the rest of the strings in the function. I will do this if we think it's worth the effort, but is it? Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mmc: core: Power cycle card on voltage switch fail
Hi Chris and Daniel, Chris: Thank you for commenting on this patch! Yes, the sdhci power cycle code needs to be removed just as Daniel's patch does, and as I suggested in another patch: [RFC] mmc: sdhci: Let core handle UHS switch failure (https://patchwork.kernel.org/patch/1517211/). Daniel: Thank you for testing the patch! Comments below. 2012/10/24 Daniel Drake d...@laptop.org: On Tue, Oct 23, 2012 at 2:39 PM, Chris Ball c...@laptop.org wrote: Dan, maybe you could see if this patch (there's a 1/2 patch too) solves your UHS problem? I tested [RFC/PATCH] mmc: core: Fixup signal voltage switch https://patchwork.kernel.org/patch/1514691/ This was previously failing on both XO-1.75 (kernel 3.0) and XO-4 (kernel 3.5) - the 1.8V switch would fail, but it would not successfully switch back to 3.3V. Can you enlighten me a bit; what is XO? :) Host controller hardware? Testing on XO-4, it worked fine: the 1.8V failed switch was detected, it came back as 3.3V and everything was fine. Ok, so since you didn't have the card_busy function at this point, the failure was detected by the sdhci code, right? It power-cycles the card, returns -EAGAIN, the new code in mmc_sd_get_cid will try 10 times and then come back to 3.3V. Is this what happened, did you get this print? pr_warning(%s: Skipping voltage switch\n, mmc_hostname(host)); Testing on XO-1.75, it didn't work: it thought that the 1.8V switch was successful so it left it at that, then the card reacted in a very unstable manner (failed/retried reads, huge amount of kernel log spam, etc). This points to that the way of checking if the card is busy or not may not work on XO-1.75? But then it seems strange that the card was semi-usable... So I came up with the attached sdhci patch. That fixes the XO-1.75 case, which now correctly detects the 1.8V switch failure and goes to 3.3V. However, that broke XO-4, which now just does: mmc2: Signal voltage switch failed, power cycling card (retries = 10) mmc2: error -110 whilst initialising SD card (no more time to debug exactly whats happening) Ok, so failure is detected in mmc_set_signal_voltage by your card_busy function, good. Then the card is power cycled, and should be initialized again, but this fails, it's probably mmc_send_app_op_cond which returns -110. Maybe the power cycle fails? So the strange thing here is that on kernel 3.5, without my patch, 1.8V switch fails and fall-back to 3.3V fails. With my patch, 1.8V fails and fall-back to 3.3V succeeds. But, when we move the detection and error-handling to my patch (the upper layer), the fall-back fails. I'm thinking of a couple of things that could have gone wrong here... If you used v2 of the patch, is assumes that the signal voltage is reset to 3.30 V in mmc_power_up, but this was introduced quite recently, in v3.6 (mmc: core: reset signal voltage on power up), but you used v1, right? So then there are at least two other differences: 1) The way the card is power cycled. sdhci toggles the SDHCI_POWER_ON bit in SDHCI_POWER_CONTROL, but my patch calls mmc_power_off / mmc_power_up. Do you know if mmc_power_off / up is a good way to power-cycle the card? 2) The way the clock is stopped. sdhci clears SDHCI_CLOCK_CARD_EN in SDHCI_CLOCK_CONTROL, my patch sets ios.clock = 0 and calls mmc_set_ios. But maybe this is not the issue, since the 1.8V switch fails in all cases. I don't really know how to proceed. Do you have the complete dmesgs from the 3.5 kernel: with my patch and without your patch + with my patch and with your patch? With these I could try to do a deeper analysis. By the way, in your patch 0001-update-sdhci-for-voltage-changing-fixes.txt you don't check if the signal voltage switch succeeds electrically: ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); if (ctrl SDHCI_CTRL_VDD_180) { but maybe we can assume that this will be fine after the delay of 5 ms in mmc_set_signal_voltage? Also in sdhci_card_busy you don't do runtime_pm_get before you check the busy status, but since the busy check returns busy, perhaps this is not the issue here either. All tests with the same 32GB SD storage card. I wonder if this difference in behaviour is related to the difference in kernel versions on each platform (3.0 vs 3.5). I would like to test with 3.5 or newer running on both, but this requires a bit of setup work for the XO-1.75 first (long story). And I'm out of time at this point :( this was a borrowed SD card. Ok, I hope you'll be able to get time and SD-card in the future. :) Btw, what brand was the SD-card? Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc-utils: Support enable/disable of eMMC H/W Reset function
Hi, 2012/10/19 Chris Ball c...@laptop.org: (Note: one-time programmable fuse.) $ mmc hwreset enable /dev/mmcblk0 $ mmc hwreset disable /dev/mmcblk0 --- mmc.c | 10 ++ mmc.h | 4 mmc_cmds.c | 59 +++ mmc_cmds.h | 2 ++ 4 files changed, 75 insertions(+) we've played around with this feature a bit, and if the reset pin is not connected to for instance a GPIO, enabling the reset can give really interesting results :) But if it's connected on the other hand, it works fine... Acked-By: Johan Rudholm johan.rudh...@stericsson.com Kind regards, Johan -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2] mmc: core: Fixup signal voltage switch
When switching SD and SDIO cards from 3.3V to 1.8V signal levels, the clock should be gated for 5 ms during the step. After enabling the clock, the host should wait for at least 1 ms before checking for failure. Failure by the card to switch is indicated by dat[0:3] being pulled low. The host should check for this condition and power-cycle the card if failure is indicated. Add a retry mechanism for the SDIO case. If the voltage switch fails repeatedly, give up and continue the initialization using the original voltage. Signed-off-by: Johan Rudholm johan.rudh...@stericsson.com --- This rfc/patch is based on the following two patches: [PATCH v2 1/2] mmc: core: Proper signal voltage switch [PATCH v2 2/2] mmc: core: Power cycle card on voltage switch fail This patch has been tested with a couple of UHS micro-SD cards, one of which sometimes requires up to five power cycles before it accepts the signal voltage switch. The patch has also been tested with various other micro-SD cards, as well as one SDIO WLAN chip (cw1200) to check for regressions. The patch has been tested with CONFIG_MMC_CLKGATE. I'd be very grateful if someone could help me test this patch with a SDIO card that supports 1.8V and perhaps also a combo card (which does seem to be rare these days?)? Changelog: v1 - v2 - Removed reset of signal voltage in mmc_sd_get_cid, since mmc: core: reset signal voltage on power up previous two patches - v1: - Keep calls to mmc_host_clk_hold / mmc_host_clk_release - Add retry-loop / power cycle in sdio.c - Fall back to 3.3 V if the switch repeatedly fails - Add an extra argument to the card_busy host_ops function, which can be used to signal polling use of the function --- drivers/mmc/core/core.c | 31 ++- drivers/mmc/core/core.h |1 + drivers/mmc/core/sd.c| 29 - drivers/mmc/core/sdio.c | 28 ++-- include/linux/mmc/host.h |6 ++ 5 files changed, 87 insertions(+), 8 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 06c42cf..160760c 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1243,8 +1243,37 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11 host-ios.signal_voltage = signal_voltage; if (host-ops-start_signal_voltage_switch) { + u32 clock; + mmc_host_clk_hold(host); + clock = host-ios.clock; + if (cmd11) { + host-ios.clock = 0; + mmc_set_ios(host); + } + err = host-ops-start_signal_voltage_switch(host, host-ios); + + if (err cmd11) { + host-ios.clock = clock; + mmc_set_ios(host); + } else if (cmd11) { + /* Stop clock for at least 5 ms according to spec */ + mmc_delay(5); + host-ios.clock = clock; + mmc_set_ios(host); + + /* Wait for at least 1 ms according to spec */ + mmc_delay(1); + + /* +* Failure to switch is indicated by the card holding +* dat[0:3] low +*/ + if (host-ops-card_busy + host-ops-card_busy(host, 0)) + err = -EAGAIN; + } mmc_host_clk_release(host); } @@ -1284,7 +1313,7 @@ void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type) * If a host does all the power sequencing itself, ignore the * initial MMC_POWER_UP stage. */ -static void mmc_power_up(struct mmc_host *host) +void mmc_power_up(struct mmc_host *host) { int bit; diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index 3bdafbc..5a5170d 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -45,6 +45,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, void mmc_set_timing(struct mmc_host *host, unsigned int timing); void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type); void mmc_power_off(struct mmc_host *host); +void mmc_power_up(struct mmc_host *host); static inline void mmc_delay(unsigned int ms) { diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 74972c2..0b456de 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -713,6 +713,14 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr) { int err; u32 max_current; + int retries = 10; + +try_again: + if (!retries) { + ocr = ~SD_OCR_S18R; + pr_warning(%s: Skipping voltage switch\n, + mmc_hostname(host
Re: [RESEND PATCH 2/2] mmc: mmci: Switching off HWFC for SDIO depends on MCLK
2012/10/10 Linus Walleij linus.wall...@linaro.org: On Wed, Oct 10, 2012 at 6:03 PM, Ulf Hansson ulf.hans...@stericsson.com wrote: From: Ulf Hansson ulf.hans...@linaro.org For writes, HWFC shall be switched off when transfer size = 8 bytes and when MCLK rate is above 50 MHz. For 50MHz and below it shall be switched off when transfer size 8 bytes. Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Acked-by: Linus Walleij linus.wall...@linaro.org Acked-by: Johan Rudholm johan.rudh...@stericsson.com -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH 1/2] mmc: mmci: Fix incorrect handling of HW flow control for SDIO
2012/10/10 Linus Walleij linus.wall...@linaro.org: On Wed, Oct 10, 2012 at 6:03 PM, Ulf Hansson ulf.hans...@stericsson.com wrote: From: Ulf Hansson ulf.hans...@linaro.org For data writes = 8 bytes, HW flow control was disabled but never re-enabled when the transfer was completed. This meant that a following read request would give buffer overrun errors. Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Looks correct to me: Acked-by: Linus Walleij linus.wall...@linaro.org Acked-by: Johan Rudholm johan.rudh...@stericsson.com -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html