diff mbox series

[v6,7/9] mmc: core: Adjust ACMD22 to SDUC

Message ID 20240904145256.3670679-8-avri.altman@wdc.com (mailing list archive)
State New
Headers show
Series Add SDUC Support | expand

Commit Message

Avri Altman Sept. 4, 2024, 2:52 p.m. UTC
ACMD22 is used to verify the previously write operation.  Normally, it
returns the number of written sectors as u32.  SDUC, however, returns it
as u64.  This is not a superfluous requirement, because SDUC writes may
exceeds 2TB.  For Linux mmc however, the previously write operation
could not be more than the block layer limits, thus we make room for a
u64 and cast the returning value to u32.

Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/mmc/core/block.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

Comments

Adrian Hunter Sept. 6, 2024, 10:57 a.m. UTC | #1
On 4/09/24 17:52, Avri Altman wrote:
> ACMD22 is used to verify the previously write operation.  Normally, it
> returns the number of written sectors as u32.  SDUC, however, returns it
> as u64.  This is not a superfluous requirement, because SDUC writes may
> exceeds 2TB.  For Linux mmc however, the previously write operation
> could not be more than the block layer limits, thus we make room for a
> u64 and cast the returning value to u32.
> 
> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> ---
>  drivers/mmc/core/block.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 50d37c4f5a50..f36611512a1d 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -50,6 +50,7 @@
>  #include <linux/mmc/sd.h>
>  
>  #include <linux/uaccess.h>
> +#include <asm/unaligned.h>
>  
>  #include "queue.h"
>  #include "block.h"
> @@ -994,11 +995,10 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
>  	int err;
>  	u32 result;
>  	__be32 *blocks;
> -
> +	u8 resp_sz;

Could do the initialization here i.e.

	u8 resp_sz = mmc_card_ult_capacity(card) ? 8 : 4;

>  	struct mmc_request mrq = {};
>  	struct mmc_command cmd = {};
>  	struct mmc_data data = {};
> -
>  	struct scatterlist sg;
>  
>  	err = mmc_app_cmd(card->host, card);
> @@ -1009,7 +1009,14 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
>  	cmd.arg = 0;
>  	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
>  
> -	data.blksz = 4;
> +	/*
> +	 * Normally, ACMD22 returns the number of written sectors as u32.
> +	 * SDUC, however, returns it as u64.  This is not a superfluous
> +	 * requirement, because SDUC writes may exceed 2TB.
> +	 */
> +	resp_sz = mmc_card_ult_capacity(card) ? 8 : 4;
> +
> +	data.blksz = resp_sz;
>  	data.blocks = 1;
>  	data.flags = MMC_DATA_READ;
>  	data.sg = &sg;
> @@ -1019,15 +1026,25 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
>  	mrq.cmd = &cmd;
>  	mrq.data = &data;
>  
> -	blocks = kmalloc(4, GFP_KERNEL);
> +	blocks = kmalloc(resp_sz, GFP_KERNEL);

Separate issue, but this should probably be GFP_NOIO, or
just have the buffer in mmc_host or mmc_card

>  	if (!blocks)
>  		return -ENOMEM;
>  
> -	sg_init_one(&sg, blocks, 4);
> +	sg_init_one(&sg, blocks, resp_sz);
>  
>  	mmc_wait_for_req(card->host, &mrq);
>  
> -	result = ntohl(*blocks);
> +	if (mmc_card_ult_capacity(card)) {
> +		u64 blocks_64 = get_unaligned_be64(blocks);
> +		/*
> +		 * For Linux mmc however, the previously write operation could
> +		 * not be more than the block layer limits, thus just make room
> +		 * for a u64 and cast the response back to u32.
> +		 */
> +		result = blocks_64 > UINT_MAX ? UINT_MAX : (u32)blocks_64;

Perhaps:
		result = clamp_val(get_unaligned_be64(blocks), 0, UINT_MAX);

> +	} else {
> +		result = ntohl(*blocks);
> +	}
>  	kfree(blocks);
>  
>  	if (cmd.error || data.error)
Avri Altman Sept. 6, 2024, 11:07 a.m. UTC | #2
> On 4/09/24 17:52, Avri Altman wrote:
> > ACMD22 is used to verify the previously write operation.  Normally, it
> > returns the number of written sectors as u32.  SDUC, however, returns
> > it as u64.  This is not a superfluous requirement, because SDUC writes
> > may exceeds 2TB.  For Linux mmc however, the previously write
> > operation could not be more than the block layer limits, thus we make
> > room for a
> > u64 and cast the returning value to u32.
> >
> > Signed-off-by: Avri Altman <avri.altman@wdc.com>
> > ---
> >  drivers/mmc/core/block.c | 29 +++++++++++++++++++++++------
> >  1 file changed, 23 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
> > 50d37c4f5a50..f36611512a1d 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -50,6 +50,7 @@
> >  #include <linux/mmc/sd.h>
> >
> >  #include <linux/uaccess.h>
> > +#include <asm/unaligned.h>
> >
> >  #include "queue.h"
> >  #include "block.h"
> > @@ -994,11 +995,10 @@ static int mmc_sd_num_wr_blocks(struct mmc_card
> *card, u32 *written_blocks)
> >       int err;
> >       u32 result;
> >       __be32 *blocks;
> > -
> > +     u8 resp_sz;
> 
> Could do the initialization here i.e.
> 
>         u8 resp_sz = mmc_card_ult_capacity(card) ? 8 : 4;
Done.

