diff mbox series

[v4,4/9] mmc: core: Add close-ended Ext memory addressing

Message ID 20240825074141.3171549-5-avri.altman@wdc.com
State New
Headers show
Series Add SDUC Support | expand

Commit Message

Avri Altman Aug. 25, 2024, 7:41 a.m. UTC
In a multi-block data transfer, CMD23 shall precede CMD22. Prepare CMD22
in advance as an additional extension of the mrq, to be handle by the
host once CMD23 is done.

Tested-by: Ricky WU <ricky_wu@realtek.com>
Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/mmc/core/block.c |  7 +++++++
 drivers/mmc/core/core.c  | 18 ++++++++++++++++++
 drivers/mmc/core/queue.h |  1 +
 include/linux/mmc/core.h |  1 +
 4 files changed, 27 insertions(+)

Comments

Avri Altman Aug. 26, 2024, 5:32 a.m. UTC | #1
> In a multi-block data transfer, CMD23 shall precede CMD22. Prepare CMD22
> in advance as an additional extension of the mrq, to be handle by the host
> once CMD23 is done.
I am floundering about the close-ended part of this series.
My main concern is an amid stream of fixes & quirks of bogus hw,
that tends to apply extra logic specifically around acmd12 & acmd23.

Unless someone think it's absolutely necessary to be included,
I would like to drop patches 4, 5, and 6.
What do you think?

Thanks,
Avri

