public inbox for linux-block@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mmc: block: use single block write in retry
@ 2026-03-10 16:04 Bin Liu
  2026-03-16 15:01 ` Ulf Hansson
                   ` (2 more replies)
  0 siblings, 3 replies; 30+ messages in thread
From: Bin Liu @ 2026-03-10 16:04 UTC (permalink / raw)
  To: linux-block, linux-mmc; +Cc: ulf.hansson, axboe

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.

[0] https://www.ti.com/lit/pdf/sprz582
Signed-off-by: Bin Liu <b-liu@ti.com>
---
 block/blk-mq-debugfs.c   |  1 +
 drivers/mmc/core/block.c | 12 ++++++++++--
 include/linux/blk-mq.h   |  3 +++
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 28167c9baa55..5f97d07fa3c8 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -243,6 +243,7 @@ static const char *const rqf_name[] = {
 	RQF_NAME(ZONE_WRITE_PLUGGING),
 	RQF_NAME(TIMED_OUT),
 	RQF_NAME(RESV),
+	RQF_NAME(XFER_SINGLE_BLK),
 };
 #undef RQF_NAME
 
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 05ee76cb0a08..00016584d70d 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1401,6 +1401,9 @@ static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
 		    rq_data_dir(req) == WRITE &&
 		    (md->flags & MMC_BLK_REL_WR);
 
+	if (req->rq_flags & RQF_XFER_SINGLE_BLK)
+		recovery_mode = 1;
+
 	memset(brq, 0, sizeof(struct mmc_blk_request));
 
 	mmc_crypto_prepare_req(mqrq);
