2021-05-06 03:31:34

by oracleks043021

[permalink] [raw]
Subject: [PATCH RESEND v4] Clean up and show effect of ERASE_GROUP_DEF

From: Kimito Sakata <[email protected]>

Replaced unused pointer with NULL on calls to strtol().
Added logic to print High Capacity mode parameters of Erase unit size,
Erase Timeout, Write protect Group Size if the EXT_CSD_ERASE_GrOUP_DEF
bit 0 is enabled.

Tested on X86 but the changes should work on all platforms.

Co-developed-by: Bean Huo <[email protected]>
Signed-off-by: Kimito Sakata <[email protected]>
Reviewed-by: Kenneth Gibbons <[email protected]>

Changelog:
V3--V4:
1. Replace unused pointer var with NULL.
2. Added msg if ERASE_GROUP_DEF enabled for HC.
v2--v3:
1. Remove redundant ifndef

V1--V2:
1. refactor Kimito's original patch
2. change to use MMC_IOC_MULTI_CMD
3. add checkup if eMMC devie supports secure erase/trim


---
mmc_cmds.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/mmc_cmds.c b/mmc_cmds.c
index 3e36ff2..afa85b7 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -2518,6 +2518,22 @@ static int erase(int dev_fd, __u32 argin, __u32 start, __u32 end)
{
int ret = 0;
struct mmc_ioc_multi_cmd *multi_cmd;
+ __u8 ext_csd[512];
+
+
+ ret = read_extcsd(dev_fd, ext_csd);
+ if (ret) {
+ fprintf(stderr, "Could not read EXT_CSD\n");
+ exit(1);
+ }
+ if (ext_csd[EXT_CSD_ERASE_GROUP_DEF] & 0x01) {
+ fprintf(stderr, "High Capacity Erase Unit Size=%d bytes\n" \
+ "High Capacity Erase Timeout=%d ms\n" \
+ "High Capacity Write Protect Group Size=%d bytes\n",
+ ext_csd[224]*0x80000,
+ ext_csd[223]*300,
+ ext_csd[221]*ext_csd[224]*0x80000);
+ }

multi_cmd = calloc(1, sizeof(struct mmc_ioc_multi_cmd) +
3 * sizeof(struct mmc_ioc_cmd));
@@ -2559,7 +2575,6 @@ int do_erase(int nargs, char **argv)
{
int dev_fd, ret;
char *print_str;
- char **eptr = NULL;
__u8 ext_csd[512], checkup_mask = 0;
__u32 arg, start, end;

@@ -2569,14 +2584,14 @@ int do_erase(int nargs, char **argv)
}

if (strstr(argv[2], "0x") || strstr(argv[2], "0X"))
- start = strtol(argv[2], eptr, 16);
+ start = strtol(argv[2], NULL, 16);
else
- start = strtol(argv[2], eptr, 10);
+ start = strtol(argv[2], NULL, 10);

if (strstr(argv[3], "0x") || strstr(argv[3], "0X"))
- end = strtol(argv[3], eptr, 16);
+ end = strtol(argv[3], NULL, 16);
else
- end = strtol(argv[3], eptr, 10);
+ end = strtol(argv[3], NULL, 10);

if (end < start) {
fprintf(stderr, "erase start [0x%08x] > erase end [0x%08x]\n",
--
2.31.1


2021-05-06 06:46:26

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCH RESEND v4] Clean up and show effect of ERASE_GROUP_DEF

This patch is only the incremental change from your last version.
I guess you forgot to amend your changes.

To merge your changes after you tested them, you can use:
#git status
It will show you which file you changed
#git add <the file you changed>
#git commit --amend
Will open your default editor with the commit message.
Update your commit history, with the appropriate v3 -> v4 changes.

Also add my reviewed-by tag, below Kenneth Gibbons:
Reviewed-by: Avri Altman <[email protected]>

Save the patch to some directory:
#git format-patch -1 -v4 -o <some path>

Now send the patch. Use git send email:
#cd <the directory that contains the patch>
#git send-email <patch file name> --to 'Ulf Hansson <[email protected]>' --to [email protected] --cc [email protected]

It should have the same subject as before, be v4, without the RESEND label.
So before sending you should see the subject:
[PATCH v4] mmc-utils: Add eMMC erase command support

git send-email will automatically cc me, Bean, and Kenneth.

Thanks,
Avri

P.S.
If by accident you saved those changes as a separate commit, instead of amending them,
Just squash them into the previous one.
Ping me privately if you need help with that.

>
> From: Kimito Sakata <[email protected]>
>
> Replaced unused pointer with NULL on calls to strtol().
> Added logic to print High Capacity mode parameters of Erase unit size,
> Erase Timeout, Write protect Group Size if the EXT_CSD_ERASE_GrOUP_DEF
> bit 0 is enabled.
>
> Tested on X86 but the changes should work on all platforms.
>
> Co-developed-by: Bean Huo <[email protected]>
> Signed-off-by: Kimito Sakata <[email protected]>
> Reviewed-by: Kenneth Gibbons <[email protected]>
>
> Changelog:
> V3--V4:
> 1. Replace unused pointer var with NULL.
> 2. Added msg if ERASE_GROUP_DEF enabled for HC.
> v2--v3:
> 1. Remove redundant ifndef
>
> V1--V2:
> 1. refactor Kimito's original patch
> 2. change to use MMC_IOC_MULTI_CMD
> 3. add checkup if eMMC devie supports secure erase/trim
>
>
> ---
> mmc_cmds.c | 25 ++++++++++++++++++++-----
> 1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/mmc_cmds.c b/mmc_cmds.c
> index 3e36ff2..afa85b7 100644
> --- a/mmc_cmds.c
> +++ b/mmc_cmds.c
> @@ -2518,6 +2518,22 @@ static int erase(int dev_fd, __u32 argin, __u32
> start, __u32 end)
> {
> int ret = 0;
> struct mmc_ioc_multi_cmd *multi_cmd;
> + __u8 ext_csd[512];
> +
> +
> + ret = read_extcsd(dev_fd, ext_csd);
> + if (ret) {
> + fprintf(stderr, "Could not read EXT_CSD\n");
> + exit(1);
> + }
> + if (ext_csd[EXT_CSD_ERASE_GROUP_DEF] & 0x01) {
> + fprintf(stderr, "High Capacity Erase Unit Size=%d bytes\n" \
> + "High Capacity Erase Timeout=%d ms\n" \
> + "High Capacity Write Protect Group Size=%d bytes\n",
> + ext_csd[224]*0x80000,
> + ext_csd[223]*300,
> + ext_csd[221]*ext_csd[224]*0x80000);
> + }
>
> multi_cmd = calloc(1, sizeof(struct mmc_ioc_multi_cmd) +
> 3 * sizeof(struct mmc_ioc_cmd));
> @@ -2559,7 +2575,6 @@ int do_erase(int nargs, char **argv)
> {
> int dev_fd, ret;
> char *print_str;
> - char **eptr = NULL;
> __u8 ext_csd[512], checkup_mask = 0;
> __u32 arg, start, end;
>
> @@ -2569,14 +2584,14 @@ int do_erase(int nargs, char **argv)
> }
>
> if (strstr(argv[2], "0x") || strstr(argv[2], "0X"))
> - start = strtol(argv[2], eptr, 16);
> + start = strtol(argv[2], NULL, 16);
> else
> - start = strtol(argv[2], eptr, 10);
> + start = strtol(argv[2], NULL, 10);
>
> if (strstr(argv[3], "0x") || strstr(argv[3], "0X"))
> - end = strtol(argv[3], eptr, 16);
> + end = strtol(argv[3], NULL, 16);
> else
> - end = strtol(argv[3], eptr, 10);
> + end = strtol(argv[3], NULL, 10);
>
> if (end < start) {
> fprintf(stderr, "erase start [0x%08x] > erase end [0x%08x]\n",
> --
> 2.31.1