[PATCH v3] i2c_tools.c: add i2ctransfer utility

Tito farmatito at tiscali.it
Mon Jan 7 14:07:20 UTC 2019


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 at gmail.com>
> Signed-off-by: Nikolaus Voss <nikolaus.voss at 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 */
> 


More information about the busybox mailing list