@@ -1540,10 +1543,13 @@ static void mmc_blk_cqe_complete_rq(struct mmc_queue *mq, struct request *req)
 		err = 0;
 
 	if (err) {
-		if (mqrq->retries++ < MMC_CQE_RETRIES)
+		if (mqrq->retries++ < MMC_CQE_RETRIES) {
+			if (rq_data_dir(req) == WRITE)
+				req->rq_flags |= RQF_XFER_SINGLE_BLK;
 			blk_mq_requeue_request(req, true);
-		else
+		} else {
 			blk_mq_end_request(req, BLK_STS_IOERR);
+		}
 	} else if (mrq->data) {
 		if (blk_update_request(req, BLK_STS_OK, mrq->data->bytes_xfered))
 			blk_mq_requeue_request(req, true);
@@ -2085,6 +2091,8 @@ static void mmc_blk_mq_complete_rq(struct mmc_queue *mq, struct request *req)
 	} else if (!blk_rq_bytes(req)) {
 		__blk_mq_end_request(req, BLK_STS_IOERR);
 	} else if (mqrq->retries++ < MMC_MAX_RETRIES) {
+		if (rq_data_dir(req) == WRITE)
+			req->rq_flags |= RQF_XFER_SINGLE_BLK;
 		blk_mq_requeue_request(req, true);
 	} else {
 		if (mmc_card_removed(mq->card))
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 18a2388ba581..06b6e1c4fca3 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -64,6 +64,7 @@ enum rqf_flags {
 	/* ->timeout has been called, don't expire again */
 	__RQF_TIMED_OUT,
 	__RQF_RESV,
+	__RQF_XFER_SINGLE_BLK,
 	__RQF_BITS
 };
 
@@ -85,6 +86,8 @@ enum rqf_flags {
 			((__force req_flags_t)(1 << __RQF_ZONE_WRITE_PLUGGING))
 #define RQF_TIMED_OUT		((__force req_flags_t)(1 << __RQF_TIMED_OUT))
 #define RQF_RESV		((__force req_flags_t)(1 << __RQF_RESV))
+#define RQF_XFER_SINGLE_BLK	\
+			((__force req_flags_t)(1 << __RQF_XFER_SINGLE_BLK))
 
 /* flags that prevent us from merging requests: */
 #define RQF_NOMERGE_FLAGS \
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH] mmc: block: use single block write in retry
  2026-03-10 16:04 [PATCH] mmc: block: use single block write in retry Bin Liu
@ 2026-03-16 15:01 ` Ulf Hansson
  2026-03-16 16:22   ` Bin Liu
  2026-03-17 12:46 ` Shawn Lin
  2026-03-24 14:34 ` [PATCH v2 0/2] " Bin Liu
  2 siblings, 1 reply; 30+ messages in thread
From: Ulf Hansson @ 2026-03-16 15:01 UTC (permalink / raw)
  To: Bin Liu; +Cc: linux-block, linux-mmc, axboe

On Tue, 10 Mar 2026 at 17:04, Bin Liu <b-liu@ti.com> wrote:
>
> 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.
>
> [0] https://www.ti.com/lit/pdf/sprz582

It's great that a link like the above can be provided, still it would
be nice to explain a bit more about the problem in the commit message
too. Can you please do that?

> Signed-off-by: Bin Liu <b-liu@ti.com>
> ---
>  block/blk-mq-debugfs.c   |  1 +
>  drivers/mmc/core/block.c | 12 ++++++++++--
>  include/linux/blk-mq.h   |  3 +++
>  3 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> index 28167c9baa55..5f97d07fa3c8 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
> @@ -243,6 +243,7 @@ static const char *const rqf_name[] = {
>         RQF_NAME(ZONE_WRITE_PLUGGING),
>         RQF_NAME(TIMED_OUT),
>         RQF_NAME(RESV),
> +       RQF_NAME(XFER_SINGLE_BLK),
>  };
>  #undef RQF_NAME
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 05ee76cb0a08..00016584d70d 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -1401,6 +1401,9 @@ static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
>                     rq_data_dir(req) == WRITE &&
>                     (md->flags & MMC_BLK_REL_WR);
>
> +       if (req->rq_flags & RQF_XFER_SINGLE_BLK)
> +               recovery_mode = 1;
> +

I don't get this, sorry.

Under what circumstances do we need to fall back to using "single
block"? Or is this just for debugging?

>         memset(brq, 0, sizeof(struct mmc_blk_request));
>
>         mmc_crypto_prepare_req(mqrq);
> @@ -1540,10 +1543,13 @@ static void mmc_blk_cqe_complete_rq(struct mmc_queue *mq, struct request *req)
>                 err = 0;
>
>         if (err) {
> -               if (mqrq->retries++ < MMC_CQE_RETRIES)
> +               if (mqrq->retries++ < MMC_CQE_RETRIES) {
> +                       if (rq_data_dir(req) == WRITE)
> +                               req->rq_flags |= RQF_XFER_SINGLE_BLK;
>                         blk_mq_requeue_request(req, true);
> -               else
> +               } else {
>                         blk_mq_end_request(req, BLK_STS_IOERR);
> +               }
>         } else if (mrq->data) {
>                 if (blk_update_request(req, BLK_STS_OK, mrq->data->bytes_xfered))
>                         blk_mq_requeue_request(req, true);
> @@ -2085,6 +2091,8 @@ static void mmc_blk_mq_complete_rq(struct mmc_queue *mq, struct request *req)
>         } else if (!blk_rq_bytes(req)) {
>                 __blk_mq_end_request(req, BLK_STS_IOERR);
>         } else if (mqrq->retries++ < MMC_MAX_RETRIES) {
> +               if (rq_data_dir(req) == WRITE)
> +                       req->rq_flags |= RQF_XFER_SINGLE_BLK;
>                 blk_mq_requeue_request(req, true);
>         } else {
>                 if (mmc_card_removed(mq->card))
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index 18a2388ba581..06b6e1c4fca3 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -64,6 +64,7 @@ enum rqf_flags {
>         /* ->timeout has been called, don't expire again */
>         __RQF_TIMED_OUT,
>         __RQF_RESV,
> +       __RQF_XFER_SINGLE_BLK,
>         __RQF_BITS
>  };
>
> @@ -85,6 +86,8 @@ enum rqf_flags {
>                         ((__force req_flags_t)(1 << __RQF_ZONE_WRITE_PLUGGING))
>  #define RQF_TIMED_OUT          ((__force req_flags_t)(1 << __RQF_TIMED_OUT))
>  #define RQF_RESV               ((__force req_flags_t)(1 << __RQF_RESV))
> +#define RQF_XFER_SINGLE_BLK    \
> +                       ((__force req_flags_t)(1 << __RQF_XFER_SINGLE_BLK))
>
>  /* flags that prevent us from merging requests: */
>  #define RQF_NOMERGE_FLAGS \
> --
> 2.34.1
>

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] mmc: block: use single block write in retry
  2026-03-16 15:01 ` Ulf Hansson
@ 2026-03-16 16:22   ` Bin Liu
  2026-03-24 12:44     ` Ulf Hansson
  0 siblings, 1 reply; 30+ messages in thread
From: Bin Liu @ 2026-03-16 16:22 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: linux-block, linux-mmc, axboe

Hi Ulf,

On Mon, Mar 16, 2026 at 04:01:38PM +0100, Ulf Hansson wrote:
> On Tue, 10 Mar 2026 at 17:04, Bin Liu <b-liu@ti.com> wrote:
> >
> > 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.
> >
> > [0] https://www.ti.com/lit/pdf/sprz582
> 
> It's great that a link like the above can be provided, still it would
> be nice to explain a bit more about the problem in the commit message
> too. Can you please do that?

Sure I will add the errata details in the commit message in v2.

> 
> > Signed-off-by: Bin Liu <b-liu@ti.com>
> > ---
> >  block/blk-mq-debugfs.c   |  1 +
> >  drivers/mmc/core/block.c | 12 ++++++++++--
> >  include/linux/blk-mq.h   |  3 +++
> >  3 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> > index 28167c9baa55..5f97d07fa3c8 100644
> > --- a/block/blk-mq-debugfs.c
> > +++ b/block/blk-mq-debugfs.c
> > @@ -243,6 +243,7 @@ static const char *const rqf_name[] = {
> >         RQF_NAME(ZONE_WRITE_PLUGGING),
> >         RQF_NAME(TIMED_OUT),
> >         RQF_NAME(RESV),
> > +       RQF_NAME(XFER_SINGLE_BLK),
> >  };
> >  #undef RQF_NAME
> >
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> > index 05ee76cb0a08..00016584d70d 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -1401,6 +1401,9 @@ static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
> >                     rq_data_dir(req) == WRITE &&
> >                     (md->flags & MMC_BLK_REL_WR);
> >
> > +       if (req->rq_flags & RQF_XFER_SINGLE_BLK)
> > +               recovery_mode = 1;
> > +
> 
> I don't get this, sorry.

No worries, the logic of this solution is not that obvious.

In summary of this patch, in both CQE-enabled and non-CQE cases, in
*_complete_rq(), flag RQF_XFER_SINGLE_BLK is set in req->rq_flags when
WRITE request fails. This is to mark that single-block write, according
to errata i2493, should be used in retrying this WRITE request. 

In retrying, mmc_blk_data_prep() is called again with
RQF_XFER_SINGLE_BLK set, this makes recovery_mode to be forced to 1,

then a few lines down in mmc_blk_data_prep():

	if (recovery_mode)
		brq->data.blocks = queue_physical_block_size(mq->queue) >> 9;

The req block size is set to 1 to use single block write.

Please let me know if this explains the logic of this patch.

> 
> Under what circumstances do we need to fall back to using "single
> block"? Or is this just for debugging?

When multiple block write fails, use single block write. This is
the software workaround documented in the errata i2493.

This is not for debugging. When multiple block write fails, switching
to single block write will sccuseed.

-Bin.

> 
> >         memset(brq, 0, sizeof(struct mmc_blk_request));
> >
> >         mmc_crypto_prepare_req(mqrq);
> > @@ -1540,10 +1543,13 @@ static void mmc_blk_cqe_complete_rq(struct mmc_queue *mq, struct request *req)
> >                 err = 0;
> >
> >         if (err) {
> > -               if (mqrq->retries++ < MMC_CQE_RETRIES)
> > +               if (mqrq->retries++ < MMC_CQE_RETRIES) {
> > +                       if (rq_data_dir(req) == WRITE)
> > +                               req->rq_flags |= RQF_XFER_SINGLE_BLK;
> >                         blk_mq_requeue_request(req, true);
> > -               else
> > +               } else {
> >                         blk_mq_end_request(req, BLK_STS_IOERR);
> > +               }
> >         } else if (mrq->data) {
> >                 if (blk_update_request(req, BLK_STS_OK, mrq->data->bytes_xfered))
> >                         blk_mq_requeue_request(req, true);
> > @@ -2085,6 +2091,8 @@ static void mmc_blk_mq_complete_rq(struct mmc_queue *mq, struct request *req)
> >         } else if (!blk_rq_bytes(req)) {
> >                 __blk_mq_end_request(req, BLK_STS_IOERR);
> >         } else if (mqrq->retries++ < MMC_MAX_RETRIES) {
> > +               if (rq_data_dir(req) == WRITE)
> > +                       req->rq_flags |= RQF_XFER_SINGLE_BLK;
> >                 blk_mq_requeue_request(req, true);
> >         } else {
> >                 if (mmc_card_removed(mq->card))
> > diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> > index 18a2388ba581..06b6e1c4fca3 100644
> > --- a/include/linux/blk-mq.h
> > +++ b/include/linux/blk-mq.h
> > @@ -64,6 +64,7 @@ enum rqf_flags {
> >         /* ->timeout has been called, don't expire again */
> >         __RQF_TIMED_OUT,
> >         __RQF_RESV,
> > +       __RQF_XFER_SINGLE_BLK,
> >         __RQF_BITS
> >  };
> >
> > @@ -85,6 +86,8 @@ enum rqf_flags {
> >                         ((__force req_flags_t)(1 << __RQF_ZONE_WRITE_PLUGGING))
> >  #define RQF_TIMED_OUT          ((__force req_flags_t)(1 << __RQF_TIMED_OUT))
> >  #define RQF_RESV               ((__force req_flags_t)(1 << __RQF_RESV))
> > +#define RQF_XFER_SINGLE_BLK    \
> > +                       ((__force req_flags_t)(1 << __RQF_XFER_SINGLE_BLK))
> >
> >  /* flags that prevent us from merging requests: */
> >  #define RQF_NOMERGE_FLAGS \
> > --
> > 2.34.1
> >
> 
> Kind regards
> Uffe

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] mmc: block: use single block write in retry
  2026-03-10 16:04 [PATCH] mmc: block: use single block write in retry Bin Liu
  2026-03-16 15:01 ` Ulf Hansson
@ 2026-03-17 12:46 ` Shawn Lin
  2026-03-17 13:24   ` Bin Liu
  2026-03-24 14:34 ` [PATCH v2 0/2] " Bin Liu
  2 siblings, 1 reply; 30+ messages in thread
From: Shawn Lin @ 2026-03-17 12:46 UTC (permalink / raw)
  To: Bin Liu; +Cc: shawn.lin, ulf.hansson, axboe, linux-block, linux-mmc

在 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
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.

More importantly, what is the failure rate of this issue? If it occurs
frequently, we should consider this a significant hardware defect. In
that case, the host driver could register a multi_io_quirk to apply
certain restrictions on the write path. This approach has been used
previously on TI OMAP platforms.

> 
> [0] https://www.ti.com/lit/pdf/sprz582
> Signed-off-by: Bin Liu <b-liu@ti.com>
> ---
>   block/blk-mq-debugfs.c   |  1 +
>   drivers/mmc/core/block.c | 12 ++++++++++--
>   include/linux/blk-mq.h   |  3 +++
>   3 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> index 28167c9baa55..5f97d07fa3c8 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
> @@ -243,6 +243,7 @@ static const char *const rqf_name[] = {
>   	RQF_NAME(ZONE_WRITE_PLUGGING),
>   	RQF_NAME(TIMED_OUT),
>   	RQF_NAME(RESV),
> +	RQF_NAME(XFER_SINGLE_BLK),
>   };
>   #undef RQF_NAME
>   
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 05ee76cb0a08..00016584d70d 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -1401,6 +1401,9 @@ static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
>   		    rq_data_dir(req) == WRITE &&
>   		    (md->flags & MMC_BLK_REL_WR);
>   
> +	if (req->rq_flags & RQF_XFER_SINGLE_BLK)
> +		recovery_mode = 1;
> +
>   	memset(brq, 0, sizeof(struct mmc_blk_request));
>   
>   	mmc_crypto_prepare_req(mqrq);
> @@ -1540,10 +1543,13 @@ static void mmc_blk_cqe_complete_rq(struct mmc_queue *mq, struct request *req)
>   		err = 0;
>   
>   	if (err) {
> -		if (mqrq->retries++ < MMC_CQE_RETRIES)
> +		if (mqrq->retries++ < MMC_CQE_RETRIES) {
> +			if (rq_data_dir(req) == WRITE)
> +				req->rq_flags |= RQF_XFER_SINGLE_BLK;
>   			blk_mq_requeue_request(req, true);
> -		else
> +		} else {
>   			blk_mq_end_request(req, BLK_STS_IOERR);
> +		}
>   	} else if (mrq->data) {
>   		if (blk_update_request(req, BLK_STS_OK, mrq->data->bytes_xfered))
>   			blk_mq_requeue_request(req, true);
> @@ -2085,6 +2091,8 @@ static void mmc_blk_mq_complete_rq(struct mmc_queue *mq, struct request *req)
>   	} else if (!blk_rq_bytes(req)) {
>   		__blk_mq_end_request(req, BLK_STS_IOERR);
>   	} else if (mqrq->retries++ < MMC_MAX_RETRIES) {
> +		if (rq_data_dir(req) == WRITE)
> +			req->rq_flags |= RQF_XFER_SINGLE_BLK;
>   		blk_mq_requeue_request(req, true);
>   	} else {
>   		if (mmc_card_removed(mq->card))
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index 18a2388ba581..06b6e1c4fca3 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -64,6 +64,7 @@ enum rqf_flags {
>   	/* ->timeout has been called, don't expire again */
>   	__RQF_TIMED_OUT,
>   	__RQF_RESV,
> +	__RQF_XFER_SINGLE_BLK,
>   	__RQF_BITS
>   };
>   
> @@ -85,6 +86,8 @@ enum rqf_flags {
>   			((__force req_flags_t)(1 << __RQF_ZONE_WRITE_PLUGGING))
>   #define RQF_TIMED_OUT		((__force req_flags_t)(1 << __RQF_TIMED_OUT))
>   #define RQF_RESV		((__force req_flags_t)(1 << __RQF_RESV))
> +#define RQF_XFER_SINGLE_BLK	\
> +			((__force req_flags_t)(1 << __RQF_XFER_SINGLE_BLK))
>   
>   /* flags that prevent us from merging requests: */
>   #define RQF_NOMERGE_FLAGS \
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] mmc: block: use single block write in retry
  2026-03-17 12:46 ` Shawn Lin
@ 2026-03-17 13:24   ` Bin Liu
  2026-03-18  1:43     ` Shawn Lin
  0 siblings, 1 reply; 30+ messages in thread
From: Bin Liu @ 2026-03-17 13:24 UTC (permalink / raw)
  To: Shawn Lin; +Cc: ulf.hansson, axboe, linux-block, linux-mmc

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. 

> frequently, we should consider this a significant hardware defect. In
> that case, the host driver could register a multi_io_quirk to apply
> certain restrictions on the write path. This approach has been used
> previously on TI OMAP platforms.

-Bin.

> 
> > 
> > [0] https://www.ti.com/lit/pdf/sprz582
> > Signed-off-by: Bin Liu <b-liu@ti.com>
> > ---
> >   block/blk-mq-debugfs.c   |  1 +
> >   drivers/mmc/core/block.c | 12 ++++++++++--
> >   include/linux/blk-mq.h   |  3 +++
> >   3 files changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> > index 28167c9baa55..5f97d07fa3c8 100644
> > --- a/block/blk-mq-debugfs.c
> > +++ b/block/blk-mq-debugfs.c
> > @@ -243,6 +243,7 @@ static const char *const rqf_name[] = {
> >   	RQF_NAME(ZONE_WRITE_PLUGGING),
> >   	RQF_NAME(TIMED_OUT),
> >   	RQF_NAME(RESV),
> > +	RQF_NAME(XFER_SINGLE_BLK),
> >   };
> >   #undef RQF_NAME
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> > index 05ee76cb0a08..00016584d70d 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -1401,6 +1401,9 @@ static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
> >   		    rq_data_dir(req) == WRITE &&
> >   		    (md->flags & MMC_BLK_REL_WR);
> > +	if (req->rq_flags & RQF_XFER_SINGLE_BLK)
> > +		recovery_mode = 1;
> > +
> >   	memset(brq, 0, sizeof(struct mmc_blk_request));
> >   	mmc_crypto_prepare_req(mqrq);
> > @@ -1540,10 +1543,13 @@ static void mmc_blk_cqe_complete_rq(struct mmc_queue *mq, struct request *req)
> >   		err = 0;
> >   	if (err) {
> > -		if (mqrq->retries++ < MMC_CQE_RETRIES)
> > +		if (mqrq->retries++ < MMC_CQE_RETRIES) {
> > +			if (rq_data_dir(req) == WRITE)
> > +				req->rq_flags |= RQF_XFER_SINGLE_BLK;
> >   			blk_mq_requeue_request(req, true);
> > -		else
> > +		} else {
> >   			blk_mq_end_request(req, BLK_STS_IOERR);
> > +		}
> >   	} else if (mrq->data) {
> >   		if (blk_update_request(req, BLK_STS_OK, mrq->data->bytes_xfered))
> >   			blk_mq_requeue_request(req, true);
> > @@ -2085,6 +2091,8 @@ static void mmc_blk_mq_complete_rq(struct mmc_queue *mq, struct request *req)
> >   	} else if (!blk_rq_bytes(req)) {
> >   		__blk_mq_end_request(req, BLK_STS_IOERR);
> >   	} else if (mqrq->retries++ < MMC_MAX_RETRIES) {
> > +		if (rq_data_dir(req) == WRITE)
> > +			req->rq_flags |= RQF_XFER_SINGLE_BLK;
> >   		blk_mq_requeue_request(req, true);
> >   	} else {
> >   		if (mmc_card_removed(mq->card))
> > diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> > index 18a2388ba581..06b6e1c4fca3 100644
> > --- a/include/linux/blk-mq.h
> > +++ b/include/linux/blk-mq.h
> > @@ -64,6 +64,7 @@ enum rqf_flags {
> >   	/* ->timeout has been called, don't expire again */
> >   	__RQF_TIMED_OUT,
> >   	__RQF_RESV,
> > +	__RQF_XFER_SINGLE_BLK,
> >   	__RQF_BITS
> >   };
> > @@ -85,6 +86,8 @@ enum rqf_flags {
> >   			((__force req_flags_t)(1 << __RQF_ZONE_WRITE_PLUGGING))
> >   #define RQF_TIMED_OUT		((__force req_flags_t)(1 << __RQF_TIMED_OUT))
> >   #define RQF_RESV		((__force req_flags_t)(1 << __RQF_RESV))
> > +#define RQF_XFER_SINGLE_BLK	\
> > +			((__force req_flags_t)(1 << __RQF_XFER_SINGLE_BLK))
> >   /* flags that prevent us from merging requests: */
> >   #define RQF_NOMERGE_FLAGS \
> > 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] mmc: block: use single block write in retry
  2026-03-17 13:24   ` Bin Liu
@ 2026-03-18  1:43     ` Shawn Lin
  2026-03-18 14:55       ` Bin Liu
  2026-03-18 15:17       ` Bin Liu
  0 siblings, 2 replies; 30+ messages in thread
From: Shawn Lin @ 2026-03-18  1:43 UTC (permalink / raw)
  To: Bin Liu; +Cc: shawn.lin, ulf.hansson, axboe, linux-block, linux-mmc

在 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) {
+		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)


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH] mmc: block: use single block write in retry
  2026-03-18  1:43     ` Shawn Lin
@ 2026-03-18 14:55       ` Bin Liu
  2026-03-18 15:10         ` Bin Liu
  2026-03-18 15:17       ` Bin Liu
  1 sibling, 1 reply; 30+ messages in thread
From: Bin Liu @ 2026-03-18 14:55 UTC (permalink / raw)
  To: Shawn Lin; +Cc: ulf.hansson, axboe, linux-block, linux-mmc

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?

I tested the following patch in CQE mode, it fixes the data corruption
problem, however the kernel prints Buffer I/O error messages which is
confusing. Please see the test log below.

# hexdump ff00-4096.dat
0000000 00ff 00ff 00ff 00ff 00ff 00ff 00ff 00ff
*
0001000
# dd if=/dev/zero of=/dev/mmcblk0p1 bs=1024
count=4
4+0 records in
4+0 records out
# hexdump -n 4096 /dev/mmcblk0p1
0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
0001000
# dd if=ff00-4096.dat of=/dev/mmcblk0p1
8+0 records in
8+0 records out
[  390.990581] mmc0: running CQE recovery
[  390.995129] mmc0: running CQE recovery
[  390.999684] mmc0: running CQE recovery
[  391.004002] I/O error, dev mmcblk0, sector 2048 op 0x1:(WRITE) flags 0x800000 phys_seg 8 prio class 2
[  391.013596] Buffer I/O error on dev mmcblk0p1, logical block 0, lost async page write
[  391.021678] Buffer I/O error on dev mmcblk0p1, logical block 1, lost async page write
[  391.029681] Buffer I/O error on dev mmcblk0p1, logical block 2, lost async page write
[  391.037754] Buffer I/O error on dev mmcblk0p1, logical block 3, lost async page write
[  391.045821] Buffer I/O error on dev mmcblk0p1, logical block 4, lost async page write
[  391.053922] Buffer I/O error on dev mmcblk0p1, logical block 5, lost async page write
[  391.061926] Buffer I/O error on dev mmcblk0p1, logical block 6, lost async page write
[  391.069982] Buffer I/O error on dev mmcblk0p1, logical block 7, lost async page write
# hexdump -n 4096 /dev/mmcblk0p1
0000000 00ff 00ff 00ff 00ff 00ff 00ff 00ff 00ff
*
0001000

-Bin.

> 
> 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) {
> +		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)
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] mmc: block: use single block write in retry
  2026-03-18 14:55       ` Bin Liu
@ 2026-03-18 15:10         ` Bin Liu
  0 siblings, 0 replies; 30+ messages in thread
