From 839efa66696bec32d6b0ceb34e6a77705ea6732f Mon Sep 17 00:00:00 2001 From: David Alessio Date: Tue, 20 Aug 2019 20:29:21 +0000 Subject: [PATCH] Merged in david_alessio/apps/bugfix/i2ctool-dev-clobbers-slave-config (pull request #198) Bugfix/i2ctool dev clobbers slave config * friendlier I2C "knock-knock" uses only read request While many I2C slave devices have multiple indexed registers, some have only one register and it's not indexed. For example, the I2C bus switch TCA9548A has only a Control Register, attempting to index to "Reg[0]" alters its contents to 0x00 disabling all subordinate buses. This patch fixes that problem by simply trying to read something/anything from the slave. This also helps coax out slaves with register files that start from a higher index, i.e. the AT24CS0x, FLASH chips with a UUID that appears as a 2nd I2C slave at (address+8), report their serial number at Reg[80]-Reg[8F] and will NAK a read of Reg[0]. * modify get/set to prevent write of reg index if not specifed Approved-by: Gregory Nutt --- system/i2c/i2c_common.c | 1 + system/i2c/i2c_dev.c | 49 +++++------------------ system/i2c/i2c_get.c | 76 +++++++++++++++++++++++------------ system/i2c/i2c_main.c | 10 +++-- system/i2c/i2c_set.c | 87 +++++++++++++++++++++++++++-------------- system/i2c/i2ctool.h | 3 +- 6 files changed, 128 insertions(+), 98 deletions(-) diff --git a/system/i2c/i2c_common.c b/system/i2c/i2c_common.c index 0e4a505d5..4dda65d90 100644 --- a/system/i2c/i2c_common.c +++ b/system/i2c/i2c_common.c @@ -171,6 +171,7 @@ int i2ctool_common_args(FAR struct i2ctool_s *i2ctool, FAR char **arg) } i2ctool->regaddr = (uint8_t)value; + i2ctool->hasregindx = true; return ret; case 's': diff --git a/system/i2c/i2c_dev.c b/system/i2c/i2c_dev.c index 6c5dafd73..22f66907e 100644 --- a/system/i2c/i2c_dev.c +++ b/system/i2c/i2c_dev.c @@ -55,14 +55,8 @@ int i2ccmd_dev(FAR struct i2ctool_s *i2ctool, int argc, char **argv) { - struct i2c_msg_s msg[2]; + struct i2c_msg_s msg; FAR char *ptr; - union - { - uint16_t data16; - uint8_t data8; - } u; - uint8_t regaddr; uint8_t saveaddr; long first; @@ -163,41 +157,15 @@ int i2ccmd_dev(FAR struct i2ctool_s *i2ctool, int argc, char **argv) /* Set up data structures */ - regaddr = i2ctool->regaddr; + regaddr = i2ctool->regaddr; - msg[0].frequency = i2ctool->freq; - msg[0].addr = addr; - msg[0].flags = I2C_M_NOSTOP; - msg[0].buffer = ®addr; - msg[0].length = 1; + msg.frequency = i2ctool->freq; + msg.addr = addr; + msg.flags = I2C_M_READ; + msg.buffer = ®addr; + msg.length = 1; - msg[1].frequency = i2ctool->freq; - msg[1].addr = addr; - msg[1].flags = I2C_M_READ; - - if (i2ctool->width == 8) - { - msg[1].buffer = &u.data8; - msg[1].length = 1; - } - else - { - msg[1].buffer = (uint8_t*)&u.data16; - msg[1].length = 2; - } - - if (i2ctool->start) - { - ret = i2cdev_transfer(fd, &msg[0], 1); - if (ret == OK) - { - ret = i2cdev_transfer(fd, &msg[1], 1); - } - } - else - { - ret = i2cdev_transfer(fd, msg, 2); - } + ret = i2cdev_transfer(fd, &msg, 1); if (ret == OK) { @@ -207,6 +175,7 @@ int i2ccmd_dev(FAR struct i2ctool_s *i2ctool, int argc, char **argv) { i2ctool_printf(i2ctool, "-- "); } + i2ctool_flush(i2ctool); } i2ctool_printf(i2ctool, "\n"); diff --git a/system/i2c/i2c_get.c b/system/i2c/i2c_get.c index f2388057d..75dce4a42 100644 --- a/system/i2c/i2c_get.c +++ b/system/i2c/i2c_get.c @@ -182,38 +182,64 @@ int i2ctool_get(FAR struct i2ctool_s *i2ctool, int fd, uint8_t regaddr, /* Set up data structures */ - msg[0].frequency = i2ctool->freq; - msg[0].addr = i2ctool->addr; - msg[0].flags = I2C_M_NOSTOP; - msg[0].buffer = ®addr; - msg[0].length = 1; - - msg[1].frequency = i2ctool->freq; - msg[1].addr = i2ctool->addr; - msg[1].flags = I2C_M_READ; - - if (i2ctool->width == 8) + if (i2ctool->hasregindx) { - msg[1].buffer = &u.data8; - msg[1].length = 1; - } - else - { - msg[1].buffer = (uint8_t*)&u.data16; - msg[1].length = 2; - } + msg[0].frequency = i2ctool->freq; + msg[0].addr = i2ctool->addr; + msg[0].flags = I2C_M_NOSTOP; + msg[0].buffer = ®addr; + msg[0].length = 1; - if (i2ctool->start) - { - ret = i2cdev_transfer(fd, &msg[0], 1); - if (ret== OK) + msg[1].frequency = i2ctool->freq; + msg[1].addr = i2ctool->addr; + msg[1].flags = I2C_M_READ; + + if (i2ctool->width == 8) { - ret = i2cdev_transfer(fd, &msg[1], 1); + msg[1].buffer = &u.data8; + msg[1].length = 1; + } + else + { + msg[1].buffer = (uint8_t*)&u.data16; + msg[1].length = 2; + } + + if (i2ctool->start) + { + ret = i2cdev_transfer(fd, &msg[0], 1); + if (ret== OK) + { + ret = i2cdev_transfer(fd, &msg[1], 1); + } + } + else + { + ret = i2cdev_transfer(fd, msg, 2); } } else { - ret = i2cdev_transfer(fd, msg, 2); + /* no register index "-r" has been specified so + * we do a pure read (no write of index) + */ + + msg[0].frequency = i2ctool->freq; + msg[0].addr = i2ctool->addr; + msg[0].flags = I2C_M_READ; + + if (i2ctool->width == 8) + { + msg[0].buffer = &u.data8; + msg[0].length = 1; + } + else + { + msg[0].buffer = (uint8_t*)&u.data16; + msg[0].length = 2; + } + + ret = i2cdev_transfer(fd, msg, 1); } /* Return the result of the read operation */ diff --git a/system/i2c/i2c_main.c b/system/i2c/i2c_main.c index 8ace1898f..56727d34e 100644 --- a/system/i2c/i2c_main.c +++ b/system/i2c/i2c_main.c @@ -124,9 +124,6 @@ static int i2ccmd_help(FAR struct i2ctool_s *i2ctool, int argc, char **argv) i2ctool_printf(i2ctool, " [-b bus] is the I2C bus number (decimal). " "Default: %d Current: %d\n", CONFIG_I2CTOOL_MINBUS, i2ctool->bus); - i2ctool_printf(i2ctool, " [-r regaddr] is the I2C device register address (hex). " - "Default: 00 Current: %02x\n", - i2ctool->regaddr); i2ctool_printf(i2ctool, " [-w width] is the data width (8 or 16 decimal). " "Default: 8 Current: %d\n", i2ctool->width); @@ -139,6 +136,11 @@ static int i2ccmd_help(FAR struct i2ctool_s *i2ctool, int argc, char **argv) i2ctool_printf(i2ctool, " [-f freq] I2C frequency. " "Default: %d Current: %d\n", CONFIG_I2CTOOL_DEFFREQ, i2ctool->freq); + + i2ctool_printf(i2ctool, "\nSpecial non-sticky options:\n"); + i2ctool_printf(i2ctool, " [-r regaddr] is the I2C device register index (hex). " + "Default: not used/sent\n"); + i2ctool_printf(i2ctool, "\nNOTES:\n"); #ifndef CONFIG_DISABLE_ENVIRON i2ctool_printf(i2ctool, "o An environment variable like $PATH may be used for any argument.\n"); @@ -380,6 +382,8 @@ int i2c_main(int argc, char *argv[]) g_i2ctool.freq = CONFIG_I2CTOOL_DEFFREQ; } + g_i2ctool.hasregindx = false; + /* Parse and process the command line */ i2c_setup(&g_i2ctool); diff --git a/system/i2c/i2c_set.c b/system/i2c/i2c_set.c index ea1f64466..4a1bcd687 100644 --- a/system/i2c/i2c_set.c +++ b/system/i2c/i2c_set.c @@ -207,43 +207,72 @@ int i2ctool_set(FAR struct i2ctool_s *i2ctool, int fd, uint8_t regaddr, } u; int ret; - /* Set up data structures */ - msg[0].frequency = i2ctool->freq; - msg[0].addr = i2ctool->addr; - msg[0].flags = I2C_M_NOSTOP; - msg[0].buffer = ®addr; - msg[0].length = 1; - - msg[1].frequency = i2ctool->freq; - msg[1].addr = i2ctool->addr; - msg[1].flags = 0; - - if (i2ctool->width == 8) + if (i2ctool->hasregindx) { - u.data8 = (uint8_t)value; - msg[1].buffer = &u.data8; - msg[1].length = 1; - } - else - { - u.data16 = value; - msg[1].buffer = (uint8_t*)&u.data16; - msg[1].length = 2; - } + /* Set up data structures */ - if (i2ctool->start) - { - ret = i2cdev_transfer(fd, &msg[0], 1); - if (ret == OK) + msg[0].frequency = i2ctool->freq; + msg[0].addr = i2ctool->addr; + msg[0].flags = I2C_M_NOSTOP; + msg[0].buffer = ®addr; + msg[0].length = 1; + + msg[1].frequency = i2ctool->freq; + msg[1].addr = i2ctool->addr; + msg[1].flags = 0; + + if (i2ctool->width == 8) { - ret = i2cdev_transfer(fd, &msg[1], 1); + u.data8 = (uint8_t)value; + msg[1].buffer = &u.data8; + msg[1].length = 1; + } + else + { + u.data16 = value; + msg[1].buffer = (uint8_t*)&u.data16; + msg[1].length = 2; + } + + if (i2ctool->start) + { + ret = i2cdev_transfer(fd, &msg[0], 1); + if (ret == OK) + { + ret = i2cdev_transfer(fd, &msg[1], 1); + } + } + else + { + msg[1].flags |= I2C_M_NOSTART; + ret = i2cdev_transfer(fd, msg, 2); } } else { - msg[1].flags |= I2C_M_NOSTART; - ret = i2cdev_transfer(fd, msg, 2); + /* no register index "-r" has been specified so + * we don't send a register index, just do the write + */ + + msg[0].frequency = i2ctool->freq; + msg[0].addr = i2ctool->addr; + msg[0].flags = 0; + + if (i2ctool->width == 8) + { + u.data8 = (uint8_t)value; + msg[0].buffer = &u.data8; + msg[0].length = 1; + } + else + { + u.data16 = value; + msg[0].buffer = (uint8_t*)&u.data16; + msg[0].length = 2; + } + + ret = i2cdev_transfer(fd, msg, 1); } return ret; diff --git a/system/i2c/i2ctool.h b/system/i2c/i2ctool.h index ddc5f50ec..3ad6c8752 100644 --- a/system/i2c/i2ctool.h +++ b/system/i2c/i2ctool.h @@ -140,6 +140,7 @@ struct i2ctool_s uint8_t width; /* [-w width] is the data width (8 or 16) */ bool start; /* [-s|n], send|don't send start between command and data */ bool autoincr; /* [-i|j], Auto increment|don't increment regaddr on repititions */ + bool hasregindx; /* true with the use of -r */ uint32_t freq; /* [-f freq] I2C frequency */ /* Output streams */ @@ -192,7 +193,7 @@ int i2ccmd_verf(FAR struct i2ctool_s *i2ctool, int argc, FAR char **argv); /* I2C access functions */ -int i2ctool_get(FAR struct i2ctool_s *i2ctool, int fd, uint8_t addr, +int i2ctool_get(FAR struct i2ctool_s *i2ctool, int fd, uint8_t regaddr, FAR uint16_t *result); int i2ctool_set(FAR struct i2ctool_s *i2ctool, int fd, uint8_t regaddr, uint16_t value);