Hey Wolfram,
On Mon, Mar 06, 2017 at 03:29:31PM +0100, Wolfram Sang wrote:
> Soooo, finally, finally, here is another version of my 'i2ctransfer' tool.
> This
> time, I'll keep at it until it is upstream. It was lying around long enough.
> Thanks must go to Renesas for further funding this work! Kudos.
\o/
> [...]
> diff --git a/tools/i2ctransfer.8 b/tools/i2ctransfer.8
> new file mode 100644
> index 0000000..f6fb94a
> --- /dev/null
> +++ b/tools/i2ctransfer.8
> @@ -0,0 +1,153 @@
> +.TH i2ctransfer 8 "February 2017"
> +.SH "NAME"
> +i2ctransfer \- send user-defined I2C messages in one transfer
> +
> +.SH SYNOPSIS
> +.B i2ctransfer
> +.RB [ -f ]
> +.RB [ -y ]
> +.RB [ -v ]
> +.I i2cbus desc
> +.RI [ data ]
> +.RI [ desc
> +.RI [ data ]]
You could join the previous two lines.
> +.RI ...
> +.br
> +.B i2ctransfer
> +.B -V
> +
> +.SH DESCRIPTION
> +.B i2ctransfer
> +is a program to create I2C messages and send them combined as one transfer.
> +For read messages, the contents of the received buffers are printed to
> stdout, one line per read message.
> +.br
> +Please note the difference between a
> +.I transfer
> +and a
> +.I message
> +here.
> +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.
> +Messages within the transfer are concatenated using the REPEATED START
> condition which is described there as well.
> +Some devices keep their internal states for REPEATED START but reset them
> after a STOP.
> +Also, you cannot be interrupted by another I2C master during one transfer,
> but it might happen between multiple transfers.
Well, unless you loose arbitration. Maybe this is too picky to be
relevant here?
Also in single-master setups you can be interrupted if a driver chooses
to start sending a transfer between two of yours. I think this is the
more relevant reason you want to use a single transfer.
> +This programm helps you to create proper transfers for your needs.
> [...]
> diff --git a/tools/i2ctransfer.c b/tools/i2ctransfer.c
> new file mode 100644
> index 0000000..e8ff4be
> --- /dev/null
> +++ b/tools/i2ctransfer.c
> @@ -0,0 +1,347 @@
> [...]
> +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;
> + }
Do you need this check? I hope the kernel doesn't rely on userspace to
not send a transfer the adapter doesn't support? If the kernel checks
appropriatly it's a waste of time to duplicate the check in i2ctransfer?
> + return 0;
> +}
> +
> +static void print_msgs(struct i2c_msg *msgs, __u32 nmsgs, unsigned flags)
> +{
> + FILE *output = flags & PRINT_STDERR ? stderr : stdout;
> + unsigned i;
> + __u16 j;
> +
> + for (i = 0; i < nmsgs; i++) {
> + int read = msgs[i].flags & I2C_M_RD;
> + int print_buf = (read && (flags & PRINT_READ_BUF)) ||
> + (!read && (flags & PRINT_WRITE_BUF));
> +
> + if (flags & PRINT_HEADER)
> + fprintf(output, "msg %u: addr 0x%02x, %s, len %u",
> + i, msgs[i].addr, read ? "read" : "write",
> msgs[i].len);
> +
> + if (msgs[i].len && print_buf) {
> + if (flags & PRINT_HEADER)
> + fprintf(output, ", buf ");
> + for (j = 0; j < msgs[i].len - 1; j++)
> + fprintf(output, "0x%02x ", msgs[i].buf[j]);
> + /* Print final byte with newline */
> + fprintf(output, "0x%02x\n", msgs[i].buf[j]);
> + } else if (flags & PRINT_HEADER) {
> + 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");
Does it kill kittens? :-)
> + 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;
> + int force = 0, yes = 0, version = 0, verbose = 0;
> + struct i2c_msg msgs[I2C_RDRW_IOCTL_MAX_MSGS];
Should this limit be described in the man page?
> + switch (state) {
> + case PARSE_GET_DESC:
> + flags = 0;
> +
> + switch (*arg_ptr++) {
> + case 'r': flags |= I2C_M_RD; break;
This doesn't match kernel coding style and I'd put it on separate lines.
> + case 'w': break;
> +[...]
> + close(file);
> +
> + for (i = 0; i < nmsgs; i++)
> + free(msgs[i].buf);
> +
> + exit(0);
return EXIT_SUCCESS; ?
> +
> + err_out_with_arg:
> + fprintf(stderr, "Error: faulty argument is '%s'\n", argv[arg_idx]);
> + err_out:
> + close(file);
> +
> + for (i = 0; i <= nmsgs; i++)
> + free(msgs[i].buf);
> +
> + exit(1);
return EXIT_FAILURE; ?
Apart from the exit code this is exactly the trailer of the good path,
so these could share code.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