From: Bin Liu @ 2026-03-18 15:10 UTC (permalink / raw)
  To: Shawn Lin; +Cc: ulf.hansson, axboe, linux-block, linux-mmc

On Wed, Mar 18, 2026 at 09:55:47AM -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?
> 
> I tested the following patch in CQE mode, it fixes the data corruption
> problem, however the kernel prints Buffer I/O error messages which is
> confusing. Please see the test log below.

The patch doesn't seem to catch the CQE error path. I added a few
pr_err() in the places you patches, but none of them is printed on
console. And this time data corruption happens, so the patch doesn't
seem to fix the problem. Here is the test log:


# hexdump -n 4096 /dev/mmcblk0p1
0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
0001000
# dd if=ff00-4096.dat of=/dev/mmcblk0p1
[   89.208865] mmc0: running CQE recovery
[   89.214757] mmc0: running CQE recovery
[   89.219414] mmc0: running CQE recovery
[   89.223743] I/O error, dev mmcblk0, sector 2048 op 0x1:(WRITE) flags 0x800800 phys_seg 1 prio class 2
[   89.233152] Buffer I/O error on dev mmcblk0p1, logical block 0, lost async page write
8+0 records in
8+0 records out
# hexdump -n 4096 /dev/mmcblk0p1
0000000 00ff 00ff 00ff 00ff 00ff 00ff 00ff 00ff
*
0000600 0000 0000 0000 0000 0000 0000 0000 0000
*
0001000

-Bin.

> 
> # hexdump ff00-4096.dat
> 0000000 00ff 00ff 00ff 00ff 00ff 00ff 00ff 00ff
> *
> 0001000
> # dd if=/dev/zero of=/dev/mmcblk0p1 bs=1024
> count=4
> 4+0 records in
> 4+0 records out
> # hexdump -n 4096 /dev/mmcblk0p1
> 0000000 0000 0000 0000 0000 0000 0000 0000 0000
> *
> 0001000
> # dd if=ff00-4096.dat of=/dev/mmcblk0p1
> 8+0 records in
> 8+0 records out
> [  390.990581] mmc0: running CQE recovery
> [  390.995129] mmc0: running CQE recovery
> [  390.999684] mmc0: running CQE recovery
> [  391.004002] I/O error, dev mmcblk0, sector 2048 op 0x1:(WRITE) flags 0x800000 phys_seg 8 prio class 2
> [  391.013596] Buffer I/O error on dev mmcblk0p1, logical block 0, lost async page write
> [  391.021678] Buffer I/O error on dev mmcblk0p1, logical block 1, lost async page write
> [  391.029681] Buffer I/O error on dev mmcblk0p1, logical block 2, lost async page write
> [  391.037754] Buffer I/O error on dev mmcblk0p1, logical block 3, lost async page write
> [  391.045821] Buffer I/O error on dev mmcblk0p1, logical block 4, lost async page write
> [  391.053922] Buffer I/O error on dev mmcblk0p1, logical block 5, lost async page write
> [  391.061926] Buffer I/O error on dev mmcblk0p1, logical block 6, lost async page write
> [  391.069982] Buffer I/O error on dev mmcblk0p1, logical block 7, lost async page write
> # hexdump -n 4096 /dev/mmcblk0p1
> 0000000 00ff 00ff 00ff 00ff 00ff 00ff 00ff 00ff
> *
> 0001000
> 
> -Bin.
> 
> > 
> > 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) {
> > +		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)
> > 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] mmc: block: use single block write in retry
  2026-03-18  1:43     ` Shawn Lin
  2026-03-18 14:55       ` Bin Liu
@ 2026-03-18 15:17       ` Bin Liu
  2026-03-23 13:05         ` Bin Liu
  1 sibling, 1 reply; 30+ messages in thread
From: Bin Liu @ 2026-03-18 15:17 UTC (permalink / raw)
  To: Shawn Lin; +Cc: ulf.hansson, axboe, linux-block, linux-mmc

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.

