mmc: block: Remove error check of hw_reset on reset
PATCH v4 1/2 mmc: block: Remove error check of hw_reset on reset
Before switching back to the right partition in mmc_blk_reset there used to be a check if hw_reset was even supported. This return value was removed, so there is no reason to check. Furthermore ensure part_curr is not falsely set to a valid value on reset or partition switch error.
As part of this change the code paths of mmc_blk_reset calls were checked to ensure no commands are issued after a failed mmc_blk_reset directly without going through the block layer.
---
-v4: Only partition switch if necessary and fix one mmc_blk_reset call -v3: Ensure invalid part_curr on error
-v2: Do not attempt to switch partitions if reset failed
drivers/mmc/core/block.c | 42 ++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 17 deletions(-)
drivers/mmc/core/block.c +25 -17
@@ -134,6 +134,7 @@ struct mmc_blk_data {* track of the current selected device partition. */ unsigned int part_curr; +#define MMC_BLK_PART_INVALID UINT_MAX/* Unknown partition active */ int area_type;/* debugfs files (only in main mmc_blk_data) */
@@ -987,33 +988,39 @@ static unsigned int mmc_blk_data_timeout_ms(struct mmc_host *host,return ms; } +/* +* Attempts to reset the card and get back to the requested partition. +* Therefore any error here must result in cancelling the block layer +* request, it must not be reattempted without going through the mmc_blk +* partition sanity checks. +*/ static int mmc_blk_reset (struct mmc_blk_data *md,struct mmc_host *host,int type) {int err; +struct mmc_blk_data *main_md =dev_get_drvdata (&host->card->dev);if (md->reset_done & type)return -EEXIST ; md->reset_done |= type; err =mmc_hw_reset (host->card); +/* +* A successful reset will leave the card in the main partition, but +* upon failure it might not be, so set it to MMC_BLK_PART_INVALID +* in that case. +*/ + main_md->part_curr = err ?MMC_BLK_PART_INVALID : main_md->part_type; +if (err) +return err;/* Ensure we switch back to the correct partition */ -if (err) { -struct mmc_blk_data *main_md = -dev_get_drvdata (&host->card->dev); -int part_err; - - main_md->part_curr = main_md->part_type; - part_err =mmc_blk_part_switch (host->card, md->part_type); -if (part_err) { -/* -* We have failed to get back into the correct -* partition, so we need to abort the whole request. -*/ -return -ENODEV ; - } - } -return err; +if (mmc_blk_part_switch (host->card, md->part_type)) +/* +* We have failed to get back into the correct +* partition, so we need to abort the whole request. +*/ +return -ENODEV ; +return 0 ; }static inline void mmc_blk_reset_success (struct mmc_blk_data *md,int type)
@@ -1868,7 +1875,8 @@ static void mmc_blk_mq_rw_recovery(struct mmc_queue *mq, struct request *req)/* Reset before last retry */ if (mqrq->retries +1 ==MMC_MAX_RETRIES ) -mmc_blk_reset (md, card->host, type); +if (mmc_blk_reset (md, card->host, type)) +return ;/* Command errors fail fast, so use all MMC_MAX_RETRIES */ if (brq->sbc.error || brq->cmd.error)
Signature · 5 lines
2.37.3 Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz Managing Director: Dr. Jan Peter Berns. Commercial register of local courts: Freiburg HRB381782
Quoted 6 lines · Christian Löhle
Before switching back to the right partition in
mmc_blk_resetthere used to be a check ifhw_resetwas even supported. This return value was removed, so there is no reason to check. Furthermore ensurepart_curris not falsely set to a valid value on reset or partition switch error.
Quoted 8 lines
As part of this change the code paths of
mmc_blk_resetcalls were checked to ensure no commands are issued after a failedmmc_blk_resetdirectly without going through the block layer.Fixes fefdd3c91e0a ("mmc: core: Drop superfluous validations in mmc_hw|sw_reset()")Signed-off-by Christian Loehle <[email protected]>
One minor comment, otherwise:
Quoted 77 lines
---
(quoted diff) +0
-3-v4: Only partition switch if necessary and fix one mmc_blk_reset call -v3: Ensure invalid part_curr on error -v2: Do not attempt to switch partitions if reset failed drivers/mmc/core/block.c | 42 ++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 17 deletions(-)drivers/mmc/core/block.c +23
-16@@ -134,6 +134,7 @@ struct mmc_blk_data {* track of the current selected device partition. */ unsigned int part_curr; +#define MMC_BLK_PART_INVALID UINT_MAX/* Unknown partition active */ int area_type;/* debugfs files (only in main mmc_blk_data) */ @@ -987,33 +988,39 @@ static unsigned int mmc_blk_data_timeout_ms(struct mmc_host *host,return ms; } +/* +* Attempts to reset the card and get back to the requested partition. +* Therefore any error here must result in cancelling the block layer +* request, it must not be reattempted without going through the mmc_blk +* partition sanity checks. +*/ static int mmc_blk_reset (struct mmc_blk_data *md,struct mmc_host *host,int type) {int err; +struct mmc_blk_data *main_md =dev_get_drvdata (&host->card->dev);if (md->reset_done & type)return -EEXIST ; md->reset_done |= type; err =mmc_hw_reset (host->card); +/* +* A successful reset will leave the card in the main partition, but +* upon failure it might not be, so set it to MMC_BLK_PART_INVALID +* in that case. +*/ + main_md->part_curr = err ?MMC_BLK_PART_INVALID : main_md->part_type; +if (err) +return err;/* Ensure we switch back to the correct partition */ -if (err) { -struct mmc_blk_data *main_md = -dev_get_drvdata (&host->card->dev); -int part_err; - - main_md->part_curr = main_md->part_type; - part_err =mmc_blk_part_switch (host->card, md->part_type); -if (part_err) { -/* -* We have failed to get back into the correct -* partition, so we need to abort the whole request. -*/ -return -ENODEV ; - } - } -return err; +if (mmc_blk_part_switch (host->card, md->part_type)) +/* +* We have failed to get back into the correct +* partition, so we need to abort the whole request. +*/ +return -ENODEV ; +return 0 ; }static inline void mmc_blk_reset_success (struct mmc_blk_data *md,int type)
drivers/mmc/core/block.c static void mmc_blk_mq_rw_recovery(struct mmc_queue *mq, struct request *req) · line 1878
Quoted 5 lines
drivers/mmc/core/block.c +1
-1@@ -1868,7 +1875,8 @@ static void mmc_blk_mq_rw_recovery(struct mmc_queue *mq, struct request *req)/* Reset before last retry */ if (mqrq->retries +1 ==MMC_MAX_RETRIES ) -mmc_blk_reset (md, card->host, type); +if (mmc_blk_reset (md, card->host, type))
Kernel style is to avoid nested "if" statements, so it should be like this:
if (mqrq->retries + 1 == MMC_MAX_RETRIES &&
mmc_blk_reset(md, card->host, type) return;
Quoted 4 lines
drivers/mmc/core/block.c +1
-0+return ;/* Command errors fail fast, so use all MMC_MAX_RETRIES */ if (brq->sbc.error || brq->cmd.error)