[U-Boot] [PATCH 0/5] i2c patches. Again!
Below are my i2c patches again, (and the bootm one but that is pretty minor) Now decently numbered and with the remarks of Heiko applied.
Thanks go to - Heiko for the comments - Eric and Wolfgang for teaching me a little bit more git (btw: what happend to sccs? :-) ) - everyone for having patience with me while I am struggling to submit patches in a proper way.
I have a few more things I'm working on, but would like to see feedback or at least acks for these, so I avoid a lot of rework because something in this patches is not considered proper.
(I saw some code in i2c that can be refactored; also might implement subcommand handling in a few other files).
Have fun, Frans.
Frans Meulenbroeks (5): cmd_i2c.c: reduced subaddress length to 3 bytes cmd_bootm.c: made subcommand array static cmd_i2c.c: reworked subcommand handling cmd_i2c.c: sorted commands alphabetically cmd_i2c.c: added i2c read to memory function
common/cmd_bootm.c | 2 +- common/cmd_i2c.c | 160 +++++++++++++++++++++++++++++++++++++--------------- 2 files changed, 116 insertions(+), 46 deletions(-)
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 | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c index e54fc20..b51e3f4 100644 --- a/common/cmd_i2c.c +++ b/common/cmd_i2c.c @@ -1302,25 +1302,24 @@ 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" "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 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 | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c index b51e3f4..9e7143a 100644 --- a/common/cmd_i2c.c +++ b/common/cmd_i2c.c @@ -154,6 +154,63 @@ 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 +1329,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, "", ""), @@ -1315,6 +1373,7 @@ U_BOOT_CMD( "i2c mw chip address[.0, .1, .2] value [count] - write to I2C device (fill)\n" "i2c nm chip address[.0, .1, .2] - write to I2C device (constant address)\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" @@ -1322,8 +1381,7 @@ U_BOOT_CMD( "i2c speed [speed] - show or set I2C bus speed" );
-#if defined(CONFIG_I2C_MUX) - +#if defined(CONFIG_I2C_MUX) int i2c_mux_add_device(I2C_MUX_DEVICE *dev) { I2C_MUX_DEVICE *devtmp = i2c_mux_devices;
Hi Frans,
[dzu@pollux u-boot-testing (master)]$ git am ~/p1 Applying: cmd_i2c.c: added i2c read to memory function /work/dzu/git/u-boot-testing/.git/rebase-apply/patch:95: trailing whitespace. #if defined(CONFIG_I2C_MUX) error: patch failed: common/cmd_i2c.c:1272 error: common/cmd_i2c.c: patch does not apply Patch failed at 0001 cmd_i2c.c: added i2c read to memory function When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort".
You really want to run "scripts/checkpatch.pl" from a Linux source tree on your patches to catch such style problems (beware that in U-Boot we use common sense for the output and tolerate some warnings). Whitespace problems can also be easily seen if you do a "git config --global color.diff=auto". Then such problems can be spotted in "git diff" output.
Currently I don't have time to diagnose the error in line 1272 - maybe you can do that.
Cheers Detlev
Hi Detlev,
Wrt the whitespace: I'll take more care next time.
wrt the patch not applying: did you apply the patches 1 to 4 first? I'm especially triggered by the fact that you are applying p1, but this is patch 5. Also your testing tree does not show that the others have been applied (but of course it could be a different git or you could have unrolled it them).
The i2c patches cannot be applied out of order!! The new i2c read command is added to the command table to at least that patch need to be applied first.
I've ran the patches on the main git head on two different systems and did not run into problems. Also I just did do a checkout of the file just before the patch was applied. I could apply the patch on it without a problem. I also did a diff on the patch as I've sent it and the email copy is equal to the copy on my system.
Frans (puzzled).
2010/2/26 Detlev Zundel dzu@denx.de:
Hi Frans,
[dzu@pollux u-boot-testing (master)]$ git am ~/p1 Applying: cmd_i2c.c: added i2c read to memory function /work/dzu/git/u-boot-testing/.git/rebase-apply/patch:95: trailing whitespace. #if defined(CONFIG_I2C_MUX) error: patch failed: common/cmd_i2c.c:1272 error: common/cmd_i2c.c: patch does not apply Patch failed at 0001 cmd_i2c.c: added i2c read to memory function When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort".
You really want to run "scripts/checkpatch.pl" from a Linux source tree on your patches to catch such style problems (beware that in U-Boot we use common sense for the output and tolerate some warnings). Whitespace problems can also be easily seen if you do a "git config --global color.diff=auto". Then such problems can be spotted in "git diff" output.
Currently I don't have time to diagnose the error in line 1272 - maybe you can do that.
Cheers Detlev
-- I have always observed that the pretensions of all people are in exact inverse ratio to their merits; this is one of the axioms of morals. -- Joseph Lagrange -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu@denx.de
Hi Frans,
Wrt the whitespace: I'll take more care next time.
No problem.
wrt the patch not applying: did you apply the patches 1 to 4 first? I'm especially triggered by the fact that you are applying p1, but this is patch 5.
Yes, my bad, sorry - disregard this part of my mail. I was looking through your patches in the order of the mbox file, not realizing my fault.
Cheers Detlev
participants (2)
-
Detlev Zundel -
Frans Meulenbroeks