-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)
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] mmc: block: use single block write in retry
  2026-03-18 15:17       ` Bin Liu
@ 2026-03-23 13:05         ` Bin Liu
  2026-03-24  7:34           ` Shawn Lin
  0 siblings, 1 reply; 30+ messages in thread
From: Bin Liu @ 2026-03-23 13:05 UTC (permalink / raw)
  To: Shawn Lin; +Cc: ulf.hansson, axboe, linux-block, linux-mmc

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)
> > 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] mmc: block: use single block write in retry
  2026-03-23 13:05         ` Bin Liu
@ 2026-03-24  7:34           ` Shawn Lin
  0 siblings, 0 replies; 30+ messages in thread
From: Shawn Lin @ 2026-03-24  7:34 UTC (permalink / raw)
  To: Bin Liu; +Cc: shawn.lin, ulf.hansson, axboe, linux-block, linux-mmc

Hi Bin

On 2026/03/23 Monday 21:05, Bin Liu wrote:
> 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?
> 

Sorry for late reply. I haven't had time to investigate it deeply but I
think it would not block your original patch for folks to review it.

> 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)
>>>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] mmc: block: use single block write in retry
  2026-03-16 16:22   ` Bin Liu
@ 2026-03-24 12:44     ` Ulf Hansson
  0 siblings, 0 replies; 30+ messages in thread
From: Ulf Hansson @ 2026-03-24 12:44 UTC (permalink / raw)
  To: Bin Liu; +Cc: linux-block, linux-mmc, axboe

On Mon, 16 Mar 2026 at 17:23, Bin Liu <b-liu@ti.com> wrote:
>
> Hi Ulf,
>
> On Mon, Mar 16, 2026 at 04:01:38PM +0100, Ulf Hansson wrote:
> > On Tue, 10 Mar 2026 at 17:04, Bin Liu <b-liu@ti.com> wrote:
> > >
> > > 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.
> > >
> > > [0] https://www.ti.com/lit/pdf/sprz582
> >
> > It's great that a link like the above can be provided, still it would
> > be nice to explain a bit more about the problem in the commit message
> > too. Can you please do that?
>
> Sure I will add the errata details in the commit message in v2.
>
> >
> > > Signed-off-by: Bin Liu <b-liu@ti.com>
> > > ---
> > >  block/blk-mq-debugfs.c   |  1 +
> > >  drivers/mmc/core/block.c | 12 ++++++++++--
> > >  include/linux/blk-mq.h   |  3 +++
> > >  3 files changed, 14 insertions(+), 2 deletions(-)

Please split this patch into two, one for the blk-mq core and one for mmc core.

> > >
> > > diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> > > index 28167c9baa55..5f97d07fa3c8 100644
> > > --- a/block/blk-mq-debugfs.c
> > > +++ b/block/blk-mq-debugfs.c
> > > @@ -243,6 +243,7 @@ static const char *const rqf_name[] = {
> > >         RQF_NAME(ZONE_WRITE_PLUGGING),
> > >         RQF_NAME(TIMED_OUT),
> > >         RQF_NAME(RESV),
> > > +       RQF_NAME(XFER_SINGLE_BLK),
> > >  };
> > >  #undef RQF_NAME
> > >
> > > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> > > index 05ee76cb0a08..00016584d70d 100644
> > > --- a/drivers/mmc/core/block.c
> > > +++ b/drivers/mmc/core/block.c
> > > @@ -1401,6 +1401,9 @@ static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
> > >                     rq_data_dir(req) == WRITE &&
> > >                     (md->flags & MMC_BLK_REL_WR);
> > >
> > > +       if (req->rq_flags & RQF_XFER_SINGLE_BLK)
> > > +               recovery_mode = 1;
> > > +
> >
> > I don't get this, sorry.
>
> No worries, the logic of this solution is not that obvious.
>
> In summary of this patch, in both CQE-enabled and non-CQE cases, in
> *_complete_rq(), flag RQF_XFER_SINGLE_BLK is set in req->rq_flags when
> WRITE request fails. This is to mark that single-block write, according
> to errata i2493, should be used in retrying this WRITE request.
>
> In retrying, mmc_blk_data_prep() is called again with
> RQF_XFER_SINGLE_BLK set, this makes recovery_mode to be forced to 1,
>
> then a few lines down in mmc_blk_data_prep():
>
>         if (recovery_mode)
>                 brq->data.blocks = queue_physical_block_size(mq->queue) >> 9;
>
> The req block size is set to 1 to use single block write.
>
> Please let me know if this explains the logic of this patch.

Yes, that makes more sense now, thanks for clarifying!

>
> >
> > Under what circumstances do we need to fall back to using "single
> > block"? Or is this just for debugging?
>
> When multiple block write fails, use single block write. This is
> the software workaround documented in the errata i2493.
>
> This is not for debugging. When multiple block write fails, switching
> to single block write will sccuseed.

Okay, again, thanks for clarifying!

[...]

I noticed your conversation with Shawn, where he pointed out, that we
already have something similar for READ, which is implemented
internally within the mmc block device driver.

I don't have a strong opinion on how to do this, so assuming Jens is
okay with the approach in $subject patch, I am happy to proceed.

If we decide to move forward with this, perhaps we should re-work the
internal MMC fallback for READ to align with this approach too.

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v2 0/2] mmc: block: use single block write in retry
  2026-03-10 16:04 [PATCH] mmc: block: use single block write in retry Bin Liu
  2026-03-16 15:01 ` Ulf Hansson
  2026-03-17 12:46 ` Shawn Lin
@ 2026-03-24 14:34 ` Bin Liu
  2026-03-24 14:34   ` [PATCH v2 1/2] block: define new rqf_flags RQF_XFER_SINGLE_BLK Bin Liu
                     ` (3 more replies)
  2 siblings, 4 replies; 30+ messages in thread
From: Bin Liu @ 2026-03-24 14:34 UTC (permalink / raw)
  To: linux-block, linux-mmc; +Cc: ulf.hansson, axboe, shawn.lin, Bin Liu

Due to errata i2493[0], multi-block write would still fail in retries.

With i2493, the MMC interface has the potential of write failures when
issuing multi-block writes operating in HS200 mode with excessive IO
supply noise.

While the errata provides guidance in hardware design and layout to
minimize the IO supply noise, in theory the write failure cannot be
resolved in hardware. The software solution to ensure the data integrity
is to add minimum 5us delay between block writes. Single-block write is
the practical way to introduce the delay.

This patch set 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.

link to v1:
    https://lore.kernel.org/linux-mmc/20260310160408.3976760-1-b-liu@ti.com/T/#u

[0] https://www.ti.com/lit/pdf/sprz582
Signed-off-by: Bin Liu <b-liu@ti.com>

Bin Liu (2):
  block: define new rqf_flags RQF_XFER_SINGLE_BLK
  mmc: block: use single block write in retry

 block/blk-mq-debugfs.c   |  1 +
 drivers/mmc/core/block.c | 12 ++++++++++--
 include/linux/blk-mq.h   |  3 +++
 3 files changed, 14 insertions(+), 2 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v2 1/2] block: define new rqf_flags RQF_XFER_SINGLE_BLK
  2026-03-24 14:34 ` [PATCH v2 0/2] " Bin Liu
@ 2026-03-24 14:34   ` Bin Liu
  2026-03-24 18:48     ` Jens Axboe
  2026-03-24 14:34   ` [PATCH v2 2/2] mmc: block: use single block write in retry Bin Liu
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 30+ messages in thread
From: Bin Liu @ 2026-03-24 14:34 UTC (permalink / raw)
  To: linux-block, linux-mmc; +Cc: ulf.hansson, axboe, shawn.lin, Bin Liu

This new flag will be used to handle multi-block transfer failure, in
which case single-block transfer will be used.

Signed-off-by: Bin Liu <b-liu@ti.com>
---
v2: new patch, separate the new flag definition.

 block/blk-mq-debugfs.c | 1 +
 include/linux/blk-mq.h | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 28167c9baa55..5f97d07fa3c8 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -243,6 +243,7 @@ static const char *const rqf_name[] = {
 	RQF_NAME(ZONE_WRITE_PLUGGING),
 	RQF_NAME(TIMED_OUT),
 	RQF_NAME(RESV),
+	RQF_NAME(XFER_SINGLE_BLK),
 };
 #undef RQF_NAME
 
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 18a2388ba581..06b6e1c4fca3 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -64,6 +64,7 @@ enum rqf_flags {
 	/* ->timeout has been called, don't expire again */
 	__RQF_TIMED_OUT,
 	__RQF_RESV,
+	__RQF_XFER_SINGLE_BLK,
 	__RQF_BITS
 };
 
@@ -85,6 +86,8 @@ enum rqf_flags {
 			((__force req_flags_t)(1 << __RQF_ZONE_WRITE_PLUGGING))
 #define RQF_TIMED_OUT		((__force req_flags_t)(1 << __RQF_TIMED_OUT))
 #define RQF_RESV		((__force req_flags_t)(1 << __RQF_RESV))
+#define RQF_XFER_SINGLE_BLK	\
+			((__force req_flags_t)(1 << __RQF_XFER_SINGLE_BLK))
 
 /* flags that prevent us from merging requests: */
 #define RQF_NOMERGE_FLAGS \
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 2/2] mmc: block: use single block write in retry
  2026-03-24 14:34 ` [PATCH v2 0/2] " Bin Liu
  2026-03-24 14:34   ` [PATCH v2 1/2] block: define new rqf_flags RQF_XFER_SINGLE_BLK Bin Liu
@ 2026-03-24 14:34   ` Bin Liu
  2026-03-24 14:57   ` [PATCH v2 0/2] " Ulf Hansson
  2026-03-25 13:49   ` [PATCH v3] " Bin Liu
  3 siblings, 0 replies; 30+ messages in thread
From: Bin Liu @ 2026-03-24 14:34 UTC (permalink / raw)
  To: linux-block, linux-mmc; +Cc: ulf.hansson, axboe, shawn.lin, Bin Liu

Due to errata i2493[0], multi-block write would still fail in retries.

With i2493, the MMC interface has the potential of write failures when
issuing multi-block writes operating in HS200 mode with excessive IO
supply noise.

While the errata provides guidance in hardware design and layout to
minimize the IO supply noise, in theory the write failure cannot be
resolved in hardware. The software solution to ensure the data integrity
is to add minimum 5us delay between block writes. Single-block write is
the practical way to introduce the delay.

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.

[0] https://www.ti.com/lit/pdf/sprz582
Signed-off-by: Bin Liu <b-liu@ti.com>
---
v2: add errata details in the commit message.

 drivers/mmc/core/block.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 05ee76cb0a08..00016584d70d 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1401,6 +1401,9 @@ static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
 		    rq_data_dir(req) == WRITE &&
 		    (md->flags & MMC_BLK_REL_WR);
 
+	if (req->rq_flags & RQF_XFER_SINGLE_BLK)
+		recovery_mode = 1;
+
 	memset(brq, 0, sizeof(struct mmc_blk_request));
 
 	mmc_crypto_prepare_req(mqrq);
@@ -1540,10 +1543,13 @@ static void mmc_blk_cqe_complete_rq(struct mmc_queue *mq, struct request *req)
 		err = 0;
 
 	if (err) {
-		if (mqrq->retries++ < MMC_CQE_RETRIES)
+		if (mqrq->retries++ < MMC_CQE_RETRIES) {
+			if (rq_data_dir(req) == WRITE)
+				req->rq_flags |= RQF_XFER_SINGLE_BLK;
 			blk_mq_requeue_request(req, true);
-		else
+		} else {
 			blk_mq_end_request(req, BLK_STS_IOERR);
+		}
 	} else if (mrq->data) {
 		if (blk_update_request(req, BLK_STS_OK, mrq->data->bytes_xfered))
 			blk_mq_requeue_request(req, true);
@@ -2085,6 +2091,8 @@ static void mmc_blk_mq_complete_rq(struct mmc_queue *mq, struct request *req)
 	} else if (!blk_rq_bytes(req)) {
 		__blk_mq_end_request(req, BLK_STS_IOERR);
 	} else if (mqrq->retries++ < MMC_MAX_RETRIES) {
+		if (rq_data_dir(req) == WRITE)
+			req->rq_flags |= RQF_XFER_SINGLE_BLK;
 		blk_mq_requeue_request(req, true);
 	} else {
 		if (mmc_card_removed(mq->card))
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 0/2] mmc: block: use single block write in retry
  2026-03-24 14:34 ` [PATCH v2 0/2] " Bin Liu
  2026-03-24 14:34   ` [PATCH v2 1/2] block: define new rqf_flags RQF_XFER_SINGLE_BLK Bin Liu
  2026-03-24 14:34   ` [PATCH v2 2/2] mmc: block: use single block write in retry Bin Liu
@ 2026-03-24 14:57   ` Ulf Hansson
  2026-03-24 19:17     ` Jens Axboe
  2026-03-25 13:49   ` [PATCH v3] " Bin Liu
  3 siblings, 1 reply; 30+ messages in thread
From: Ulf Hansson @ 2026-03-24 14:57 UTC (permalink / raw)
  To: axboe; +Cc: Bin Liu, linux-block, linux-mmc, shawn.lin

On Tue, 24 Mar 2026 at 15:34, Bin Liu <b-liu@ti.com> wrote:
>
> Due to errata i2493[0], multi-block write would still fail in retries.
>
> With i2493, the MMC interface has the potential of write failures when
> issuing multi-block writes operating in HS200 mode with excessive IO
> supply noise.
>
> While the errata provides guidance in hardware design and layout to
> minimize the IO supply noise, in theory the write failure cannot be
> resolved in hardware. The software solution to ensure the data integrity
> is to add minimum 5us delay between block writes. Single-block write is
> the practical way to introduce the delay.
>
> This patch set 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.
>
> link to v1:
>     https://lore.kernel.org/linux-mmc/20260310160408.3976760-1-b-liu@ti.com/T/#u
>
> [0] https://www.ti.com/lit/pdf/sprz582
> Signed-off-by: Bin Liu <b-liu@ti.com>
>
> Bin Liu (2):
>   block: define new rqf_flags RQF_XFER_SINGLE_BLK
>   mmc: block: use single block write in retry
>
>  block/blk-mq-debugfs.c   |  1 +
>  drivers/mmc/core/block.c | 12 ++++++++++--
>  include/linux/blk-mq.h   |  3 +++
>  3 files changed, 14 insertions(+), 2 deletions(-)
>
> --
> 2.34.1
>

Jens, if you think this approach is okay, feel free to take the series
via your tree. Or if you prefer me to handle it, please let me know.

For the series:
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 1/2] block: define new rqf_flags RQF_XFER_SINGLE_BLK
  2026-03-24 14:34   ` [PATCH v2 1/2] block: define new rqf_flags RQF_XFER_SINGLE_BLK Bin Liu
@ 2026-03-24 18:48     ` Jens Axboe
  2026-03-24 19:11       ` Bin Liu
  0 siblings, 1 reply; 30+ messages in thread
From: Jens Axboe @ 2026-03-24 18:48 UTC (permalink / raw)
  To: Bin Liu, linux-block, linux-mmc; +Cc: ulf.hansson, shawn.lin

On 3/24/26 8:34 AM, Bin Liu wrote:
> This new flag will be used to handle multi-block transfer failure, in
> which case single-block transfer will be used.

Maybe I missed this in previous discussions, but why can't this flag go
into struct mmc_queue_req?

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 1/2] block: define new rqf_flags RQF_XFER_SINGLE_BLK
  2026-03-24 18:48     ` Jens Axboe
@ 2026-03-24 19:11       ` Bin Liu
  2026-03-24 19:16         ` Jens Axboe
  0 siblings, 1 reply; 30+ messages in thread
From: Bin Liu @ 2026-03-24 19:11 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, linux-mmc, ulf.hansson, shawn.lin

Hi Jens,

On Tue, Mar 24, 2026 at 12:48:11PM -0600, Jens Axboe wrote:
> On 3/24/26 8:34 AM, Bin Liu wrote:
> > This new flag will be used to handle multi-block transfer failure, in
> > which case single-block transfer will be used.
> 
> Maybe I missed this in previous discussions, but why can't this flag go
> into struct mmc_queue_req?

You didn't miss anything. We didn't discuss about it until now.

I added the flag in struct request because it already has rq_flags to
host all the flag bits. But I don't see struct mmc_queue_req has any
member for this flag purpose. Do you mean we should add a new flag
member into struct mmc_queue_req?

-Bin.

> 
> -- 
> Jens Axboe

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 1/2] block: define new rqf_flags RQF_XFER_SINGLE_BLK
  2026-03-24 19:11       ` Bin Liu
@ 2026-03-24 19:16         ` Jens Axboe
  2026-03-24 20:41           ` Bin Liu
  0 siblings, 1 reply; 30+ messages in thread
From: Jens Axboe @ 2026-03-24 19:16 UTC (permalink / raw)
  To: Bin Liu; +Cc: linux-block, linux-mmc, ulf.hansson, shawn.lin

On 3/24/26 1:11 PM, Bin Liu wrote:
> Hi Jens,
> 
> On Tue, Mar 24, 2026 at 12:48:11PM -0600, Jens Axboe wrote:
>> On 3/24/26 8:34 AM, Bin Liu wrote:
>>> This new flag will be used to handle multi-block transfer failure, in
>>> which case single-block transfer will be used.
>>
>> Maybe I missed this in previous discussions, but why can't this flag go
>> into struct mmc_queue_req?
> 
> You didn't miss anything. We didn't discuss about it until now.
> 
> I added the flag in struct request because it already has rq_flags to
> host all the flag bits. But I don't see struct mmc_queue_req has any
> member for this flag purpose. Do you mean we should add a new flag
> member into struct mmc_queue_req?

It's a bad idea to add driver specific flag to the core flags,
particularly when you already have per-rq data associated with the
driver. Then it should surely go there. The block layer doesn't care
about RQF_XFER_SINGLE_BLK, that's strictly a device property. Hence it
should go in the driver, NOT in the block layer.

So yes, add a bool or flags or whatever you need on the driver side.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 0/2] mmc: block: use single block write in retry
  2026-03-24 14:57   ` [PATCH v2 0/2] " Ulf Hansson
@ 2026-03-24 19:17     ` Jens Axboe
  0 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2026-03-24 19:17 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: Bin Liu, linux-block, linux-mmc, shawn.lin

On 3/24/26 8:57 AM, Ulf Hansson wrote:
> Jens, if you think this approach is okay, feel free to take the series
> via your tree. Or if you prefer me to handle it, please let me know.

See comments on patch 1, I think that needs a rework. And then you won't
have any block related changes either.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 1/2] block: define new rqf_flags RQF_XFER_SINGLE_BLK
  2026-03-24 19:16         ` Jens Axboe
@ 2026-03-24 20:41           ` Bin Liu
  0 siblings, 0 replies; 30+ messages in thread
From: Bin Liu @ 2026-03-24 20:41 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, linux-mmc, ulf.hansson, shawn.lin

Hi Jens,

On Tue, Mar 24, 2026 at 01:16:34PM -0600, Jens Axboe wrote:
> On 3/24/26 1:11 PM, Bin Liu wrote:
> > Hi Jens,
> > 
> > On Tue, Mar 24, 2026 at 12:48:11PM -0600, Jens Axboe wrote:
> >> On 3/24/26 8:34 AM, Bin Liu wrote:
> >>> This new flag will be used to handle multi-block transfer failure, in
> >>> which case single-block transfer will be used.
> >>
> >> Maybe I missed this in previous discussions, but why can't this flag go
> >> into struct mmc_queue_req?
> > 
> > You didn't miss anything. We didn't discuss about it until now.
> > 
> > I added the flag in struct request because it already has rq_flags to
> > host all the flag bits. But I don't see struct mmc_queue_req has any
> > member for this flag purpose. Do you mean we should add a new flag
> > member into struct mmc_queue_req?
> 
> It's a bad idea to add driver specific flag to the core flags,
> particularly when you already have per-rq data associated with the
> driver. Then it should surely go there. The block layer doesn't care
> about RQF_XFER_SINGLE_BLK, that's strictly a device property. Hence it
> should go in the driver, NOT in the block layer.
> 
> So yes, add a bool or flags or whatever you need on the driver side.

