[PATCH v2 1/3] mmc: Check support for TRIM operations

[PATCH v2 1/3] mmc: Check support for TRIM operations

[Loic Poulain]

When secure/insecure TRIM operations are supported.
When used as erase command argument it applies the
erase operation to write blocks instead of erase
groups.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
v2: Add mmc unit test change to the series

 drivers/mmc/mmc.c | 3 +++
 include/mmc.h     | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 210703ea46..e5f5ccb5f4 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -2432,6 +2432,9 @@ static int mmc_startup_v4(struct mmc *mmc)
 
 	mmc->wr_rel_set = ext_csd[EXT_CSD_WR_REL_SET];
 
+	mmc->can_trim =
+		!!(ext_csd[EXT_CSD_SEC_FEATURE] & EXT_CSD_SEC_FEATURE_TRIM_EN);
+
 	return 0;
 error:
 	if (mmc->ext_csd) {
diff --git a/include/mmc.h b/include/mmc.h
index 571fa625d0..f6e23625ca 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -241,6 +241,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
 #define EXT_CSD_HC_WP_GRP_SIZE		221	/* RO */
 #define EXT_CSD_HC_ERASE_GRP_SIZE	224	/* RO */
 #define EXT_CSD_BOOT_MULT		226	/* RO */
+#define EXT_CSD_SEC_FEATURE		231	/* RO */
 #define EXT_CSD_GENERIC_CMD6_TIME       248     /* RO */
 #define EXT_CSD_BKOPS_SUPPORT		502	/* RO */
 
@@ -315,6 +316,8 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
 #define EXT_CSD_WR_DATA_REL_USR		(1 << 0)	/* user data area WR_REL */
 #define EXT_CSD_WR_DATA_REL_GP(x)	(1 << ((x)+1))	/* GP part (x+1) WR_REL */
 
+#define EXT_CSD_SEC_FEATURE_TRIM_EN	(1 << 4) /* Support secure & insecure trim */
+
 #define R1_ILLEGAL_COMMAND		(1 << 22)
 #define R1_APP_CMD			(1 << 5)
 
@@ -687,6 +690,7 @@ struct mmc {
 	uint tran_speed;
 	uint legacy_speed; /* speed for the legacy mode provided by the card */
 	uint read_bl_len;
+	bool can_trim;
 #if CONFIG_IS_ENABLED(MMC_WRITE)
 	uint write_bl_len;
 	uint erase_grp_size;	/* in 512-byte sectors */
-- 
2.34.1

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

[Loic Poulain]

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

[PATCH v2 3/3] test: dm: mmc: Check block erasing boundaries

[Loic Poulain]

Verify that erasing blocks does not impact adjacent ones.
- Write four blocks [0 1 2 3]
- Erase two blocks [ 1 2 ]
- Verify [0 1 2 3 ]

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
v2: Add this change to the series

 test/dm/mmc.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/test/dm/mmc.c b/test/dm/mmc.c
index f744452ff2..b1eb8bee2f 100644
--- a/test/dm/mmc.c
+++ b/test/dm/mmc.c
@@ -30,7 +30,7 @@ static int dm_test_mmc_blk(struct unit_test_state *uts)
 	struct udevice *dev;
 	struct blk_desc *dev_desc;
 	int i;
-	char write[1024], read[1024];
+	char write[4 * 512], read[4 * 512];
 
 	ut_assertok(uclass_get_device(UCLASS_MMC, 0, &dev));
 	ut_assertok(blk_get_device_by_str("mmc", "0", &dev_desc));
@@ -39,14 +39,14 @@ static int dm_test_mmc_blk(struct unit_test_state *uts)
 	ut_asserteq(512, dev_desc->blksz);
 	for (i = 0; i < sizeof(write); i++)
 		write[i] = i;
-	ut_asserteq(2, blk_dwrite(dev_desc, 0, 2, write));
-	ut_asserteq(2, blk_dread(dev_desc, 0, 2, read));
+	ut_asserteq(4, blk_dwrite(dev_desc, 0, 4, write));
+	ut_asserteq(4, blk_dread(dev_desc, 0, 4, read));
 	ut_asserteq_mem(write, read, sizeof(write));
 
-	/* Now erase them */
-	memset(write, '\0', sizeof(write));
-	ut_asserteq(2, blk_derase(dev_desc, 0, 2));
-	ut_asserteq(2, blk_dread(dev_desc, 0, 2, read));
+	/* Now erase two of them [1 - 2] and verify all blocks */
+	memset(&write[512], '\0', 2 * 512);
+	ut_asserteq(2, blk_derase(dev_desc, 1, 2));
+	ut_asserteq(4, blk_dread(dev_desc, 0, 4, read));
 	ut_asserteq_mem(write, read, sizeof(write));
 
 	return 0;
-- 
2.34.1

Re: [PATCH v2 3/3] test: dm: mmc: Check block erasing boundaries

[Simon Glass]

Hi Loic,

On Thu, 26 Jan 2023 at 02:24, Loic Poulain <loic.poulain@linaro.org> wrote:
>
> Verify that erasing blocks does not impact adjacent ones.
> - Write four blocks [0 1 2 3]
> - Erase two blocks [ 1 2 ]
> - Verify [0 1 2 3 ]
>
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
> v2: Add this change to the series
>
>  test/dm/mmc.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

This looks good, but can you add the trim command to
sandbox_mmc_send_cmd()? Then you can test that as well.


>
> diff --git a/test/dm/mmc.c b/test/dm/mmc.c
> index f744452ff2..b1eb8bee2f 100644
> --- a/test/dm/mmc.c
> +++ b/test/dm/mmc.c
> @@ -30,7 +30,7 @@ static int dm_test_mmc_blk(struct unit_test_state *uts)
>         struct udevice *dev;
>         struct blk_desc *dev_desc;
>         int i;
> -       char write[1024], read[1024];
> +       char write[4 * 512], read[4 * 512];
>
>         ut_assertok(uclass_get_device(UCLASS_MMC, 0, &dev));
>         ut_assertok(blk_get_device_by_str("mmc", "0", &dev_desc));
> @@ -39,14 +39,14 @@ static int dm_test_mmc_blk(struct unit_test_state *uts)
>         ut_asserteq(512, dev_desc->blksz);
>         for (i = 0; i < sizeof(write); i++)
>                 write[i] = i;
> -       ut_asserteq(2, blk_dwrite(dev_desc, 0, 2, write));
> -       ut_asserteq(2, blk_dread(dev_desc, 0, 2, read));
> +       ut_asserteq(4, blk_dwrite(dev_desc, 0, 4, write));
> +       ut_asserteq(4, blk_dread(dev_desc, 0, 4, read));
>         ut_asserteq_mem(write, read, sizeof(write));
>
> -       /* Now erase them */
> -       memset(write, '\0', sizeof(write));
> -       ut_asserteq(2, blk_derase(dev_desc, 0, 2));
> -       ut_asserteq(2, blk_dread(dev_desc, 0, 2, read));
> +       /* Now erase two of them [1 - 2] and verify all blocks */
> +       memset(&write[512], '\0', 2 * 512);
> +       ut_asserteq(2, blk_derase(dev_desc, 1, 2));
> +       ut_asserteq(4, blk_dread(dev_desc, 0, 4, read));
>         ut_asserteq_mem(write, read, sizeof(write));
>
>         return 0;
> --
> 2.34.1
>

Regards,
Simon

Re: [PATCH v2 1/3] mmc: Check support for TRIM operations

[Simon Glass]

On Thu, 26 Jan 2023 at 02:24, Loic Poulain <loic.poulain@linaro.org> wrote:
>
> When secure/insecure TRIM operations are supported.
> When used as erase command argument it applies the
> erase operation to write blocks instead of erase
> groups.
>
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
> v2: Add mmc unit test change to the series
>
>  drivers/mmc/mmc.c | 3 +++
>  include/mmc.h     | 4 ++++
>  2 files changed, 7 insertions(+)

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

Re: [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@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@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@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

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

[Loic Poulain]

Hi Simon,

On Sat, 28 Jan 2023 at 23:01, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Loic,
>
> On Thu, 26 Jan 2023 at 02:24, Loic Poulain <loic.poulain@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@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@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?

It would indeed make sense to return an error since mmc_erase does not
perform what we expect. Now, since this behavior exists for a while,
we may also want to keep it for legacy, though it should be a corner
case...

Regards,
Loic

RE: [PATCH v2 1/3] mmc: Check support for TRIM operations

[Jaehoon Chung]

> -----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 1/3] mmc: Check support for TRIM operations
> 
> When secure/insecure TRIM operations are supported.
> When used as erase command argument it applies the
> erase operation to write blocks instead of erase
> groups.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
> v2: Add mmc unit test change to the series
> 
>  drivers/mmc/mmc.c | 3 +++
>  include/mmc.h     | 4 ++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 210703ea46..e5f5ccb5f4 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -2432,6 +2432,9 @@ static int mmc_startup_v4(struct mmc *mmc)
> 
>  	mmc->wr_rel_set = ext_csd[EXT_CSD_WR_REL_SET];
> 
> +	mmc->can_trim =
> +		!!(ext_csd[EXT_CSD_SEC_FEATURE] & EXT_CSD_SEC_FEATURE_TRIM_EN);
> +
>  	return 0;
>  error:
>  	if (mmc->ext_csd) {
> diff --git a/include/mmc.h b/include/mmc.h
> index 571fa625d0..f6e23625ca 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -241,6 +241,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
>  #define EXT_CSD_HC_WP_GRP_SIZE		221	/* RO */
>  #define EXT_CSD_HC_ERASE_GRP_SIZE	224	/* RO */
>  #define EXT_CSD_BOOT_MULT		226	/* RO */
> +#define EXT_CSD_SEC_FEATURE		231	/* RO */
>  #define EXT_CSD_GENERIC_CMD6_TIME       248     /* RO */
>  #define EXT_CSD_BKOPS_SUPPORT		502	/* RO */
> 
> @@ -315,6 +316,8 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
>  #define EXT_CSD_WR_DATA_REL_USR		(1 << 0)	/* user data area WR_REL */
>  #define EXT_CSD_WR_DATA_REL_GP(x)	(1 << ((x)+1))	/* GP part (x+1) WR_REL */
> 
> +#define EXT_CSD_SEC_FEATURE_TRIM_EN	(1 << 4) /* Support secure & insecure trim */
> +
>  #define R1_ILLEGAL_COMMAND		(1 << 22)
>  #define R1_APP_CMD			(1 << 5)
> 
> @@ -687,6 +690,7 @@ struct mmc {
>  	uint tran_speed;
>  	uint legacy_speed; /* speed for the legacy mode provided by the card */
>  	uint read_bl_len;
> +	bool can_trim;
>  #if CONFIG_IS_ENABLED(MMC_WRITE)
>  	uint write_bl_len;
>  	uint erase_grp_size;	/* in 512-byte sectors */
> --
> 2.34.1

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

[Jaehoon Chung]

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

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

[Loic Poulain]

Hi Jaehoon,

On Mon, 6 Feb 2023 at 06:05, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>
> 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?

Not at the moment, but it can be used to support eMMC 'Secure Erase' arg.

Regards,
Loic

Re: [PATCH v2 3/3] test: dm: mmc: Check block erasing boundaries

[Loic Poulain]

On Fri, 27 Jan 2023 at 15:30, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Loic,
>
> On Thu, 26 Jan 2023 at 02:24, Loic Poulain <loic.poulain@linaro.org> wrote:
> >
> > Verify that erasing blocks does not impact adjacent ones.
> > - Write four blocks [0 1 2 3]
> > - Erase two blocks [ 1 2 ]
> > - Verify [0 1 2 3 ]
> >
> > Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> > ---
> > v2: Add this change to the series
> >
> >  test/dm/mmc.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
>
> This looks good, but can you add the trim command to
> sandbox_mmc_send_cmd()? Then you can test that as well.

The TRIM option is not exposed, it is internally managed inside mmc
depending on the need for it. So from a testing perspective, I can
only test that the mmc erase is doing what we ask correctly.

Regards,
Loic

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

[Jaehoon Chung]

Hi Loic,

> -----Original Message-----
> From: Loic Poulain <loic.poulain@linaro.org>
> Sent: Wednesday, February 8, 2023 5:09 PM
> To: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: sjg@chromium.org; peng.fan@nxp.com; u-boot@lists.denx.de
> Subject: Re: [PATCH v2 2/3] mmc: erase: Use TRIM erase when available
> 
> Hi Jaehoon,
> 
> On Mon, 6 Feb 2023 at 06:05, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> >
> > 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?
> 
> Not at the moment, but it can be used to support eMMC 'Secure Erase' arg.

I had mis-read. I had read the MMC_TRIM_ARG as MMC_ERASE_ARG. Thanks for kindly explanation. :)

Best Regards,
Jaehoon Chung

> 
> Regards,
> Loic

Re: [PATCH v2 1/3] mmc: Check support for TRIM operations

[Jaehoon Chung]

On 1/26/23 18:24, Loic Poulain wrote:
> When secure/insecure TRIM operations are supported.
> When used as erase command argument it applies the
> erase operation to write blocks instead of erase
> groups.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>


Applied to u-boot-mmc/master.

Best Regards,
Jaehoon Chung


> ---
> v2: Add mmc unit test change to the series
> 
>  drivers/mmc/mmc.c | 3 +++
>  include/mmc.h     | 4 ++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 210703ea46..e5f5ccb5f4 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -2432,6 +2432,9 @@ static int mmc_startup_v4(struct mmc *mmc)
>  
>  	mmc->wr_rel_set = ext_csd[EXT_CSD_WR_REL_SET];
>  
> +	mmc->can_trim =
> +		!!(ext_csd[EXT_CSD_SEC_FEATURE] & EXT_CSD_SEC_FEATURE_TRIM_EN);
> +
>  	return 0;
>  error:
>  	if (mmc->ext_csd) {
> diff --git a/include/mmc.h b/include/mmc.h
> index 571fa625d0..f6e23625ca 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -241,6 +241,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
>  #define EXT_CSD_HC_WP_GRP_SIZE		221	/* RO */
>  #define EXT_CSD_HC_ERASE_GRP_SIZE	224	/* RO */
>  #define EXT_CSD_BOOT_MULT		226	/* RO */
> +#define EXT_CSD_SEC_FEATURE		231	/* RO */
>  #define EXT_CSD_GENERIC_CMD6_TIME       248     /* RO */
>  #define EXT_CSD_BKOPS_SUPPORT		502	/* RO */
>  
> @@ -315,6 +316,8 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
>  #define EXT_CSD_WR_DATA_REL_USR		(1 << 0)	/* user data area WR_REL */
>  #define EXT_CSD_WR_DATA_REL_GP(x)	(1 << ((x)+1))	/* GP part (x+1) WR_REL */
>  
> +#define EXT_CSD_SEC_FEATURE_TRIM_EN	(1 << 4) /* Support secure & insecure trim */
> +
>  #define R1_ILLEGAL_COMMAND		(1 << 22)
>  #define R1_APP_CMD			(1 << 5)
>  
> @@ -687,6 +690,7 @@ struct mmc {
>  	uint tran_speed;
>  	uint legacy_speed; /* speed for the legacy mode provided by the card */
>  	uint read_bl_len;
> +	bool can_trim;
>  #if CONFIG_IS_ENABLED(MMC_WRITE)
>  	uint write_bl_len;
>  	uint erase_grp_size;	/* in 512-byte sectors */

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

[Jaehoon Chung]

On 1/26/23 18:24, Loic Poulain 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@linaro.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>


Applied to u-boot-mmc/master.

Best Regards,
Jaehoon Chung


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

Re: [PATCH v2 3/3] test: dm: mmc: Check block erasing boundaries

[Jaehoon Chung]

On 1/26/23 18:24, Loic Poulain wrote:
> Verify that erasing blocks does not impact adjacent ones.
> - Write four blocks [0 1 2 3]
> - Erase two blocks [ 1 2 ]
> - Verify [0 1 2 3 ]
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>


Applied to u-boot-mmc/master.

Best Regards,
Jaehoon Chung

> ---
> v2: Add this change to the series
> 
>  test/dm/mmc.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/test/dm/mmc.c b/test/dm/mmc.c
> index f744452ff2..b1eb8bee2f 100644
> --- a/test/dm/mmc.c
> +++ b/test/dm/mmc.c
> @@ -30,7 +30,7 @@ static int dm_test_mmc_blk(struct unit_test_state *uts)
>  	struct udevice *dev;
>  	struct blk_desc *dev_desc;
>  	int i;
> -	char write[1024], read[1024];
> +	char write[4 * 512], read[4 * 512];
>  
>  	ut_assertok(uclass_get_device(UCLASS_MMC, 0, &dev));
>  	ut_assertok(blk_get_device_by_str("mmc", "0", &dev_desc));
> @@ -39,14 +39,14 @@ static int dm_test_mmc_blk(struct unit_test_state *uts)
>  	ut_asserteq(512, dev_desc->blksz);
>  	for (i = 0; i < sizeof(write); i++)
>  		write[i] = i;
> -	ut_asserteq(2, blk_dwrite(dev_desc, 0, 2, write));
> -	ut_asserteq(2, blk_dread(dev_desc, 0, 2, read));
> +	ut_asserteq(4, blk_dwrite(dev_desc, 0, 4, write));
> +	ut_asserteq(4, blk_dread(dev_desc, 0, 4, read));
>  	ut_asserteq_mem(write, read, sizeof(write));
>  
> -	/* Now erase them */
> -	memset(write, '\0', sizeof(write));
> -	ut_asserteq(2, blk_derase(dev_desc, 0, 2));
> -	ut_asserteq(2, blk_dread(dev_desc, 0, 2, read));
> +	/* Now erase two of them [1 - 2] and verify all blocks */
> +	memset(&write[512], '\0', 2 * 512);
> +	ut_asserteq(2, blk_derase(dev_desc, 1, 2));
> +	ut_asserteq(4, blk_dread(dev_desc, 0, 4, read));
>  	ut_asserteq_mem(write, read, sizeof(write));
>  
>  	return 0;