[U-Boot] [PATCH] cmd_i2c.c: reduced subaddress length to 3 bytes

according to some of the comments the subaddress length is 1 or 2, but we are being prepared for the case it becomes 3. However the code also accepted 4. This repairs this by changing the constand 4 to 3.
Signed-off-by: Frans Meulenbroeks fransmeulenbroeks@gmail.com --- common/cmd_i2c.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c index 62cbd33..7531702 100644 --- a/common/cmd_i2c.c +++ b/common/cmd_i2c.c @@ -193,7 +193,7 @@ int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) for (j = 0; j < 8; j++) { if (argv[2][j] == '.') { alen = argv[2][j+1] - '0'; - if (alen > 4) { + if (alen > 3) { cmd_usage(cmdtp); return 1; } @@ -287,7 +287,7 @@ int do_i2c_mw ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) for (j = 0; j < 8; j++) { if (argv[2][j] == '.') { alen = argv[2][j+1] - '0'; - if (alen > 4) { + if (alen > 3) { cmd_usage(cmdtp); return 1; } @@ -361,7 +361,7 @@ int do_i2c_crc (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) for (j = 0; j < 8; j++) { if (argv[2][j] == '.') { alen = argv[2][j+1] - '0'; - if (alen > 4) { + if (alen > 3) { cmd_usage(cmdtp); return 1; } @@ -451,7 +451,7 @@ mod_i2c_mem(cmd_tbl_t *cmdtp, int incrflag, int flag, int argc, char *argv[]) for (j = 0; j < 8; j++) { if (argv[2][j] == '.') { alen = argv[2][j+1] - '0'; - if (alen > 4) { + if (alen > 3) { cmd_usage(cmdtp); return 1; } @@ -607,7 +607,7 @@ int do_i2c_loop(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) for (j = 0; j < 8; j++) { if (argv[2][j] == '.') { alen = argv[2][j+1] - '0'; - if (alen > 4) { + if (alen > 3) { cmd_usage(cmdtp); return 1; }

Signed-off-by: Frans Meulenbroeks fransmeulenbroeks@gmail.com --- common/cmd_bootm.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 23ab0c4..827d542 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -462,7 +462,7 @@ static int bootm_start_standalone(ulong iflag, int argc, char *argv[])
/* we overload the cmd field with our state machine info instead of a * function pointer */ -cmd_tbl_t cmd_bootm_sub[] = { +static cmd_tbl_t cmd_bootm_sub[] = { U_BOOT_CMD_MKENT(start, 0, 1, (void *)BOOTM_STATE_START, "", ""), U_BOOT_CMD_MKENT(loados, 0, 1, (void *)BOOTM_STATE_LOADOS, "", ""), #if defined(CONFIG_PPC) || defined(CONFIG_M68K) || defined(CONFIG_SPARC)

Signed-off-by: Frans Meulenbroeks fransmeulenbroeks@gmail.com --- common/cmd_i2c.c | 77 +++++++++++++++++++++++++++++++---------------------- 1 files changed, 45 insertions(+), 32 deletions(-)
diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c index 7531702..e54fc20 100644 --- a/common/cmd_i2c.c +++ b/common/cmd_i2c.c @@ -1242,46 +1242,59 @@ int do_i2c_bus_speed(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) return ret; }
-int do_i2c(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) +int do_i2c_mm(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) { - /* Strip off leading 'i2c' command argument */ - argc--; - argv++; + return mod_i2c_mem (cmdtp, 1, flag, argc, argv); +} + +int do_i2c_nm(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) +{ + return mod_i2c_mem (cmdtp, 0, flag, argc, argv); +}
+int do_i2c_reset(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) +{ + i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); + return 0; +} + +static cmd_tbl_t cmd_i2c_sub[] = { #if defined(CONFIG_I2C_MUX) - if (!strncmp(argv[0], "bu", 2)) - return do_i2c_add_bus(cmdtp, flag, argc, argv); + U_BOOT_CMD_MKENT(bus, 1, 1, do_i2c_add_bus, "", ""), #endif /* CONFIG_I2C_MUX */ - if (!strncmp(argv[0], "sp", 2)) - return do_i2c_bus_speed(cmdtp, flag, argc, argv); + U_BOOT_CMD_MKENT(crc32, 3, 1, do_i2c_crc, "", ""), #if defined(CONFIG_I2C_MULTI_BUS) - if (!strncmp(argv[0], "de", 2)) - return do_i2c_bus_num(cmdtp, flag, argc, argv); + U_BOOT_CMD_MKENT(dev, 1, 1, do_i2c_bus_num, "", ""), #endif /* CONFIG_I2C_MULTI_BUS */ - if (!strncmp(argv[0], "md", 2)) - return do_i2c_md(cmdtp, flag, argc, argv); - if (!strncmp(argv[0], "mm", 2)) - return mod_i2c_mem (cmdtp, 1, flag, argc, argv); - if (!strncmp(argv[0], "mw", 2)) - return do_i2c_mw(cmdtp, flag, argc, argv); - if (!strncmp(argv[0], "nm", 2)) - return mod_i2c_mem (cmdtp, 0, flag, argc, argv); - if (!strncmp(argv[0], "cr", 2)) - return do_i2c_crc(cmdtp, flag, argc, argv); - if (!strncmp(argv[0], "pr", 2)) - return do_i2c_probe(cmdtp, flag, argc, argv); - if (!strncmp(argv[0], "re", 2)) { - i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); - return 0; - } - if (!strncmp(argv[0], "lo", 2)) - return do_i2c_loop(cmdtp, flag, argc, argv); + U_BOOT_CMD_MKENT(loop, 3, 1, do_i2c_loop, "", ""), + U_BOOT_CMD_MKENT(md, 3, 1, do_i2c_md, "", ""), + U_BOOT_CMD_MKENT(mm, 2, 1, do_i2c_mm, "", ""), + U_BOOT_CMD_MKENT(mw, 3, 1, do_i2c_mw, "", ""), + U_BOOT_CMD_MKENT(nm, 2, 1, do_i2c_nm, "", ""), + U_BOOT_CMD_MKENT(probe, 0, 1, do_i2c_probe, "", ""), + U_BOOT_CMD_MKENT(reset, 0, 1, do_i2c_reset, "", ""), #if defined(CONFIG_CMD_SDRAM) - if (!strncmp(argv[0], "sd", 2)) - return do_sdram(cmdtp, flag, argc, argv); + U_BOOT_CMD_MKENT(sdram, 1, 1, do_i2c_sdram, "", ""), #endif - cmd_usage(cmdtp); - return 0; + U_BOOT_CMD_MKENT(speed, 1, 1, do_i2c_bus_speed, "", ""), +}; + +int do_i2c(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) +{ + cmd_tbl_t *c; + + /* Strip off leading 'i2c' command argument */ + argc--; + argv++; + + c = find_cmd_tbl(argv[0], &cmd_i2c_sub[0], ARRAY_SIZE(cmd_i2c_sub)); + + if (c) { + return c->cmd(cmdtp, flag, argc, argv); + } else { + cmd_usage(cmdtp); + return 1; + } }
/***************************************************/

Signed-off-by: Frans Meulenbroeks fransmeulenbroeks@gmail.com --- common/cmd_i2c.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c index e54fc20..1dd4df3 100644 --- a/common/cmd_i2c.c +++ b/common/cmd_i2c.c @@ -1302,13 +1302,14 @@ int do_i2c(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) U_BOOT_CMD( i2c, 6, 1, do_i2c, "I2C sub-system", - "speed [speed] - show or set I2C bus speed\n" #if defined(CONFIG_I2C_MUX) - "i2c bus [muxtype:muxaddr:muxchannel] - add a new bus reached over muxes\n" + "bus [muxtype:muxaddr:muxchannel] - add a new bus reached over muxes\ni2c " #endif /* CONFIG_I2C_MUX */ + "crc32 chip address[.0, .1, .2] count - compute CRC32 checksum\n" #if defined(CONFIG_I2C_MULTI_BUS) "i2c dev [dev] - show or set current I2C bus\n" #endif /* CONFIG_I2C_MULTI_BUS */ + "i2c loop chip address[.0, .1, .2] [# of objects] - looping read of device\n" "i2c md chip address[.0, .1, .2] [# of objects] - read from I2C device\n" "i2c mm chip address[.0, .1, .2] - write to I2C device (auto-incrementing)\n" "i2c mw chip address[.0, .1, .2] value [count] - write to I2C device (fill)\n" @@ -1316,11 +1317,10 @@ U_BOOT_CMD( "i2c crc32 chip address[.0, .1, .2] count - compute CRC32 checksum\n" "i2c probe - show devices on the I2C bus\n" "i2c reset - re-init the I2C Controller\n" - "i2c loop chip address[.0, .1, .2] [# of objects] - looping read of device" #if defined(CONFIG_CMD_SDRAM) - "\n" - "i2c sdram chip - print SDRAM configuration information" + "i2c sdram chip - print SDRAM configuration information\n" #endif + "i2c speed [speed] - show or set I2C bus speed" );
#if defined(CONFIG_I2C_MUX)

Signed-off-by: Frans Meulenbroeks fransmeulenbroeks@gmail.com --- common/cmd_i2c.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 60 insertions(+), 0 deletions(-)
diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c index 1dd4df3..552b4e5 100644 --- a/common/cmd_i2c.c +++ b/common/cmd_i2c.c @@ -154,6 +154,64 @@ int i2c_set_bus_speed(unsigned int) */ #define DISP_LINE_LEN 16
+/* + * Syntax: + * i2c read {i2c_chip} {devaddr}{.0, .1, .2} {len} {memaddr} + */ + +int do_i2c_read ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{ + u_char chip; + uint devaddr, alen, length; + u_char *memaddr; + int j; + + if (argc != 5) { + cmd_usage(cmdtp); + return 1; + } + + /* + * I2C chip address + */ + chip = simple_strtoul(argv[1], NULL, 16); + + /* + * I2C data address within the chip. This can be 1 or + * 2 bytes long. Some day it might be 3 bytes long :-). + */ + devaddr = simple_strtoul(argv[2], NULL, 16); + alen = 1; + for (j = 0; j < 8; j++) { + if (argv[2][j] == '.') { + alen = argv[2][j+1] - '0'; + if (alen > 3) { + cmd_usage(cmdtp); + return 1; + } + break; + } else if (argv[2][j] == '\0') + break; + } + + /* + * Length is the number of objects, not number of bytes. + */ + length = simple_strtoul(argv[3], NULL, 16); + + /* + * memaddr is the address where to store things in memory + */ + memaddr = (u_char *)simple_strtoul(argv[4], NULL, 16); + + if (i2c_read(chip, devaddr, alen, memaddr, length) != 0) + { + puts ("Error reading the chip.\n"); + return 1; + } + return 0; +} + int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) { u_char chip; @@ -1272,6 +1330,7 @@ static cmd_tbl_t cmd_i2c_sub[] = { U_BOOT_CMD_MKENT(mw, 3, 1, do_i2c_mw, "", ""), U_BOOT_CMD_MKENT(nm, 2, 1, do_i2c_nm, "", ""), U_BOOT_CMD_MKENT(probe, 0, 1, do_i2c_probe, "", ""), + U_BOOT_CMD_MKENT(read, 5, 1, do_i2c_read, "", ""), U_BOOT_CMD_MKENT(reset, 0, 1, do_i2c_reset, "", ""), #if defined(CONFIG_CMD_SDRAM) U_BOOT_CMD_MKENT(sdram, 1, 1, do_i2c_sdram, "", ""), @@ -1316,6 +1375,7 @@ U_BOOT_CMD( "i2c nm chip address[.0, .1, .2] - write to I2C device (constant address)\n" "i2c crc32 chip address[.0, .1, .2] count - compute CRC32 checksum\n" "i2c probe - show devices on the I2C bus\n" + "i2c read chip address[.0, .1, .2] length memaddress - read to memory \n" "i2c reset - re-init the I2C Controller\n" #if defined(CONFIG_CMD_SDRAM) "i2c sdram chip - print SDRAM configuration information\n"

Hello Frans,
Frans Meulenbroeks wrote:
Signed-off-by: Frans Meulenbroeks fransmeulenbroeks@gmail.com
common/cmd_i2c.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 60 insertions(+), 0 deletions(-)
diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c index 1dd4df3..552b4e5 100644 --- a/common/cmd_i2c.c +++ b/common/cmd_i2c.c @@ -154,6 +154,64 @@ int i2c_set_bus_speed(unsigned int)
[...]
- /*
* memaddr is the address where to store things in memory
*/
- memaddr = (u_char *)simple_strtoul(argv[4], NULL, 16);
- if (i2c_read(chip, devaddr, alen, memaddr, length) != 0)
- {
as Wolfgang suggested to your first post, incorrect brace style, please fix.
puts ("Error reading the chip.\n");
return 1;
- }
- return 0;
+}
int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) { u_char chip; @@ -1272,6 +1330,7 @@ static cmd_tbl_t cmd_i2c_sub[] = { U_BOOT_CMD_MKENT(mw, 3, 1, do_i2c_mw, "", ""), U_BOOT_CMD_MKENT(nm, 2, 1, do_i2c_nm, "", ""), U_BOOT_CMD_MKENT(probe, 0, 1, do_i2c_probe, "", ""),
- U_BOOT_CMD_MKENT(read, 5, 1, do_i2c_read, "", ""), U_BOOT_CMD_MKENT(reset, 0, 1, do_i2c_reset, "", ""),
#if defined(CONFIG_CMD_SDRAM) U_BOOT_CMD_MKENT(sdram, 1, 1, do_i2c_sdram, "", ""), @@ -1316,6 +1375,7 @@ U_BOOT_CMD( "i2c nm chip address[.0, .1, .2] - write to I2C device (constant address)\n" "i2c crc32 chip address[.0, .1, .2] count - compute CRC32 checksum\n" "i2c probe - show devices on the I2C bus\n"
- "i2c read chip address[.0, .1, .2] length memaddress - read to memory \n" "i2c reset - re-init the I2C Controller\n"
#if defined(CONFIG_CMD_SDRAM) "i2c sdram chip - print SDRAM configuration information\n"
Rest looks good, thanks!
bye Heiko

Hello Frans,
First, thanks for your patches! They look good to me, just one nitpicking comment.
Frans Meulenbroeks wrote:
Signed-off-by: Frans Meulenbroeks fransmeulenbroeks@gmail.com
common/cmd_i2c.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c index e54fc20..1dd4df3 100644 --- a/common/cmd_i2c.c +++ b/common/cmd_i2c.c @@ -1302,13 +1302,14 @@ int do_i2c(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) U_BOOT_CMD( i2c, 6, 1, do_i2c, "I2C sub-system",
- "speed [speed] - show or set I2C bus speed\n"
#if defined(CONFIG_I2C_MUX)
- "i2c bus [muxtype:muxaddr:muxchannel] - add a new bus reached over muxes\n"
- "bus [muxtype:muxaddr:muxchannel] - add a new bus reached over muxes\ni2c "
#endif /* CONFIG_I2C_MUX */
- "crc32 chip address[.0, .1, .2] count - compute CRC32 checksum\n"
#if defined(CONFIG_I2C_MULTI_BUS) "i2c dev [dev] - show or set current I2C bus\n" #endif /* CONFIG_I2C_MULTI_BUS */
- "i2c loop chip address[.0, .1, .2] [# of objects] - looping read of device\n" "i2c md chip address[.0, .1, .2] [# of objects] - read from I2C device\n" "i2c mm chip address[.0, .1, .2] - write to I2C device (auto-incrementing)\n" "i2c mw chip address[.0, .1, .2] value [count] - write to I2C device (fill)\n"
@@ -1316,11 +1317,10 @@ U_BOOT_CMD( "i2c crc32 chip address[.0, .1, .2] count - compute CRC32 checksum\n"
You should delete this here, otherwise it will show the "i2c crc32" command twice in this help list.
"i2c probe - show devices on the I2C bus\n" "i2c reset - re-init the I2C Controller\n"
- "i2c loop chip address[.0, .1, .2] [# of objects] - looping read of device"
#if defined(CONFIG_CMD_SDRAM)
- "\n"
- "i2c sdram chip - print SDRAM configuration information"
- "i2c sdram chip - print SDRAM configuration information\n"
#endif
- "i2c speed [speed] - show or set I2C bus speed"
);
#if defined(CONFIG_I2C_MUX)
bye Heiko
participants (2)
-
Frans Meulenbroeks
-
Heiko Schocher