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

[Adrian Hunter <adrian.hunter@intel.com>]

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
>