Linux I2C development
 help / color / mirror / Atom feed
* [PATCH v4 0/2] i2c-tools: Make tools accept bus path
@ 2026-07-06  4:27 Brigham Campbell
  2026-07-06  4:27 ` [PATCH v4 1/2] i2c-tools: Allow passing device file paths Brigham Campbell
  2026-07-06  4:27 ` [PATCH v4 2/2] i2c-tools: Document device paths as I2CBUS arg Brigham Campbell
  0 siblings, 2 replies; 7+ messages in thread
From: Brigham Campbell @ 2026-07-06  4:27 UTC (permalink / raw)
  To: Jean Delvare, linux-i2c
  Cc: Wolfram Sang, Brigham Campbell, Gero Schwäricke

The purpose of this series is to make i2c-tools userspace utilities
accept an i2c bus character device file path as an argument. This is
useful, for example, if a sysadmin creates udev rules to create a
persistent symlink to an i2c bus and wishes to use that symlink with
i2c-tools.

I've tested these changes by running i2cdetect, i2cget, and i2cset.
Please be aware that my current test setup has an i801 bus controller
with what seems to be no slave devices connected, so my testing has been
superficial, involving mostly failed read/writes. This patch series only
touches code that actually opens the character device file, so I'm
satisfied with this testing. If maintainers prefer, I'm happy to test
more rigorously.

Since v1:
 * Scrapped brittle checking against device major number 89 in favor of
   an i2c_bus_hint variant struct.
Since v2:
 * Scrapped i2c_bus_hint variant in favor of combining the lookup and
   open steps into one function. No more awkward variant.
Since v3:
 * Rebased onto `master` and fixed minor conflicts.

Signed-off-by: Brigham Campbell <me@brighamcampbell.com>
---
Brigham Campbell (2):
      i2c-tools: Allow passing device file paths
      i2c-tools: Document device paths as I2CBUS arg

 tools/i2cbusses.c   | 40 ++++++++++++++++++++++++++++++++++------
 tools/i2cbusses.h   |  4 +++-
 tools/i2cdetect.8   |  8 ++++----
 tools/i2cdetect.c   | 14 +++++---------
 tools/i2cdump.8     |  7 ++++---
 tools/i2cdump.c     | 14 +++++---------
 tools/i2cget.8      |  9 +++++----
 tools/i2cget.c      | 13 +++++--------
 tools/i2cset.8      | 12 ++++++------
 tools/i2cset.c      | 13 +++++--------
 tools/i2ctransfer.8 |  2 +-
 tools/i2ctransfer.c | 13 +++++--------
 12 files changed, 82 insertions(+), 67 deletions(-)
---
base-commit: 5852f5375eea6ca9dabf31f4772cb8b8ecc905d8
change-id: 20260530-accept-device-path-539164c9c1df

Best regards,
--  
Brigham Campbell <me@brighamcampbell.com>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v4 1/2] i2c-tools: Allow passing device file paths
  2026-07-06  4:27 [PATCH v4 0/2] i2c-tools: Make tools accept bus path Brigham Campbell
@ 2026-07-06  4:27 ` Brigham Campbell
  2026-07-09 12:18   ` Wolfram Sang
  2026-07-09 12:40   ` Gero Schwäricke
  2026-07-06  4:27 ` [PATCH v4 2/2] i2c-tools: Document device paths as I2CBUS arg Brigham Campbell
  1 sibling, 2 replies; 7+ messages in thread
From: Brigham Campbell @ 2026-07-06  4:27 UTC (permalink / raw)
  To: Jean Delvare, linux-i2c
  Cc: Wolfram Sang, Brigham Campbell, Gero Schwäricke

Make i2c-tools binaries accept a device file path or symlink directly.
This is useful, for example, for use with stable symlinks generated by
udev.

In the case of conflict, the bus parameter is parsed with the following
precedence in order to maintain backwards compatibility:

number -> name -> device path

Suggested-by: Gero Schwäricke <gero.schwaericke@sevenlab.de>
Signed-off-by: Brigham Campbell <me@brighamcampbell.com>
---
 tools/i2cbusses.c   | 40 ++++++++++++++++++++++++++++++++++------
 tools/i2cbusses.h   |  4 +++-
 tools/i2cdetect.c   | 12 ++++--------
 tools/i2cdump.c     | 12 ++++--------
 tools/i2cget.c      | 11 ++++-------
 tools/i2cset.c      | 11 ++++-------
 tools/i2ctransfer.c | 11 ++++-------
 7 files changed, 57 insertions(+), 44 deletions(-)

diff --git a/tools/i2cbusses.c b/tools/i2cbusses.c
index d687615..54f3238 100644
--- a/tools/i2cbusses.c
+++ b/tools/i2cbusses.c
@@ -65,7 +65,7 @@ static enum adt i2c_get_funcs(int i2cbus)
 	char filename[20];
 	enum adt ret;
 
-	file = open_i2c_dev(i2cbus, filename, sizeof(filename), 1);
+	file = open_i2c_dev_num(i2cbus, filename, sizeof(filename), 1);
 	if (file < 0)
 		return adt_unknown;
 
@@ -348,10 +348,6 @@ static int lookup_i2c_bus_by_name(const char *bus_name)
 		}
 	}
 
-	if (i2cbus == -1)
-		fprintf(stderr, "Error: I2C bus name doesn't match any "
-			"bus present!\n");
-
 done:
 	free_adapters(adapters);
 	return i2cbus;
@@ -410,7 +406,7 @@ int parse_i2c_address(const char *address_arg, int all_addrs)
 	return address;
 }
 
-int open_i2c_dev(int i2cbus, char *filename, size_t size, int quiet)
+int open_i2c_dev_num(int i2cbus, char *filename, size_t size, int quiet)
 {
 	int file, len;
 
@@ -446,6 +442,38 @@ int open_i2c_dev(int i2cbus, char *filename, size_t size, int quiet)
 	return file;
 }
 
+int open_i2c_dev_path(const char *i2cbus_arg, int quiet)
+{
+	int file = open(i2cbus_arg, O_RDWR);
+
+	if (file < 0 && !quiet) {
+		fprintf(stderr, "Error: Could not open file "
+			"`%s': %s\n", i2cbus_arg, strerror(errno));
+		if (errno == EACCES)
+			fprintf(stderr, "Run as root?\n");
+	}
+
+	return file;
+}
+
+int open_i2c_dev(char *i2cbus_arg, char **filename, size_t size, int quiet)
+{
+	int file, i2cbus;
+
+	i2cbus = lookup_i2c_bus(i2cbus_arg);
+
+	if (i2cbus < 0) {
+		*filename = i2cbus_arg;
+		file = open_i2c_dev_path(i2cbus_arg, quiet);
+		if (file < 0)
+			fprintf(stderr, "Failed to open `%s' as a bus name "
+					"and as a path.\n", i2cbus_arg);
+		return file;
+	}
+
+	return open_i2c_dev_num(i2cbus, *filename, size, quiet);
+}
+
 int set_slave_addr(int file, int address, int force)
 {
 	/* With force, let the user read from/write to the registers
diff --git a/tools/i2cbusses.h b/tools/i2cbusses.h
index 6f901b6..5de3056 100644
--- a/tools/i2cbusses.h
+++ b/tools/i2cbusses.h
@@ -31,7 +31,9 @@ void free_adapters(struct i2c_adap *adapters);
 
 int lookup_i2c_bus(const char *i2cbus_arg);
 int parse_i2c_address(const char *address_arg, int all_addrs);
-int open_i2c_dev(int i2cbus, char *filename, size_t size, int quiet);
+int open_i2c_dev(char *i2cbus_arg, char **filename, size_t size, int quiet);
+int open_i2c_dev_num(int i2cbus, char *filename, size_t size, int quiet);
+int open_i2c_dev_path(const char *i2cbus_arg, int quiet);
 int set_slave_addr(int file, int address, int force);
 
 #define MISSING_FUNC_FMT	"Error: Adapter does not have %s capability\n"
diff --git a/tools/i2cdetect.c b/tools/i2cdetect.c
index 3c22f17..b62f395 100644
--- a/tools/i2cdetect.c
+++ b/tools/i2cdetect.c
@@ -213,8 +213,9 @@ static void print_i2c_busses(void)
 int main(int argc, char *argv[])
 {
 	char *end;
-	int i2cbus, file, res;
-	char filename[20];
+	int file, res;
+	char filename_buf[20];
+	char *filename = filename_buf;
 	unsigned long funcs;
 	int mode = MODE_AUTO;
 	int first = 0x08, last = 0x77;
@@ -277,11 +278,6 @@ int main(int argc, char *argv[])
 		help();
 		exit(1);
 	}
-	i2cbus = lookup_i2c_bus(argv[optind]);
-	if (i2cbus < 0) {
-		help();
-		exit(1);
-	}
 
 	/* read address range if present */
 	if (argc == optind + 3 && mode != MODE_FUNC) {
@@ -321,7 +317,7 @@ int main(int argc, char *argv[])
 		exit(1);
 	}
 
-	file = open_i2c_dev(i2cbus, filename, sizeof(filename), 0);
+	file = open_i2c_dev(argv[optind], &filename, sizeof(filename_buf), 0);
 	if (file < 0) {
 		exit(1);
 	}
diff --git a/tools/i2cdump.c b/tools/i2cdump.c
index d315e2f..ce3be2c 100644
--- a/tools/i2cdump.c
+++ b/tools/i2cdump.c
@@ -100,9 +100,10 @@ static int check_funcs(int file, int size, int pec)
 int main(int argc, char *argv[])
 {
 	char *end;
-	int i, j, res, i2cbus, address, size, file;
+	int i, j, res, address, size, file;
 	int bank = 0, bankreg = 0x4E, old_bank = 0;
-	char filename[20];
+	char filename_buf[20];
+	char *filename = filename_buf;
 	int block[256];
 	int pec = 0, even = 0;
 	int opt;
@@ -135,11 +136,6 @@ int main(int argc, char *argv[])
 		help();
 		exit(1);
 	}
-	i2cbus = lookup_i2c_bus(argv[optind]);
-	if (i2cbus < 0) {
-		help();
-		exit(1);
-	}
 
 	if (argc < optind + 2) {
 		fprintf(stderr, "Error: No address specified!\n");
@@ -235,7 +231,7 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	file = open_i2c_dev(i2cbus, filename, sizeof(filename), 0);
+	file = open_i2c_dev(argv[optind], &filename, sizeof(filename_buf), 0);
 	if (file < 0
 	 || check_funcs(file, size, pec)
 	 || set_slave_addr(file, address, force))
diff --git a/tools/i2cget.c b/tools/i2cget.c
index 17d78bf..e08ee81 100644
--- a/tools/i2cget.c
+++ b/tools/i2cget.c
@@ -169,9 +169,10 @@ static int confirm(const char *filename, int address, int size, int daddress,
 int main(int argc, char *argv[])
 {
 	char *end;
-	int res, i2cbus, address, size, file;
+	int res, address, size, file;
 	int daddress;
-	char filename[20];
+	char filename_buf[20];
+	char *filename = filename_buf;
 	int pec = 0;
 	int opt;
 	int force = 0, yes = 0, version = 0, all_addrs = 0;
@@ -199,10 +200,6 @@ int main(int argc, char *argv[])
 	if (argc < optind + 2)
 		help(1);
 
-	i2cbus = lookup_i2c_bus(argv[optind]);
-	if (i2cbus < 0)
-		help(1);
-
 	address = parse_i2c_address(argv[optind+1], all_addrs);
 	if (address < 0)
 		help(1);
@@ -251,7 +248,7 @@ int main(int argc, char *argv[])
 		length = I2C_SMBUS_BLOCK_MAX;
 	}
 
-	file = open_i2c_dev(i2cbus, filename, sizeof(filename), 0);
+	file = open_i2c_dev(argv[optind], &filename, sizeof(filename_buf), 0);
 	if (file < 0
 	 || check_funcs(file, size, daddress, pec)
 	 || set_slave_addr(file, address, force))
diff --git a/tools/i2cset.c b/tools/i2cset.c
index aaf7faf..8371570 100644
--- a/tools/i2cset.c
+++ b/tools/i2cset.c
@@ -151,9 +151,10 @@ int main(int argc, char *argv[])
 {
 	char *end;
 	const char *maskp = NULL;
-	int res, i2cbus, address, size, file;
+	int res, address, size, file;
 	int value, daddress, vmask = 0;
-	char filename[20];
+	char filename_buf[20];
+	char *filename = filename_buf;
 	int pec = 0;
 	int opt;
 	int force = 0, yes = 0, version = 0, readback = 0, all_addrs = 0;
@@ -183,10 +184,6 @@ int main(int argc, char *argv[])
 	if (argc < optind + 3)
 		help(1);
 
-	i2cbus = lookup_i2c_bus(argv[optind]);
-	if (i2cbus < 0)
-		help(1);
-
 	address = parse_i2c_address(argv[optind+1], all_addrs);
 	if (address < 0)
 		help(1);
@@ -301,7 +298,7 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	file = open_i2c_dev(i2cbus, filename, sizeof(filename), 0);
+	file = open_i2c_dev(argv[optind], &filename, sizeof(filename_buf), 0);
 	if (file < 0
 	 || check_funcs(file, size, pec)
 	 || set_slave_addr(file, address, force))
diff --git a/tools/i2ctransfer.c b/tools/i2ctransfer.c
index a49b734..d9d1396 100644
--- a/tools/i2ctransfer.c
+++ b/tools/i2ctransfer.c
@@ -257,8 +257,9 @@ static enum supported_flags_retval
 
 int main(int argc, char *argv[])
 {
-	char filename[20];
-	int i2cbus, address = -1, file, opt, nmsgs = 0, nmsgs_sent, i;
+	char filename_buf[20];
+	char *filename = filename_buf;
+	int address = -1, file, opt, nmsgs = 0, nmsgs_sent, i;
 	int force = 0, yes = 0, version = 0, verbose = 0, all_addrs = 0, binary = 0;
 	struct i2c_msg msgs[I2C_RDRW_IOCTL_MAX_MSGS];
 	enum parse_state state = PARSE_GET_DESC;
@@ -293,11 +294,7 @@ int main(int argc, char *argv[])
 		exit(1);
 	}
 
-	i2cbus = lookup_i2c_bus(argv[optind++]);
-	if (i2cbus < 0)
-		exit(1);
-
-	file = open_i2c_dev(i2cbus, filename, sizeof(filename), 0);
+	file = open_i2c_dev(argv[optind++], &filename, sizeof(filename_buf), 0);
 	if (file < 0 || get_funcs(file, &funcs) || check_funcs(funcs))
 		exit(1);
 

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v4 2/2] i2c-tools: Document device paths as I2CBUS arg
  2026-07-06  4:27 [PATCH v4 0/2] i2c-tools: Make tools accept bus path Brigham Campbell
  2026-07-06  4:27 ` [PATCH v4 1/2] i2c-tools: Allow passing device file paths Brigham Campbell
@ 2026-07-06  4:27 ` Brigham Campbell
  2026-07-11 19:43   ` Wolfram Sang
  1 sibling, 1 reply; 7+ messages in thread
From: Brigham Campbell @ 2026-07-06  4:27 UTC (permalink / raw)
  To: Jean Delvare, linux-i2c; +Cc: Wolfram Sang, Brigham Campbell

i2c-tools now accept device file paths in addition to the i2c adapter
number and i2c adapter name. Add this capability to documentation.

Signed-off-by: Brigham Campbell <me@brighamcampbell.com>
---
 tools/i2cdetect.8   |  8 ++++----
 tools/i2cdetect.c   |  2 +-
 tools/i2cdump.8     |  7 ++++---
 tools/i2cdump.c     |  2 +-
 tools/i2cget.8      |  9 +++++----
 tools/i2cget.c      |  2 +-
 tools/i2cset.8      | 12 ++++++------
 tools/i2cset.c      |  2 +-
 tools/i2ctransfer.8 |  2 +-
 tools/i2ctransfer.c |  2 +-
 10 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/tools/i2cdetect.8 b/tools/i2cdetect.8
index 5935b2b..fdfff62 100644
--- a/tools/i2cdetect.8
+++ b/tools/i2cdetect.8
@@ -26,10 +26,10 @@ i2cdetect \- detect I2C chips
 .SH DESCRIPTION
 i2cdetect is a userspace program to scan an I2C bus for devices. It
 outputs a table with the list of detected devices on the specified bus.
-\fIi2cbus\fR indicates the number or name of the I2C bus to be scanned, and
-should correspond to one of the busses listed by \fIi2cdetect -l\fR.
-The optional parameters \fIfirst\fR and \fIlast\fR restrict the scanning
-range (default: from 0x08 to 0x77).
+\fIi2cbus\fR indicates the number, name, or device file path of the I2C
+bus to be scanned, and should correspond to one of the busses listed by
+\fIi2cdetect -l\fR.  The optional parameters \fIfirst\fR and \fIlast\fR
+restrict the scanning range (default: from 0x08 to 0x77).
 .PP
 As there is no standard I2C detection command, i2cdetect uses arbitrary
 SMBus commands (namely SMBus quick write and SMBus receive byte) to probe
diff --git a/tools/i2cdetect.c b/tools/i2cdetect.c
index b62f395..78a0955 100644
--- a/tools/i2cdetect.c
+++ b/tools/i2cdetect.c
@@ -38,7 +38,7 @@ static void help(void)
 		"Usage: i2cdetect [-y] [-a] [-q|-r] I2CBUS [FIRST LAST]\n"
 		"       i2cdetect -F I2CBUS\n"
 		"       i2cdetect -l\n"
-		"  I2CBUS is an integer or an I2C bus name\n"
+		"  I2CBUS is an integer, I2C bus name, or I2C bus device path\n"
 		"  If provided, FIRST and LAST limit the probing range.\n");
 }
 
diff --git a/tools/i2cdump.8 b/tools/i2cdump.8
index 6ede625..103051d 100644
--- a/tools/i2cdump.8
+++ b/tools/i2cdump.8
@@ -52,9 +52,10 @@ scripts.
 Allow using addresses between 0x00 - 0x07 and 0x78 - 0x7f. Not recommended.
 .PP
 At least two options must be provided to i2cdump. \fIi2cbus\fR indicates the
-number or name of the I2C bus to be scanned. This number should correspond to one
-of the busses listed by \fIi2cdetect -l\fR. \fIaddress\fR indicates the
-address to be scanned on that bus, and is an integer between 0x08 and 0x77.
+number, name, or device file path of the I2C bus to be scanned. This number
+should correspond to one of the busses listed by \fIi2cdetect -l\fR.
+\fIaddress\fR indicates the address to be scanned on that bus, and is an integer
+between 0x08 and 0x77.
 .PP
 The \fImode\fR parameter, if specified, is one of the letters \fBb\fP, \fBw\fP,
 or \fBi\fP, corresponding to a read size of a single byte, a 16-bit
diff --git a/tools/i2cdump.c b/tools/i2cdump.c
index ce3be2c..6389e68 100644
--- a/tools/i2cdump.c
+++ b/tools/i2cdump.c
@@ -32,7 +32,7 @@ static void help(void)
 {
 	fprintf(stderr,
 		"Usage: i2cdump [-f] [-y] [-r first-last] [-a] I2CBUS ADDRESS [MODE [BANK [BANKREG]]]\n"
-		"  I2CBUS is an integer or an I2C bus name\n"
+		"  I2CBUS is an integer, I2C bus name, or I2C bus device path\n"
 		"  ADDRESS is an integer (0x08 - 0x77, or 0x00 - 0x7f if -a is given)\n"
 		"  MODE is one of:\n"
 		"    b (byte, default)\n"
diff --git a/tools/i2cget.8 b/tools/i2cget.8
index 69586cc..ba261a9 100644
--- a/tools/i2cget.8
+++ b/tools/i2cget.8
@@ -45,10 +45,11 @@ scripts. Use with caution.
 .B -a
 Allow using addresses between 0x00 - 0x07 and 0x78 - 0x7f. Not recommended.
 .PP
-There are two required options to i2cget. \fIi2cbus\fR indicates the number
-or name of the I2C bus to be scanned.  This number should correspond to one of
-the busses listed by \fIi2cdetect -l\fR. \fIchip-address\fR specifies the
-address of the chip on that bus, and is an integer between 0x08 and 0x77.
+There are two required options to i2cget. \fIi2cbus\fR indicates the number,
+name, or device file path of the I2C bus to be scanned.  This number should
+correspond to one of the busses listed by \fIi2cdetect -l\fR.
+\fIchip-address\fR specifies the address of the chip on that bus, and is an
+integer between 0x08 and 0x77.
 .PP
 \fIdata-address\fR specifies the address on that chip to read from, and is
 an integer between 0x00 and 0xFF. If omitted, the currently active register
diff --git a/tools/i2cget.c b/tools/i2cget.c
index e08ee81..759f016 100644
--- a/tools/i2cget.c
+++ b/tools/i2cget.c
@@ -35,7 +35,7 @@ static void __attribute__ ((noreturn)) help(int status)
 {
 	fprintf(stderr,
 		"Usage: i2cget [-f] [-y] [-a] I2CBUS CHIP-ADDRESS [DATA-ADDRESS [MODE [LENGTH]]]\n"
-		"  I2CBUS is an integer or an I2C bus name\n"
+		"  I2CBUS is an integer, I2C bus name, or I2C bus device path\n"
 		"  ADDRESS is an integer (0x08 - 0x77, or 0x00 - 0x7f if -a is given)\n"
 		"  MODE is one of:\n"
 		"    b (read byte data, default)\n"
diff --git a/tools/i2cset.8 b/tools/i2cset.8
index e4e1870..d7a2c05 100644
--- a/tools/i2cset.8
+++ b/tools/i2cset.8
@@ -65,12 +65,12 @@ apply as those of option \fB-m\fR.
 .B -a
 Allow using addresses between 0x00 - 0x07 and 0x78 - 0x7f. Not recommended.
 .PP
-There are three required options to i2cset. \fIi2cbus\fR indicates the number
-or name of the I2C bus to be scanned.  This number should correspond to one of
-the busses listed by \fIi2cdetect -l\fR. \fIchip-address\fR specifies the
-address of the chip on that bus, and is an integer between 0x08 and 0x77.
-\fIdata-address\fR specifies the address on that chip to write to, and is an
-integer between 0x00 and 0xFF.
+There are three required options to i2cset. \fIi2cbus\fR indicates the number,
+name, or device file path of the I2C bus to be scanned. This number should
+correspond to one of the busses listed by \fIi2cdetect -l\fR. \fIchip-address\fR
+specifies the address of the chip on that bus, and is an integer between 0x08
+and 0x77. \fIdata-address\fR specifies the address on that chip to write to, and
+is an integer between 0x00 and 0xFF.
 .PP
 The \fIvalue\fR parameter, if specified, is the value to write to that
 location on the chip. If this parameter is omitted, then a short write is
diff --git a/tools/i2cset.c b/tools/i2cset.c
index 8371570..27e3c7f 100644
--- a/tools/i2cset.c
+++ b/tools/i2cset.c
@@ -32,7 +32,7 @@ static void __attribute__ ((noreturn)) help(int status)
 {
 	fprintf(stderr,
 		"Usage: i2cset [-f] [-y] [-m MASK] [-r] [-a] I2CBUS CHIP-ADDRESS DATA-ADDRESS [VALUE] ... [MODE]\n"
-		"  I2CBUS is an integer or an I2C bus name\n"
+		"  I2CBUS is an integer, I2C bus name, or I2C bus device path\n"
 		"  ADDRESS is an integer (0x08 - 0x77, or 0x00 - 0x7f if -a is given)\n"
 		"  MODE is one of:\n"
 		"    c (byte, no value)\n"
diff --git a/tools/i2ctransfer.8 b/tools/i2ctransfer.8
index de03c7d..20fd454 100644
--- a/tools/i2ctransfer.8
+++ b/tools/i2ctransfer.8
@@ -84,7 +84,7 @@ This is mainly meant to be used in scripts.
 .PP
 The first parameter
 .I i2cbus
-indicates the number or name of the I2C bus to be used.
+indicates the number, name, or device file path of the I2C bus to be used.
 This number should correspond to one of the busses listed by
 .B i2cdetect -l.
 
diff --git a/tools/i2ctransfer.c b/tools/i2ctransfer.c
index d9d1396..925c2b4 100644
--- a/tools/i2ctransfer.c
+++ b/tools/i2ctransfer.c
@@ -59,7 +59,7 @@ static void help(void)
 		"           -v verbose mode\n"
 		"           -V version info\n"
 		"           -y yes to all confirmations\n"
-		"  I2CBUS is an integer or an I2C bus name\n"
+		"  I2CBUS is an integer, I2C bus name, or I2C bus device path\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"

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] i2c-tools: Allow passing device file paths
  2026-07-06  4:27 ` [PATCH v4 1/2] i2c-tools: Allow passing device file paths Brigham Campbell
@ 2026-07-09 12:18   ` Wolfram Sang
  2026-07-09 12:40   ` Gero Schwäricke
  1 sibling, 0 replies; 7+ messages in thread
From: Wolfram Sang @ 2026-07-09 12:18 UTC (permalink / raw)
  To: Brigham Campbell; +Cc: Jean Delvare, linux-i2c, Gero Schwäricke

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

On Sun, Jul 05, 2026 at 10:27:53PM -0600, Brigham Campbell wrote:
> Make i2c-tools binaries accept a device file path or symlink directly.
> This is useful, for example, for use with stable symlinks generated by
> udev.
> 
> In the case of conflict, the bus parameter is parsed with the following
> precedence in order to maintain backwards compatibility:
> 
> number -> name -> device path
> 
> Suggested-by: Gero Schwäricke <gero.schwaericke@sevenlab.de>
> Signed-off-by: Brigham Campbell <me@brighamcampbell.com>

I general, I like the approach. There is only some inconsistency in the
error reporting which is done twice now:

$ sudo tools/i2cdetect -F "i915 gmbus dpe"
Error: Could not open file `i915 gmbus dpe': No such file or directory
Failed to open `i915 gmbus dpe' as a bus name and as a path.

Here is my proposed solution as it is easier to discuss code. What do
you think? In a second step, we could move open_i2c_dev_num() up in the
code, make it static, and remove it from the header. Then, only
open_i2c_dev() would stay in the public header. I'd think the tools
themselves shouldn't know about the differences how to reach the
adapter. Or?

From: Wolfram Sang <wsa+renesas@sang-engineering.com>
Date: Thu, 9 Jul 2026 14:09:41 +0200
Subject: [PATCH i2c-tools] improve error reporting

---
 tools/i2cbusses.c | 21 +++++----------------
 tools/i2cbusses.h |  1 -
 2 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/tools/i2cbusses.c b/tools/i2cbusses.c
index 54f3238..67e268a 100644
--- a/tools/i2cbusses.c
+++ b/tools/i2cbusses.c
@@ -442,20 +442,6 @@ int open_i2c_dev_num(int i2cbus, char *filename, size_t size, int quiet)
 	return file;
 }
 
-int open_i2c_dev_path(const char *i2cbus_arg, int quiet)
-{
-	int file = open(i2cbus_arg, O_RDWR);
-
-	if (file < 0 && !quiet) {
-		fprintf(stderr, "Error: Could not open file "
-			"`%s': %s\n", i2cbus_arg, strerror(errno));
-		if (errno == EACCES)
-			fprintf(stderr, "Run as root?\n");
-	}
-
-	return file;
-}
-
 int open_i2c_dev(char *i2cbus_arg, char **filename, size_t size, int quiet)
 {
 	int file, i2cbus;
@@ -464,10 +450,13 @@ int open_i2c_dev(char *i2cbus_arg, char **filename, size_t size, int quiet)
 
 	if (i2cbus < 0) {
 		*filename = i2cbus_arg;
-		file = open_i2c_dev_path(i2cbus_arg, quiet);
-		if (file < 0)
+		file = open(i2cbus_arg, O_RDWR);
+		if (file < 0 && !quiet) {
 			fprintf(stderr, "Failed to open `%s' as a bus name "
 					"and as a path.\n", i2cbus_arg);
+			if (errno == EACCES)
+				fprintf(stderr, "Run as root?\n");
+		}
 		return file;
 	}
 
diff --git a/tools/i2cbusses.h b/tools/i2cbusses.h
index 5de3056..542d291 100644
--- a/tools/i2cbusses.h
+++ b/tools/i2cbusses.h
@@ -33,7 +33,6 @@ int lookup_i2c_bus(const char *i2cbus_arg);
 int parse_i2c_address(const char *address_arg, int all_addrs);
 int open_i2c_dev(char *i2cbus_arg, char **filename, size_t size, int quiet);
 int open_i2c_dev_num(int i2cbus, char *filename, size_t size, int quiet);
-int open_i2c_dev_path(const char *i2cbus_arg, int quiet);
 int set_slave_addr(int file, int address, int force);
 
 #define MISSING_FUNC_FMT	"Error: Adapter does not have %s capability\n"




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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] i2c-tools: Allow passing device file paths
  2026-07-06  4:27 ` [PATCH v4 1/2] i2c-tools: Allow passing device file paths Brigham Campbell
  2026-07-09 12:18   ` Wolfram Sang
@ 2026-07-09 12:40   ` Gero Schwäricke
  2026-07-11 19:24     ` Wolfram Sang
  1 sibling, 1 reply; 7+ messages in thread
From: Gero Schwäricke @ 2026-07-09 12:40 UTC (permalink / raw)
  To: Brigham Campbell, Jean Delvare, linux-i2c; +Cc: Wolfram Sang

Hi Brigham,

I just finished reviewing your patch. Aaand I saw Wolfram beat me to it
by 20 minutes. I hope you forgive me for sending it as is even though it
may now contain duplicates to his findings.

On Mon Jul 6, 2026 at 6:27 AM CEST, Brigham Campbell wrote:
[...]
> @@ -410,7 +406,7 @@ int parse_i2c_address(const char *address_arg, int all_addrs)
>  	return address;
>  }
>  
> -int open_i2c_dev(int i2cbus, char *filename, size_t size, int quiet)
> +int open_i2c_dev_num(int i2cbus, char *filename, size_t size, int quiet)
>  {
>  	int file, len;
>  

The lookup function uses suffix `_by_name`, so for consistency maybe use
`_by_nr` instead? That is what it's called in `struct i2c_adap`.

> @@ -446,6 +442,38 @@ int open_i2c_dev(int i2cbus, char *filename, size_t size, int quiet)
>  	return file;
>  }
>  
> +int open_i2c_dev_path(const char *i2cbus_arg, int quiet)
> +{
> +	int file = open(i2cbus_arg, O_RDWR);
> +
> +	if (file < 0 && !quiet) {
> +		fprintf(stderr, "Error: Could not open file "
> +			"`%s': %s\n", i2cbus_arg, strerror(errno));
> +		if (errno == EACCES)
> +			fprintf(stderr, "Run as root?\n");
> +	}
> +
> +	return file;
> +}
> +
> +int open_i2c_dev(char *i2cbus_arg, char **filename, size_t size, int quiet)
> +{
> +	int file, i2cbus;
> +
> +	i2cbus = lookup_i2c_bus(i2cbus_arg);
> +
> +	if (i2cbus < 0) {
> +		*filename = i2cbus_arg;
> +		file = open_i2c_dev_path(i2cbus_arg, quiet);
> +		if (file < 0)
> +			fprintf(stderr, "Failed to open `%s' as a bus name "
> +					"and as a path.\n", i2cbus_arg);
> +		return file;
> +	}
> +
> +	return open_i2c_dev_num(i2cbus, *filename, size, quiet);
> +}
> +

This is a minimally invasive change that makes this work, but it also
spaghettifies the code base a little. `lookup_i2c_bus()` tries to parse
the input to a bus number or, if that doesn't work, tries to resolve it
as a bus name. And if that fails we try to open as path. The issue is
that `lookup_i2c_bus()` is not very descriptive. Without reading the
code it's not understandable what's happening. Instead I would propose
to inline `lookup_i2c_bus()` and then have the logic inside
`open_i2c_dev()` be imparative like:

1. try to convert input to i2cbus bus number, if it worked
   `return open_i2c_dev_by_nr()` with the bus number, otherwise,
2. try `lookup_i2c_bus_by_name()`, if it worked we now have the bus
   number, so we `return open_i2c_dev_by_nr()` with it, otherwise,
3. try to open as path.

I think we can also inline `open_i2c_dev_path()` and remove it's
`fprintf()` in the failure case. We're just speculating here that it's a
path, so printing at the end that the given input could not be found as
bus number, bus name, or path is sufficient.

>  int set_slave_addr(int file, int address, int force)
>  {
>  	/* With force, let the user read from/write to the registers
> diff --git a/tools/i2cbusses.h b/tools/i2cbusses.h
> index 6f901b6..5de3056 100644
> --- a/tools/i2cbusses.h
> +++ b/tools/i2cbusses.h
> @@ -31,7 +31,9 @@ void free_adapters(struct i2c_adap *adapters);
>  
>  int lookup_i2c_bus(const char *i2cbus_arg);
>  int parse_i2c_address(const char *address_arg, int all_addrs);
> -int open_i2c_dev(int i2cbus, char *filename, size_t size, int quiet);
> +int open_i2c_dev(char *i2cbus_arg, char **filename, size_t size, int quiet);
> +int open_i2c_dev_num(int i2cbus, char *filename, size_t size, int quiet);
> +int open_i2c_dev_path(const char *i2cbus_arg, int quiet);
>  int set_slave_addr(int file, int address, int force);
>  
>  #define MISSING_FUNC_FMT	"Error: Adapter does not have %s capability\n"

I don't think there's a need to expose _num() and _path(). In fact, you
can even remove lookup_i2c_bus() as that is not used anymore by any
tool.

> diff --git a/tools/i2cdetect.c b/tools/i2cdetect.c
> index 3c22f17..b62f395 100644
> --- a/tools/i2cdetect.c
> +++ b/tools/i2cdetect.c
> @@ -213,8 +213,9 @@ static void print_i2c_busses(void)
>  int main(int argc, char *argv[])
>  {
>  	char *end;
> -	int i2cbus, file, res;
> -	char filename[20];
> +	int file, res;
> +	char filename_buf[20];
> +	char *filename = filename_buf;
>  	unsigned long funcs;
>  	int mode = MODE_AUTO;
>  	int first = 0x08, last = 0x77;
> @@ -277,11 +278,6 @@ int main(int argc, char *argv[])
>  		help();
>  		exit(1);
>  	}
> -	i2cbus = lookup_i2c_bus(argv[optind]);
> -	if (i2cbus < 0) {
> -		help();
> -		exit(1);
> -	}
>  
>  	/* read address range if present */
>  	if (argc == optind + 3 && mode != MODE_FUNC) {
> @@ -321,7 +317,7 @@ int main(int argc, char *argv[])
>  		exit(1);
>  	}
>  
> -	file = open_i2c_dev(i2cbus, filename, sizeof(filename), 0);
> +	file = open_i2c_dev(argv[optind], &filename, sizeof(filename_buf), 0);
>  	if (file < 0) {
>  		exit(1);
>  	}

Removing this will change the error path, so a call that would
previously fail in lookup_i2c_bus() may now fail somewhere else,
changing error code and message. Same for all other tools.

@Jean Is that acceptable or against the targeted stability requirements
of i2c-tools?

Best,
Gero

[...] the rest is tool changes similar to i2cdetect

-- 


sevenlab engineering GmbH <https://sevenlab.de>
serious engineering.

Geschäftsführer: Christian J. Pereira
Amtsgericht Köln, HRB 121730
Anschrift: Hansaring 20, 50670 Köln

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] i2c-tools: Allow passing device file paths
  2026-07-09 12:40   ` Gero Schwäricke
@ 2026-07-11 19:24     ` Wolfram Sang
  0 siblings, 0 replies; 7+ messages in thread
From: Wolfram Sang @ 2026-07-11 19:24 UTC (permalink / raw)
  To: Gero Schwäricke; +Cc: Brigham Campbell, Jean Delvare, linux-i2c

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


> code it's not understandable what's happening. Instead I would propose
> to inline `lookup_i2c_bus()` and then have the logic inside
> `open_i2c_dev()` be imparative like:
> 
> 1. try to convert input to i2cbus bus number, if it worked
>    `return open_i2c_dev_by_nr()` with the bus number, otherwise,
> 2. try `lookup_i2c_bus_by_name()`, if it worked we now have the bus
>    number, so we `return open_i2c_dev_by_nr()` with it, otherwise,
> 3. try to open as path.

I like this approach.

> > -	i2cbus = lookup_i2c_bus(argv[optind]);
> > -	if (i2cbus < 0) {
> > -		help();
> > -		exit(1);
> > -	}
> >  
> >  	/* read address range if present */
> >  	if (argc == optind + 3 && mode != MODE_FUNC) {
> > @@ -321,7 +317,7 @@ int main(int argc, char *argv[])
> >  		exit(1);
> >  	}
> >  
> > -	file = open_i2c_dev(i2cbus, filename, sizeof(filename), 0);
> > +	file = open_i2c_dev(argv[optind], &filename, sizeof(filename_buf), 0);
> >  	if (file < 0) {
> >  		exit(1);
> >  	}
> 
> Removing this will change the error path, so a call that would
> previously fail in lookup_i2c_bus() may now fail somewhere else,
> changing error code and message. Same for all other tools.

You mean that help() is not printed in some cases then? The error code
is 1 in all cases, or am I missing something?

> @Jean Is that acceptable or against the targeted stability requirements
> of i2c-tools?

I am also curious about Jean's opinion here. In my book it is okay. If
not, we could also release the new version as 5.0, I'd say. Would even
match adding the DDR5 support nicely ;)


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 2/2] i2c-tools: Document device paths as I2CBUS arg
  2026-07-06  4:27 ` [PATCH v4 2/2] i2c-tools: Document device paths as I2CBUS arg Brigham Campbell
@ 2026-07-11 19:43   ` Wolfram Sang
  0 siblings, 0 replies; 7+ messages in thread
From: Wolfram Sang @ 2026-07-11 19:43 UTC (permalink / raw)
  To: Brigham Campbell; +Cc: Jean Delvare, linux-i2c

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

On Sun, Jul 05, 2026 at 10:27:54PM -0600, Brigham Campbell wrote:
> i2c-tools now accept device file paths in addition to the i2c adapter
> number and i2c adapter name. Add this capability to documentation.
> 
> Signed-off-by: Brigham Campbell <me@brighamcampbell.com>

Looks okay to me. Thank you!


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-07-11 19:44 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-06  4:27 [PATCH v4 0/2] i2c-tools: Make tools accept bus path Brigham Campbell
2026-07-06  4:27 ` [PATCH v4 1/2] i2c-tools: Allow passing device file paths Brigham Campbell
2026-07-09 12:18   ` Wolfram Sang
2026-07-09 12:40   ` Gero Schwäricke
2026-07-11 19:24     ` Wolfram Sang
2026-07-06  4:27 ` [PATCH v4 2/2] i2c-tools: Document device paths as I2CBUS arg Brigham Campbell
2026-07-11 19:43   ` Wolfram Sang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox