diff mbox series

[1/7] i2cget: Add support for I2C block data

Message ID 20210608172858.0fbd301f@endymion
State New
Headers show
Series Rework block read support among i2cget and i2cdump | expand

Commit Message

Jean Delvare June 8, 2021, 3:28 p.m. UTC 
From: Crestez Dan Leonard <leonard.crestez@intel.com>

This adds mode 'i' for I2C_SMBUS_I2C_BLOCK_DATA. This is the same mode
letter from i2cdump.

Length is optional and defaults to 32 (maximum).

The intended use is debugging i2c devices with shell commands.

[JD: Fix the build (wrong variable name)
     Ensure PEC isn't used in I2C block mode
     Don't print the lenth (doesn't add value and could be mistaken as
       an offset
     Various cleanups]

Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
Signed-off-by: Jean Delvare <jdelvare@suse.de>
---
 tools/i2cget.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 55 insertions(+), 10 deletions(-)

Comments

Wolfram Sang June 27, 2021, 10:11 a.m. UTC | #1 
> +		"  LENGTH is the I2C block data length\n");


I think it would be good to state the allowed length values. Same for
patch 2.
Jean Delvare July 12, 2021, 9:36 a.m. UTC | #2 
On Sun, 27 Jun 2021 12:11:26 +0200, Wolfram Sang wrote:
> > +		"  LENGTH is the I2C block data length\n");  

> 

> I think it would be good to state the allowed length values. Same for

> patch 2.


Good idea. Done, thanks.

-- 
Jean Delvare
SUSE L3 Support
diff mbox series

Patch

--- i2c-tools.orig/tools/i2cget.c	2021-06-08 16:49:55.025098861 +0200
+++ i2c-tools/tools/i2cget.c	2021-06-08 16:54:15.707669058 +0200
@@ -41,14 +41,16 @@  static void help(void) __attribute__ ((n
 static void help(void)
 {
 	fprintf(stderr,
-		"Usage: i2cget [-f] [-y] [-a] I2CBUS CHIP-ADDRESS [DATA-ADDRESS [MODE]]\n"
+		"Usage: i2cget [-f] [-y] [-a] I2CBUS CHIP-ADDRESS [DATA-ADDRESS [MODE [LENGTH]]]\n"
 		"  I2CBUS is an integer or an I2C bus name\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"
 		"    w (read word data)\n"
 		"    c (write byte/read byte)\n"
-		"    Append p for SMBus PEC\n");
+		"    i (read I2C block data)\n"
+		"    Append p for SMBus PEC\n"
+		"  LENGTH is the I2C block data length\n");
 	exit(1);
 }
 
@@ -89,6 +91,13 @@  static int check_funcs(int file, int siz
 			return -1;
 		}
 		break;
+
+	case I2C_SMBUS_I2C_BLOCK_DATA:
+		if (!(funcs & I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
+			fprintf(stderr, MISSING_FUNC_FMT, "SMBus read I2C block data");
+			return -1;
+		}
+		break;
 	}
 
 	if (pec
@@ -101,7 +110,7 @@  static int check_funcs(int file, int siz
 }
 
 static int confirm(const char *filename, int address, int size, int daddress,
-		   int pec)
+		   int length, int pec)
 {
 	int dont = 0;
 
@@ -132,11 +141,15 @@  static int confirm(const char *filename,
 		fprintf(stderr, "current data\naddress");
 	else
 		fprintf(stderr, "data address\n0x%02x", daddress);
-	fprintf(stderr, ", using %s.\n",
-		size == I2C_SMBUS_BYTE ? (daddress < 0 ?
-		"read byte" : "write byte/read byte") :
-		size == I2C_SMBUS_BYTE_DATA ? "read byte data" :
-		"read word data");
+	if (size == I2C_SMBUS_I2C_BLOCK_DATA)
+		fprintf(stderr, ", %d %s using read I2C block data.\n",
+			length, length > 1 ? "bytes" : "byte");
+	else
+		fprintf(stderr, ", using %s.\n",
+			size == I2C_SMBUS_BYTE ? (daddress < 0 ?
+			"read byte" : "write byte/read byte") :
+			size == I2C_SMBUS_BYTE_DATA ? "read byte data" :
+			"read word data");
 	if (pec)
 		fprintf(stderr, "PEC checking enabled.\n");
 
@@ -159,6 +172,8 @@  int main(int argc, char *argv[])
 	int pec = 0;
 	int flags = 0;
 	int force = 0, yes = 0, version = 0, all_addrs = 0;
+	int length;
+	unsigned char block_data[I2C_SMBUS_BLOCK_MAX];
 
 	/* handle (optional) flags first */
 	while (1+flags < argc && argv[1+flags][0] == '-') {
@@ -209,11 +224,30 @@  int main(int argc, char *argv[])
 		case 'b': size = I2C_SMBUS_BYTE_DATA; break;
 		case 'w': size = I2C_SMBUS_WORD_DATA; break;
 		case 'c': size = I2C_SMBUS_BYTE; break;
+		case 'i': size = I2C_SMBUS_I2C_BLOCK_DATA; break;
 		default:
 			fprintf(stderr, "Error: Invalid mode!\n");
 			help();
 		}
 		pec = argv[flags+4][1] == 'p';
+		if (size == I2C_SMBUS_I2C_BLOCK_DATA && pec) {
+			fprintf(stderr, "Error: PEC not supported for I2C block data!\n");
+			help();
+		}
+	}
+
+	if (argc > flags + 5) {
+		if (size != I2C_SMBUS_I2C_BLOCK_DATA) {
+			fprintf(stderr, "Error: Length only valid for I2C block data!\n");
+			help();
+		}
+		length = strtol(argv[flags+5], &end, 0);
+		if (*end || length < 1 || length > I2C_SMBUS_BLOCK_MAX) {
+			fprintf(stderr, "Error: Length invalid!\n");
+			help();
+		}
+	} else {
+		length = I2C_SMBUS_BLOCK_MAX;
 	}
 
 	file = open_i2c_dev(i2cbus, filename, sizeof(filename), 0);
@@ -222,7 +256,7 @@  int main(int argc, char *argv[])
 	 || set_slave_addr(file, address, force))
 		exit(1);
 
-	if (!yes && !confirm(filename, address, size, daddress, pec))
+	if (!yes && !confirm(filename, address, size, daddress, length, pec))
 		exit(0);
 
 	if (pec && ioctl(file, I2C_PEC, 1) < 0) {
@@ -244,6 +278,9 @@  int main(int argc, char *argv[])
 	case I2C_SMBUS_WORD_DATA:
 		res = i2c_smbus_read_word_data(file, daddress);
 		break;
+	case I2C_SMBUS_I2C_BLOCK_DATA:
+		res = i2c_smbus_read_i2c_block_data(file, daddress, length, block_data);
+		break;
 	default: /* I2C_SMBUS_BYTE_DATA */
 		res = i2c_smbus_read_byte_data(file, daddress);
 	}
@@ -254,7 +291,15 @@  int main(int argc, char *argv[])
 		exit(2);
 	}
 
-	printf("0x%0*x\n", size == I2C_SMBUS_WORD_DATA ? 4 : 2, res);
+	if (size == I2C_SMBUS_I2C_BLOCK_DATA) {
+		int i;
+
+		for (i = 0; i < res - 1; ++i)
+			printf("0x%02hhx ", block_data[i]);
+		printf("0x%02hhx\n", block_data[res - 1]);
+	} else {
+		printf("0x%0*x\n", size == I2C_SMBUS_WORD_DATA ? 4 : 2, res);
+	}
 
 	exit(0);
 }