[PATCH] i2c_tools.c: add i2ctransfer utility
Xabier Oneca -- xOneca
xoneca at gmail.com
Mon Dec 17 09:37:00 UTC 2018
Hi Nikolaus,
Some comments about your patch follow:
> i2ctransfer sends and receives user defined i2c messages
> ---
> miscutils/i2c_tools.c | 216 +++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 215 insertions(+), 1 deletion(-)
>
> diff --git a/miscutils/i2c_tools.c b/miscutils/i2c_tools.c
> index 6a2134063..76f128e6d 100644
> --- a/miscutils/i2c_tools.c
> +++ b/miscutils/i2c_tools.c
> @@ -36,17 +36,26 @@
> //config: help
> //config: Detect I2C chips.
> //config:
> +//config:config I2CTRANSFER
> +//config: bool "i2ctransfer (4.0 kb)"
> +//config: default y
> +//config: select PLATFORM_LINUX
> +//config: help
> +//config: Send user-defined I2C messages in one transfer.
> +//config:
>
> //applet:IF_I2CGET(APPLET(i2cget, BB_DIR_USR_SBIN, BB_SUID_DROP))
> //applet:IF_I2CSET(APPLET(i2cset, BB_DIR_USR_SBIN, BB_SUID_DROP))
> //applet:IF_I2CDUMP(APPLET(i2cdump, BB_DIR_USR_SBIN, BB_SUID_DROP))
> //applet:IF_I2CDETECT(APPLET(i2cdetect, BB_DIR_USR_SBIN, BB_SUID_DROP))
> +//applet:IF_I2CTRANSFER(APPLET(i2ctransfer, BB_DIR_USR_SBIN, BB_SUID_DROP))
> /* not NOEXEC: if hw operation stalls, use less memory in "hung" process */
>
> //kbuild:lib-$(CONFIG_I2CGET) += i2c_tools.o
> //kbuild:lib-$(CONFIG_I2CSET) += i2c_tools.o
> //kbuild:lib-$(CONFIG_I2CDUMP) += i2c_tools.o
> //kbuild:lib-$(CONFIG_I2CDETECT) += i2c_tools.o
> +//kbuild:lib-$(CONFIG_I2CTRANSFER) += i2c_tools.o
>
> /*
> * Unsupported stuff:
> @@ -80,12 +89,18 @@
> #define I2C_FUNCS 0x0705
> #define I2C_PEC 0x0708
> #define I2C_SMBUS 0x0720
> +#define I2C_RDWR 0x0707
> +#define I2C_RDWR_IOCTL_MAX_MSGS 42
> struct i2c_smbus_ioctl_data {
> __u8 read_write;
> __u8 command;
> __u32 size;
> union i2c_smbus_data *data;
> };
> +struct i2c_rdwr_ioctl_data {
> + struct i2c_msg *msgs; /* pointers to i2c_msgs */
> + __u32 nmsgs; /* number of i2c_msgs */
> +};
> /* end linux/i2c-dev.h */
>
> /*
> @@ -262,7 +277,7 @@ static int i2c_bus_lookup(const char *bus_str)
> return xstrtou_range(bus_str, 10, 0, 0xfffff);
> }
>
> -#if ENABLE_I2CGET || ENABLE_I2CSET || ENABLE_I2CDUMP
> +#if ENABLE_I2CGET || ENABLE_I2CSET || ENABLE_I2CDUMP || ENABLE_I2CTRANSFER
> static int i2c_parse_bus_addr(const char *addr_str)
> {
> /* Slave address must be in range 0x03 - 0x77. */
> @@ -1373,3 +1388,202 @@ int i2cdetect_main(int argc UNUSED_PARAM, char **argv)
> return 0;
> }
> #endif /* ENABLE_I2CDETECT */
> +
> +#if ENABLE_I2CTRANSFER
> +static void check_i2c_func(int fd)
> +{
> + unsigned long funcs;
> +
> + get_funcs_matrix(fd, &funcs);
> +
> + if (!(funcs & I2C_FUNC_I2C))
> + bb_error_msg_and_die("warning: adapter does not support I2C transfers");
> +}
> +
> +//usage:#define i2ctransfer_trivial_usage
> +//usage: "[-f] [-y] I2CBUS DESC [DATA] [DESC [DATA]]..."
> +//usage:#define i2ctransfer_full_usage "\n\n"
> +//usage: "Send user-defined I2C messages in one transfer"
> +//usage: "\n"
> +//usage: "\nI2CBUS I2C bus number"
> +//usage: "\nDESC describes the transfer in the form: {r|w}LENGTH[@address]"
> +int i2ctransfer_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
> +int i2ctransfer_main(int argc UNUSED_PARAM, char **argv)
> +{
> + const unsigned opt_f = (1 << 0), opt_y = (1 << 1);
> +
> + int bus_num, bus_addr = - 1;
> + int fd;
> + unsigned opts;
> + int nmsgs = 0, nmsgs_sent, i, j;
> + struct i2c_msg msgs[I2C_RDWR_IOCTL_MAX_MSGS];
> + unsigned buf_idx = 0;
> + struct i2c_rdwr_ioctl_data rdwr;
> + enum parse_state {
> + PARSE_GET_DESC,
> + PARSE_GET_DATA,
> + } state = PARSE_GET_DESC;
> +
> + for (i = 0; i < I2C_RDWR_IOCTL_MAX_MSGS; i++)
> + msgs[i].buf = NULL;
> +
> + opts = getopt32(argv, "^"
> + "fy"
> + "\0" "-2" /* minimum 2 args */
> + );
> + argv += optind;
> + argc -= optind;
> + argc--; /* now argv[argc] is last arg */
argc is not used. No need to fix its value.
> +
> + bus_num = i2c_bus_lookup(argv[0]);
> + fd = i2c_dev_open(bus_num);
> + check_i2c_func(fd);
> +
> + while (*++argv) {
> + char *arg_ptr = *argv;
> + unsigned long len, raw_data;
> + __u16 flags;
> + __u8 data, *buf;
> + char *end;
> +
> + if (nmsgs > I2C_RDWR_IOCTL_MAX_MSGS)
> + bb_error_msg_and_die("Error: Too many messages (max: %d)\n",
> + I2C_RDWR_IOCTL_MAX_MSGS);
Off-by-one error? Seems you meant to use >=.
> +
> + switch (state) {
> + case PARSE_GET_DESC:
> + flags = 0;
> +
> + switch (*arg_ptr++) {
> + case 'r': flags |= I2C_M_RD; break;
> + case 'w': break;
> + default:
> + bb_error_msg_and_die("Error: Invalid direction: %s\n",
> + *argv);
Maybe bb_show_usage() here?
> + }
> +
> + len = strtoul(arg_ptr, &end, 0);
> + if (len > 0xffff || arg_ptr == end)
> + bb_error_msg_and_die("Error: Length invalid: %s\n", *argv);
> +
> + arg_ptr = end;
> + if (*arg_ptr) {
> + if (*arg_ptr++ != '@')
> + bb_error_msg_and_die("Error: Unknown separator after length: %s\n",
> + *argv);
> +
> + /* We skip 10-bit support for now. If we want it,
> + * it should be marked with a 't' flag before
> + * the address here.
> + */
> +
> + if (!(opts & opt_f)) {
> + bus_addr = i2c_parse_bus_addr(arg_ptr);
> + i2c_set_slave_addr(fd, bus_addr, opts & opt_f);
Here opts & opt_f == 0 always.
> + } else {
> + /* 'force' allows whole address range */
Not sure about this statement. It seems to be a '-a' for this:
https://manpages.debian.org/unstable/i2c-tools/i2ctransfer.8.en.html#OPTIONS
It seems none of the I2C functions of Busybox but i2cdetect support -a.
> + bus_addr = strtol(arg_ptr, &end, 0);
Can xstrtou_range(arg_ptr, 0, 0, 0x7f) be used here?
> + if (arg_ptr == end || *end || bus_addr > 0x7f)
> + bb_error_msg_and_die("Error: Invalid chip address: %s\n",
> + *argv);
> + }
> + } else {
> + /* Reuse last address if possible */
> + if (bus_addr < 0)
> + bb_error_msg_and_die("Error: No address given: %s\n",
> + *argv);
> + }
> +
> + msgs[nmsgs].addr = bus_addr;
> + msgs[nmsgs].flags = flags;
> + msgs[nmsgs].len = len;
> +
> + if (len) {
> + buf = xmalloc(len);
> + if (!buf)
> + bb_error_msg_and_die("Error: No memory for buffer: %s\n", *argv);
xmalloc dies on error. No need to check buf.
> + memset(buf, 0, len);
xmalloc + memset = xzalloc
> + msgs[nmsgs].buf = buf;
> + }
> +
> + if (flags & I2C_M_RD || len == 0) {
> + nmsgs++;
> + } else {
> + buf_idx = 0;
> + state = PARSE_GET_DATA;
> + }
> + break;
> +
> + case PARSE_GET_DATA:
> + raw_data = strtoul(arg_ptr, &end, 0);
> + if (raw_data > 0xff || arg_ptr == end)
> + bb_error_msg_and_die("Error: Invalid data byte: %s\n", *argv);
> +
> + data = raw_data;
> + len = msgs[nmsgs].len;
> +
> + while (buf_idx < len) {
> + msgs[nmsgs].buf[buf_idx++] = data;
> +
> + if (!*end)
> + break;
> +
> + switch (*end) {
> + /* Pseudo randomness (8 bit AXR with a=13 and b=27) */
> + case 'p':
> + data = (data ^ 27) + 13;
> + data = (data << 1) | (data >> 7);
> + break;
> + case '+': data++; break;
> + case '-': data--; break;
> + case '=': break;
> + default:
> + bb_error_msg_and_die("Error: Invalid data byte suffix: %s\n",
> + *argv);
> + }
> + }
> +
> + if (buf_idx == len) {
> + nmsgs++;
> + state = PARSE_GET_DESC;
> + }
> +
> + break;
> +
> + default:
> + /* Should never happen */
> + bb_error_msg_and_die("Internal Error: Unknown state in state machine!\n");
> + }
> + }
> +
> + if (state != PARSE_GET_DESC || nmsgs == 0)
> + bb_error_msg_and_die("Error: Incomplete message\n");
> +
> + if (!(opts & opt_y))
> + confirm_action(bus_addr, 0, 0, 0);
> +
> + rdwr.msgs = msgs;
> + rdwr.nmsgs = nmsgs;
> + nmsgs_sent = ioctl(fd, I2C_RDWR, &rdwr);
> + if (nmsgs_sent < 0)
> + bb_error_msg("Error: Sending messages failed: %s\n", strerror(errno));
> + else if (nmsgs_sent < nmsgs)
> + bb_error_msg("Warning: only %d/%d messages were sent\n", nmsgs_sent, nmsgs);
> +
> + for (i = 0; i < nmsgs_sent; i++) {
> + if (msgs[i].len && msgs[i].flags & I2C_M_RD) {
> + for (j = 0; j < msgs[i].len - 1; j++)
> + printf("0x%02x ", msgs[i].buf[j]);
> + /* Print final byte with newline */
> + printf("0x%02x\n", msgs[i].buf[j]);
> + }
> + }
> +
> + close(fd);
> +
> + for (i = 0; i < nmsgs; i++)
> + free(msgs[i].buf);
> +
> + return 0;
> +}
> +#endif /* ENABLE_I2CTRANSFER */
> --
> 2.17.1
Cheers,
Xabier Oneca_,,_
More information about the busybox
mailing list