> 
> Tested-by: Ricky WU <ricky_wu@realtek.com>
> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> ---
>  drivers/mmc/core/block.c |  7 +++++++
>  drivers/mmc/core/core.c  | 18 ++++++++++++++++++
> drivers/mmc/core/queue.h |  1 +  include/linux/mmc/core.h |  1 +
>  4 files changed, 27 insertions(+)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
> 8816b3f0a312..7020a568fb79 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -1713,6 +1713,13 @@ static void mmc_blk_rw_rq_prep(struct
> mmc_queue_req *mqrq,
>  			(do_data_tag ? (1 << 29) : 0);
>  		brq->sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
>  		brq->mrq.sbc = &brq->sbc;
> +
> +		if (mmc_card_ult_capacity(card)) {
> +			brq->ext.opcode = SD_ADDR_EXT;
> +			brq->ext.arg = (u32)((blk_rq_pos(req) >> 32) & 0x3F);
> +			brq->ext.flags = MMC_RSP_R1 | MMC_CMD_AC;
> +			brq->mrq.ext = &brq->ext;
> +		}
>  	} else if (mmc_card_ult_capacity(card)) {
>  		mmc_blk_wait_for_idle(mq, card->host);
>  		mmc_send_ext_addr(card->host, blk_rq_pos(req)); diff --git
> a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
> d6c819dd68ed..4808e42d7855 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -184,6 +184,14 @@ void mmc_request_done(struct mmc_host *host,
> struct mmc_request *mrq)
>  				mrq->sbc->resp[2], mrq->sbc->resp[3]);
>  		}
> 
> +		if (mrq->ext) {
> +			pr_debug("%s: req done <CMD%u>: %d: %08x %08x
> %08x %08x\n",
> +				 mmc_hostname(host), mrq->ext->opcode,
> +				 mrq->ext->error,
> +				 mrq->ext->resp[0], mrq->ext->resp[1],
> +				 mrq->ext->resp[2], mrq->ext->resp[3]);
> +		}
> +
>  		pr_debug("%s: req done (CMD%u): %d: %08x %08x %08x
> %08x\n",
>  			mmc_hostname(host), cmd->opcode, err,
>  			cmd->resp[0], cmd->resp[1],
> @@ -270,6 +278,12 @@ static void mmc_mrq_pr_debug(struct mmc_host
> *host, struct mmc_request *mrq,
>  			 mrq->sbc->arg, mrq->sbc->flags);
>  	}
> 
> +	if (mrq->ext) {
> +		pr_debug("<%s: starting CMD%u arg %08x flags %08x>\n",
> +			 mmc_hostname(host), mrq->ext->opcode,
> +			 mrq->ext->arg, mrq->ext->flags);
> +	}
> +
>  	if (mrq->cmd) {
>  		pr_debug("%s: starting %sCMD%u arg %08x flags %08x\n",
>  			 mmc_hostname(host), cqe ? "CQE direct " : "", @@ -
> 309,6 +323,10 @@ static int mmc_mrq_prep(struct mmc_host *host, struct
> mmc_request *mrq)
>  		mrq->sbc->error = 0;
>  		mrq->sbc->mrq = mrq;
>  	}
> +	if (mrq->ext) {
> +		mrq->ext->error = 0;
> +		mrq->ext->mrq = mrq;
> +	}
>  	if (mrq->data) {
>  		if (mrq->data->blksz > host->max_blk_size ||
>  		    mrq->data->blocks > host->max_blk_count || diff --git
> a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h index
> 1498840a4ea0..7e191d7f0461 100644
> --- a/drivers/mmc/core/queue.h
> +++ b/drivers/mmc/core/queue.h
> @@ -40,6 +40,7 @@ struct mmc_blk_ioc_data;  struct mmc_blk_request {
>  	struct mmc_request	mrq;
>  	struct mmc_command	sbc;
> +	struct mmc_command	ext;
>  	struct mmc_command	cmd;
>  	struct mmc_command	stop;
>  	struct mmc_data		data;
> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h index
> 2c7928a50907..5560e70cb8d4 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -142,6 +142,7 @@ struct mmc_data {
>  struct mmc_host;
>  struct mmc_request {
>  	struct mmc_command	*sbc;		/* SET_BLOCK_COUNT for
> multiblock */
> +	struct mmc_command	*ext;		/* SD_ADDR_EXT for SDUC */
>  	struct mmc_command	*cmd;
>  	struct mmc_data		*data;
>  	struct mmc_command	*stop;
> --
> 2.25.1
Adrian Hunter Aug. 26, 2024, 6:51 a.m. UTC | #2
On 26/08/24 08:32, Avri Altman wrote:
>> In a multi-block data transfer, CMD23 shall precede CMD22. Prepare CMD22
>> in advance as an additional extension of the mrq, to be handle by the host
>> once CMD23 is done.
> I am floundering about the close-ended part of this series.
> My main concern is an amid stream of fixes & quirks of bogus hw,
> that tends to apply extra logic specifically around acmd12 & acmd23.
> 
> Unless someone think it's absolutely necessary to be included,
> I would like to drop patches 4, 5, and 6.
> What do you think?

What are the downsides to supporting open-ended only?

> 
> Thanks,
> Avri
> 
>>
>> Tested-by: Ricky WU <ricky_wu@realtek.com>
>> Signed-off-by: Avri Altman <avri.altman@wdc.com>
>> ---
>>  drivers/mmc/core/block.c |  7 +++++++
>>  drivers/mmc/core/core.c  | 18 ++++++++++++++++++
>> drivers/mmc/core/queue.h |  1 +  include/linux/mmc/core.h |  1 +
>>  4 files changed, 27 insertions(+)
>>
>> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
>> 8816b3f0a312..7020a568fb79 100644
>> --- a/drivers/mmc/core/block.c
>> +++ b/drivers/mmc/core/block.c
>> @@ -1713,6 +1713,13 @@ static void mmc_blk_rw_rq_prep(struct
>> mmc_queue_req *mqrq,
>>  			(do_data_tag ? (1 << 29) : 0);
>>  		brq->sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
>>  		brq->mrq.sbc = &brq->sbc;
>> +
>> +		if (mmc_card_ult_capacity(card)) {
>> +			brq->ext.opcode = SD_ADDR_EXT;
>> +			brq->ext.arg = (u32)((blk_rq_pos(req) >> 32) & 0x3F);
>> +			brq->ext.flags = MMC_RSP_R1 | MMC_CMD_AC;
>> +			brq->mrq.ext = &brq->ext;
>> +		}
>>  	} else if (mmc_card_ult_capacity(card)) {
>>  		mmc_blk_wait_for_idle(mq, card->host);
>>  		mmc_send_ext_addr(card->host, blk_rq_pos(req)); diff --git
>> a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
>> d6c819dd68ed..4808e42d7855 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -184,6 +184,14 @@ void mmc_request_done(struct mmc_host *host,
>> struct mmc_request *mrq)
>>  				mrq->sbc->resp[2], mrq->sbc->resp[3]);
>>  		}
>>
>> +		if (mrq->ext) {
>> +			pr_debug("%s: req done <CMD%u>: %d: %08x %08x
>> %08x %08x\n",
>> +				 mmc_hostname(host), mrq->ext->opcode,
>> +				 mrq->ext->error,
>> +				 mrq->ext->resp[0], mrq->ext->resp[1],
>> +				 mrq->ext->resp[2], mrq->ext->resp[3]);
>> +		}
>> +
>>  		pr_debug("%s: req done (CMD%u): %d: %08x %08x %08x
>> %08x\n",
>>  			mmc_hostname(host), cmd->opcode, err,
>>  			cmd->resp[0], cmd->resp[1],
>> @@ -270,6 +278,12 @@ static void mmc_mrq_pr_debug(struct mmc_host
>> *host, struct mmc_request *mrq,
>>  			 mrq->sbc->arg, mrq->sbc->flags);
>>  	}
>>
>> +	if (mrq->ext) {
>> +		pr_debug("<%s: starting CMD%u arg %08x flags %08x>\n",
>> +			 mmc_hostname(host), mrq->ext->opcode,
>> +			 mrq->ext->arg, mrq->ext->flags);
>> +	}
>> +
>>  	if (mrq->cmd) {
>>  		pr_debug("%s: starting %sCMD%u arg %08x flags %08x\n",
>>  			 mmc_hostname(host), cqe ? "CQE direct " : "", @@ -
>> 309,6 +323,10 @@ static int mmc_mrq_prep(struct mmc_host *host, struct
>> mmc_request *mrq)
>>  		mrq->sbc->error = 0;
>>  		mrq->sbc->mrq = mrq;
>>  	}
>> +	if (mrq->ext) {
>> +		mrq->ext->error = 0;
>> +		mrq->ext->mrq = mrq;
>> +	}
>>  	if (mrq->data) {
>>  		if (mrq->data->blksz > host->max_blk_size ||
>>  		    mrq->data->blocks > host->max_blk_count || diff --git
>> a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h index
>> 1498840a4ea0..7e191d7f0461 100644
>> --- a/drivers/mmc/core/queue.h
>> +++ b/drivers/mmc/core/queue.h
>> @@ -40,6 +40,7 @@ struct mmc_blk_ioc_data;  struct mmc_blk_request {
>>  	struct mmc_request	mrq;
>>  	struct mmc_command	sbc;
>> +	struct mmc_command	ext;
>>  	struct mmc_command	cmd;
>>  	struct mmc_command	stop;
>>  	struct mmc_data		data;
>> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h index
>> 2c7928a50907..5560e70cb8d4 100644
>> --- a/include/linux/mmc/core.h
>> +++ b/include/linux/mmc/core.h
>> @@ -142,6 +142,7 @@ struct mmc_data {
>>  struct mmc_host;
>>  struct mmc_request {
>>  	struct mmc_command	*sbc;		/* SET_BLOCK_COUNT for
>> multiblock */
>> +	struct mmc_command	*ext;		/* SD_ADDR_EXT for SDUC */
>>  	struct mmc_command	*cmd;
>>  	struct mmc_data		*data;
>>  	struct mmc_command	*stop;
>> --
>> 2.25.1
>
Avri Altman Aug. 26, 2024, 7:11 a.m. UTC | #3
> On 26/08/24 08:32, Avri Altman wrote:
> >> In a multi-block data transfer, CMD23 shall precede CMD22. Prepare
> >> CMD22 in advance as an additional extension of the mrq, to be handle
> >> by the host once CMD23 is done.
> > I am floundering about the close-ended part of this series.
> > My main concern is an amid stream of fixes & quirks of bogus hw, that
> > tends to apply extra logic specifically around acmd12 & acmd23.
> >
> > Unless someone think it's absolutely necessary to be included, I would
> > like to drop patches 4, 5, and 6.
> > What do you think?
> 
> What are the downsides to supporting open-ended only?
I guess all the reasons for inventing close-ended in the first place...
Still, it could be added later, should those cards become ubiquitous, if any.

Thanks,
Avri
> 
> >
> > Thanks,
> > Avri
> >
> >>
> >> Tested-by: Ricky WU <ricky_wu@realtek.com>
> >> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> >> ---
> >>  drivers/mmc/core/block.c |  7 +++++++  drivers/mmc/core/core.c  | 18
> >> ++++++++++++++++++ drivers/mmc/core/queue.h |  1 +
> >> include/linux/mmc/core.h |  1 +
> >>  4 files changed, 27 insertions(+)
> >>
> >> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> >> index
> >> 8816b3f0a312..7020a568fb79 100644
> >> --- a/drivers/mmc/core/block.c
> >> +++ b/drivers/mmc/core/block.c
> >> @@ -1713,6 +1713,13 @@ static void mmc_blk_rw_rq_prep(struct
> >> mmc_queue_req *mqrq,
> >>                      (do_data_tag ? (1 << 29) : 0);
> >>              brq->sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
> >>              brq->mrq.sbc = &brq->sbc;
> >> +
> >> +            if (mmc_card_ult_capacity(card)) {
> >> +                    brq->ext.opcode = SD_ADDR_EXT;
> >> +                    brq->ext.arg = (u32)((blk_rq_pos(req) >> 32) & 0x3F);
> >> +                    brq->ext.flags = MMC_RSP_R1 | MMC_CMD_AC;
> >> +                    brq->mrq.ext = &brq->ext;
> >> +            }
> >>      } else if (mmc_card_ult_capacity(card)) {
> >>              mmc_blk_wait_for_idle(mq, card->host);
> >>              mmc_send_ext_addr(card->host, blk_rq_pos(req)); diff
> >> --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
> >> d6c819dd68ed..4808e42d7855 100644
> >> --- a/drivers/mmc/core/core.c
> >> +++ b/drivers/mmc/core/core.c
> >> @@ -184,6 +184,14 @@ void mmc_request_done(struct mmc_host
> *host,
> >> struct mmc_request *mrq)
> >>                              mrq->sbc->resp[2], mrq->sbc->resp[3]);
> >>              }
> >>
> >> +            if (mrq->ext) {
> >> +                    pr_debug("%s: req done <CMD%u>: %d: %08x %08x
> >> %08x %08x\n",
> >> +                             mmc_hostname(host), mrq->ext->opcode,
> >> +                             mrq->ext->error,
> >> +                             mrq->ext->resp[0], mrq->ext->resp[1],
> >> +                             mrq->ext->resp[2], mrq->ext->resp[3]);
> >> +            }
> >> +
> >>              pr_debug("%s: req done (CMD%u): %d: %08x %08x %08x
> >> %08x\n",
> >>                      mmc_hostname(host), cmd->opcode, err,
> >>                      cmd->resp[0], cmd->resp[1], @@ -270,6 +278,12 @@
> >> static void mmc_mrq_pr_debug(struct mmc_host *host, struct
> >> mmc_request *mrq,
> >>                       mrq->sbc->arg, mrq->sbc->flags);
> >>      }
> >>
> >> +    if (mrq->ext) {
> >> +            pr_debug("<%s: starting CMD%u arg %08x flags %08x>\n",
> >> +                     mmc_hostname(host), mrq->ext->opcode,
> >> +                     mrq->ext->arg, mrq->ext->flags);
> >> +    }
> >> +
> >>      if (mrq->cmd) {
> >>              pr_debug("%s: starting %sCMD%u arg %08x flags %08x\n",
> >>                       mmc_hostname(host), cqe ? "CQE direct " : "",
> >> @@ -
> >> 309,6 +323,10 @@ static int mmc_mrq_prep(struct mmc_host *host,
> >> struct mmc_request *mrq)
> >>              mrq->sbc->error = 0;
> >>              mrq->sbc->mrq = mrq;
> >>      }
> >> +    if (mrq->ext) {
> >> +            mrq->ext->error = 0;
> >> +            mrq->ext->mrq = mrq;
> >> +    }
> >>      if (mrq->data) {
> >>              if (mrq->data->blksz > host->max_blk_size ||
> >>                  mrq->data->blocks > host->max_blk_count || diff
> >> --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h index
> >> 1498840a4ea0..7e191d7f0461 100644
> >> --- a/drivers/mmc/core/queue.h
> >> +++ b/drivers/mmc/core/queue.h
> >> @@ -40,6 +40,7 @@ struct mmc_blk_ioc_data;  struct mmc_blk_request {
> >>      struct mmc_request      mrq;
> >>      struct mmc_command      sbc;
> >> +    struct mmc_command      ext;
> >>      struct mmc_command      cmd;
> >>      struct mmc_command      stop;
> >>      struct mmc_data         data;
> >> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> >> index
> >> 2c7928a50907..5560e70cb8d4 100644
> >> --- a/include/linux/mmc/core.h
> >> +++ b/include/linux/mmc/core.h
> >> @@ -142,6 +142,7 @@ struct mmc_data {  struct mmc_host;  struct
> >> mmc_request {
> >>      struct mmc_command      *sbc;           /* SET_BLOCK_COUNT for
> >> multiblock */
> >> +    struct mmc_command      *ext;           /* SD_ADDR_EXT for SDUC */
> >>      struct mmc_command      *cmd;
> >>      struct mmc_data         *data;
> >>      struct mmc_command      *stop;
> >> --
> >> 2.25.1
> >
diff mbox series

Patch

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 8816b3f0a312..7020a568fb79 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1713,6 +1713,13 @@  static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
 			(do_data_tag ? (1 << 29) : 0);
 		brq->sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
 		brq->mrq.sbc = &brq->sbc;
+
+		if (mmc_card_ult_capacity(card)) {
+			brq->ext.opcode = SD_ADDR_EXT;
+			brq->ext.arg = (u32)((blk_rq_pos(req) >> 32) & 0x3F);
+			brq->ext.flags = MMC_RSP_R1 | MMC_CMD_AC;
+			brq->mrq.ext = &brq->ext;
+		}
 	} else if (mmc_card_ult_capacity(card)) {
 		mmc_blk_wait_for_idle(mq, card->host);
 		mmc_send_ext_addr(card->host, blk_rq_pos(req));
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index d6c819dd68ed..4808e42d7855 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -184,6 +184,14 @@  void mmc_request_done(struct mmc_host *host, struct mmc_request *mrq)
 				mrq->sbc->resp[2], mrq->sbc->resp[3]);
 		}
 
+		if (mrq->ext) {
+			pr_debug("%s: req done <CMD%u>: %d: %08x %08x %08x %08x\n",
+				 mmc_hostname(host), mrq->ext->opcode,
+				 mrq->ext->error,
+				 mrq->ext->resp[0], mrq->ext->resp[1],
+				 mrq->ext->resp[2], mrq->ext->resp[3]);
+		}
+
 		pr_debug("%s: req done (CMD%u): %d: %08x %08x %08x %08x\n",
 			mmc_hostname(host), cmd->opcode, err,
 			cmd->resp[0], cmd->resp[1],
@@ -270,6 +278,12 @@  static void mmc_mrq_pr_debug(struct mmc_host *host, struct mmc_request *mrq,
 			 mrq->sbc->arg, mrq->sbc->flags);
 	}
 
+	if (mrq->ext) {
+		pr_debug("<%s: starting CMD%u arg %08x flags %08x>\n",
+			 mmc_hostname(host), mrq->ext->opcode,
+			 mrq->ext->arg, mrq->ext->flags);
+	}
+
 	if (mrq->cmd) {
 		pr_debug("%s: starting %sCMD%u arg %08x flags %08x\n",
 			 mmc_hostname(host), cqe ? "CQE direct " : "",
@@ -309,6 +323,10 @@  static int mmc_mrq_prep(struct mmc_host *host, struct mmc_request *mrq)
 		mrq->sbc->error = 0;
 		mrq->sbc->mrq = mrq;
 	}
+	if (mrq->ext) {
+		mrq->ext->error = 0;
+		mrq->ext->mrq = mrq;
+	}
 	if (mrq->data) {
 		if (mrq->data->blksz > host->max_blk_size ||
 		    mrq->data->blocks > host->max_blk_count ||
diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
index 1498840a4ea0..7e191d7f0461 100644
--- a/drivers/mmc/core/queue.h
+++ b/drivers/mmc/core/queue.h
@@ -40,6 +40,7 @@  struct mmc_blk_ioc_data;
 struct mmc_blk_request {
 	struct mmc_request	mrq;
 	struct mmc_command	sbc;
+	struct mmc_command	ext;
 	struct mmc_command	cmd;
 	struct mmc_command	stop;
 	struct mmc_data		data;
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 2c7928a50907..5560e70cb8d4 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -142,6 +142,7 @@  struct mmc_data {
 struct mmc_host;
 struct mmc_request {
 	struct mmc_command	*sbc;		/* SET_BLOCK_COUNT for multiblock */
+	struct mmc_command	*ext;		/* SD_ADDR_EXT for SDUC */
 	struct mmc_command	*cmd;
 	struct mmc_data		*data;
 	struct mmc_command	*stop;