> 
> >       struct mmc_request mrq = {};
> >       struct mmc_command cmd = {};
> >       struct mmc_data data = {};
> > -
> >       struct scatterlist sg;
> >
> >       err = mmc_app_cmd(card->host, card); @@ -1009,7 +1009,14 @@
> > static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32
> *written_blocks)
> >       cmd.arg = 0;
> >       cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
> >
> > -     data.blksz = 4;
> > +     /*
> > +      * Normally, ACMD22 returns the number of written sectors as u32.
> > +      * SDUC, however, returns it as u64.  This is not a superfluous
> > +      * requirement, because SDUC writes may exceed 2TB.
> > +      */
> > +     resp_sz = mmc_card_ult_capacity(card) ? 8 : 4;
> > +
> > +     data.blksz = resp_sz;
> >       data.blocks = 1;
> >       data.flags = MMC_DATA_READ;
> >       data.sg = &sg;
> > @@ -1019,15 +1026,25 @@ static int mmc_sd_num_wr_blocks(struct
> mmc_card *card, u32 *written_blocks)
> >       mrq.cmd = &cmd;
> >       mrq.data = &data;
> >
> > -     blocks = kmalloc(4, GFP_KERNEL);
> > +     blocks = kmalloc(resp_sz, GFP_KERNEL);
> 
> Separate issue, but this should probably be GFP_NOIO, or just have the buffer in
> mmc_host or mmc_card
OK.  Mark this to be fixed in a subsequent patch.

> 
> >       if (!blocks)
> >               return -ENOMEM;
> >
> > -     sg_init_one(&sg, blocks, 4);
> > +     sg_init_one(&sg, blocks, resp_sz);
> >
> >       mmc_wait_for_req(card->host, &mrq);
> >
> > -     result = ntohl(*blocks);
> > +     if (mmc_card_ult_capacity(card)) {
> > +             u64 blocks_64 = get_unaligned_be64(blocks);
> > +             /*
> > +              * For Linux mmc however, the previously write operation could
> > +              * not be more than the block layer limits, thus just make room
> > +              * for a u64 and cast the response back to u32.
> > +              */
> > +             result = blocks_64 > UINT_MAX ? UINT_MAX :
> > + (u32)blocks_64;
> 
> Perhaps:
>                 result = clamp_val(get_unaligned_be64(blocks), 0, UINT_MAX);
Done.

Thanks,
Avri
> 
> > +     } else {
> > +             result = ntohl(*blocks);
> > +     }
> >       kfree(blocks);
> >
> >       if (cmd.error || data.error)
diff mbox series

Patch

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 50d37c4f5a50..f36611512a1d 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -50,6 +50,7 @@ 
 #include <linux/mmc/sd.h>
 
 #include <linux/uaccess.h>
+#include <asm/unaligned.h>
 
 #include "queue.h"
 #include "block.h"
@@ -994,11 +995,10 @@  static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
 	int err;
 	u32 result;
 	__be32 *blocks;
-
+	u8 resp_sz;
 	struct mmc_request mrq = {};
 	struct mmc_command cmd = {};
 	struct mmc_data data = {};
-
 	struct scatterlist sg;
 
 	err = mmc_app_cmd(card->host, card);
@@ -1009,7 +1009,14 @@  static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
 	cmd.arg = 0;
 	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
 
-	data.blksz = 4;
+	/*
+	 * Normally, ACMD22 returns the number of written sectors as u32.
+	 * SDUC, however, returns it as u64.  This is not a superfluous
+	 * requirement, because SDUC writes may exceed 2TB.
+	 */
+	resp_sz = mmc_card_ult_capacity(card) ? 8 : 4;
+
+	data.blksz = resp_sz;
 	data.blocks = 1;
 	data.flags = MMC_DATA_READ;
 	data.sg = &sg;
@@ -1019,15 +1026,25 @@  static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
 	mrq.cmd = &cmd;
 	mrq.data = &data;
 
-	blocks = kmalloc(4, GFP_KERNEL);
+	blocks = kmalloc(resp_sz, GFP_KERNEL);
 	if (!blocks)
 		return -ENOMEM;
 
-	sg_init_one(&sg, blocks, 4);
+	sg_init_one(&sg, blocks, resp_sz);
 
 	mmc_wait_for_req(card->host, &mrq);
 
-	result = ntohl(*blocks);
+	if (mmc_card_ult_capacity(card)) {
+		u64 blocks_64 = get_unaligned_be64(blocks);
+		/*
+		 * For Linux mmc however, the previously write operation could
+		 * not be more than the block layer limits, thus just make room
+		 * for a u64 and cast the response back to u32.
+		 */
+		result = blocks_64 > UINT_MAX ? UINT_MAX : (u32)blocks_64;
+	} else {
+		result = ntohl(*blocks);
+	}
 	kfree(blocks);
 
 	if (cmd.error || data.error)