* [PATCH v6 0/2] mmc: core: hw_reset changes @ 2015-01-09 11:08 Johan Rudholm 2015-01-09 11:08 ` [PATCH v6 1/2] mmc: core: refactor the hw_reset routines Johan Rudholm 2015-01-09 11:08 ` [PATCH v6 2/2] mmc: sd: add hw_reset callback Johan Rudholm 0 siblings, 2 replies; 5+ messages in thread From: Johan Rudholm @ 2015-01-09 11:08 UTC (permalink / raw) To: linux-mmc, Chris Ball, Ulf Hansson Cc: Adrian Hunter, Guennadi Liakhovetski, David Lanzendörfer, Jesper Nilsson, Johan Rudholm 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v6 1/2] mmc: core: refactor the hw_reset routines 2015-01-09 11:08 [PATCH v6 0/2] mmc: core: hw_reset changes Johan Rudholm @ 2015-01-09 11:08 ` Johan Rudholm 2015-01-12 12:55 ` Ulf Hansson 2015-01-09 11:08 ` [PATCH v6 2/2] mmc: sd: add hw_reset callback Johan Rudholm 1 sibling, 1 reply; 5+ messages in thread From: Johan Rudholm @ 2015-01-09 11:08 UTC (permalink / raw) To: linux-mmc, Chris Ball, Ulf Hansson Cc: Adrian Hunter, Guennadi Liakhovetski, David Lanzendörfer, Jesper Nilsson, Johan Rudholm 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 <johanru@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/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1795,6 +1795,37 @@ 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_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; + + if (!mmc_can_reset(card)) + return -EOPNOTSUPP; + + mmc_host_clk_hold(host); + mmc_set_clock(host, host->f_init); + + host->ops->hw_reset(host); + + mmc_host_clk_release(host); + + return 0; +} + static const struct mmc_bus_ops mmc_ops = { .remove = mmc_remove, .detect = mmc_detect, @@ -1805,6 +1836,7 @@ static const struct mmc_bus_ops mmc_ops = { .power_restore = mmc_power_restore, .alive = mmc_alive, .shutdown = mmc_shutdown, + .hw_reset = mmc_mmc_hw_reset, }; /* diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h index cb2b040..160448f 100644 --- a/include/linux/mmc/core.h +++ b/include/linux/mmc/core.h @@ -182,7 +182,6 @@ extern int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen); extern int mmc_set_blockcount(struct mmc_card *card, unsigned int blockcount, bool is_rel_write); extern int mmc_hw_reset(struct mmc_host *host); -extern int mmc_hw_reset_check(struct mmc_host *host); extern int mmc_can_reset(struct mmc_card *card); extern void mmc_set_data_timeout(struct mmc_data *, const struct mmc_card *); -- 1.7.2.5 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v6 1/2] mmc: core: refactor the hw_reset routines 2015-01-09 11:08 ` [PATCH v6 1/2] mmc: core: refactor the hw_reset routines Johan Rudholm @ 2015-01-12 12:55 ` Ulf Hansson 2015-01-12 14:37 ` Johan Rudholm 0 siblings, 1 reply; 5+ messages in thread From: Ulf Hansson @ 2015-01-12 12:55 UTC (permalink / raw) To: Johan Rudholm Cc: linux-mmc, Chris Ball, Adrian Hunter, Guennadi Liakhovetski, David Lanzendörfer, Jesper Nilsson, Johan Rudholm On 9 January 2015 at 12:08, Johan Rudholm <johan.rudholm@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 <johanru@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. > -} > - > -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 *); ->Please rename to reset() > }; > > 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/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -1795,6 +1795,37 @@ 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_mmc_hw_reset(struct mmc_host *host) Please rename to mmc_reset() > +{ > + struct mmc_card *card = host->card; > + > + 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); > + > + mmc_host_clk_release(host); > + > + return 0; > +} > + > static const struct mmc_bus_ops mmc_ops = { > .remove = mmc_remove, > .detect = mmc_detect, > @@ -1805,6 +1836,7 @@ static const struct mmc_bus_ops mmc_ops = { > .power_restore = mmc_power_restore, > .alive = mmc_alive, > .shutdown = mmc_shutdown, > + .hw_reset = mmc_mmc_hw_reset, > }; > > /* > diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h > index cb2b040..160448f 100644 > --- a/include/linux/mmc/core.h > +++ b/include/linux/mmc/core.h > @@ -182,7 +182,6 @@ extern int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen); > extern int mmc_set_blockcount(struct mmc_card *card, unsigned int blockcount, > bool is_rel_write); > extern int mmc_hw_reset(struct mmc_host *host); > -extern int mmc_hw_reset_check(struct mmc_host *host); > extern int mmc_can_reset(struct mmc_card *card); > > extern void mmc_set_data_timeout(struct mmc_data *, const struct mmc_card *); > -- > 1.7.2.5 > I am overall very happy with this approach. Nice work Johan! Kind regards Uffe ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v6 1/2] mmc: core: refactor the hw_reset routines 2015-01-12 12:55 ` Ulf Hansson @ 2015-01-12 14:37 ` Johan Rudholm 0 siblings, 0 replies; 5+ messages in thread From: Johan Rudholm @ 2015-01-12 14:37 UTC (permalink / raw) To: Ulf Hansson Cc: Johan Rudholm, linux-mmc, Chris Ball, Adrian Hunter, Guennadi Liakhovetski, David Lanzendörfer, Jesper Nilsson 2015-01-12 13:55 GMT+01:00 Ulf Hansson <ulf.hansson@linaro.org>: > On 9 January 2015 at 12:08, Johan Rudholm <johan.rudholm@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 <johanru@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 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 *); > > ->Please rename to reset() Ok. >> }; >> >> 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/mmc/core/mmc.c >> +++ b/drivers/mmc/core/mmc.c >> @@ -1795,6 +1795,37 @@ 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_mmc_hw_reset(struct mmc_host *host) > > Please rename to mmc_reset() Ok. >> +{ >> + struct mmc_card *card = host->card; >> + >> + 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); >> + >> + mmc_host_clk_release(host); >> + >> + return 0; >> +} >> + >> static const struct mmc_bus_ops mmc_ops = { >> .remove = mmc_remove, >> .detect = mmc_detect, >> @@ -1805,6 +1836,7 @@ static const struct mmc_bus_ops mmc_ops = { >> .power_restore = mmc_power_restore, >> .alive = mmc_alive, >> .shutdown = mmc_shutdown, >> + .hw_reset = mmc_mmc_hw_reset, >> }; >> >> /* >> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h >> index cb2b040..160448f 100644 >> --- a/include/linux/mmc/core.h >> +++ b/include/linux/mmc/core.h >> @@ -182,7 +182,6 @@ extern int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen); >> extern int mmc_set_blockcount(struct mmc_card *card, unsigned int blockcount, >> bool is_rel_write); >> extern int mmc_hw_reset(struct mmc_host *host); >> -extern int mmc_hw_reset_check(struct mmc_host *host); >> extern int mmc_can_reset(struct mmc_card *card); >> >> extern void mmc_set_data_timeout(struct mmc_data *, const struct mmc_card *); >> -- >> 1.7.2.5 >> > > I am overall very happy with this approach. Nice work Johan! Thank you, and thank you for the detailed review work! //Johan ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v6 2/2] mmc: sd: add hw_reset callback 2015-01-09 11:08 [PATCH v6 0/2] mmc: core: hw_reset changes Johan Rudholm 2015-01-09 11:08 ` [PATCH v6 1/2] mmc: core: refactor the hw_reset routines Johan Rudholm @ 2015-01-09 11:08 ` Johan Rudholm 1 sibling, 0 replies; 5+ messages in thread From: Johan Rudholm @ 2015-01-09 11:08 UTC (permalink / raw) To: linux-mmc, Chris Ball, Ulf Hansson Cc: Adrian Hunter, Guennadi Liakhovetski, David Lanzendörfer, Jesper Nilsson, Johan Rudholm 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 <johanru@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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-01-12 14:38 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-01-09 11:08 [PATCH v6 0/2] mmc: core: hw_reset changes Johan Rudholm 2015-01-09 11:08 ` [PATCH v6 1/2] mmc: core: refactor the hw_reset routines Johan Rudholm 2015-01-12 12:55 ` Ulf Hansson 2015-01-12 14:37 ` Johan Rudholm 2015-01-09 11:08 ` [PATCH v6 2/2] mmc: sd: add hw_reset callback Johan Rudholm
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox