[PATCH i2c-tools v4] i2ctransfer: Add optional message modifier flags

[PATCH i2c-tools v4] i2ctransfer: Add optional message modifier flags

[Benoît Monin]

Allow setting protocol mangling and repeated start elision flags of an i2c
message with a set of optional command-line flags. These optional flags
are parsed at the beginning of the DESC field up to a read or write flag.

For example, to read one byte from address 0x50 followed by a stop, then
write two bytes at 0x54 on bus 0, one would call i2ctransfer as follow:

    i2ctransfer 0 pr1@0x50 w2@0x54 0x10 0x20

Since the new flags are optional, this patch preserves the compatibility
of the i2ctransfer syntax.

Handling of the message flags is done in add_flag_if_supported(). This
function checks if the flag is defined at compile time and if the adapter
supports the required functionality to handle the flag at runtime.

Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
I2C messages can be modified with a set of flags covered by the protocol
mangling and the skip repeated start functionalities.

The patch adds the parsing of optional flags to i2ctransfer message
description. Those command-line flags then set the i2c message flags
alongside the read/write flag.

I wrote these changes to test the insertion of I2C_M_STOP flag in
multi-message transactions with i2ctransfer, but the other flags may be
useful for various test scenarios. 
---
Changes in v4:
- Rebase on current upstream and drop first patch that has been merged.
- Return an enum from add_flag_if_supported(), to distinguish between the various possible cases.
- Link to v3: https://lore.kernel.org/r/20260127-msg-flags-v3-0-e7539945db2b@bootlin.com

Changes in v3:
- Use ifdefs to check that the flags and functionalities are known
  at compile-time.
- Check that the adapter supports the requested flags before using them
  in i2ctransfer.
- Add a warning in i2ctransfer man page about the risk of using these
  flags.
- Link to v2: https://lore.kernel.org/r/20251223-msg-flags-v2-0-8d934a4366e2@bootlin.com

Changes in v2:
- Document the flags in i2ctransfer.8 man page.
- Link to v1: https://lore.kernel.org/r/20251128-msg-flags-v1-0-6353f26fa6bc@bootlin.com
---
 tools/i2ctransfer.8 |  28 +++++++++-
 tools/i2ctransfer.c | 145 +++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 159 insertions(+), 14 deletions(-)

diff --git a/tools/i2ctransfer.8 b/tools/i2ctransfer.8
index 4bdf436..ca41241 100644
--- a/tools/i2ctransfer.8
+++ b/tools/i2ctransfer.8
@@ -96,8 +96,11 @@ The number of blocks is limited by the Linux Kernel and defined by I2C_RDWR_IOCT
 .I desc
 blocks are composed like this:
 
-.I {r|w}<length_of_message>[@address]
+.I [inpst]{r|w}<length_of_message>[@address]
 
+.TP
+.B [inpst]
+If supported, specifies optional MESSAGE MODIFIER FLAGS. See the section below for details.
 .TP
 .B {r|w}
 specifies if the message is read or write
@@ -141,6 +144,29 @@ decrease value by 1 until end of message (i.e. 0xff- means 0xff, 0xfe, 0xfd, ...
 p
 use value as seed for an 8 bit pseudo random sequence (i.e. 0p means 0x00, 0x50, 0xb0, ...)
 
+.SH "MESSAGE MODIFIER FLAGS"
+.PP
+These optional flags can be used to change the handling of a message when it is sent on the bus.
+They allow you to deviate from the I2C standard to work around device issues.
+They should be used with care, as they can confuse your I2C bus.
+Note that not all flags (or any) may be supported by your particular hardware and your kernel version.
+
+.TP
+.B i
+ignore NACK from client, treat them as ACK.
+.TP
+.B n
+in a read message, master ACK/NACK bit is skipped.
+.TP
+.B p
+emit a STOP after the message.
+.TP
+.B s
+skip repeated start.
+.TP
+.B t
+toggle read/write bit.
+
 .SH EXAMPLES
 .PP
 On bus 0, from an EEPROM at address 0x50, read 8 byte from offset 0x64
diff --git a/tools/i2ctransfer.c b/tools/i2ctransfer.c
index 4db98e3..003c70b 100644
--- a/tools/i2ctransfer.c
+++ b/tools/i2ctransfer.c
@@ -34,6 +34,14 @@ enum parse_state {
 	PARSE_GET_DATA,
 };
 
+enum supported_flags_retval {
+	FLAGS_GOT_RW,
+	FLAGS_GOT_OPTIONAL,
+	FLAGS_UNEXPECTED,
+	FLAGS_UNKNOWN,
+	FLAGS_UNSUPPORTED,
+};
+
 #define PRINT_STDERR	(1 << 0)
 #define PRINT_READ_BUF	(1 << 1)
 #define PRINT_WRITE_BUF	(1 << 2)
@@ -52,9 +60,16 @@ static void help(void)
 		"           -V version info\n"
 		"           -y yes to all confirmations\n"
 		"  I2CBUS is an integer or an I2C bus name\n"
-		"  DESC describes the transfer in the form: {r|w}LENGTH[@address]\n"
-		"    1) read/write-flag 2) LENGTH (range 0-65535, or '?')\n"
-		"    3) I2C address (use last one if omitted)\n"
+		"  DESC describes the transfer in the form: [inpst]{r|w}LENGTH[@address]\n"
+		"    1) optional message modifier flags, if supported\n"
+		"       i: ignore NACK from client\n"
+		"       n: no master ACK/NACK bit in a read message\n"
+		"       p: emit a STOP after the message\n"
+		"       s: skip repeated start\n"
+		"       t: toggle read/write bit\n"
+		"    2) mandatory read/write flag\n"
+		"    3) LENGTH (range 0-65535, or '?')\n"
+		"    4) I2C address (use last one if omitted)\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"
@@ -66,17 +81,20 @@ static void help(void)
 		"  # i2ctransfer 0 w17@0x50 0x42 0xff-\n");
 }
 
-static int check_funcs(int file)
+static int get_funcs(int file, unsigned long *funcs)
 {
-	unsigned long funcs;
-
 	/* check adapter functionality */
-	if (ioctl(file, I2C_FUNCS, &funcs) < 0) {
+	if (ioctl(file, I2C_FUNCS, funcs) < 0) {
 		fprintf(stderr, "Error: Could not get the adapter "
 			"functionality matrix: %s\n", strerror(errno));
 		return -1;
 	}
 
+	return 0;
+}
+
+static int check_funcs(unsigned long funcs)
+{
 	if (!(funcs & I2C_FUNC_I2C)) {
 		fprintf(stderr, MISSING_FUNC_FMT, "I2C transfers");
 		return -1;
@@ -140,6 +158,104 @@ static int confirm(const char *filename, struct i2c_msg *msgs, __u32 nmsgs)
 	return 1;
 }
 
+/*
+ * Parse one message flag and check if the adapter supports it.
+ * Return:
+ * FLAGS_GOT_RW if we have the direction of the message (read or write)
+ * FLAGS_GOT_OPTIONAL if the argument is a valid and supported optional flag
+ * FLAGS_UNEXPECTED if the argument is the beginning of the size or address
+ * FLAGS_UNKNOWN for unknown flag
+ * FLAGS_UNSUPPORTED for flag unsupported by the adapter
+ */
+static enum supported_flags_retval
+	add_flag_if_supported(__u16 *flags, unsigned long funcs, char arg)
+{
+	int mangling = 0, nostart = 0;
+
+	/* Get the message flag from the argument */
+	switch (arg) {
+	/* mandatory direction flag: ends the parsing of flags */
+	case 'r':
+		*flags |= I2C_M_RD;
+		return FLAGS_GOT_RW;
+	case 'w':
+		return FLAGS_GOT_RW;
+
+	/* optional flags */
+#ifdef I2C_M_IGNORE_NAK
+	case 'i':
+		*flags |= I2C_M_IGNORE_NAK;
+		mangling = 1;
+		break;
+#endif
+#ifdef I2C_M_NO_RD_ACK
+	case 'n':
+		*flags |= I2C_M_NO_RD_ACK;
+		mangling = 1;
+		break;
+#endif
+#ifdef I2C_M_STOP
+	case 'p':
+		*flags |= I2C_M_STOP;
+		mangling = 1;
+		break;
+#endif
+#ifdef I2C_M_NOSTART
+	case 's':
+		*flags |= I2C_M_NOSTART;
+		nostart = 1;
+		break;
+#endif
+#ifdef I2C_M_REV_DIR_ADDR
+	case 't':
+		*flags |= I2C_M_REV_DIR_ADDR;
+		mangling = 1;
+		break;
+#endif
+
+	/* unexpected next part of the message: length or address */
+	case '0':
+	case '1':
+	case '2':
+	case '3':
+	case '4':
+	case '5':
+	case '6':
+	case '7':
+	case '8':
+	case '9':
+	case '?':
+	case '@':
+		fprintf(stderr, "Error: Unexpected flag '%c'\n", arg);
+		return FLAGS_UNEXPECTED;
+
+	default:
+		fprintf(stderr, "Error: Unknown flag '%c'\n", arg);
+		return FLAGS_UNKNOWN;
+	}
+
+	/* Check that the adapter supports the requested flags */
+#ifdef I2C_FUNC_PROTOCOL_MANGLING
+	if (funcs & I2C_FUNC_PROTOCOL_MANGLING)
+		mangling = 0;
+#endif
+	if (mangling) {
+		fprintf(stderr, MISSING_FUNC_FMT, "protocol mangling");
+		return FLAGS_UNSUPPORTED;
+	}
+
+#ifdef I2C_FUNC_NOSTART
+	if (funcs & I2C_FUNC_NOSTART)
+		nostart = 0;
+#endif
+	if (nostart) {
+		fprintf(stderr, MISSING_FUNC_FMT, "repeated start skipping");
+		return FLAGS_UNSUPPORTED;
+	}
+
+	return FLAGS_GOT_OPTIONAL;
+}
+
 int main(int argc, char *argv[])
 {
 	char filename[20];
@@ -148,6 +264,7 @@ int main(int argc, char *argv[])
 	struct i2c_msg msgs[I2C_RDRW_IOCTL_MAX_MSGS];
 	enum parse_state state = PARSE_GET_DESC;
 	unsigned int buf_idx = 0;
+	unsigned long funcs;
 
 	memset(msgs, 0, sizeof(msgs));
 
@@ -182,7 +299,7 @@ int main(int argc, char *argv[])
 		exit(1);
 
 	file = open_i2c_dev(i2cbus, filename, sizeof(filename), 0);
-	if (file < 0 || check_funcs(file))
+	if (file < 0 || get_funcs(file, &funcs) || check_funcs(funcs))
 		exit(1);
 
 	while (optind < argc) {
@@ -191,6 +308,7 @@ int main(int argc, char *argv[])
 		__u16 flags;
 		__u8 data, *buf;
 		char *end;
+		int ret;
 
 		if (nmsgs == I2C_RDRW_IOCTL_MAX_MSGS) {
 			fprintf(stderr, "Error: Too many messages (max: %d)\n",
@@ -202,11 +320,12 @@ int main(int argc, char *argv[])
 		case PARSE_GET_DESC:
 			flags = 0;
 
-			switch (*arg_ptr++) {
-			case 'r': flags |= I2C_M_RD; break;
-			case 'w': break;
-			default:
-				fprintf(stderr, "Error: Invalid direction\n");
+			for (ret = FLAGS_GOT_OPTIONAL; *arg_ptr && ret == FLAGS_GOT_OPTIONAL; arg_ptr++)
+				ret = add_flag_if_supported(&flags, funcs, *arg_ptr);
+			if (ret == FLAGS_UNKNOWN || ret == FLAGS_UNEXPECTED || ret == FLAGS_UNSUPPORTED)
+				goto err_out_with_arg;
+			if (ret != FLAGS_GOT_RW) {
+				fprintf(stderr, "Error: Missing direction flag 'r' or 'w'\n");
 				goto err_out_with_arg;
 			}
 

---
base-commit: ec1267371e378fc6aaf1cd7686b5b9b31293255a
change-id: 20251127-msg-flags-3d2b2da9ae28

Best regards,
--  
Benoît Monin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH i2c-tools v4] i2ctransfer: Add optional message modifier flags

[Wolfram Sang]

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

Hi Benoît,

Looks mostly good, I think we are nearly there.

> +	/* unexpected next part of the message: length or address */
> +	case '0':
> +	case '1':
> +	case '2':
> +	case '3':
> +	case '4':
> +	case '5':
> +	case '6':
> +	case '7':
> +	case '8':
> +	case '9':
> +	case '?':
> +	case '@':
> +		fprintf(stderr, "Error: Unexpected flag '%c'\n", arg);
> +		return FLAGS_UNEXPECTED;

I suggest to drop this error message. The above characters are not
exactly flags. By removing the printout here...

> +			for (ret = FLAGS_GOT_OPTIONAL; *arg_ptr && ret == FLAGS_GOT_OPTIONAL; arg_ptr++)
> +				ret = add_flag_if_supported(&flags, funcs, *arg_ptr);
> +			if (ret == FLAGS_UNKNOWN || ret == FLAGS_UNEXPECTED || ret == FLAGS_UNSUPPORTED)

... and simplifying this to ...

+			if (ret == FLAGS_UNKNOWN || ret == FLAGS_UNSUPPORTED)

... we get the error message of a missing r/w-flag instead. I think this
is more helpful to the user? What do you think?

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]