[PATCH 0/3] cmd: fix return code of sf command

Heinrich Schuchardt

2 Jan 2023 2 Jan '23

6:21 p.m.

For 'sf write', 'sf read', 'sf erase' using excessive values of offset or size should not display the command usage but only the error message.

Use CMD_RET_* constants. Simplify do_spi_flash().

Heinrich Schuchardt (3): cmd: fix return code of 'sf write' and 'sf read' cmd: simplify do_spi_flash() cmd: fix return code of 'sf erase'

cmd/sf.c | 45 +++++++++++++++++++-------------------------- 1 file changed, 19 insertions(+), 26 deletions(-)

-- 2.37.2

Show replies by date

Heinrich Schuchardt

2 Jan 2 Jan

6:21 p.m.

New subject: [PATCH 1/3] cmd: fix return code of 'sf write' and 'sf read'

If the offset or the size passed to the 'sf write' or 'sf read' command exceeds the size of the SPI flash displaying the command usage is not helpful. Return CMD_RET_FAILURE instead of CMD_RET_USAGE.

Use the CMD_RET_* constants instead of 0, 1, -1.

Simplify a logical expression in the final return statement.

Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- cmd/sf.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/cmd/sf.c b/cmd/sf.c index cf92ac4109..272521bcec 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -281,33 +281,33 @@ static int do_spi_flash_read_write(int argc, char *const argv[]) loff_t offset, len, maxsize;

if (argc < 3) - return -1; + return CMD_RET_USAGE;

addr = hextoul(argv[1], &endp); if (*argv[1] == 0 || *endp != 0) - return -1; + return CMD_RET_USAGE;

if (mtd_arg_off_size(argc - 2, &argv[2], &dev, &offset, &len, &maxsize, MTD_DEV_TYPE_NOR, flash->size)) - return -1; + return CMD_RET_FAILURE;

/* Consistency checking */ if (offset + len > flash->size) { printf("ERROR: attempting %s past flash size (%#x)\n", argv[0], flash->size); - return 1; + return CMD_RET_FAILURE; }

if (strncmp(argv[0], "read", 4) != 0 && flash->flash_is_unlocked && !flash->flash_is_unlocked(flash, offset, len)) { printf("ERROR: flash area is locked\n"); - return 1; + return CMD_RET_FAILURE; }

buf = map_physmem(addr, len, MAP_WRBACK); if (!buf && addr) { puts("Failed to map physical memory\n"); - return 1; + return CMD_RET_FAILURE; }

if (strcmp(argv[0], "update") == 0) { @@ -332,7 +332,7 @@ static int do_spi_flash_read_write(int argc, char *const argv[])

unmap_physmem(buf, len);

- return ret == 0 ? 0 : 1; + return ret ? CMD_RET_FAILURE : CMD_RET_SUCCESS; }

static int do_spi_flash_erase(int argc, char *const argv[])

-- 2.37.2

Heinrich Schuchardt

6:21 p.m.

New subject: [PATCH 2/3] cmd: simplify do_spi_flash()

CMD_RET_USAGE == -1. The special handling of this value at the end of do_spi_flash() does not make any sense.

