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

[Simon Glass]

Hi Loic,

On Thu, 26 Jan 2023 at 02:24, Loic Poulain <loic.poulain at linaro.org> wrote:
>
> 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 at linaro.org>
> ---
> v2: Add mmc unit test change to the series
>
>  drivers/mmc/mmc_write.c | 34 +++++++++++++++++++++++-----------
>  1 file changed, 23 insertions(+), 11 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

Please see below

>
> 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;
>         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);

Should this return an error, or just go ahead?

> +               }
> +       }
>
>         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
>

Regards,
Simon