| Message ID | 20240827122342.3314173-4-avri.altman@wdc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add SDUC Support | expand |
On 27/08/24 15:23, Avri Altman wrote: > For open-ended read/write - just send CMD22 before issuing the command. > While at it, make sure that the rw command arg is properly casting the > lower 32 bits, as it can be larger now. > > Tested-by: Ricky WU <ricky_wu@realtek.com> > Signed-off-by: Avri Altman <avri.altman@wdc.com> > --- > drivers/mmc/core/block.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c > index 2c9963248fcb..8816b3f0a312 100644 > --- a/drivers/mmc/core/block.c > +++ b/drivers/mmc/core/block.c > @@ -180,6 +180,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq, > static void mmc_blk_hsq_req_done(struct mmc_request *mrq); > static int mmc_spi_err_check(struct mmc_card *card); > static int mmc_blk_busy_cb(void *cb_data, bool *busy); > +static int mmc_blk_wait_for_idle(struct mmc_queue *mq, struct mmc_host *host); > > static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk) > { > @@ -1664,7 +1665,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq, > > brq->mrq.cmd = &brq->cmd; > > - brq->cmd.arg = blk_rq_pos(req); > + brq->cmd.arg = blk_rq_pos(req) & 0xFFFFFFFF; > if (!mmc_card_blockaddr(card)) > brq->cmd.arg <<= 9; > brq->cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC; > @@ -1712,6 +1713,9 @@ 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; > + } 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); + init_completion(&mrq->cmd_completion); mmc_retune_hold(host); > } > } >
> On 27/08/24 15:23, Avri Altman wrote: > > For open-ended read/write - just send CMD22 before issuing the command. > > While at it, make sure that the rw command arg is properly casting the > > lower 32 bits, as it can be larger now. > > > > Tested-by: Ricky WU <ricky_wu@realtek.com> > > Signed-off-by: Avri Altman <avri.altman@wdc.com> > > --- > > drivers/mmc/core/block.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index > > 2c9963248fcb..8816b3f0a312 100644 > > --- a/drivers/mmc/core/block.c > > +++ b/drivers/mmc/core/block.c > > @@ -180,6 +180,7 @@ static void mmc_blk_rw_rq_prep(struct > > mmc_queue_req *mqrq, static void mmc_blk_hsq_req_done(struct > > mmc_request *mrq); static int mmc_spi_err_check(struct mmc_card > > *card); static int mmc_blk_busy_cb(void *cb_data, bool *busy); > > +static int mmc_blk_wait_for_idle(struct mmc_queue *mq, struct > > +mmc_host *host); > > > > static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk) { @@ > > -1664,7 +1665,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req > > *mqrq, > > > > brq->mrq.cmd = &brq->cmd; > > > > - brq->cmd.arg = blk_rq_pos(req); > > + brq->cmd.arg = blk_rq_pos(req) & 0xFFFFFFFF; > > if (!mmc_card_blockaddr(card)) > > brq->cmd.arg <<= 9; > > brq->cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC; > @@ > > -1712,6 +1713,9 @@ 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; > > + } 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); > + Will give it a try. Thanks, Avri > init_completion(&mrq->cmd_completion); > > mmc_retune_hold(host); > > > } > > } > >
> > On 27/08/24 15:23, Avri Altman wrote: > > > For open-ended read/write - just send CMD22 before issuing the command. > > > While at it, make sure that the rw command arg is properly casting > > > the lower 32 bits, as it can be larger now. > > > > > > Tested-by: Ricky WU <ricky_wu@realtek.com> > > > Signed-off-by: Avri Altman <avri.altman@wdc.com> > > > --- > > > drivers/mmc/core/block.c | 6 +++++- > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c > > > index > > > 2c9963248fcb..8816b3f0a312 100644 > > > --- a/drivers/mmc/core/block.c > > > +++ b/drivers/mmc/core/block.c > > > @@ -180,6 +180,7 @@ static void mmc_blk_rw_rq_prep(struct > > > mmc_queue_req *mqrq, static void mmc_blk_hsq_req_done(struct > > > mmc_request *mrq); static int mmc_spi_err_check(struct mmc_card > > > *card); static int mmc_blk_busy_cb(void *cb_data, bool *busy); > > > +static int mmc_blk_wait_for_idle(struct mmc_queue *mq, struct > > > +mmc_host *host); > > > > > > static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk) { @@ > > > -1664,7 +1665,7 @@ static void mmc_blk_rw_rq_prep(struct > > > mmc_queue_req *mqrq, > > > > > > brq->mrq.cmd = &brq->cmd; > > > > > > - brq->cmd.arg = blk_rq_pos(req); > > > + brq->cmd.arg = blk_rq_pos(req) & 0xFFFFFFFF; > > > if (!mmc_card_blockaddr(card)) > > > brq->cmd.arg <<= 9; > > > brq->cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC; > > @@ > > > -1712,6 +1713,9 @@ 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; > > > + } 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); > > + > 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.: 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. And there is that thing of adding dword to mmc_command. What do you think? Thanks, Avri > > Thanks, > Avri > > > init_completion(&mrq->cmd_completion); > > > > mmc_retune_hold(host); > > > > > } > > > } > > >
> > > > (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.: > 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. > > And there is that thing of adding dword to mmc_command. > > What do you think? > > Thanks, > Avri > > > > Thanks, > > Avri > > > > > init_completion(&mrq->cmd_completion); > > > > > > mmc_retune_hold(host); > > > > > > > } > > > > } > > > >
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); >>>> >>>>> } >>>>> } >>>>> >
On 4/09/24 14:11, Adrian Hunter wrote: > 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 */ Although retries is also way bigger than needed. However, maybe start by just making mmc_command bigger and see if anyone complains. > int error; /* command error */ > > >>> >>> Thanks, >>> Avri >>>> >>>> Thanks, >>>> Avri >>>> >>>>> init_completion(&mrq->cmd_completion); >>>>> >>>>> mmc_retune_hold(host); >>>>> >>>>>> } >>>>>> } >>>>>> >> >
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index 2c9963248fcb..8816b3f0a312 100644 --- a/drivers/mmc/core/block.c +++ b/drivers/mmc/core/block.c @@ -180,6 +180,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq, static void mmc_blk_hsq_req_done(struct mmc_request *mrq); static int mmc_spi_err_check(struct mmc_card *card); static int mmc_blk_busy_cb(void *cb_data, bool *busy); +static int mmc_blk_wait_for_idle(struct mmc_queue *mq, struct mmc_host *host); static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk) { @@ -1664,7 +1665,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq, brq->mrq.cmd = &brq->cmd; - brq->cmd.arg = blk_rq_pos(req); + brq->cmd.arg = blk_rq_pos(req) & 0xFFFFFFFF; if (!mmc_card_blockaddr(card)) brq->cmd.arg <<= 9; brq->cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC; @@ -1712,6 +1713,9 @@ 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; + } 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)); } }