Re: [PATCH v5 3/9] mmc: core: Add open-ended Ext memory addressing

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

On 4/09/24 12:55, Avri Altman wrote:
>>>>>                       (do_data_tag ? (1 << 29) : 0);
>>>>>               brq->sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
>>>>>               brq->mrq.sbc = &brq->sbc;
>>>>> +     } 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));
>>>>
>>>> Did you consider having mmc_start_request() send CMD22?
>>>> e.g.
>>>>
>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
>>>> d6c819dd68ed..22677a01c0e3 100644
>>>> --- a/drivers/mmc/core/core.c
>>>> +++ b/drivers/mmc/core/core.c
>>>> @@ -336,6 +336,9 @@ int mmc_start_request(struct mmc_host *host,
>>>> struct mmc_request *mrq)  {
>>>>         int err;
>>>>
>>>> +       if (mrq->cmd && mrq->cmd->ext_addr)
>>>> +               mmc_send_ext_addr(card->host, mrq->cmd->ext_addr);
> Oh , and yes - To state the obvious: async reqs are working fine now.
> Not sure why it wasn't when I was calling it from mmc_blk_rw_rq_prep.
> 
> Thanks,
> Avri
> 
>>>> +
>>> Will give it a try.
>> In the proposed form, this doesn't work.
>> Mainly because mmc_send_ext_addr eventually calls mmc_start_request by
>> itself:
>> mmc_wait_for_cmd() -> mmc_wait_for_req() -> __mmc_start_req() ->
>> mmc_start_request().
>>
>> Also, since need to call CMD22 for any address, it is ok mrq->cmd->ext_addr to be
>> 0x0, Then need it to be a little bit bulkier,  e.g.:

Sorry about that

>> by adding a "check_sduc" argument to mmc_start_request, if (mrq->cmd &&
>> check_sduc)) And make it true in mmc_blk_mq_issue_rw_rq and
>> mmc_blk_read_single, false otherwise, But this seems to be an invalid option to
>> me.

Probably better to put has_ext_addr flag into mmc_command.

>>
>> And there is that thing of adding dword to mmc_command.
>>
>> What do you think?

Adding to mmc_command should be OK.  If you want to save space,
it looks like 'flags' has many unused bits.

diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index f0ac2e469b32..ceb521e3598f 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -17,7 +17,7 @@ struct mmc_command {
 #define MMC_CMD23_ARG_REL_WR	(1 << 31)
 #define MMC_CMD23_ARG_TAG_REQ	(1 << 29)
 	u32			resp[4];
-	unsigned int		flags;		/* expected response type */
+	u16			flags;		/* expected response type */
 #define MMC_RSP_PRESENT	(1 << 0)
 #define MMC_RSP_136	(1 << 1)		/* 136 bit response */
 #define MMC_RSP_CRC	(1 << 2)		/* expect valid crc */
@@ -76,6 +76,10 @@ struct mmc_command {
  */
 #define mmc_cmd_type(cmd)	((cmd)->flags & MMC_CMD_MASK)
 
+/* For SDUC */
+	u8			has_ext_addr;
+	u8			ext_addr;
+
 	unsigned int		retries;	/* max number of retries */
 	int			error;		/* command error */
 

>>
>> Thanks,
>> Avri
>>>
>>> Thanks,
>>> Avri
>>>
>>>>         init_completion(&mrq->cmd_completion);
>>>>
>>>>         mmc_retune_hold(host);
>>>>
>>>>>       }
>>>>>  }
>>>>>
>