On 27/01/24 10:28, Victor Shih wrote:
> On Tue, Jan 23, 2024 at 2:28 PM Victor Shih <victorshihgli@xxxxxxxxx> wrote:
>>
>> From: Victor Shih <victor.shih@xxxxxxxxxxxxxxxxxxx>
>>
>> Embed UHS-II access/control functionality into the MMC request
>> processing flow.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
>> Signed-off-by: Jason Lai <jason.lai@xxxxxxxxxxxxxxxxxxx>
>> Signed-off-by: Victor Shih <victor.shih@xxxxxxxxxxxxxxxxxxx>
>> ---
>>
>> Updates in V13:
>> - Separate __mmc_go_idle() into one patch for re-factorring the code.
>> - Move mmc_decode_scr declaration to sd.h.
>> - Ues uhs2_sd_tran to stead MMC_UHS2_SD_TRAN.
>> - Drop unnecessary comment.
>>
>> Updates in V12:
>> - Use mmc_op_multi() to check DCMD which supports multi read/write
>> in mmc_uhs2_prepare_cmd().
>>
>> Updates in V10:
>> - Move some definitions of PatchV9[02/23] to PatchV10[06/23].
>> - Move some definitions of PatchV9[05/23] to PatchV10[06/23].
>> - Drop do_multi in the mmc_blk_rw_rq_prep().
>> - Use tmode_half_duplex to instead of uhs2_tmode0_flag.
>> - Move entire control of the tmode into mmc_uhs2_prepare_cmd().
>>
>> Updates in V8:
>> - Add MMC_UHS2_SUPPORT to be cleared in sd_uhs2_detect().
>> - Modify return value in sd_uhs2_attach().
>>
>> Updates in V7:
>> - Add mmc_uhs2_card_prepare_cmd helper function in sd_ops.h.
>> - Drop uhs2_state in favor of ios->timing.
>> - Remove unnecessary functions.
>>
>> ---
>>
>> drivers/mmc/core/core.c | 10 +-
>> drivers/mmc/core/sd.c | 10 +-
>> drivers/mmc/core/sd.h | 5 +
>> drivers/mmc/core/sd_ops.c | 9 +
>> drivers/mmc/core/sd_ops.h | 17 +
>> drivers/mmc/core/sd_uhs2.c | 1115 ++++++++++++++++++++++++++++++++++--
>> include/linux/mmc/core.h | 13 +
>> include/linux/mmc/host.h | 15 +
>> 8 files changed, 1155 insertions(+), 39 deletions(-)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 2edf31492a5d..be77cebe1fb8 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -334,6 +334,8 @@ static int mmc_mrq_prep(struct mmc_host *host, struct mmc_request *mrq)
>>
>> int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
>> {
>> + struct uhs2_command uhs2_cmd;
>> + __be32 payload[4]; /* for maximum size */
>> int err;
>>
>> init_completion(&mrq->cmd_completion);
>> @@ -351,6 +353,8 @@ int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
>> if (err)
>> return err;
>>
>> + mmc_uhs2_card_prepare_cmd(host, mrq, uhs2_cmd, payload);
>> +
>> led_trigger_event(host->led, LED_FULL);
>> __mmc_start_request(host, mrq);
>>
>
> Hi, Adrian
>
> I referenced your comments of the V9:
>
> Refer:
> https://patchwork.kernel.org/project/linux-mmc/patch/20230721101349.12387-7-victorshihgli@xxxxxxxxx/
>
> My understanding is as follows, please correct me if there are any mistakes.
> There is already "struct uhs2_command *uhs2_cmd" in struct mmc_command.
> If I also put "__be32 payload[4]" in struct mmc_command.
> The code will become:
> mmc_uhs2_card_prepare_cmd(host, mrq, *mrq->cmd->uhs2_cmd, mrq->cmd->payload);
> But a null pointer problem occurs when sending commands like COM0(mmc_go_idle).
> In this case I just can only plan for the time being as follows:
>
> if (mrq->cmd->uhs2_cmd)
> mmc_uhs2_card_prepare_cmd(host, mrq, *mrq->cmd->uhs2_cmd,
> mrq->cmd->payload);
> else
> mmc_uhs2_card_prepare_cmd(host, mrq, uhs2_cmd, payload);
>
> Would you give me any other advice?
struct uhs2_command uhs2_cmd should not be on the stack local to
mmc_start_request(). Probably moving it to struct mmc_request
is as good as any other option. So starting like below and
then with whatever other changes are needed to make it work.
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index be77cebe1fb8..68496c51a521 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -334,8 +334,6 @@ static int mmc_mrq_prep(struct mmc_host *host, struct mmc_request *mrq)
int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
{
- struct uhs2_command uhs2_cmd;
- __be32 payload[4]; /* for maximum size */
int err;
init_completion(&mrq->cmd_completion);
@@ -353,7 +351,7 @@ int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
if (err)
return err;
- mmc_uhs2_card_prepare_cmd(host, mrq, uhs2_cmd, payload);
+ mmc_uhs2_card_prepare_cmd(host, mrq);
led_trigger_event(host->led, LED_FULL);
__mmc_start_request(host, mrq);
@@ -434,8 +432,6 @@ EXPORT_SYMBOL(mmc_wait_for_req_done);
*/
int mmc_cqe_start_req(struct mmc_host *host, struct mmc_request *mrq)
{
- struct uhs2_command uhs2_cmd;
- __be32 payload[4]; /* for maximum size */
int err;
/*
@@ -456,7 +452,7 @@ int mmc_cqe_start_req(struct mmc_host *host, struct mmc_request *mrq)
if (err)
goto out_err;
- mmc_uhs2_card_prepare_cmd(host, mrq, uhs2_cmd, payload);
+ mmc_uhs2_card_prepare_cmd(host, mrq);
err = host->cqe_ops->cqe_request(host, mrq);
if (err)
diff --git a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h
index d3a3465c7669..e3af68a52de8 100644
--- a/drivers/mmc/core/sd_ops.h
+++ b/drivers/mmc/core/sd_ops.h
@@ -24,14 +24,10 @@ int mmc_app_sd_status(struct mmc_card *card, void *ssr);
int mmc_app_cmd(struct mmc_host *host, struct mmc_card *card);
void mmc_uhs2_prepare_cmd(struct mmc_host *host, struct mmc_request *mrq);
-static inline void mmc_uhs2_card_prepare_cmd(struct mmc_host *host, struct mmc_request *mrq,
- struct uhs2_command uhs2_cmd, __be32 payload[4])
+static inline void mmc_uhs2_card_prepare_cmd(struct mmc_host *host, struct mmc_request *mrq)
{
- if (host->uhs2_sd_tran) {
- uhs2_cmd.payload = payload;
- mrq->cmd->uhs2_cmd = &uhs2_cmd;
+ if (host->uhs2_sd_tran)
mmc_uhs2_prepare_cmd(host, mrq);
- }
}
static inline int mmc_sd_can_poweroff_notify(struct mmc_card *card)
diff --git a/drivers/mmc/core/sd_uhs2.c b/drivers/mmc/core/sd_uhs2.c
index c46729d85644..9cabb6937dc1 100644
--- a/drivers/mmc/core/sd_uhs2.c
+++ b/drivers/mmc/core/sd_uhs2.c
@@ -1194,6 +1194,7 @@ void mmc_uhs2_prepare_cmd(struct mmc_host *host, struct mmc_request *mrq)
u8 plen;
cmd = mrq->cmd;
+ cmd->uhs2_cmd = &mrq->uhs2_cmd;
header = host->card->uhs2_config.node_id;
if ((cmd->flags & MMC_CMD_MASK) == MMC_CMD_ADTC)
header |= UHS2_PACKET_TYPE_DCMD;
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index f30f6be86f66..83c901794c17 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -23,13 +23,18 @@ enum mmc_blk_status {
MMC_BLK_NEW_REQUEST,
};
+#define UHS2_MAX_PAYLOAD_LEN 2
+#define UHS2_MAX_RESP_LEN 20
+
struct uhs2_command {
u16 header;
u16 arg;
- __be32 *payload;
- u32 payload_len;
- u32 packet_len;
+ __be32 payload[UHS2_MAX_PAYLOAD_LEN];
+ u8 payload_len;
+ u8 packet_len; // TODO: is this really needed?
u8 tmode_half_duplex;
+ u8 uhs2_resp_len; /* UHS2 native cmd resp len */
+ u8 uhs2_resp[UHS2_MAX_RESP_LEN]; /* UHS2 native cmd resp */
};
struct mmc_command {
@@ -119,8 +124,6 @@ struct mmc_command {
struct mmc_request *mrq; /* associated request */
struct uhs2_command *uhs2_cmd; /* UHS2 command */
- u8 *uhs2_resp; /* UHS2 native cmd resp */
- u8 uhs2_resp_len; /* UHS2 native cmd resp len */
};
struct mmc_data {
@@ -179,6 +182,7 @@ struct mmc_request {
const struct bio_crypt_ctx *crypto_ctx;
int crypto_key_slot;
#endif
+ struct uhs2_command uhs2_cmd;
};
struct mmc_card;
