| Message ID | 20240825074141.3171549-5-avri.altman@wdc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add SDUC Support | expand |
> 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
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 >
> 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 --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;