To avoid future confusion use the CMD_RET_* constants and simplify the code.

Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- cmd/sf.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/cmd/sf.c b/cmd/sf.c index 272521bcec..2a88430681 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -579,21 +579,19 @@ static int do_spi_flash(struct cmd_tbl *cmdtp, int flag, int argc,

/* need at least two arguments */ if (argc < 2) - goto usage; + return CMD_RET_USAGE;

cmd = argv[1]; --argc; ++argv;

- if (strcmp(cmd, "probe") == 0) { - ret = do_spi_flash_probe(argc, argv); - goto done; - } + if (strcmp(cmd, "probe") == 0) + return do_spi_flash_probe(argc, argv);

/* The remaining commands require a selected device */ if (!flash) { puts("No SPI flash selected. Please run `sf probe'\n"); - return 1; + return CMD_RET_FAILURE; }

if (strcmp(cmd, "read") == 0 || strcmp(cmd, "write") == 0 || @@ -606,14 +604,9 @@ static int do_spi_flash(struct cmd_tbl *cmdtp, int flag, int argc, else if (IS_ENABLED(CONFIG_CMD_SF_TEST) && !strcmp(cmd, "test")) ret = do_spi_flash_test(argc, argv); else - ret = -1; - -done: - if (ret != -1) - return ret; + ret = CMD_RET_USAGE;

-usage: - return CMD_RET_USAGE; + return ret; }

#ifdef CONFIG_SYS_LONGHELP

-- 2.37.2

Heinrich Schuchardt

6:21 p.m.

New subject: [PATCH 3/3] cmd: fix return code of 'sf erase'

If the offset or the size passed to the 'sf erase' command exceeds the size of the SPI flash displaying the command usage is not helpful. Return CMD_RET_FAILURE instead of CMD_RET_USAGE.

Use the CMD_RET_* constants instead of 0, 1, -1.

Simplify a logical expression in the final return statement.

Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- cmd/sf.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/cmd/sf.c b/cmd/sf.c index 2a88430681..a28cccb338 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -343,27 +343,27 @@ static int do_spi_flash_erase(int argc, char *const argv[]) ulong size;

if (argc < 3) - return -1; + return CMD_RET_USAGE;

if (mtd_arg_off(argv[1], &dev, &offset, &len, &maxsize, MTD_DEV_TYPE_NOR, flash->size)) - return -1; + return CMD_RET_FAILURE;

ret = sf_parse_len_arg(argv[2], &size); if (ret != 1) - return -1; + return CMD_RET_USAGE;

/* Consistency checking */ if (offset + size > flash->size) { printf("ERROR: attempting %s past flash size (%#x)\n", argv[0], flash->size); - return 1; + return CMD_RET_FAILURE; }

if (flash->flash_is_unlocked && !flash->flash_is_unlocked(flash, offset, len)) { printf("ERROR: flash area is locked\n"); - return 1; + return CMD_RET_FAILURE; }

ret = spi_flash_erase(flash, offset, size); @@ -373,7 +373,7 @@ static int do_spi_flash_erase(int argc, char *const argv[]) else printf("OK\n");

- return ret == 0 ? 0 : 1; + return ret ? CMD_RET_FAILURE : CMD_RET_SUCCESS; }

static int do_spi_protect(int argc, char *const argv[])

-- 2.37.2

Dhruva Gole

3 Jan 3 Jan

10:13 a.m.

New subject: [PATCH 3/3] cmd: fix return code of 'sf erase'

On 02/01/23 22:51, Heinrich Schuchardt wrote:

...

If the offset or the size passed to the 'sf erase' command exceeds the size of the SPI flash displaying the command usage is not helpful. Return CMD_RET_FAILURE instead of CMD_RET_USAGE.

Use the CMD_RET_* constants instead of 0, 1, -1.

Simplify a logical expression in the final return statement.

Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com

This series makes the code more readable and so,

Acked-by: Dhruva Gole d-gole@ti.com

...

cmd/sf.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/cmd/sf.c b/cmd/sf.c index 2a88430681..a28cccb338 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -343,27 +343,27 @@ static int do_spi_flash_erase(int argc, char *const argv[]) ulong size;

if (argc < 3)
return -1;
return CMD_RET_USAGE;
if (mtd_arg_off(argv[1], &dev, &offset, &len, &maxsize, MTD_DEV_TYPE_NOR, flash->size))
return -1;
return CMD_RET_FAILURE;
ret = sf_parse_len_arg(argv[2], &size); if (ret != 1)
return -1;
return CMD_RET_USAGE;
/* Consistency checking */ if (offset + size > flash->size) { printf("ERROR: attempting %s past flash size (%#x)\n", argv[0], flash->size);
return 1;
return CMD_RET_FAILURE;
}

if (flash->flash_is_unlocked && !flash->flash_is_unlocked(flash, offset, len)) { printf("ERROR: flash area is locked\n");
return 1;
return CMD_RET_FAILURE;
}

ret = spi_flash_erase(flash, offset, size);
@@ -373,7 +373,7 @@ static int do_spi_flash_erase(int argc, char *const argv[]) else printf("OK\n");

return ret == 0 ? 0 : 1;

return ret ? CMD_RET_FAILURE : CMD_RET_SUCCESS; }

static int do_spi_protect(int argc, char *const argv[])

-- Thanks and Regards, Dhruva Gole

Jagan Teki

26 Jan 26 Jan

4:36 p.m.

On Mon, Jan 2, 2023 at 10:51 PM Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:

...

For 'sf write', 'sf read', 'sf erase' using excessive values of offset or size should not display the command usage but only the error message.

Use CMD_RET_* constants. Simplify do_spi_flash().

Heinrich Schuchardt (3): cmd: fix return code of 'sf write' and 'sf read' cmd: simplify do_spi_flash() cmd: fix return code of 'sf erase'

Applied to u-boot-spi/master

840

Age (days ago)

864

Last active (days ago)

List overview

Download

5 comments

3 participants

tags (0)

participants (3)

Dhruva Gole
Heinrich Schuchardt
Jagan Teki