On Tue, 11 Oct 2022 at 12:15, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>
> On 10/10/22 17:03, Christian Löhle wrote:
> > 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.
> >
> > Fixes: fefdd3c91e0a ("mmc: core: Drop superfluous validations in mmc_hw|sw_reset()")
> > Cc: stable@xxxxxxxxxxxxxxx
> >
> > Signed-off-by: Christian Loehle <cloehle@xxxxxxxxxxxxxx>
> > ---
> > -v3: Ensure invalid part_curr on error
> > -v2: Do not attempt to switch partitions if reset failed
> >
> > drivers/mmc/core/block.c | 29 ++++++++++++++---------------
> > 1 file changed, 14 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> > index ce89611a136e..45a44edcc31a 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -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) */
> > @@ -991,29 +992,27 @@ 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);
> > + int part_err;
> >
> > if (md->reset_done & type)
> > return -EEXIST;
> >
> > md->reset_done |= type;
> > + main_md->part_curr = MMC_BLK_PART_INVALID;
>
> This forces a partition switch even if it is not necessary.
Right, very good point!
>
> > err = mmc_hw_reset(host->card);
>
> This would be better:
>
> /*
> * 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;
>
Yes! And the comment is valuable too!
[...]
Kind regards
Uffe