Sure I will add a variable in struct mmc_queue_req to carry this flag.
Thank you for the comments.

-Bin.
> 
> -- 
> Jens Axboe

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v3] mmc: block: use single block write in retry
  2026-03-24 14:34 ` [PATCH v2 0/2] " Bin Liu
                     ` (2 preceding siblings ...)
  2026-03-24 14:57   ` [PATCH v2 0/2] " Ulf Hansson
@ 2026-03-25 13:49   ` Bin Liu
  2026-03-25 14:12     ` Jens Axboe
  2026-03-26 12:33     ` Ulf Hansson
  3 siblings, 2 replies; 30+ messages in thread
From: Bin Liu @ 2026-03-25 13:49 UTC (permalink / raw)
  To: linux-block, linux-mmc; +Cc: ulf.hansson, axboe, shawn.lin, Bin Liu

Due to errata i2493[0], multi-block write would still fail in retries.

With i2493, the MMC interface has the potential of write failures when
issuing multi-block writes operating in HS200 mode with excessive IO
supply noise.

While the errata provides guidance in hardware design and layout to
minimize the IO supply noise, in theory the write failure cannot be
resolved in hardware. The software solution to ensure the data integrity
is to add minimum 5us delay between block writes. Single-block write is
the practical way to introduce the delay.

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.

[0] https://www.ti.com/lit/pdf/sprz582
Cc: stable@vger.kernel.org
Signed-off-by: Bin Liu <b-liu@ti.com>
---
v3: move the flag to struct mmc_queue_req to reduce the scope
    cc stable@

v2: add errata details in the commit message.

link to v2:
    https://lore.kernel.org/linux-mmc/20260310160408.3976760-1-b-liu@ti.com/T/#mb201075c9c466a78b58dbb0bf1f39139f0928591

link to v1:
    https://lore.kernel.org/linux-mmc/20260310160408.3976760-1-b-liu@ti.com/T/#u

 drivers/mmc/core/block.c | 12 ++++++++++--
 drivers/mmc/core/queue.h |  3 +++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 05ee76cb0a08..db8c99c73a61 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1401,6 +1401,9 @@ static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
 		    rq_data_dir(req) == WRITE &&
 		    (md->flags & MMC_BLK_REL_WR);
 
+	if (mqrq->flags & MQRQ_XFER_SINGLE_BLOCK)
+		recovery_mode = 1;
+
 	memset(brq, 0, sizeof(struct mmc_blk_request));
 
 	mmc_crypto_prepare_req(mqrq);
