Re: [PATCH] mmc: block: use single block write in retry

[Bin Liu <b-liu@ti.com>]

Hi,

On Wed, Mar 18, 2026 at 10:17:35AM -0500, Bin Liu wrote:
> Hi Shawn,
> 
> On Wed, Mar 18, 2026 at 09:43:10AM +0800, Shawn Lin wrote:
> > 在 2026/03/17 星期二 21:24, Bin Liu 写道:
> > > Hi Shawn,
> > > 
> > > On Tue, Mar 17, 2026 at 08:46:40PM +0800, Shawn Lin wrote:
> > > > 在 2026/03/11 星期三 0:04, Bin Liu 写道:
> > > > > Due to errata i2493[0], multi-block write would still fail in retries.
> > > > > 
> > > > > This patch reuses recovery_mode flag, and switches to single-block
> > > > > write in retry when multi-block write fails. It covers both CQE and
> > > > > non-CQE cases.
> > > > 
> > > > MMC core natively retries single for multi block read. So I assume
> > > > what you are trying to do is make it the same for write path. We didn't
> > > 
> > > Yes I am trying something similar for writ path, but
> > > 
> > > > resort to upper block layer for help for read case in the past, so  I
> > > > think you could still achieve that within the scope of MMC core.
> > > 
> > > unlike the read path solution, I didn't solve the issue within the scope
> > > of MMC core, becaure it would intruduce more code than this patch does,
> > > especially the CQE and non-CQE cases have different failure path. (I
> > > didn't try to figure out how the single block read retry covers both
> > > failure cases...)
> > > 
> > > I feel the solution in this patch is more clever with less code
> > > introduced.
> > > 
> > > > 
> > > > More importantly, what is the failure rate of this issue? If it occurs
> > > 
> > > Very rare. For example, on the EVM, the issue can only be triggered by
> > > writing 3 or more blocks of data pattern 0xFF00.
> > 
> > I manually modified cqhci and sdhci to inject errors for multi-block
> > writes under both CQE and non-CQE modes. The following patch appears to
> > work by testing. Could you please test it?
> > 
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> > index 05ee76cb0a08..ca60991a3305 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -1642,13 +1642,13 @@ static int mmc_blk_cqe_issue_flush(struct mmc_queue
> > *mq, struct request *req)
> >  	return mmc_blk_cqe_start_req(mq->card->host, mrq);
> >  }
> > 
> > -static int mmc_blk_hsq_issue_rw_rq(struct mmc_queue *mq, struct request
> > *req)
> > +static int mmc_blk_hsq_issue_rw_rq(struct mmc_queue *mq, struct request
> > *req, int recovery_mode)
> >  {
> >  	struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
> >  	struct mmc_host *host = mq->card->host;
> >  	int err;
> > 
> > -	mmc_blk_rw_rq_prep(mqrq, mq->card, 0, mq);
> > +	mmc_blk_rw_rq_prep(mqrq, mq->card, recovery_mode, mq);
> >  	mqrq->brq.mrq.done = mmc_blk_hsq_req_done;
> >  	mmc_pre_req(host, &mqrq->brq.mrq);
> > 
> > @@ -1663,13 +1663,23 @@ static int mmc_blk_cqe_issue_rw_rq(struct mmc_queue
> > *mq, struct request *req)
> >  {
> >  	struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
> >  	struct mmc_host *host = mq->card->host;
> > +	int err;
> > 
> > -	if (host->hsq_enabled)
> > -		return mmc_blk_hsq_issue_rw_rq(mq, req);
> > +	if (host->hsq_enabled) {
> > +		err = mmc_blk_hsq_issue_rw_rq(mq, req, 0);
> > +		if (err)
> > +			return mmc_blk_hsq_issue_rw_rq(mq, req, 1);
> > +	}
> > 
> >  	mmc_blk_data_prep(mq, mqrq, 0, NULL, NULL);
> > 
> > -	return mmc_blk_cqe_start_req(mq->card->host, &mqrq->brq.mrq);
> > +	err = mmc_blk_cqe_start_req(mq->card->host, &mqrq->brq.mrq);
> > +	if (err) {
> 
> In the CQE case, when the problem happens due to the errata, this
> function won't return error. The MMC write error is reported in
> mmc_blk_cqe_complete_rq() if you see my original patch.

Which route should we go forward, continue to refine this patch or any
more comments to my original patch?

Thanks,
-Bin.

> 
> -Bin.
> 
> > +		mmc_blk_data_prep(mq, mqrq, 1, NULL, NULL);
> > +		return mmc_blk_cqe_start_req(mq->card->host, &mqrq->brq.mrq);
> > +	}
> > +
> > +	return 0;
> >  }
> > 
> >  static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
> > @@ -1776,24 +1786,26 @@ static int mmc_blk_fix_state(struct mmc_card *card,
> > struct request *req)
> >  	return err;
> >  }
> > 
> > -#define MMC_READ_SINGLE_RETRIES	2
> > +#define MMC_RW_SINGLE_RETRIES	2
> > 
> > -/* Single (native) sector read during recovery */
> > -static void mmc_blk_read_single(struct mmc_queue *mq, struct request *req)
> > +/* Single (native) sector read or write during recovery */
> > +static void mmc_blk_rw_single(struct mmc_queue *mq, struct request *req)
> >  {
> >  	struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
> >  	struct mmc_request *mrq = &mqrq->brq.mrq;
> >  	struct mmc_card *card = mq->card;
> >  	struct mmc_host *host = card->host;
> >  	blk_status_t error = BLK_STS_OK;
> > -	size_t bytes_per_read = queue_physical_block_size(mq->queue);
> > +	size_t bytes_per_rw = queue_physical_block_size(mq->queue);
> > +
> > 
> >  	do {
> >  		u32 status;
> >  		int err;
> >  		int retries = 0;
> > 
> > -		while (retries++ <= MMC_READ_SINGLE_RETRIES) {
> > +		while (retries++ <= MMC_RW_SINGLE_RETRIES) {
> >  			mmc_blk_rw_rq_prep(mqrq, card, 1, mq);
> > 
> >  			mmc_wait_for_req(host, mrq);
> > @@ -1821,13 +1833,13 @@ static void mmc_blk_read_single(struct mmc_queue
> > *mq, struct request *req)
> >  		else
> >  			error = BLK_STS_OK;
> > 
> > -	} while (blk_update_request(req, error, bytes_per_read));
> > +	} while (blk_update_request(req, error, bytes_per_rw));
> > 
> >  	return;
> > 
> >  error_exit:
> >  	mrq->data->bytes_xfered = 0;
> > -	blk_update_request(req, BLK_STS_IOERR, bytes_per_read);
> > +	blk_update_request(req, BLK_STS_IOERR, bytes_per_rw);
> >  	/* Let it try the remaining request again */
> >  	if (mqrq->retries > MMC_MAX_RETRIES - 1)
> >  		mqrq->retries = MMC_MAX_RETRIES - 1;
> > @@ -1969,12 +1981,9 @@ static void mmc_blk_mq_rw_recovery(struct mmc_queue
> > *mq, struct request *req)
> >  		return;
> >  	}
> > 
> > -	if (rq_data_dir(req) == READ && brq->data.blocks >
> > -			queue_physical_block_size(mq->queue) >> 9) {
> > -		/* Read one (native) sector at a time */
> > -		mmc_blk_read_single(mq, req);
> > -		return;
> > -	}
> > +	/* Perform one (native) sector at a time */
> > +	if (brq->data.blocks > queue_physical_block_size(mq->queue) >> 9)
> > +		mmc_blk_rw_single(mq, req);
> >  }
> > 
> >  static inline bool mmc_blk_rq_error(struct mmc_blk_request *brq)
> >