[RFC] new applets: i2c-tools
tito
farmatito at tiscali.it
Tue Dec 9 20:48:19 UTC 2014
On Tuesday 09 December 2014 18:09:54 walter harms wrote:
> hi,
> i did not try compile, i was only watching ...
>
> hope the comments will help
> re,
> wh
>
> Am 09.12.2014 15:51, schrieb Bartosz Golaszewski:
> > Hi,
> >
> > I've started porting i2c-tools to busybox.
> >
> > This patch implements i2cget together with necessary library functions - a lot
> > of this code can be reused in other i2c utils.
> >
> > Bloatcheck:
> >
> > function old new delta
> > i2cget_main - 1464 +1464
> > .rodata 138898 139779 +881
> > packed_usage 29706 29825 +119
> > i2c_smbus_access - 48 +48
> > run_applet_no_and_exit 588 596 +8
> > err_msg_and_usage_and_die - 8 +8
> > applet_names 2451 2458 +7
> > applet_main 1428 1432 +4
> > applet_nameofs 714 716 +2
> > ------------------------------------------------------------------------------
> > (add/remove: 4/0 grow/shrink: 6/0 up/down: 2541/0) Total: 2541 bytes
> >
> > I'd like to get some comments before implementing the rest.
> >
> > Signed-off-by: Bartosz Golaszewski <bartekgola at gmail.com>
> > ---
> > miscutils/i2c_tools.c | 451 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 451 insertions(+)
> > create mode 100644 miscutils/i2c_tools.c
> >
> > diff --git a/miscutils/i2c_tools.c b/miscutils/i2c_tools.c
> > new file mode 100644
> > index 0000000..3036ae6
> > --- /dev/null
> > +++ b/miscutils/i2c_tools.c
> > @@ -0,0 +1,451 @@
> > +/* vi: set sw=4 ts=4: */
> > +/*
> > + * Minimal i2c-tools implementation for busybox.
> > + * Parts of code ported from i2c-tools:
> > + * http://www.lm-sensors.org/wiki/I2CTools.
> > + *
> > + * Copyright (C) 2014 by Bartosz Golaszewski <bartekgola at gmail.com>
> > + *
> > + * Licensed under GPLv2 or later, see file LICENSE in this source tree.
> > + */
> > +
> > +//config:config I2CGET
> > +//config: bool "i2cget"
> > +//config: default y
> > +//config: select PLATFORM_LINUX
> > +//config: help
> > +//config: Read from I2C/SMBus chip registers.
> > +
> > +//applet:IF_I2CGET(APPLET(i2cget, BB_DIR_USR_SBIN, BB_SUID_DROP))
> > +
> > +//kbuild:lib-$(CONFIG_I2CGET) += i2c_tools.o
> > +
> > +//usage:#define i2cget_trivial_usage
> > +//usage: "[-f] [-y] I2CBUS CHIP-ADDRESS [DATA-ADDRESS [MODE]]"
> > +//usage:#define i2cget_full_usage "\n\n"
> > +//usage: "Read from I2C/SMBus chip registers\n"
> > +//usage: "\n I2CBUS is an i2c bus number"
> > +//usage: "\n ADDRESS is an integer (0x03 - 0x77)"
> > +//usage: "\n MODE is one of:"
> > +//usage: "\n b (read byte data, default)"
> > +//usage: "\n w (read word data)"
> > +//usage: "\n c (write byte/read byte)"
> > +//usage: "\n\n -f - force access to device"
> > +//usage: "\n -y - disable interactive mode"
> > +
> > +#include "libbb.h"
> > +
> > +/*
> > + * /dev/i2c-X ioctl commands. The ioctl's parameter is always an unsigned long,
> > + * except for:
> > + * - I2C_FUNCS, takes pointer to an unsigned long
> > + * - I2C_RDWR, takes pointer to struct i2c_rdwr_ioctl_data
> > + * - I2C_SMBUS, takes pointer to struct i2c_smbus_ioctl_data
> > + */
> > +
> > +/* Number of times a device address should be polled when not acknowledging */
> > +#define I2C_RETRIES 0x0701
> > +/* Set timeout in units of 10 ms */
> > +#define I2C_TIMEOUT 0x0702
> > +
> > +/*
> > + * NOTE: Slave address is 7 or 10 bits, but 10-bit addresses
> > + * are NOT supported! (due to code brokenness)
> > + */
> > +
> > +/* Use this slave address */
> > +#define I2C_SLAVE 0x0703
> > +/* Use this slave address, even if it is already in use by a driver! */
> > +#define I2C_SLAVE_FORCE 0x0706
> > +/* 0 for 7 bit addrs, != 0 for 10 bit */
> > +#define I2C_TENBIT 0x0704
> > +/* Get the adapter functionality mask */
> > +#define I2C_FUNCS 0x0705
> > +/* Combined R/W transfer (one STOP only) */
> > +#define I2C_RDWR 0x0707
> > +/* != 0 to use PEC with SMBus */
> > +#define I2C_PEC 0x0708
> > +/* SMBus transfer */
> > +#define I2C_SMBUS 0x0720
> > +
> > +/* This is the structure as used in the I2C_SMBUS ioctl call */
> > +struct i2c_smbus_ioctl_data {
> > + uint8_t read_write;
> > + uint8_t command;
> > + uint32_t size;
> > + union i2c_smbus_data *data;
> > +};
> > +
> > +/* This is the structure as used in the I2C_RDWR ioctl call */
> > +struct i2c_rdwr_ioctl_data {
> > + struct i2c_msg *msgs; /* pointers to i2c_msgs */
> > + uint32_t nmsgs; /* number of i2c_msgs */
> > +};
> > +
> > +/* As specified in SMBus standard */
> > +#define I2C_SMBUS_BLOCK_MAX 32
> > +/* Not specified but we use same structure */
> > +#define I2C_SMBUS_I2C_BLOCK_MAX 32
> > +
> > +/*
> > + * Data for SMBus Messages
> > + */
> > +union i2c_smbus_data {
> > + uint8_t byte;
> > + uint16_t word;
> > + /* block[0] is used for length and one more for PEC */
> > + uint8_t block[I2C_SMBUS_BLOCK_MAX + 2];
> > +};
> > +
> > +#define I2C_RDRW_IOCTL_MAX_MSGS 42
> > +
> > +/* smbus_access read or write markers */
> > +#define I2C_SMBUS_READ 1
> > +#define I2C_SMBUS_WRITE 0
> > +
> > +/* SMBus transaction types (size parameter in the below functions). */
> > +#define I2C_SMBUS_QUICK 0
> > +#define I2C_SMBUS_BYTE 1
> > +#define I2C_SMBUS_BYTE_DATA 2
> > +#define I2C_SMBUS_WORD_DATA 3
> > +#define I2C_SMBUS_PROC_CALL 4
> > +#define I2C_SMBUS_BLOCK_DATA 5
> > +#define I2C_SMBUS_I2C_BLOCK_BROKEN 6
> > +#define I2C_SMBUS_BLOCK_PROC_CALL 7
> > +#define I2C_SMBUS_I2C_BLOCK_DATA 8
> > +
> > +/* Defines to determine what functionality is present */
> > +#define I2C_FUNC_I2C 0x00000001
> > +#define I2C_FUNC_10BIT_ADDR 0x00000002
> > +#define I2C_FUNC_PROTOCOL_MANGLING 0x00000004
> > +#define I2C_FUNC_SMBUS_PEC 0x00000008
> > +#define I2C_FUNC_SMBUS_BLOCK_PROC_CALL 0x00008000
> > +#define I2C_FUNC_SMBUS_QUICK 0x00010000
> > +#define I2C_FUNC_SMBUS_READ_BYTE 0x00020000
> > +#define I2C_FUNC_SMBUS_WRITE_BYTE 0x00040000
> > +#define I2C_FUNC_SMBUS_READ_BYTE_DATA 0x00080000
> > +#define I2C_FUNC_SMBUS_WRITE_BYTE_DATA 0x00100000
> > +#define I2C_FUNC_SMBUS_READ_WORD_DATA 0x00200000
> > +#define I2C_FUNC_SMBUS_WRITE_WORD_DATA 0x00400000
> > +#define I2C_FUNC_SMBUS_PROC_CALL 0x00800000
> > +#define I2C_FUNC_SMBUS_READ_BLOCK_DATA 0x01000000
> > +#define I2C_FUNC_SMBUS_WRITE_BLOCK_DATA 0x02000000
> > +#define I2C_FUNC_SMBUS_READ_I2C_BLOCK 0x04000000
> > +#define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK 0x08000000
> > +
> > +/*
> > + * This is needed for ioctl_or_perror_and_die() since it only accepts pointer.
> > + */
> > +#define INT_TO_PTR(i) ((void*)((long)i))
> > +
> > +static int32_t i2c_smbus_access(int fd, char read_write, uint8_t command,
> > + int size, union i2c_smbus_data *data)
> > +{
> > + struct i2c_smbus_ioctl_data args;
> > +
> > + args.read_write = read_write;
> > + args.command = command;
> > + args.size = size;
> > + args.data = data;
> > +
> > + return ioctl(fd, I2C_SMBUS, &args);
> > +}
> > +
> > +static int32_t i2c_smbus_read_byte(int fd)
> > +{
> > + union i2c_smbus_data data;
> > + int err;
> > +
> > + err = i2c_smbus_access(fd, I2C_SMBUS_READ, 0, I2C_SMBUS_BYTE, &data);
> > + if (err < 0)
> > + return err;
> > +
> > + return 0x0FF & data.byte;
> > +}
>
> nitpicking:
> is that 0x0ff needed ? so far i can see data.byte should be uint8_t ?
>
> > +
> > +static int32_t i2c_smbus_write_byte(int fd, uint8_t val)
> > +{
> > + return i2c_smbus_access(fd, I2C_SMBUS_WRITE,
> > + val, I2C_SMBUS_BYTE, NULL);
> > +}
> > +
> > +static int32_t i2c_smbus_read_byte_data(int fd, uint8_t cmd)
> > +{
> > + union i2c_smbus_data data;
> > + int err;
> > +
> > + err = i2c_smbus_access(fd, I2C_SMBUS_READ, cmd,
> > + I2C_SMBUS_BYTE_DATA, &data);
> > + if (err < 0)
> > + return err;
> > +
> > + return 0x0FF & data.byte;
> > +}
> as i2c_smbus_read_byte; 0x0ff is confusing me 0x00ff or 0xff
>
> > +
> > +static int32_t i2c_smbus_read_word_data(int fd, uint8_t cmd)
> > +{
> > + union i2c_smbus_data data;
> > + int err;
> > +
> > + err = i2c_smbus_access(fd, I2C_SMBUS_READ, cmd,
> > + I2C_SMBUS_WORD_DATA, &data);
> > + if (err < 0)
> > + return err;
> > +
> > + return 0x0FFFF & data.word;
> > +}
> > +
> > +static void NORETURN err_msg_and_usage_and_die(const char *msg)
> > +{
> > + bb_error_msg(msg);
> > + bb_show_usage();
> > +}
> > +
> +static int i2c_bus_lookup(const char *bus_str)
> +{
> + unsigned long i2cbus;
> + char *end;
> +
> + i2cbus = bb_strtoul(bus_str, &end, 10);
> + if (*end || !*bus_str)
> + err_msg_and_usage_and_die("Bus address is not a number");
> + if (i2cbus > 0xfffff)
> + err_msg_and_usage_and_die("Bus address out of range");
> +
> + return i2cbus;
> +}
> +
Maybe ?
static int i2c_bus_lookup(const char *bus_str)
{
unsigned long i2cbus = xstrtou_range(bus_str, 10, 0, 0xfffff);
return i2cbus;
}
> > +static int i2c_parse_addr(const char *addr_str)
> > +{
> > + long addr;
> > + char *end;
> > +
> > + addr = bb_strtol(addr_str, &end, 16);
> > + if (*end || !*addr_str)
> > + err_msg_and_usage_and_die("Chip address is not a number");
> > + if (addr < 0x03 || addr > 0x77)
> > + err_msg_and_usage_and_die("Chip address out of range");
> > +
> > + return addr;
> > +}
and
static int i2c_parse_addr(const char *addr_str) {
long addr = xstrto_range(addr_str, 16, 0x03, 0x77);
return addr;
}
so err_msg_and_usage_and_die(const char *msg) could be
removed as xstrto_range has error messages and dies
or eventually move the one liners to main function.
> > +
> > +/*
> > + * Opens the device file associated with given i2c bus.
> > + *
> > + * i2cbus - number of the i2c bus
> > + * filename - buffer to store the filename
> > + * len - buffer length
> > + */
> > +static int i2c_dev_open(int i2cbus, char *filename, size_t len)
> > +{
> > + int fd;
> > +
> > + snprintf(filename, len, "/dev/i2c-%d", i2cbus);
> > +
> > + fd = open(filename, O_RDWR);
> > + if (fd < 0 && errno == ENOENT) {
> > + /* Older kernels register the devices files this way. */
> > + snprintf(filename, len, "/dev/i2c/%d", i2cbus);
> > + fd = open(filename, O_RDWR);
> > + }
> > +
> > + if (fd < 0)
> > + bb_perror_msg_and_die("Could not open file: "
> > + "/dev/i2c-%d or /dev/i2c/%d",
> > + i2cbus, i2cbus);
> > +
> > + return fd;
> > +}
>
Maybe?
static int i2c_dev_open(int i2cbus, char *filename, size_t len)
{
int fd;
snprintf(filename, len, "/dev/i2c-%d", i2cbus);
fd = open_or_warn(filename, O_RDWR);
if (fd < 0 && errno == ENOENT) {
snprintf(filename, len, "/dev/i2c/%d", i2cbus);
fd = xopen(filename, O_RDWR);
}
return fd;
}
> could you please explain shortly why you have a external buffer here ?
>
> you could easly do
> filename=xasprintf("/dev/i2c-%d", i2cbus);
> or
> char filename[32];
> or
> ....
>
>
> btw:
> bb_perror_msg_and_die("Could not open file: "Could not open file: %s",filename) would work also ?
>
> > +
> > +static void i2c_set_slave_addr(int fd, int addr, int force)
> > +{
> > + ioctl_or_perror_and_die(fd, force ? I2C_SLAVE_FORCE : I2C_SLAVE,
> > + INT_TO_PTR(addr),
> > + "Could not set address to 0x%02x", addr);
> > +}
> > +
> > +static void check_funcs(int fd, int mode, int data_addr, int pec)
> > +{
> > + unsigned long funcs;
> > + const char *err;
> > +
> > + ioctl_or_perror_and_die(fd, I2C_FUNCS, &funcs,
> > + "Could not get the adapter functionality matrix");
> > +
> > + switch (mode) {
> > + case I2C_SMBUS_BYTE:
> > + if (!(funcs & I2C_FUNC_SMBUS_READ_BYTE)) {
> > + err = "SMBus receive byte";
> > + goto err_out;
> > + }
> > + if (data_addr >= 0 && !(funcs & I2C_FUNC_SMBUS_WRITE_BYTE)) {
> > + err = "SMBus send byte";
> > + goto err_out;
> > + }
> > + break;
> > + case I2C_SMBUS_BYTE_DATA:
> > + if (!(funcs & I2C_FUNC_SMBUS_READ_BYTE_DATA)) {
> > + err = "SMBus read byte";
> > + goto err_out;
> > + }
> > + break;
> > + case I2C_SMBUS_WORD_DATA:
> > + if (!(funcs & I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> > + err = "SMBus read word";
> > + goto err_out;
> > + }
> > + break;
> > + }
> > +
> > + if (pec && !(funcs & (I2C_FUNC_SMBUS_PEC | I2C_FUNC_I2C)))
bb_error_msg ?
> > + fprintf(stderr,
> > + "Warning: Adapter does not seem to support PEC\n");
> > +
> > + return;
> > +
> > +err_out:
> > + bb_error_msg_and_die("Adapter does not have %s capability", err);
> > +}
> > +
> > +/* Return 1 if we should continue, 0 if we should abort */
> > +static int user_ack(int def)
> > +{
> > + char s[2];
> > + int ret;
> > +
> > + if (!fgets(s, 2, stdin))
> > + return 0; /* Nack by default */
> i would use read(STDIN_FILENO,s,2);
>
> > +
> > + switch (s[0]) {
>
> tolower(s[0]); ? just to avoild these double labels.
>
> > + case 'y':
> > + case 'Y':
> > + ret = 1;
> > + break;
> > + case 'n':
> > + case 'N':
> > + ret = 0;
> > + break;
> > + default:
> > + ret = def;
> > + }
> > +
> > + /* Flush extra characters */
> > + while (s[0] != '\n') {
> > + int c = fgetc(stdin);
> > + if (c == EOF) {
> > + ret = 0;
> > + break;
> > + }
> > + s[0] = c;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +/*
> > + * The messages displayed here are much less elaborate than their i2c-tools
> > + * counterparts - this is done for size reduction.
> > + */
> > +static void confirm_action(int bus_addr, int mode, int data_addr, int pec)
> > +{
> > + int dont = 0;
> > +
bb_err_msg ?
> > + fprintf(stderr, "WARNING! This program can confuse your I2C bus\n");
> > +
> > + /* Don't let the user break his/her EEPROMs */
> > + if (bus_addr >= 0x50 && bus_addr <= 0x57 && pec) {
bb_error_msg_and_die ?
> > + fprintf(stderr, "This is I2C not smbus - using PEC on I2C "
> > + "devices may result in data loss. Aborting.\n");
> > + xfunc_die();
> > + }
> > +
> > + if (mode == I2C_SMBUS_BYTE && data_addr >= 0 && pec) {
bb_error_msg ?
> > + fprintf(stderr, "WARNING! May interpret a write byte command "
> > + "with PEC as a write byte data command!\n");
> > + dont++;
> > + }
> > +
> > + if (pec)
bb_error_msg( "PEC checking enabled")
> > + fprintf(stderr, "PEC checking enabled.\n");
> >
bb_ask_confirmation(); and remove function user_ack ?
> > + fprintf(stderr, "Continue? [%s] ", dont ? "y/N" : "Y/n");
> > + fflush(stderr);
> > + if (!user_ack(!dont)) {
> > +
bb_error_msg_and_die
> > + fprintf(stderr, "Aborting.\n");
> > + xfunc_die();
> > + }
> > +}
> > +
> maybe this will work alsp ?
> tolower(read_nonempty("Continue? Y/n"));
> (see: util-linux/fdisk_osf.c)
>
>
> > +int i2cget_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
> > +int i2cget_main(int argc, char **argv)
> > +{
> > + static const unsigned opt_f = (1 << 0), opt_y = (1 << 1);
> > + static const char *const optstr = "fy";
> > +
> > + unsigned opts;
> > + int bus_num, bus_addr, data_addr = -1;
> > + int mode = I2C_SMBUS_BYTE, pec = 0, fd, status;
> > + char *end;
> > + char filename[32];
> > +
> > + opts = getopt32(argv, optstr);
> > + argv += optind;
> > + argc -= optind;
> > +
> > + if (argc < 2)
> > + bb_show_usage();
> > +
> > + bus_num = i2c_bus_lookup(argv[0]);
> > + bus_addr = i2c_parse_addr(argv[1]);
> > +
> > + if (argc > 2) {
xstrtol_range ?
> > + data_addr = bb_strtol(argv[2], &end, 16);
> > + if (*end || data_addr < 0 || data_addr > 0xff)
> > + err_msg_and_usage_and_die("Data address invalid");
> > + }
> > +
> > + if (argc > 3) {
> > + switch (argv[3][0]) {
> > + case 'b': mode = I2C_SMBUS_BYTE_DATA; break;
> > + case 'w': mode = I2C_SMBUS_WORD_DATA; break;
> > + case 'c': /* Already set */ break;
bb_error_msg_and_die ?
> > + default: err_msg_and_usage_and_die("Invalid mode");
> > + }
> > + pec = argv[3][1] == 'p';
> > + }
> > +
> > + fd = i2c_dev_open(bus_num, filename, sizeof(filename));
> > + check_funcs(fd, mode, data_addr, pec);
> > + i2c_set_slave_addr(fd, bus_addr, opts && opt_f);
> > +
> > + if (!(opts && opt_y))
> > + confirm_action(bus_addr, mode, data_addr, pec);
> > +
> > + if (pec)
> > + ioctl_or_perror_and_die(fd, I2C_PEC, INT_TO_PTR(1),
> > + "Could not set PEC");
> > +
> > + switch (mode) {
> > + case I2C_SMBUS_BYTE:
> > + if (data_addr >= 0) {
> > + status = i2c_smbus_write_byte(fd, data_addr);
> > + if (status < 0)
bb_error_msg("write failed"); ?
> > + fprintf(stderr, "Warning - write failed\n");
> > + }
> > + status = i2c_smbus_read_byte(fd);
> > + break;
> > + case I2C_SMBUS_WORD_DATA:
> > + status = i2c_smbus_read_word_data(fd, data_addr);
> > + break;
> > + default: /* I2C_SMBUS_BYTE_DATA */
> > + status = i2c_smbus_read_byte_data(fd, data_addr);
> > + }
> > + close(fd);
> > +
> > + if (status < 0)
> > + bb_error_msg_and_die("Read failed");
> > +
> > + printf("0x%0*x\n", mode == I2C_SMBUS_WORD_DATA ? 4 : 2, status);
> > +
> > + return 0;
> > +}
Hi,
just a few hints to reduce code size using libbb functions.
Hope this helps.
Ciao,
Tito
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20141209/963b7553/attachment-0001.html>
More information about the busybox
mailing list