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

[Wolfram Sang <wsa@the-dreams.de>]

[-- Attachment #1: Type: text/plain, Size: 2855 bytes --]

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]