@@ -1540,10 +1543,13 @@ static void mmc_blk_cqe_complete_rq(struct mmc_queue *mq, struct request *req)
 		err = 0;
 
 	if (err) {
-		if (mqrq->retries++ < MMC_CQE_RETRIES)
+		if (mqrq->retries++ < MMC_CQE_RETRIES) {
+			if (rq_data_dir(req) == WRITE)
+				mqrq->flags |= MQRQ_XFER_SINGLE_BLOCK;
 			blk_mq_requeue_request(req, true);
-		else
+		} else {
 			blk_mq_end_request(req, BLK_STS_IOERR);
+		}
 	} else if (mrq->data) {
 		if (blk_update_request(req, BLK_STS_OK, mrq->data->bytes_xfered))
 			blk_mq_requeue_request(req, true);
@@ -2085,6 +2091,8 @@ static void mmc_blk_mq_complete_rq(struct mmc_queue *mq, struct request *req)
 	} else if (!blk_rq_bytes(req)) {
 		__blk_mq_end_request(req, BLK_STS_IOERR);
 	} else if (mqrq->retries++ < MMC_MAX_RETRIES) {
+		if (rq_data_dir(req) == WRITE)
+			mqrq->flags |= MQRQ_XFER_SINGLE_BLOCK;
 		blk_mq_requeue_request(req, true);
 	} else {
 		if (mmc_card_removed(mq->card))
diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
index 1498840a4ea0..c254e6580afd 100644
--- a/drivers/mmc/core/queue.h
+++ b/drivers/mmc/core/queue.h
@@ -61,6 +61,8 @@ enum mmc_drv_op {
 	MMC_DRV_OP_GET_EXT_CSD,
 };
 
+#define	MQRQ_XFER_SINGLE_BLOCK		BIT(0)
+
 struct mmc_queue_req {
 	struct mmc_blk_request	brq;
 	struct scatterlist	*sg;
@@ -69,6 +71,7 @@ struct mmc_queue_req {
 	void			*drv_op_data;
 	unsigned int		ioc_count;
 	int			retries;
+	u32			flags;
 };
 
 struct mmc_queue {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v3] mmc: block: use single block write in retry
  2026-03-25 13:49   ` [PATCH v3] " Bin Liu
@ 2026-03-25 14:12     ` Jens Axboe
  2026-03-25 14:21       ` Bin Liu
  2026-03-26 12:33     ` Ulf Hansson
  1 sibling, 1 reply; 30+ messages in thread
From: Jens Axboe @ 2026-03-25 14:12 UTC (permalink / raw)
  To: Bin Liu, linux-block, linux-mmc; +Cc: ulf.hansson, shawn.lin

On 3/25/26 7:49 AM, Bin Liu wrote:
> diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
> index 1498840a4ea0..c254e6580afd 100644
> --- a/drivers/mmc/core/queue.h
> +++ b/drivers/mmc/core/queue.h
> @@ -61,6 +61,8 @@ enum mmc_drv_op {
>  	MMC_DRV_OP_GET_EXT_CSD,
>  };
>  
> +#define	MQRQ_XFER_SINGLE_BLOCK		BIT(0)
> +
>  struct mmc_queue_req {
>  	struct mmc_blk_request	brq;
>  	struct scatterlist	*sg;
> @@ -69,6 +71,7 @@ struct mmc_queue_req {
>  	void			*drv_op_data;
>  	unsigned int		ioc_count;
>  	int			retries;
> +	u32			flags;
>  };
>  
>  struct mmc_queue {

I'm assuming retries is a pretty small number (didn't check), so if you
care about layout/space in mmc_queue_req, you could make this:

	u16 retries;
	u16 flags;

and not grow the size of it by 8 bytes with that addition.

Not a huge deal, obviously, but always good to consider things like that
to avoid wasting memory for nothing.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3] mmc: block: use single block write in retry
  2026-03-25 14:12     ` Jens Axboe
@ 2026-03-25 14:21       ` Bin Liu
  2026-03-25 14:23         ` Jens Axboe
  0 siblings, 1 reply; 30+ messages in thread
From: Bin Liu @ 2026-03-25 14:21 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, linux-mmc, ulf.hansson, shawn.lin

On Wed, Mar 25, 2026 at 08:12:36AM -0600, Jens Axboe wrote:
> On 3/25/26 7:49 AM, Bin Liu wrote:
> > diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
> > index 1498840a4ea0..c254e6580afd 100644
> > --- a/drivers/mmc/core/queue.h
> > +++ b/drivers/mmc/core/queue.h
> > @@ -61,6 +61,8 @@ enum mmc_drv_op {
> >  	MMC_DRV_OP_GET_EXT_CSD,
> >  };
> >  
> > +#define	MQRQ_XFER_SINGLE_BLOCK		BIT(0)
> > +
> >  struct mmc_queue_req {
> >  	struct mmc_blk_request	brq;
> >  	struct scatterlist	*sg;
> > @@ -69,6 +71,7 @@ struct mmc_queue_req {
> >  	void			*drv_op_data;
> >  	unsigned int		ioc_count;
> >  	int			retries;
> > +	u32			flags;
> >  };
> >  
> >  struct mmc_queue {
> 
> I'm assuming retries is a pretty small number (didn't check), so if you
> care about layout/space in mmc_queue_req, you could make this:
> 
> 	u16 retries;
> 	u16 flags;

I quickly went throught block.c, it appears retries would be only a
single digit number. I will update the patch to use u16.

Thanks,
-Bin.

> 
> and not grow the size of it by 8 bytes with that addition.
> 
> Not a huge deal, obviously, but always good to consider things like that
> to avoid wasting memory for nothing.
> 
> -- 
> Jens Axboe

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3] mmc: block: use single block write in retry
  2026-03-25 14:21       ` Bin Liu
@ 2026-03-25 14:23         ` Jens Axboe
  2026-03-25 14:27           ` Bin Liu
  0 siblings, 1 reply; 30+ messages in thread
From: Jens Axboe @ 2026-03-25 14:23 UTC (permalink / raw)
  To: Bin Liu; +Cc: linux-block, linux-mmc, ulf.hansson, shawn.lin

On 3/25/26 8:21 AM, Bin Liu wrote:
> On Wed, Mar 25, 2026 at 08:12:36AM -0600, Jens Axboe wrote:
>> On 3/25/26 7:49 AM, Bin Liu wrote:
>>> diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
>>> index 1498840a4ea0..c254e6580afd 100644
>>> --- a/drivers/mmc/core/queue.h
>>> +++ b/drivers/mmc/core/queue.h
>>> @@ -61,6 +61,8 @@ enum mmc_drv_op {
>>>  	MMC_DRV_OP_GET_EXT_CSD,
>>>  };
>>>  
>>> +#define	MQRQ_XFER_SINGLE_BLOCK		BIT(0)
>>> +
>>>  struct mmc_queue_req {
>>>  	struct mmc_blk_request	brq;
>>>  	struct scatterlist	*sg;
>>> @@ -69,6 +71,7 @@ struct mmc_queue_req {
>>>  	void			*drv_op_data;
>>>  	unsigned int		ioc_count;
>>>  	int			retries;
>>> +	u32			flags;
>>>  };
>>>  
>>>  struct mmc_queue {
>>
>> I'm assuming retries is a pretty small number (didn't check), so if you
>> care about layout/space in mmc_queue_req, you could make this:
>>
>> 	u16 retries;
>> 	u16 flags;
> 
> I quickly went throught block.c, it appears retries would be only a
> single digit number. I will update the patch to use u16.

Note that this is just my suggestion, it's down to Ulf. But what I would
do is a prep patch that makes retries u16 and ensures that's sane, then
do your flags patch on top of that. I think that's better than making
that unrelated size change for retries in your patch that isn't related
to that at all. It just sets you up to not grow the struct after your
change.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3] mmc: block: use single block write in retry
  2026-03-25 14:23         ` Jens Axboe
@ 2026-03-25 14:27           ` Bin Liu
  2026-03-25 15:22             ` Ulf Hansson
  0 siblings, 1 reply; 30+ messages in thread
From: Bin Liu @ 2026-03-25 14:27 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, linux-mmc, ulf.hansson, shawn.lin

On Wed, Mar 25, 2026 at 08:23:29AM -0600, Jens Axboe wrote:
> On 3/25/26 8:21 AM, Bin Liu wrote:
> > On Wed, Mar 25, 2026 at 08:12:36AM -0600, Jens Axboe wrote:
> >> On 3/25/26 7:49 AM, Bin Liu wrote:
> >>> diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
> >>> index 1498840a4ea0..c254e6580afd 100644
> >>> --- a/drivers/mmc/core/queue.h
> >>> +++ b/drivers/mmc/core/queue.h
> >>> @@ -61,6 +61,8 @@ enum mmc_drv_op {
> >>>  	MMC_DRV_OP_GET_EXT_CSD,
> >>>  };
> >>>  
> >>> +#define	MQRQ_XFER_SINGLE_BLOCK		BIT(0)
> >>> +
> >>>  struct mmc_queue_req {
> >>>  	struct mmc_blk_request	brq;
> >>>  	struct scatterlist	*sg;
> >>> @@ -69,6 +71,7 @@ struct mmc_queue_req {
> >>>  	void			*drv_op_data;
> >>>  	unsigned int		ioc_count;
> >>>  	int			retries;
> >>> +	u32			flags;
> >>>  };
> >>>  
> >>>  struct mmc_queue {
> >>
> >> I'm assuming retries is a pretty small number (didn't check), so if you
> >> care about layout/space in mmc_queue_req, you could make this:
> >>
> >> 	u16 retries;
> >> 	u16 flags;
> > 
> > I quickly went throught block.c, it appears retries would be only a
> > single digit number. I will update the patch to use u16.
> 
> Note that this is just my suggestion, it's down to Ulf. But what I would
> do is a prep patch that makes retries u16 and ensures that's sane, then
> do your flags patch on top of that. I think that's better than making
> that unrelated size change for retries in your patch that isn't related
> to that at all. It just sets you up to not grow the struct after your
> change.

Okay I will wait for Ulf's decision to make the u16 change or not.

-Bin.

> 
> -- 
> Jens Axboe

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3] mmc: block: use single block write in retry
  2026-03-25 14:27           ` Bin Liu
@ 2026-03-25 15:22             ` Ulf Hansson
  2026-03-25 15:29               ` Bin Liu
  0 siblings, 1 reply; 30+ messages in thread
From: Ulf Hansson @ 2026-03-25 15:22 UTC (permalink / raw)
  To: Bin Liu; +Cc: Jens Axboe, linux-block, linux-mmc, shawn.lin

On Wed, 25 Mar 2026 at 15:28, Bin Liu <b-liu@ti.com> wrote:
>
> On Wed, Mar 25, 2026 at 08:23:29AM -0600, Jens Axboe wrote:
> > On 3/25/26 8:21 AM, Bin Liu wrote:
> > > On Wed, Mar 25, 2026 at 08:12:36AM -0600, Jens Axboe wrote:
> > >> On 3/25/26 7:49 AM, Bin Liu wrote:
> > >>> diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
> > >>> index 1498840a4ea0..c254e6580afd 100644
> > >>> --- a/drivers/mmc/core/queue.h
> > >>> +++ b/drivers/mmc/core/queue.h
> > >>> @@ -61,6 +61,8 @@ enum mmc_drv_op {
> > >>>   MMC_DRV_OP_GET_EXT_CSD,
> > >>>  };
> > >>>
> > >>> +#define  MQRQ_XFER_SINGLE_BLOCK          BIT(0)
> > >>> +
> > >>>  struct mmc_queue_req {
> > >>>   struct mmc_blk_request  brq;
> > >>>   struct scatterlist      *sg;
> > >>> @@ -69,6 +71,7 @@ struct mmc_queue_req {
> > >>>   void                    *drv_op_data;
> > >>>   unsigned int            ioc_count;
> > >>>   int                     retries;
> > >>> + u32                     flags;
> > >>>  };
> > >>>
> > >>>  struct mmc_queue {
> > >>
> > >> I'm assuming retries is a pretty small number (didn't check), so if you
> > >> care about layout/space in mmc_queue_req, you could make this:
> > >>
> > >>    u16 retries;
> > >>    u16 flags;
> > >
> > > I quickly went throught block.c, it appears retries would be only a
> > > single digit number. I will update the patch to use u16.
> >
> > Note that this is just my suggestion, it's down to Ulf. But what I would
> > do is a prep patch that makes retries u16 and ensures that's sane, then
> > do your flags patch on top of that. I think that's better than making
> > that unrelated size change for retries in your patch that isn't related
> > to that at all. It just sets you up to not grow the struct after your
> > change.
>
> Okay I will wait for Ulf's decision to make the u16 change or not.

I have no strong opinion, before or after works for me, while I
certainly thinks it makes sense to do the optimization.

Even ioc_count can be changed. From a brief analyze it looks like this
(needs to be double checked).
ioc_count <= 255
retries <= max 5

So maybe u8 is good enough :-)

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3] mmc: block: use single block write in retry
  2026-03-25 15:22             ` Ulf Hansson
@ 2026-03-25 15:29               ` Bin Liu
  0 siblings, 0 replies; 30+ messages in thread
From: Bin Liu @ 2026-03-25 15:29 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: Jens Axboe, linux-block, linux-mmc, shawn.lin

On Wed, Mar 25, 2026 at 04:22:15PM +0100, Ulf Hansson wrote:
> On Wed, 25 Mar 2026 at 15:28, Bin Liu <b-liu@ti.com> wrote:
> >
> > On Wed, Mar 25, 2026 at 08:23:29AM -0600, Jens Axboe wrote:
> > > On 3/25/26 8:21 AM, Bin Liu wrote:
> > > > On Wed, Mar 25, 2026 at 08:12:36AM -0600, Jens Axboe wrote:
> > > >> On 3/25/26 7:49 AM, Bin Liu wrote:
> > > >>> diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
> > > >>> index 1498840a4ea0..c254e6580afd 100644
> > > >>> --- a/drivers/mmc/core/queue.h
> > > >>> +++ b/drivers/mmc/core/queue.h
> > > >>> @@ -61,6 +61,8 @@ enum mmc_drv_op {
> > > >>>   MMC_DRV_OP_GET_EXT_CSD,
> > > >>>  };
> > > >>>
> > > >>> +#define  MQRQ_XFER_SINGLE_BLOCK          BIT(0)
> > > >>> +
> > > >>>  struct mmc_queue_req {
> > > >>>   struct mmc_blk_request  brq;
> > > >>>   struct scatterlist      *sg;
> > > >>> @@ -69,6 +71,7 @@ struct mmc_queue_req {
> > > >>>   void                    *drv_op_data;
> > > >>>   unsigned int            ioc_count;
> > > >>>   int                     retries;
> > > >>> + u32                     flags;
> > > >>>  };
> > > >>>
> > > >>>  struct mmc_queue {
> > > >>
> > > >> I'm assuming retries is a pretty small number (didn't check), so if you
> > > >> care about layout/space in mmc_queue_req, you could make this:
> > > >>
> > > >>    u16 retries;
> > > >>    u16 flags;
> > > >
> > > > I quickly went throught block.c, it appears retries would be only a
> > > > single digit number. I will update the patch to use u16.
> > >
> > > Note that this is just my suggestion, it's down to Ulf. But what I would
> > > do is a prep patch that makes retries u16 and ensures that's sane, then
> > > do your flags patch on top of that. I think that's better than making
> > > that unrelated size change for retries in your patch that isn't related
> > > to that at all. It just sets you up to not grow the struct after your
> > > change.
> >
> > Okay I will wait for Ulf's decision to make the u16 change or not.
> 
> I have no strong opinion, before or after works for me, while I

Okay, can you please just take this v3 then? I think we typically don't
mix fix and optimization in a single patch set. I am talking about other
subsystems thought...

-Bin.

> certainly thinks it makes sense to do the optimization.
> 
> Even ioc_count can be changed. From a brief analyze it looks like this
> (needs to be double checked).
> ioc_count <= 255
> retries <= max 5
> 
> So maybe u8 is good enough :-)
> 
> Kind regards
> Uffe

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3] mmc: block: use single block write in retry
  2026-03-25 13:49   ` [PATCH v3] " Bin Liu
  2026-03-25 14:12     ` Jens Axboe
@ 2026-03-26 12:33     ` Ulf Hansson
  2026-03-26 13:41       ` Bin Liu
  1 sibling, 1 reply; 30+ messages in thread
From: Ulf Hansson @ 2026-03-26 12:33 UTC (permalink / raw)
  To: Bin Liu; +Cc: linux-block, linux-mmc, axboe, shawn.lin

On Wed, 25 Mar 2026 at 14:49, Bin Liu <b-liu@ti.com> wrote:
>
> Due to errata i2493[0], multi-block write would still fail in retries.
>
> With i2493, the MMC interface has the potential of write failures when
> issuing multi-block writes operating in HS200 mode with excessive IO
> supply noise.
>
> While the errata provides guidance in hardware design and layout to
> minimize the IO supply noise, in theory the write failure cannot be
> resolved in hardware. The software solution to ensure the data integrity
> is to add minimum 5us delay between block writes. Single-block write is
> the practical way to introduce the delay.
>
> 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.
>
> [0] https://www.ti.com/lit/pdf/sprz582
> Cc: stable@vger.kernel.org
> Signed-off-by: Bin Liu <b-liu@ti.com>

Applied for next and by adding a suggested-by tag from Jens, thanks!

Please consider sending a patch on top, that decreases the data size
of struct mmc_queue_req.

