mmc: core: hw_reset changes
v6 cover 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(-)
Signature · 1 line
1.7.2.5
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.
--- 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(-)
drivers/mmc/card/mmc_test.c +7 -11
@@ -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[] = {
drivers/mmc/core/core.c +24 -46
91 lines not on this page · raw message
drivers/mmc/core/core.h +1 -0
@@ -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);
drivers/mmc/core/mmc.c +32 -0
@@ -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, };/*
include/linux/mmc/core.h +0 -1
@@ -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 *);
Signature · 1 line
1.7.2.5
Re: [PATCH v6 1/2] mmc: core: refactor the hw_reset routines
Quoted 113 lines · Johan Rudholm
Move the (e)MMC specific
hw_resetcode fromcore.cintommc.cCall the code from the newbus_opsmemberhw_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(), leavingmmc_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 <[email protected]>--- 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(-)drivers/mmc/card/mmc_test.c +7
-11@@ -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[] = {drivers/mmc/core/core.c +15
-32@@ -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 ; - } - }
drivers/mmc/core/core.c static void mmc_hw_reset_for_init(struct mmc_host *host) · line 2322
Quoted 10 lines
drivers/mmc/core/core.c +3
-3+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.
Quoted 28 lines
drivers/mmc/core/core.c +6
-11-} - -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;drivers/mmc/core/core.h +0
-0
drivers/mmc/core/core.h struct mmc_bus_ops { · line 30
Quoted 4 lines
drivers/mmc/core/core.h +1
-0@@ -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()
Quoted 13 lines
drivers/mmc/core/core.h +0
-0};void mmc_attach_bus (struct mmc_host *host,const struct mmc_bus_ops *ops);drivers/mmc/core/mmc.c +2
-0@@ -1795,6 +1795,37 @@ static int mmc_power_restore(struct mmc_host *host)return ret; } +int mmc_can_reset (struct mmc_card *card) +{
drivers/mmc/core/mmc.c static int mmc_power_restore(struct mmc_host *host) · line 1809
Quoted 10 lines
drivers/mmc/core/mmc.c +10
-0+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()
Quoted 43 lines
drivers/mmc/core/mmc.c +20
-0+{ +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, };/* include/linux/mmc/core.h +0
-2@@ -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 *); --
Quoted 2 lines
1.7.2.5
I am overall very happy with this approach. Nice work Johan!
Kind regards
Uffe
Re: [PATCH v6 1/2] mmc: core: refactor the hw_reset routines
2015-01-12 13:55 GMT+01:00 Ulf Hansson <[email protected]>:
Quoted 128 lines · Johan Rudholm
Quoted 113 lines
Move the (e)MMC specific
hw_resetcode fromcore.cintommc.cCall the code from the newbus_opsmemberhw_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(), leavingmmc_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 <[email protected]>--- 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(-)drivers/mmc/card/mmc_test.c +7
-11@@ -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[] = {drivers/mmc/core/core.c +15
-32@@ -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 ; - } - }drivers/mmc/core/core.c static void mmc_hw_reset_for_init(struct mmc_host *host) · line 2322
Quoted 10 lines
drivers/mmc/core/core.c +3
-3+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.
Quoted 3 lines
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.
Quoted 26 lines
Quoted 26 lines
drivers/mmc/core/core.c +6
-11-} - -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;drivers/mmc/core/core.h +0
-1--- a/drivers/mmc/core/core.h
Quoted 8 lines
drivers/mmc/core/core.h struct mmc_bus_ops { · line 30
Quoted 6 lines
+++
b/drivers/mmc/core/core.hdrivers/mmc/core/core.h +1
-0@@ -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.
Quoted 11 lines
Quoted 11 lines
drivers/mmc/core/core.h +0
-0};void mmc_attach_bus (struct mmc_host *host,const struct mmc_bus_ops *ops);drivers/mmc/core/mmc.c +0
-0@@ -1795,6 +1795,37 @@ static int mmc_power_restore(struct mmc_host *host)return ret; }
Quoted 14 lines
drivers/mmc/core/mmc.c static int mmc_power_restore(struct mmc_host *host) · line 1809
Quoted 12 lines
drivers/mmc/core/mmc.c +12
-0+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.
Quoted 41 lines
Quoted 41 lines
drivers/mmc/core/mmc.c +20
-0+{ +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, };/* include/linux/mmc/core.h +0
-1@@ -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);
Quoted 6 lines
Quoted 4 lines
include/linux/mmc/core.h +0
-1extern 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
PATCH v6 2/2 mmc: sd: add hw_reset callback
In reply to Johan RudholmEnable 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.
--- drivers/mmc/core/sd.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
drivers/mmc/core/sd.c +7 -0
@@ -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, };/*
Signature · 1 line
1.7.2.5