RE: [PATCH v2 2/3] mmc: erase: Use TRIM erase when available

["Jaehoon Chung" <jh80.chung@samsung.com>]

Hi,

> -----Original Message-----
> From: Loic Poulain <loic.poulain@linaro.org>
> Sent: Thursday, January 26, 2023 6:24 PM
> To: sjg@chromium.org; peng.fan@nxp.com; jh80.chung@samsung.com
> Cc: u-boot@lists.denx.de; Loic Poulain <loic.poulain@linaro.org>
> Subject: [PATCH v2 2/3] mmc: erase: Use TRIM erase when available
> 
> The default erase command applies on erase group unit, and
> simply round down to erase group size. When the start block
> is not aligned to erase group size (e.g. erasing partition)
> it causes unwanted erasing of the previous blocks, part of
> the same erase group (e.g. owned by other logical partition,
> or by the partition table itself).
> 
> To prevent this issue, a simple solution is to use TRIM as
> argument of the Erase command, which is usually supported
> with eMMC > 4.0, and allow to apply erase operation to write
> blocks instead of erase group
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
> v2: Add mmc unit test change to the series
> 
>  drivers/mmc/mmc_write.c | 34 +++++++++++++++++++++++-----------
>  1 file changed, 23 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
> index 5b7aeeb012..a6f93380dd 100644
> --- a/drivers/mmc/mmc_write.c
> +++ b/drivers/mmc/mmc_write.c
> @@ -15,7 +15,7 @@
>  #include <linux/math64.h>
>  #include "mmc_private.h"
> 
> -static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
> +static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt, u32 args)
>  {
>  	struct mmc_cmd cmd;
>  	ulong end;
> @@ -52,7 +52,7 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
>  		goto err_out;
> 
>  	cmd.cmdidx = MMC_CMD_ERASE;
> -	cmd.cmdarg = MMC_ERASE_ARG;
> +	cmd.cmdarg = args ? args : MMC_ERASE_ARG;

It there any case to pass by other value?

Best Regards,
Jaehoon Chung

>  	cmd.resp_type = MMC_RSP_R1b;
> 
>  	err = mmc_send_cmd(mmc, &cmd, NULL);
> @@ -77,7 +77,7 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt)
>  #endif
>  	int dev_num = block_dev->devnum;
>  	int err = 0;
> -	u32 start_rem, blkcnt_rem;
> +	u32 start_rem, blkcnt_rem, erase_args = 0;
>  	struct mmc *mmc = find_mmc_device(dev_num);
>  	lbaint_t blk = 0, blk_r = 0;
>  	int timeout_ms = 1000;
> @@ -97,13 +97,25 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt)
>  	 */
>  	err = div_u64_rem(start, mmc->erase_grp_size, &start_rem);
>  	err = div_u64_rem(blkcnt, mmc->erase_grp_size, &blkcnt_rem);
> -	if (start_rem || blkcnt_rem)
> -		printf("\n\nCaution! Your devices Erase group is 0x%x\n"
> -		       "The erase range would be change to "
> -		       "0x" LBAF "~0x" LBAF "\n\n",
> -		       mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1),
> -		       ((start + blkcnt + mmc->erase_grp_size - 1)
> -		       & ~(mmc->erase_grp_size - 1)) - 1);
> +	if (start_rem || blkcnt_rem) {
> +		if (mmc->can_trim) {
> +			/* Trim function applies the erase operation to write
> +			 * blocks instead of erase groups.
> +			 */
> +			erase_args = MMC_TRIM_ARG;
> +		} else {
> +			/* The card ignores all LSB's below the erase group
> +			 * size, rounding down the addess to a erase group
> +			 * boundary.
> +			 */
> +			printf("\n\nCaution! Your devices Erase group is 0x%x\n"
> +			       "The erase range would be change to "
> +			       "0x" LBAF "~0x" LBAF "\n\n",
> +			       mmc->erase_grp_size, start & ~(mmc->erase_grp_size - 1),
> +			       ((start + blkcnt + mmc->erase_grp_size - 1)
> +			       & ~(mmc->erase_grp_size - 1)) - 1);
> +		}
> +	}
> 
>  	while (blk < blkcnt) {
>  		if (IS_SD(mmc) && mmc->ssr.au) {
> @@ -113,7 +125,7 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt)
>  			blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ?
>  				mmc->erase_grp_size : (blkcnt - blk);
>  		}
> -		err = mmc_erase_t(mmc, start + blk, blk_r);
> +		err = mmc_erase_t(mmc, start + blk, blk_r, erase_args);
>  		if (err)
>  			break;
> 
> --
> 2.34.1