[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: Re: [PATCH v3] i2c_tools.c: add i2ctransfer utility
From: Tito <farmatito () tiscali ! it>
Date: 2019-01-07 14:07:20
Message-ID: 0be4f9d0-5598-de1f-c104-32cd0dd40287 () tiscali ! it
[Download RAW message or body]
Hi,
some hints to further reduce size and increase busyboxification.
Ciao,
Tito
On 07/01/19 14:29, Nikolaus Voss wrote:
> i2ctransfer sends and receives user defined i2c messages
>
> v2: apply Xabier's comments: add -a option, don't decrement argc,
> use bb_show_usage() and xzalloc()
> v3: fix possible out of bound access to msgs[nmsgs]
>
> Reviewed-by: Xabier Oneca -- xOneca <xoneca@gmail.com>
> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
> ---
> miscutils/i2c_tools.c | 206 +++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 205 insertions(+), 1 deletion(-)
>
> diff --git a/miscutils/i2c_tools.c b/miscutils/i2c_tools.c
> index 610fed5d6..fd27f5f83 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,192 @@ 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");
shorter: bb_error_msg_and_die("I2C transfers not supported");
even shorter but ugly: bb_error_msg_and_die("No 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), opt_a = (1 << 2);
> +
> + int bus_num, bus_addr = - 1;
> + int fd;
assign default values here?
unsigned opts, first = 0x03, last = 0x77;
> + 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, "^"
> + "fya"
> + "\0" "-2" /* minimum 2 args */
> + );
> + argv += optind;
> +
> + if (opts & opt_a) {
> + first = 0x00;
> + last = 0x7f;
> + }
removed
> +
> + 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)
no \n
Could this error message be shortened?
> + bb_error_msg_and_die("Error: Too many messages (max: %d)\n",
> + I2C_RDWR_IOCTL_MAX_MSGS);
> +
> + switch (state) {
> + case PARSE_GET_DESC:
> + flags = 0;
> +
> + switch (*arg_ptr++) {
> + case 'r': flags |= I2C_M_RD; break;
> + case 'w': break;
> + default:
> + bb_show_usage();
> + }
xstrtoul_range()? so no need for bb_error_msg_and_die
> +
> + len = strtoul(arg_ptr, &end, 0);
> + if (len > 0xffff || arg_ptr == end)
no \n
Could this error message be shortened?
> + bb_error_msg_and_die("Error: Length invalid: %s\n", *argv);
> +
> + arg_ptr = end;
> + if (*arg_ptr) {
> + if (*arg_ptr++ != '@')
no \n
Could this error message be shortened?
> + bb_error_msg_and_die("Error: Unknown separator after length: %s\n",
> + *argv);
> + bus_addr = xstrtou_range(arg_ptr, 0, first, last);
> +
> + if (!(opts & opt_f))
> + i2c_set_slave_addr(fd, bus_addr, opts & opt_f);
> + } else {
> + /* Reuse last address if possible */
> + if (bus_addr < 0)
no \n
Could this error message be shortened?
> + 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 = xzalloc(len);
> + 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:
xstrtoul_range() ? so no need for bb_error_msg_and_die
> + raw_data = strtoul(arg_ptr, &end, 0);
> + if (raw_data > 0xff || arg_ptr == end)
no \n
Could this error message be shortened?
> + 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:
no \n
Could this error message be shortened?
> + 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");
no \n
shorter: bb_error_msg_and_die("Unknown Internal Error");
> + }
> + }
> +
> + if (state != PARSE_GET_DESC || nmsgs == 0)
no \n
> + 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_perror_msg(const char *s, ...)
> + bb_error_msg("Error: Sending messages failed: %s\n", strerror(errno));
> + else if (nmsgs_sent < nmsgs)
no \n
> + 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);
> +
#if ENABLE_FEATURE_CLEANUP
> + for (i = 0; i < nmsgs; i++)
> + free(msgs[i].buf);
#endif
> +
remove return 0;
exit(EXIT_SUCCESS);
> +}
> +#endif /* ENABLE_I2CTRANSFER */
>
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic