Re: [PATCH v4 9/9] mmc: core: Adjust ACMD22 to SDUC

[Adrian Hunter <adrian.hunter@intel.com>]

On 25/08/24 10:41, 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.
> 
> Moreover, SD cards expect to be allowed the full 500msec busy period
> post write operations.  This is true for standard capacity SD, and even
> more so for high volume SD cards, specifically SDUC.  If CMD13 return an
> error bit, the recovery flow is entered regardless of the busy period.
> Thus, better enforce the busy period for SDUC, otherwise it might return
> a bogus bytes written.
> 
> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> ---
>  drivers/mmc/core/block.c | 37 +++++++++++++++++++++++++++++++------
>  1 file changed, 31 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index ace701273230..b73fdef1cb0d 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -48,6 +48,7 @@
>  #include <linux/mmc/sd.h>
>  
>  #include <linux/uaccess.h>
> +#include <asm/unaligned.h>
>  
>  #include "queue.h"
>  #include "block.h"
> @@ -948,13 +949,20 @@ 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;
>  
> +	/*
> +	 * SD cards, specifically high volume cards, expect to be allowed with the
> +	 * full 500msec busy period post write. Otherwise, they may not indicate
> +	 * correctly the number of bytes written.
> +	 */
> +	if (mmc_card_ult_capacity(card))
> +		mmc_delay(500);

To get here, it should have had to go through:

	/* Try to get back to "tran" state */
	if (!mmc_host_is_spi(mq->card->host) &&
	    (err || !mmc_ready_for_data(status)))
		err = mmc_blk_fix_state(mq->card, req);

which would mean the card is not indicating "busy".
Either that is not working, or it seems like an issue
with the card, in which case it could be a card quirk
perhaps.

> +
>  	err = mmc_app_cmd(card->host, card);
>  	if (err)
>  		return err;
> @@ -963,7 +971,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;
> @@ -973,15 +988,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)