| Message ID | 1425053816-19804-1-git-send-email-wsa@the-dreams.de (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
On Fri, Feb 27, 2015 at 05:16:56PM +0100, Wolfram Sang wrote: > 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. > > Signed-off-by: Wolfram Sang <wsa@the-dreams.de> > --- > > I've been missing such a tool a number of times now, so I finally got around to > writing it myself. As with all I2C tools, it can be dangerous, but it can also > be very useful when developing. I am not sure if distros should supply it, I'll > leave that to Jean's experience. For embedded build systems, I think this > should be selectable. It is RFC for now because it needs broader testing and some > more beautification. However, I've been using it already to test the i2c_quirk > infrastructure and Renesas I2C controllers. Jean, my tests went well and so I want to brush it up for inclusion into i2c-tools upstream. Any show-stoppers you see from a high-level point of view?
Hi Wolfram, On Mon, 20 Apr 2015 19:36:38 +0200, Wolfram Sang wrote: > On Fri, Feb 27, 2015 at 05:16:56PM +0100, Wolfram Sang wrote: > > 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. > > > > Signed-off-by: Wolfram Sang <wsa@the-dreams.de> > > --- > > > > I've been missing such a tool a number of times now, so I finally got around to > > writing it myself. As with all I2C tools, it can be dangerous, but it can also > > be very useful when developing. I am not sure if distros should supply it, I'll > > leave that to Jean's experience. For embedded build systems, I think this > > should be selectable. It is RFC for now because it needs broader testing and some > > more beautification. However, I've been using it already to test the i2c_quirk > > infrastructure and Renesas I2C controllers. > > Jean, my tests went well and so I want to brush it up for inclusion into > i2c-tools upstream. Any show-stoppers you see from a high-level point of > view? I think it is a good idea, just I couldn't find the time to review it, sorry :(
On Tue, Apr 21, 2015 at 07:25:51AM +0200, Jean Delvare wrote: > Hi Wolfram, > > On Mon, 20 Apr 2015 19:36:38 +0200, Wolfram Sang wrote: > > On Fri, Feb 27, 2015 at 05:16:56PM +0100, Wolfram Sang wrote: > > > 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. > > > > > > Signed-off-by: Wolfram Sang <wsa@the-dreams.de> > > > --- > > > > > > I've been missing such a tool a number of times now, so I finally got around to > > > writing it myself. As with all I2C tools, it can be dangerous, but it can also > > > be very useful when developing. I am not sure if distros should supply it, I'll > > > leave that to Jean's experience. For embedded build systems, I think this > > > should be selectable. It is RFC for now because it needs broader testing and some > > > more beautification. However, I've been using it already to test the i2c_quirk > > > infrastructure and Renesas I2C controllers. > > > > Jean, my tests went well and so I want to brush it up for inclusion into > > i2c-tools upstream. Any show-stoppers you see from a high-level point of > > view? > > I think it is a good idea, just I couldn't find the time to review it, > sorry :( No problem, no hurry. Glad to know that you like it in general.
Hi Wolfram, On Fri, 27 Feb 2015 17:16:56 +0100, Wolfram Sang wrote: > 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. > > Signed-off-by: Wolfram Sang <wsa@the-dreams.de> > --- > > I've been missing such a tool a number of times now, so I finally got around to > writing it myself. As with all I2C tools, it can be dangerous, but it can also > be very useful when developing. I am not sure if distros should supply it, I'll > leave that to Jean's experience. For embedded build systems, I think this > should be selectable. It is RFC for now because it needs broader testing and some > more beautification. However, I've been using it already to test the i2c_quirk > infrastructure and Renesas I2C controllers. I took a very quick look today and I am fine with the general idea but one thing I don't like is the user interface. All parameters look the same even though they have very different meanings. It's very easy to get it wrong and guess what, at my very first attempt I managed to do exactly that. I was lucky enough that my incorrect syntax was not accepted by the tool, but with slightly different values it would have been accepted and and the tool would have performed something different from what I wanted to do. Possibly with tragic consequences. So instead of: # i2ctransfer 0 0x50 w 0x11 0xc0 0xbd= 0x51 r 1 I would have imagined something like: # i2ctransfer 0 w@0x50 0x11 0xc0 0xbd= r@0x51 1 This at least avoids passing addresses as register values or vice-versa. What do you think? Or maybe it's just me being an idiot and other users would get it right even with your proposed syntax. My alternative proposal leaves the potential problem of mixing length and register values. I am not necessarily a fan of passing the write length on the command line, but it is similar to the read commands, and I see it allows writing the same value to a whole register range with little effort, so it does have some value. So I guess it's OK. BTW I'm curious if you actually needed the + and - suffixes in practice? I can easily imagine how the = suffix will be useful in the real world, but + and -... Or maybe just for bus driver testing purpose? I don't have time for a full review today but just a couple things which caught my eye: > +static void help(void) > +{ > + fprintf(stderr, > + "Usage: i2ctransfer [-f] [-y] [-v] [-V] I2CBUS ADDRESS FLAGS LENGTH [DATA]...\n" You should mention here that the ADDRESS FLAGS LENGTH [DATA] sequence can be repeated, it's not obvious that the trailing "..." mean that. > + " I2CBUS is an integer or an I2C bus name\n" > + " ADDRESS is an integer (0x03 - 0x77)\n" > + " FLAGS is one of:\n" > + " r (read)\n" > + " w (write)\n" These are really directions not flags. > + " LENGTH is an integer (0 - 65535)\n" > + " DATA are LENGTH bytes, for a write message. They can be shortened by a suffix:\n" > + " = (keep value constant until LENGTH)\n" > + " + (increase value by 1 until LENGTH)\n" > + " - (decrease value by 1 until LENGTH)\n" > + "\nExample (on bus 0, write 0xbd to 0xc0-0xcf of device 0x50, read a byte from device 0x51):\n" > + " # i2ctransfer 0 0x50 w 0x11 0xc0 0xbd= 0x51 r 1\n" > + ); > (...) > +} > + /* let Linux free malloced memory on termination */ I don't like this. The memory allocated in i2cbusses.c is freed explicitly, so it is inconsistent to not free yours. Freeing the memory explicitly makes the code easier to read and debug as it documents the lifetime of allocated buffers (for both humans and tools like valgrind.) So please free your memory buffers explicitly. > + exit(0); > +}
On Thu, 7 May 2015 22:08:12 +0200, Jean Delvare wrote: > Hi Wolfram, > > On Fri, 27 Feb 2015 17:16:56 +0100, Wolfram Sang wrote: > > 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. > > > > Signed-off-by: Wolfram Sang <wsa@the-dreams.de> > > --- > > > > I've been missing such a tool a number of times now, so I finally got around to > > writing it myself. As with all I2C tools, it can be dangerous, but it can also > > be very useful when developing. I am not sure if distros should supply it, I'll > > leave that to Jean's experience. For embedded build systems, I think this > > should be selectable. It is RFC for now because it needs broader testing and some > > more beautification. However, I've been using it already to test the i2c_quirk > > infrastructure and Renesas I2C controllers. > > I took a very quick look today and I am fine with the general idea but > one thing I don't like is the user interface. All parameters look the > same even though they have very different meanings. It's very easy to > get it wrong and guess what, at my very first attempt I managed to do > exactly that. I was lucky enough that my incorrect syntax was not > accepted by the tool, but with slightly different values it would have > been accepted and and the tool would have performed something different > from what I wanted to do. Possibly with tragic consequences. > > So instead of: > > # i2ctransfer 0 0x50 w 0x11 0xc0 0xbd= 0x51 r 1 > > I would have imagined something like: > > # i2ctransfer 0 w@0x50 0x11 0xc0 0xbd= r@0x51 1 > > This at least avoids passing addresses as register values or > vice-versa. What do you think? Or maybe it's just me being an idiot and > other users would get it right even with your proposed syntax. > > My alternative proposal leaves the potential problem of mixing length > and register values. I am not necessarily a fan of passing the write > length on the command line, but it is similar to the read commands, and > I see it allows writing the same value to a whole register range with > little effort, so it does have some value. So I guess it's OK. Having slept over it, I came up with a 3rd proposal: # i2ctransfer 0 w0x11@0x50 0xc0 0xbd= r1@0x51 That is, combining the slave address, direction and length into a single parameter. The advantage is that this is all more explicit and the risk of mixing up values is close to zero. Whether it is more or less readable than the previous proposals is probably a matter of taste. Also I suspect it would make the parsing and state machine more simple, but that's only a nice side effect. Wolfram (and others), please tell me what you think. I am not trying to force my views here, just suggesting alternatives for your consideration.
> Having slept over it, I came up with a 3rd proposal: > > # i2ctransfer 0 w0x11@0x50 0xc0 0xbd= r1@0x51 > > That is, combining the slave address, direction and length into a > single parameter. The advantage is that this is all more explicit and > the risk of mixing up values is close to zero. Whether it is more or > less readable than the previous proposals is probably a matter of > taste. Also I suspect it would make the parsing and state machine more > simple, but that's only a nice side effect. > > Wolfram (and others), please tell me what you think. I am not trying to > force my views here, just suggesting alternatives for your > consideration. I liked your proposal, so thanks for this input. I agree that the risk of mixing something up is high, I was okay with the printout of the messages to be sent, but a better syntax is very welcome, too. I need to think about the flags a little bit, though. Although this isn't implemented yet, PEC and 10-bit flags might be added in the future? Handling R/W as "just another" flag made this option extremly simple. But we probably can work something out. So much for the quick response, I'll have a closer look later.
I'm curious why this would not be an extension of the i2c read and write commands? Would it not make sense to add a tier above "Block" (perhaps "Extended"), and use the same syntax? Forgive me if this is out of place - I'm quite new, both here and to Linux/C. On Thu, 7 May 2015 22:08:12 +0200, Jean Delvare wrote: > Hi Wolfram, > > On Fri, 27 Feb 2015 17:16:56 +0100, Wolfram Sang wrote: > > 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. > > > > Signed-off-by: Wolfram Sang <wsa@the-dreams.de> > > --- > > > > I've been missing such a tool a number of times now, so I finally > > got around to writing it myself. As with all I2C tools, it can be > > dangerous, but it can also be very useful when developing. I am not > > sure if distros should supply it, I'll leave that to Jean's > > experience. For embedded build systems, I think this should be > > selectable. It is RFC for now because it needs broader testing and > > some more beautification. However, I've been using it already to test the i2c_quirk infrastructure and Renesas I2C controllers. > > I took a very quick look today and I am fine with the general idea but > one thing I don't like is the user interface. All parameters look the > same even though they have very different meanings. It's very easy to > get it wrong and guess what, at my very first attempt I managed to do > exactly that. I was lucky enough that my incorrect syntax was not > accepted by the tool, but with slightly different values it would have > been accepted and and the tool would have performed something > different from what I wanted to do. Possibly with tragic consequences. > > So instead of: > > # i2ctransfer 0 0x50 w 0x11 0xc0 0xbd= 0x51 r 1 > > I would have imagined something like: > > # i2ctransfer 0 w@0x50 0x11 0xc0 0xbd= r@0x51 1 > > This at least avoids passing addresses as register values or > vice-versa. What do you think? Or maybe it's just me being an idiot > and other users would get it right even with your proposed syntax. > > My alternative proposal leaves the potential problem of mixing length > and register values. I am not necessarily a fan of passing the write > length on the command line, but it is similar to the read commands, > and I see it allows writing the same value to a whole register range > with little effort, so it does have some value. So I guess it's OK. Having slept over it, I came up with a 3rd proposal: # i2ctransfer 0 w0x11@0x50 0xc0 0xbd= r1@0x51 That is, combining the slave address, direction and length into a single parameter. The advantage is that this is all more explicit and the risk of mixing up values is close to zero. Whether it is more or less readable than the previous proposals is probably a matter of taste. Also I suspect it would make the parsing and state machine more simple, but that's only a nice side effect. Wolfram (and others), please tell me what you think. I am not trying to force my views here, just suggesting alternatives for your consideration. -- Jean Delvare SUSE L3 Support -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Randy, On Fri, May 08, 2015 at 03:28:19PM +0000, Randy Grunwell wrote: > I'm curious why this would not be an extension of the i2c read and write commands? Would it not make sense to add a tier above "Block" (perhaps "Extended"), and use the same syntax? > > Forgive me if this is out of place - I'm quite new, both here and to Linux/C. one thing you should look into is how you quote mails correctly and please don't top-post. Also breaking long lines after ~72 chars is usually done. These requests might seem picky, but they simplify things for people reading several hundred mails per day. Best regards Uwe
Hi Randy, On Fri, 8 May 2015 15:28:19 +0000, Randy Grunwell wrote: > I'm curious why this would not be an extension of the i2c read and write commands? Would it not make sense to add a tier above "Block" (perhaps "Extended"), and use the same syntax? > > Forgive me if this is out of place - I'm quite new, both here and to Linux/C. No problem, asking questions is fine. The thing is that this isn't only a question of maximum length. It is also a question of which kernel interface is being used (ioctl I2C_RDWR instead of ioctl I2C_SMBUS.) Additionally, i2ctransfer supports any combination of reading and writing, so in essence it doesn't extend a specific existing tool, it extends all of them. And the command line interface will be completely different, whichever we settle for. So it seems quite obvious that a separate tool is the best way to implement the feature, as Wolfram did.
Hi Wolfram, On Fri, 8 May 2015 16:38:26 +0200, Wolfram Sang wrote: > > Having slept over it, I came up with a 3rd proposal: > > > > # i2ctransfer 0 w0x11@0x50 0xc0 0xbd= r1@0x51 > > > > That is, combining the slave address, direction and length into a > > single parameter. The advantage is that this is all more explicit and > > the risk of mixing up values is close to zero. Whether it is more or > > less readable than the previous proposals is probably a matter of > > taste. Also I suspect it would make the parsing and state machine more > > simple, but that's only a nice side effect. > > > > Wolfram (and others), please tell me what you think. I am not trying to > > force my views here, just suggesting alternatives for your > > consideration. > > I liked your proposal, so thanks for this input. I agree that the risk > of mixing something up is high, I was okay with the printout of the > messages to be sent, but a better syntax is very welcome, too. I need to > think about the flags a little bit, though. Although this isn't > implemented yet, PEC and 10-bit flags might be added in the future? This is a good point, we need to think about it. Maybe not PEC, as normally any PEC-enabled transaction would be handled by the other tools already. And I don't think the kernel can handle PEC over ioctl I2C_RDWR anyway. But 10-bit addresses, we already had a request to support than and your new tool would be perfect for that. One easy way would be to assume that the transaction either targets one or more 10-bit addressed chips, or one or more 7-bit addressed chips, but doesn't mix. In that case a simple flag (say -t) in front of the transaction will do the trick. I'd think it is sufficient, and I even suspect that some controllers may only support that, but OTOH I never worked with 10-bit addressed chips so I can't really tell. If you think it's not enough, then the address modifier could go separately before or after the address byte, i.e. either r1@0x123t or r1@t0x123. I suspect that the latter should be easier to implement. > Handling R/W as "just another" flag made this option extremly simple. > But we probably can work something out. I think the proposal above makes more sense than grouping it with the direction letter (r or w) even though it's also a letter, as it's really an address modifier, which affects neither the direction nor the length. But again it's really only a suggestion, if you can come up with something clearer and/or easier to implement, please do. > So much for the quick response, I'll have a closer look later. I wouldn't call it "quick" ;-) but you're welcome.
Hi Jean, > If you think it's not enough, then the address modifier could go > separately before or after the address byte, i.e. either r1@0x123t or > r1@t0x123. I suspect that the latter should be easier to implement. In deed, I would aim for maximum flexibility (i.e. possible to mix 7 and 10 bit addresses), and implement the latter proposal of yours. > > Handling R/W as "just another" flag made this option extremly simple. > > But we probably can work something out. > > I think the proposal above makes more sense than grouping it with the > direction letter (r or w) I agree.
> BTW I'm curious if you actually needed the + and - suffixes in practice? > I can easily imagine how the = suffix will be useful in the real > world, but + and -... Or maybe just for bus driver testing purpose? Exactly for that. While you can send complex messages with my tool, it should be clear this is mainly for testing/developing. Once you know what you need to do to access a slave, a specfic driver is the better option, be it in userspace or kernel space. Those patterns are easily recognizable when fed into an EEPROM. They can already reveal quite some problems with bus drivers, e.g. off-by-one errors when fetching data, sending STOP etc. > > +} > > + /* let Linux free malloced memory on termination */ > > I don't like this. The memory allocated in i2cbusses.c is freed > explicitly, so it is inconsistent to not free yours. Freeing the memory Well, it is documented :) > explicitly makes the code easier to read and debug as it documents the I disagree about this one. Currently, we have a LOT of error paths when parsing input fails. Getting all the error paths right with freeing memory is usually error-prone (especially with later additions) and will IMO make the code way less readable. Since the lifetime of those buffers ends right before the exit(0), it seems unnecessarily complex to me. However, I need to redo the parsing anyhow. Let's see how it looks like in the next version. If it is easy/readable to free(), I'll do it.
diff --git a/tools/Module.mk b/tools/Module.mk index d14bb0c..62f1238 100644 --- a/tools/Module.mk +++ b/tools/Module.mk @@ -14,7 +14,7 @@ TOOLS_CFLAGS := -Wstrict-prototypes -Wshadow -Wpointer-arith -Wcast-qual \ -W -Wundef -Wmissing-prototypes -Iinclude TOOLS_LDFLAGS := -Llib -li2c -TOOLS_TARGETS := i2cdetect i2cdump i2cset i2cget +TOOLS_TARGETS := i2cdetect i2cdump i2cset i2cget i2ctransfer # # Programs @@ -32,6 +32,9 @@ $(TOOLS_DIR)/i2cset: $(TOOLS_DIR)/i2cset.o $(TOOLS_DIR)/i2cbusses.o $(TOOLS_DIR) $(TOOLS_DIR)/i2cget: $(TOOLS_DIR)/i2cget.o $(TOOLS_DIR)/i2cbusses.o $(TOOLS_DIR)/util.o $(CC) $(LDFLAGS) -o $@ $^ $(TOOLS_LDFLAGS) +$(TOOLS_DIR)/i2ctransfer: $(TOOLS_DIR)/i2ctransfer.o $(TOOLS_DIR)/i2cbusses.o $(TOOLS_DIR)/util.o + $(CC) $(LDFLAGS) -o $@ $^ $(TOOLS_LDFLAGS) + # # Objects # @@ -48,6 +51,9 @@ $(TOOLS_DIR)/i2cset.o: $(TOOLS_DIR)/i2cset.c $(TOOLS_DIR)/i2cbusses.h $(TOOLS_DI $(TOOLS_DIR)/i2cget.o: $(TOOLS_DIR)/i2cget.c $(TOOLS_DIR)/i2cbusses.h $(TOOLS_DIR)/util.h version.h $(INCLUDE_DIR)/i2c/smbus.h $(CC) $(CFLAGS) $(TOOLS_CFLAGS) -c $< -o $@ +$(TOOLS_DIR)/i2ctransfer.o: $(TOOLS_DIR)/i2ctransfer.c $(TOOLS_DIR)/i2cbusses.h $(TOOLS_DIR)/util.h version.h + $(CC) $(CFLAGS) -Wno-maybe-uninitialized $(TOOLS_CFLAGS) -c $< -o $@ + $(TOOLS_DIR)/i2cbusses.o: $(TOOLS_DIR)/i2cbusses.c $(TOOLS_DIR)/i2cbusses.h $(CC) $(CFLAGS) $(TOOLS_CFLAGS) -c $< -o $@ diff --git a/tools/i2ctransfer.c b/tools/i2ctransfer.c new file mode 100644 index 0000000..30923f5 --- /dev/null +++ b/tools/i2ctransfer.c @@ -0,0 +1,296 @@ +/* + i2ctransfer.c - A user-space program to send concatenated i2c messages + Copyright (C) 2015 Wolfram Sang <wsa@sang-engineering.com> + Copyright (C) 2015 Renesas Electronics Corporation + + Based on i2cget.c: + Copyright (C) 2005-2012 Jean Delvare <jdelvare@suse.de> + + which is based on i2cset.c: + Copyright (C) 2001-2003 Frodo Looijaard <frodol@dds.nl>, and + Mark D. Studebaker <mdsxyz123@yahoo.com> + Copyright (C) 2004-2005 Jean Delvare + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. +*/ + +#include <sys/ioctl.h> +#include <errno.h> +#include <string.h> +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <linux/i2c.h> +#include <linux/i2c-dev.h> +#include "i2cbusses.h" +#include "util.h" +#include "../version.h" + +enum parse_state { + PARSE_GET_ADDR, + PARSE_GET_FLAGS, + PARSE_GET_LENGTH, + PARSE_GET_DATA +}; + +#define PRINT_STDERR (1 << 0) +#define PRINT_READ_BUF (1 << 1) +#define PRINT_WRITE_BUF (1 << 2) +#define PRINT_HEADER (1 << 3) + +static void help(void) +{ + fprintf(stderr, + "Usage: i2ctransfer [-f] [-y] [-v] [-V] I2CBUS ADDRESS FLAGS LENGTH [DATA]...\n" + " I2CBUS is an integer or an I2C bus name\n" + " ADDRESS is an integer (0x03 - 0x77)\n" + " FLAGS is one of:\n" + " r (read)\n" + " w (write)\n" + " LENGTH is an integer (0 - 65535)\n" + " DATA are LENGTH bytes, for a write message. They can be shortened by a suffix:\n" + " = (keep value constant until LENGTH)\n" + " + (increase value by 1 until LENGTH)\n" + " - (decrease value by 1 until LENGTH)\n" + "\nExample (on bus 0, write 0xbd to 0xc0-0xcf of device 0x50, read a byte from device 0x51):\n" + " # i2ctransfer 0 0x50 w 0x11 0xc0 0xbd= 0x51 r 1\n" + ); +} + +static int check_funcs(int file) +{ + unsigned long funcs; + + /* check adapter functionality */ + if (ioctl(file, I2C_FUNCS, &funcs) < 0) { + fprintf(stderr, "Error: Could not get the adapter " + "functionality matrix: %s\n", strerror(errno)); + return -1; + } + + if (!(funcs & I2C_FUNC_I2C)) { + fprintf(stderr, MISSING_FUNC_FMT, "I2C transfers"); + return -1; + } + + return 0; +} + +static void print_msgs(struct i2c_msg *msgs, __u32 nmsgs, unsigned flags) +{ + __u32 i, j; + FILE *output = flags & PRINT_STDERR ? stderr : stdout; + + for (i = 0; i < nmsgs; i++) { + int read = !!(msgs[i].flags & I2C_M_RD); + int newline = !!(flags & PRINT_HEADER); + + if (flags & PRINT_HEADER) + fprintf(output, "Msg %u: addr 0x%04x, %s, len %u", + i, msgs[i].addr, read ? "read" : "write", msgs[i].len); + if (read == !!(flags & PRINT_READ_BUF) || + !read == !!(flags & PRINT_WRITE_BUF)) { + if (flags & PRINT_HEADER) + fprintf(output, ", buf "); + for (j = 0; j < msgs[i].len; j++) + fprintf(output, "0x%02x ", msgs[i].buf[j]); + newline = 1; + } + if (newline) + fprintf(output, "\n"); + } +} + +static int confirm(const char *filename, struct i2c_msg *msgs, __u32 nmsgs) +{ + 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 c, filename[20]; + char *end; + int i2cbus, address, file, arg_idx = 1; + int force = 0, yes = 0, version = 0, verbose = 0; + unsigned flag_idx = 0, buf_idx = 0, nmsgs = 0; + unsigned long len, raw_data; + __u8 data; + __u8 *buf; + __u16 flags; + struct i2c_msg msgs[I2C_RDRW_IOCTL_MAX_MSGS]; + struct i2c_rdwr_ioctl_data rdwr; + enum parse_state state = PARSE_GET_ADDR; + + /* handle (optional) arg_idx first */ + while (arg_idx < argc && argv[arg_idx][0] == '-') { + switch (argv[arg_idx][1]) { + case 'V': version = 1; break; + case 'v': verbose = 1; break; + case 'f': force = 1; break; + case 'y': yes = 1; break; + default: + fprintf(stderr, "Error: Unsupported option " + "\"%s\"!\n", argv[arg_idx]); + help(); + exit(1); + } + arg_idx++; + } + + if (version) { + fprintf(stderr, "i2ctransfer version %s\n", VERSION); + exit(0); + } + + if (arg_idx == argc) { + help(); + exit(0); + } + + i2cbus = lookup_i2c_bus(argv[arg_idx++]); + if (i2cbus < 0) + exit(1); + + file = open_i2c_dev(i2cbus, filename, sizeof(filename), 0); + if (file < 0 || check_funcs(file)) + exit(1); + + while (arg_idx < argc) { + switch (state) { + case PARSE_GET_ADDR: + address = parse_i2c_address(argv[arg_idx++]); + if (address < 0) + exit(1); + + if (!force && set_slave_addr(file, address, 0)) + exit(1); + + msgs[nmsgs].addr = address; + state = PARSE_GET_FLAGS; + break; + + case PARSE_GET_FLAGS: + flag_idx = 0; + flags = 0; + while ((c = argv[arg_idx][flag_idx])) { + switch (c) { + case 'r': flags |= I2C_M_RD; break; + case 'w': flags &= ~I2C_M_RD; break; + default: + fprintf(stderr, "Error: Invalid flag '%c'!\n", c); + exit(1); + } + flag_idx++; + } + msgs[nmsgs].flags = flags; + arg_idx++; + state = PARSE_GET_LENGTH; + break; + + case PARSE_GET_LENGTH: + len = strtoul(argv[arg_idx++], &end, 0); + if (*end || len > 65535) { + fprintf(stderr, "Error: Length invalid!\n"); + exit(1); + } + + msgs[nmsgs].len = len; + + buf = malloc(len); + if (!buf) { + fprintf(stderr, "Error: No memory for buffer!\n"); + exit(ENOMEM); + } + memset(buf, 0, len); + msgs[nmsgs].buf = buf; + + if (flags & I2C_M_RD) { + nmsgs++; + state = PARSE_GET_ADDR; + } else { + buf_idx = 0; + state = PARSE_GET_DATA; + } + + break; + + case PARSE_GET_DATA: + raw_data = strtoul(argv[arg_idx++], &end, 0); + if (raw_data > 255) { + fprintf(stderr, "Error: Data byte '%lu' invalid!\n", raw_data); + exit(1); + } + data = raw_data; + buf[buf_idx++] = data; + + c = *end; + if (c) { + for (; buf_idx < len; buf_idx++) { + switch (c) { + case '+': data++; break; + case '-': data--; break; + case '=': break; + default: + fprintf(stderr, "Error: Invalid data byte suffix '%c'!\n", c); + exit(1); + } + + buf[buf_idx] = data; + } + } + + if (buf_idx == len) { + nmsgs++; + state = PARSE_GET_ADDR; + } + + break; + } + } + + if (state != PARSE_GET_ADDR) { + fprintf(stderr, "Error: Incomplete message\n"); + exit(1); + } + + if (nmsgs == 0) { + help(); + exit(0); + } + + if (!yes && !confirm(filename, msgs, nmsgs)) + exit(0); + + rdwr.msgs = msgs; + rdwr.nmsgs = nmsgs; + if (ioctl(file, I2C_RDWR, &rdwr) < 0) { + fprintf(stderr, "Error: Sending messages failed: %s\n", strerror(errno)); + exit(1); + } + + close(file); + + print_msgs(msgs, nmsgs, PRINT_READ_BUF | (verbose ? PRINT_HEADER | PRINT_WRITE_BUF : 0)); + + /* let Linux free malloced memory on termination */ + exit(0); +}
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. Signed-off-by: Wolfram Sang <wsa@the-dreams.de> --- I've been missing such a tool a number of times now, so I finally got around to writing it myself. As with all I2C tools, it can be dangerous, but it can also be very useful when developing. I am not sure if distros should supply it, I'll leave that to Jean's experience. For embedded build systems, I think this should be selectable. It is RFC for now because it needs broader testing and some more beautification. However, I've been using it already to test the i2c_quirk infrastructure and Renesas I2C controllers. tools/Module.mk | 8 +- tools/i2ctransfer.c | 296 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 303 insertions(+), 1 deletion(-) create mode 100644 tools/i2ctransfer.c