Re: [i2c-tools PATCH v2] i2ctransfer: add new tool

From: Wolfram Sang
Date: Mon Mar 06 2017 - 14:13:31 EST


Hi Geert,

> > This tool allows to construct and concat multiple I2C messages into one
> > single transfer. Its aim is to test I2C master controllers, and so there
> > is no SMBus fallback.
>
> Thanks for the tool!

Very welcome :)

>
> > I've been missing such a tool a number of times now, so I finally got
> > paround to writing it myself. As with all I2C tools, it can be dangerous,
>
> around
>
> Very dangerous, it inserts spurious "p" characters ;-)

Yeah, but only if you write the tool. Everyone else is safe :D

> > +A transfer may consist of multiple messages and is started with a START condition and ends with a STOP condition as described in the I2C specification.
>
> Funny, this is the other way around than on SPI (an SPI message consists
> of multiple transfers).

In deed, nice to know.

> > +It is parsed as an unsigned 16 bit integer, but note that the Linux might apply an additional upper limit (8192 as of v4.10).
>
> s/the Linux/Linux/ (or the kernel, or i2c driver?)

Missing "Kernel". Will fix.

> > +Normally, addresses outside the range of 0x03-0x77 and addresses with a kernel driver attached to them will be blocked.
>
> So 10-bit adressing needs -f?

Not supported, will add this info to the man-page.

> > +static void print_msgs(struct i2c_msg *msgs, __u32 nmsgs, unsigned flags)
>
> unsigned int nmsgs?

No... 1)

>
> > +{
> > + FILE *output = flags & PRINT_STDERR ? stderr : stdout;
> > + unsigned i;
> > + __u16 j;
>
> unsigned int, too?

No... 1)

> > +static int confirm(const char *filename, struct i2c_msg *msgs, __u32 nmsgs)
>
> unsigned int nmsgs?

No... 1)

>
> > +{
> > + fprintf(stderr, "WARNING! This program can confuse your I2C bus, cause data loss and worse!\n");
> > + fprintf(stderr, "I will send the following messages to device file %s:\n", filename);
> > + print_msgs(msgs, nmsgs, PRINT_STDERR | PRINT_HEADER | PRINT_WRITE_BUF);
> > +
> > + fprintf(stderr, "Continue? [y/N] ");
> > + fflush(stderr);
> > + if (!user_ack(0)) {
> > + fprintf(stderr, "Aborting on user request.\n");
> > + return 0;
> > + }
> > +
> > + return 1;
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > + char filename[20];
> > + int i2cbus, address = -1, file, arg_idx = 1, nmsgs = 0, nmsgs_sent, i;
>
> unsigned int i?

No... 2)

>
> > + while (arg_idx < argc) {
> > + char *arg_ptr = argv[arg_idx];
> > + unsigned long len, raw_data;
> > + __u16 flags;
>
> unsigned int flags?

No... 1)

1) I prefer to keep the type of the data source, i.e. where the value is
copied from
2) i is always int for me

Thanks for the comments,

Wolfram

Attachment: signature.asc
Description: PGP signature