Hi Loic,
On Thu, 26 Jan 2023 at 02:24, Loic Poulain <loic.poul...@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.poul...@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 <s...@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
