Re: [PATCH v4 1/2] i2c-tools: Allow passing device file paths

[Wolfram Sang <wsa+renesas@sang-engineering.com>]

[-- 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 --]