diff mbox series

[v6,3/9] mmc: core: Add open-ended Ext memory addressing

Message ID 20240904145256.3670679-4-avri.altman@wdc.com
State New
Headers show
Series Add SDUC Support | expand

Commit Message

Avri Altman Sept. 4, 2024, 2:52 p.m. UTC
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.

Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/mmc/core/block.c | 6 +++++-
 drivers/mmc/core/core.c  | 3 +++
 include/linux/mmc/core.h | 5 +++++
 3 files changed, 13 insertions(+), 1 deletion(-)

Comments

Adrian Hunter Sept. 6, 2024, 8:02 a.m. UTC | #1
On 4/09/24 17:52, 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.
> 
> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> ---
>  drivers/mmc/core/block.c | 6 +++++-
>  drivers/mmc/core/core.c  | 3 +++
>  include/linux/mmc/core.h | 5 +++++
>  3 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index cc7318089cf2..54469261bc25 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -226,6 +226,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);

Not using mmc_blk_wait_for_idle() anymore.

>  
>  static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk)
>  {
> @@ -1710,7 +1711,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;

arg is 32 bits so not needed

>  	if (!mmc_card_blockaddr(card))
>  		brq->cmd.arg <<= 9;
>  	brq->cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
> @@ -1758,6 +1759,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)) {

The 'else' isn't actually needed, is it?  Might as well keep sbc
and ext_addr separate in this patch.

> +		brq->cmd.ext_addr = (blk_rq_pos(req) >> 32) & 0x3F;

'& 0x3f' should not be needed. i.e. we either validate blk_rq_pos(req)
(no point) or we assume it is valid.

> +		brq->cmd.has_ext_addr = 1;

If you switch to bool, that could use 'true' instead of '1'

>  	}
>  }
>  
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index d6c819dd68ed..a0b2999684b3 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->has_ext_addr)
> +		mmc_send_ext_addr(host, mrq->cmd->ext_addr);
> +
>  	init_completion(&mrq->cmd_completion);
>  
>  	mmc_retune_hold(host);
> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> index f0ac2e469b32..41c21c216584 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -76,6 +76,11 @@ struct mmc_command {
>   */
>  #define mmc_cmd_type(cmd)	((cmd)->flags & MMC_CMD_MASK)
>  
> +	/* for SDUC */
> +	u8 has_ext_addr;
> +	u8 ext_addr;
> +	u16 reserved;
> +
>  	unsigned int		retries;	/* max number of retries */
>  	int			error;		/* command error */
>
Avri Altman Sept. 6, 2024, 8:20 a.m. UTC | #2
> 
> On 4/09/24 17:52, 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.
> >
> > Signed-off-by: Avri Altman <avri.altman@wdc.com>
> > ---
> >  drivers/mmc/core/block.c | 6 +++++-
> >  drivers/mmc/core/core.c  | 3 +++
> >  include/linux/mmc/core.h | 5 +++++
> >  3 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
> > cc7318089cf2..54469261bc25 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -226,6 +226,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);
> 
> Not using mmc_blk_wait_for_idle() anymore.
Done.

> 
> >
> >  static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk)  { @@
> > -1710,7 +1711,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;
> 
> arg is 32 bits so not needed
Done.

> 
> >       if (!mmc_card_blockaddr(card))
> >               brq->cmd.arg <<= 9;
> >       brq->cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
> @@
> > -1758,6 +1759,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)) {
> 
> The 'else' isn't actually needed, is it?  Might as well keep sbc and ext_addr
> separate in this patch.
Sorry - I don't follow what you mean.
Doesn't the else implies open-ended?

> 
> > +             brq->cmd.ext_addr = (blk_rq_pos(req) >> 32) & 0x3F;
> 
> '& 0x3f' should not be needed. i.e. we either validate blk_rq_pos(req) (no point)
> or we assume it is valid.
Done.

> 
> > +             brq->cmd.has_ext_addr = 1;
> 
> If you switch to bool, that could use 'true' instead of '1'
Done.

Thanks,
Avri

> 
> >       }
> >  }
> >
> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
> > d6c819dd68ed..a0b2999684b3 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->has_ext_addr)
> > +             mmc_send_ext_addr(host, mrq->cmd->ext_addr);
> > +
> >       init_completion(&mrq->cmd_completion);
> >
> >       mmc_retune_hold(host);
> > diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h index
> > f0ac2e469b32..41c21c216584 100644
> > --- a/include/linux/mmc/core.h
> > +++ b/include/linux/mmc/core.h
> > @@ -76,6 +76,11 @@ struct mmc_command {
> >   */
> >  #define mmc_cmd_type(cmd)    ((cmd)->flags & MMC_CMD_MASK)
> >
> > +     /* for SDUC */
> > +     u8 has_ext_addr;
> > +     u8 ext_addr;
> > +     u16 reserved;
> > +
> >       unsigned int            retries;        /* max number of retries */
> >       int                     error;          /* command error */
> >
Adrian Hunter Sept. 6, 2024, 8:33 a.m. UTC | #3
On 6/09/24 11:20, Avri Altman wrote:
>>
>> On 4/09/24 17:52, 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.
>>>
>>> Signed-off-by: Avri Altman <avri.altman@wdc.com>
>>> ---
>>>  drivers/mmc/core/block.c | 6 +++++-
>>>  drivers/mmc/core/core.c  | 3 +++
>>>  include/linux/mmc/core.h | 5 +++++
>>>  3 files changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
>>> cc7318089cf2..54469261bc25 100644
>>> --- a/drivers/mmc/core/block.c
>>> +++ b/drivers/mmc/core/block.c
>>> @@ -226,6 +226,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);
>>
>> Not using mmc_blk_wait_for_idle() anymore.
> Done.
> 
>>
>>>
>>>  static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk)  { @@
>>> -1710,7 +1711,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;
>>
>> arg is 32 bits so not needed
> Done.
> 
>>
>>>       if (!mmc_card_blockaddr(card))
>>>               brq->cmd.arg <<= 9;
>>>       brq->cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
>> @@
>>> -1758,6 +1759,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)) {
>>
>> The 'else' isn't actually needed, is it?  Might as well keep sbc and ext_addr
>> separate in this patch.
> Sorry - I don't follow what you mean.
> Doesn't the else implies open-ended?

Disallowing SDUC with close-ended seems like a separate
issue.  The 'else' is not actually required, is it?

> 
>>
>>> +             brq->cmd.ext_addr = (blk_rq_pos(req) >> 32) & 0x3F;
>>
>> '& 0x3f' should not be needed. i.e. we either validate blk_rq_pos(req) (no point)
>> or we assume it is valid.
> Done.
> 
>>
>>> +             brq->cmd.has_ext_addr = 1;
>>
>> If you switch to bool, that could use 'true' instead of '1'
> Done.
> 
> Thanks,
> Avri
> 
>>
>>>       }
>>>  }
>>>
>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
>>> d6c819dd68ed..a0b2999684b3 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->has_ext_addr)
>>> +             mmc_send_ext_addr(host, mrq->cmd->ext_addr);
>>> +
>>>       init_completion(&mrq->cmd_completion);
>>>
>>>       mmc_retune_hold(host);
>>> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h index
>>> f0ac2e469b32..41c21c216584 100644
>>> --- a/include/linux/mmc/core.h
>>> +++ b/include/linux/mmc/core.h
>>> @@ -76,6 +76,11 @@ struct mmc_command {
>>>   */
>>>  #define mmc_cmd_type(cmd)    ((cmd)->flags & MMC_CMD_MASK)
>>>
>>> +     /* for SDUC */
>>> +     u8 has_ext_addr;
>>> +     u8 ext_addr;
>>> +     u16 reserved;
>>> +
>>>       unsigned int            retries;        /* max number of retries */
>>>       int                     error;          /* command error */
>>>
>
Avri Altman Sept. 6, 2024, 10:11 a.m. UTC | #4
> 
> Disallowing SDUC with close-ended seems like a separate issue.  The 'else' is not
> actually required, is it?
Ahha - ok.
I need then to switch patches 3 & 4.

Thanks,
Avri
diff mbox series

Patch

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index cc7318089cf2..54469261bc25 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -226,6 +226,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)
 {
@@ -1710,7 +1711,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;
@@ -1758,6 +1759,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)) {
+		brq->cmd.ext_addr = (blk_rq_pos(req) >> 32) & 0x3F;
+		brq->cmd.has_ext_addr = 1;
 	}
 }
 
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index d6c819dd68ed..a0b2999684b3 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->has_ext_addr)
+		mmc_send_ext_addr(host, mrq->cmd->ext_addr);
+
 	init_completion(&mrq->cmd_completion);
 
 	mmc_retune_hold(host);
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index f0ac2e469b32..41c21c216584 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -76,6 +76,11 @@  struct mmc_command {
  */
 #define mmc_cmd_type(cmd)	((cmd)->flags & MMC_CMD_MASK)
 
+	/* for SDUC */
+	u8 has_ext_addr;
+	u8 ext_addr;
+	u16 reserved;
+
 	unsigned int		retries;	/* max number of retries */
 	int			error;		/* command error */