Kind regards
Uffe



> ---
> v3: move the flag to struct mmc_queue_req to reduce the scope
>     cc stable@
>
> v2: add errata details in the commit message.
>
> link to v2:
>     https://lore.kernel.org/linux-mmc/20260310160408.3976760-1-b-liu@ti.com/T/#mb201075c9c466a78b58dbb0bf1f39139f0928591
>
> link to v1:
>     https://lore.kernel.org/linux-mmc/20260310160408.3976760-1-b-liu@ti.com/T/#u
>
>  drivers/mmc/core/block.c | 12 ++++++++++--
>  drivers/mmc/core/queue.h |  3 +++
>  2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 05ee76cb0a08..db8c99c73a61 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -1401,6 +1401,9 @@ static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
>                     rq_data_dir(req) == WRITE &&
>                     (md->flags & MMC_BLK_REL_WR);
>
> +       if (mqrq->flags & MQRQ_XFER_SINGLE_BLOCK)
> +               recovery_mode = 1;
> +
>         memset(brq, 0, sizeof(struct mmc_blk_request));
>
>         mmc_crypto_prepare_req(mqrq);
> @@ -1540,10 +1543,13 @@ static void mmc_blk_cqe_complete_rq(struct mmc_queue *mq, struct request *req)
>                 err = 0;
>
>         if (err) {
> -               if (mqrq->retries++ < MMC_CQE_RETRIES)
> +               if (mqrq->retries++ < MMC_CQE_RETRIES) {
> +                       if (rq_data_dir(req) == WRITE)
> +                               mqrq->flags |= MQRQ_XFER_SINGLE_BLOCK;
>                         blk_mq_requeue_request(req, true);
> -               else
> +               } else {
>                         blk_mq_end_request(req, BLK_STS_IOERR);
> +               }
>         } else if (mrq->data) {
>                 if (blk_update_request(req, BLK_STS_OK, mrq->data->bytes_xfered))
>                         blk_mq_requeue_request(req, true);
> @@ -2085,6 +2091,8 @@ static void mmc_blk_mq_complete_rq(struct mmc_queue *mq, struct request *req)
>         } else if (!blk_rq_bytes(req)) {
>                 __blk_mq_end_request(req, BLK_STS_IOERR);
>         } else if (mqrq->retries++ < MMC_MAX_RETRIES) {
> +               if (rq_data_dir(req) == WRITE)
> +                       mqrq->flags |= MQRQ_XFER_SINGLE_BLOCK;
>                 blk_mq_requeue_request(req, true);
>         } else {
>                 if (mmc_card_removed(mq->card))
> diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
> index 1498840a4ea0..c254e6580afd 100644
> --- a/drivers/mmc/core/queue.h
> +++ b/drivers/mmc/core/queue.h
> @@ -61,6 +61,8 @@ enum mmc_drv_op {
>         MMC_DRV_OP_GET_EXT_CSD,
>  };
>
> +#define        MQRQ_XFER_SINGLE_BLOCK          BIT(0)
> +
>  struct mmc_queue_req {
>         struct mmc_blk_request  brq;
>         struct scatterlist      *sg;
> @@ -69,6 +71,7 @@ struct mmc_queue_req {
>         void                    *drv_op_data;
>         unsigned int            ioc_count;
>         int                     retries;
> +       u32                     flags;
>  };
>
>  struct mmc_queue {
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3] mmc: block: use single block write in retry
  2026-03-26 12:33     ` Ulf Hansson
@ 2026-03-26 13:41       ` Bin Liu
  0 siblings, 0 replies; 30+ messages in thread
From: Bin Liu @ 2026-03-26 13:41 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: linux-block, linux-mmc, axboe, shawn.lin

Hi Ulf,

On Thu, Mar 26, 2026 at 01:33:58PM +0100, Ulf Hansson wrote:
> On Wed, 25 Mar 2026 at 14:49, Bin Liu <b-liu@ti.com> wrote:
> >
> > Due to errata i2493[0], multi-block write would still fail in retries.
> >
> > With i2493, the MMC interface has the potential of write failures when
> > issuing multi-block writes operating in HS200 mode with excessive IO
> > supply noise.
> >
> > While the errata provides guidance in hardware design and layout to
> > minimize the IO supply noise, in theory the write failure cannot be
> > resolved in hardware. The software solution to ensure the data integrity
> > is to add minimum 5us delay between block writes. Single-block write is
> > the practical way to introduce the delay.
> >
> > 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.
> >
> > [0] https://www.ti.com/lit/pdf/sprz582
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Bin Liu <b-liu@ti.com>
> 
> Applied for next and by adding a suggested-by tag from Jens, thanks!

Thanks.

> 
> Please consider sending a patch on top, that decreases the data size
> of struct mmc_queue_req.

Will do, in a few days or next week.

-Bin.

> 
> Kind regards
> Uffe
> 
> 
> 
> > ---
> > v3: move the flag to struct mmc_queue_req to reduce the scope
> >     cc stable@
> >
> > v2: add errata details in the commit message.
> >
> > link to v2:
> >     https://lore.kernel.org/linux-mmc/20260310160408.3976760-1-b-liu@ti.com/T/#mb201075c9c466a78b58dbb0bf1f39139f0928591
> >
> > link to v1:
> >     https://lore.kernel.org/linux-mmc/20260310160408.3976760-1-b-liu@ti.com/T/#u
> >
> >  drivers/mmc/core/block.c | 12 ++++++++++--
> >  drivers/mmc/core/queue.h |  3 +++
> >  2 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> > index 05ee76cb0a08..db8c99c73a61 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -1401,6 +1401,9 @@ static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
> >                     rq_data_dir(req) == WRITE &&
> >                     (md->flags & MMC_BLK_REL_WR);
> >
> > +       if (mqrq->flags & MQRQ_XFER_SINGLE_BLOCK)
> > +               recovery_mode = 1;
> > +
> >         memset(brq, 0, sizeof(struct mmc_blk_request));
> >
> >         mmc_crypto_prepare_req(mqrq);
> > @@ -1540,10 +1543,13 @@ static void mmc_blk_cqe_complete_rq(struct mmc_queue *mq, struct request *req)
> >                 err = 0;
> >
> >         if (err) {
> > -               if (mqrq->retries++ < MMC_CQE_RETRIES)
> > +               if (mqrq->retries++ < MMC_CQE_RETRIES) {
> > +                       if (rq_data_dir(req) == WRITE)
> > +                               mqrq->flags |= MQRQ_XFER_SINGLE_BLOCK;
> >                         blk_mq_requeue_request(req, true);
> > -               else
> > +               } else {
> >                         blk_mq_end_request(req, BLK_STS_IOERR);
> > +               }
> >         } else if (mrq->data) {
> >                 if (blk_update_request(req, BLK_STS_OK, mrq->data->bytes_xfered))
> >                         blk_mq_requeue_request(req, true);
> > @@ -2085,6 +2091,8 @@ static void mmc_blk_mq_complete_rq(struct mmc_queue *mq, struct request *req)
> >         } else if (!blk_rq_bytes(req)) {
> >                 __blk_mq_end_request(req, BLK_STS_IOERR);
> >         } else if (mqrq->retries++ < MMC_MAX_RETRIES) {
> > +               if (rq_data_dir(req) == WRITE)
> > +                       mqrq->flags |= MQRQ_XFER_SINGLE_BLOCK;
> >                 blk_mq_requeue_request(req, true);
> >         } else {
> >                 if (mmc_card_removed(mq->card))
> > diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
> > index 1498840a4ea0..c254e6580afd 100644
> > --- a/drivers/mmc/core/queue.h
> > +++ b/drivers/mmc/core/queue.h
> > @@ -61,6 +61,8 @@ enum mmc_drv_op {
> >         MMC_DRV_OP_GET_EXT_CSD,
> >  };
> >
> > +#define        MQRQ_XFER_SINGLE_BLOCK          BIT(0)
> > +
> >  struct mmc_queue_req {
> >         struct mmc_blk_request  brq;
> >         struct scatterlist      *sg;
> > @@ -69,6 +71,7 @@ struct mmc_queue_req {
> >         void                    *drv_op_data;
> >         unsigned int            ioc_count;
> >         int                     retries;
> > +       u32                     flags;
> >  };
> >
> >  struct mmc_queue {
> > --
> > 2.34.1
> >

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2026-03-26 13:41 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-10 16:04 [PATCH] mmc: block: use single block write in retry Bin Liu
2026-03-16 15:01 ` Ulf Hansson
2026-03-16 16:22   ` Bin Liu
2026-03-24 12:44     ` Ulf Hansson
2026-03-17 12:46 ` Shawn Lin
2026-03-17 13:24   ` Bin Liu
2026-03-18  1:43     ` Shawn Lin
2026-03-18 14:55       ` Bin Liu
2026-03-18 15:10         ` Bin Liu
2026-03-18 15:17       ` Bin Liu
2026-03-23 13:05         ` Bin Liu
2026-03-24  7:34           ` Shawn Lin
2026-03-24 14:34 ` [PATCH v2 0/2] " Bin Liu
2026-03-24 14:34   ` [PATCH v2 1/2] block: define new rqf_flags RQF_XFER_SINGLE_BLK Bin Liu
2026-03-24 18:48     ` Jens Axboe
2026-03-24 19:11       ` Bin Liu
2026-03-24 19:16         ` Jens Axboe
2026-03-24 20:41           ` Bin Liu
2026-03-24 14:34   ` [PATCH v2 2/2] mmc: block: use single block write in retry Bin Liu
2026-03-24 14:57   ` [PATCH v2 0/2] " Ulf Hansson
2026-03-24 19:17     ` Jens Axboe
2026-03-25 13:49   ` [PATCH v3] " Bin Liu
2026-03-25 14:12     ` Jens Axboe
2026-03-25 14:21       ` Bin Liu
2026-03-25 14:23         ` Jens Axboe
2026-03-25 14:27           ` Bin Liu
2026-03-25 15:22             ` Ulf Hansson
2026-03-25 15:29               ` Bin Liu
2026-03-26 12:33     ` Ulf Hansson
2026-03-26 13:41       ` Bin Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox