
Hello Frans,
Frans Meulenbroeks wrote:
Added a new function i2c read to read to memory.
Why is this function needed? Do you read from an EEprom? If so, you can use the eeprom command, or?
That way it becomes possible to test against a value and use that to influence the boot process.
Ah, I see, but again, if you read from an eeprom, use the eeprom command.
Design decision was to stay close to the i2c md command with respect to command syntax.
Signed-off-by: Frans Meulenbroeks fransmeulenbroeks@gmail.com
common/cmd_i2c.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 68 insertions(+), 8 deletions(-)
diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c index 62cbd33..0100aa9 100644 --- a/common/cmd_i2c.c +++ b/common/cmd_i2c.c @@ -150,6 +150,64 @@ int i2c_set_bus_speed(unsigned int)
/*
- Syntax:
- i2c read {i2c_chip} {devaddr}{.0, .1, .2} {len} {memaddr}
- */
+int do_i2c_read ( cmd_tbl_t *cmdtp, 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 > 4) {
shouldn;t it be "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);
Please add a check, if it is a valid address (not NULL).
- if (i2c_read(chip, devaddr, alen, memaddr, length) != 0)
- {
puts ("Error reading the chip.\n");
return 1;
- }
- return 0;
+}
Hmm... and what is, if you read from an eeprom, and you cross pages? You don;t get what you expect!
+/*
*/
- Syntax:
- i2c md {i2c_chip} {addr}{.0, .1, .2} {len}
#define DISP_LINE_LEN 16 @@ -1249,15 +1306,17 @@ int do_i2c(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) argv++;
#if defined(CONFIG_I2C_MUX)
- if (!strncmp(argv[0], "bu", 2))
- if (!strcmp(argv[0], "bus", 3)) return do_i2c_add_bus(cmdtp, flag, argc, argv);
Why this?
#endif /* CONFIG_I2C_MUX */
- if (!strncmp(argv[0], "sp", 2))
- if (!strncmp(argv[0], "speed", 5)) return do_i2c_bus_speed(cmdtp, flag, argc, argv);
and this ... and all other?
Keep in mind, that maybe there are at least default environments, which uses i2c commands, and so you have to check, if you don;t break existing board support, if you change this!
#if defined(CONFIG_I2C_MULTI_BUS)
- if (!strncmp(argv[0], "de", 2))
- if (!strncmp(argv[0], "dev", 3)) return do_i2c_bus_num(cmdtp, flag, argc, argv);
#endif /* CONFIG_I2C_MULTI_BUS */
- if (!strncmp(argv[0], "read", 4))
return do_i2c_read(cmdtp, argc, argv);
Please sort alphabetical!
if (!strncmp(argv[0], "md", 2)) return do_i2c_md(cmdtp, flag, argc, argv); if (!strncmp(argv[0], "mm", 2)) @@ -1266,18 +1325,18 @@ int do_i2c(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) 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))
- if (!strncmp(argv[0], "crc", 3)) return do_i2c_crc(cmdtp, flag, argc, argv);
- if (!strncmp(argv[0], "pr", 2))
- if (!strncmp(argv[0], "probe", 5)) return do_i2c_probe(cmdtp, flag, argc, argv);
Add the new command here ...
- if (!strncmp(argv[0], "re", 2)) {
- if (!strncmp(argv[0], "reset", 5)) {
... and you have only here to change the command check length from 2 -> 3!
Or, you convert it, as Detlev suggested here:
http://lists.denx.de/pipermail/u-boot/2010-February/067893.html
This would be the preferred way.
While writting here, and your code is just a copy from "i2c md" maybe you can just modify the i2c md command, to something like that:
"i2c md {i2c_chip} {addr}{.0, .1, .2} {len} {memaddr}"
If there is a "memaddr", the i2c md command don;t print the values, but it writes them to the memaddr ...
bye heiko