Skip to contentSkip to conversation

i2ctools: Need capability to write SMBus block command

lm-sensors Superseded Guenter Roeck 3 participants 10 messages Jan 25, 2011 – Jan 29, 2011 6 messages not in the archive I2C subsystem host drivers

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): 20110125162106.GA8024-IzeFyvvaP7pWk0Htik3J/[email protected]

#2 lore raw
Quoted 9 lines · Guenter Roeck

Hi,

I need the capability to write a SMBus block command with i2ctools.

Not an issue writing it myself. Question is if to expand i2cset to include that capability, or to write a new command (i2csetblock ?) to do it.

Thoughts ?

Sorry, wrong mailing list. Copying linux-i2c.

Thanks,
Guenter

#3 lore raw
In reply to a message not in the archive

Hi Guenter,

Quoted 6 lines · Guenter Roeck

I need the capability to write a SMBus block command with i2ctools.

Not an issue writing it myself. Question is if to expand i2cset to include that capability, or to write a new command (i2csetblock ?) to do it.

Thoughts ?

This would probably be best discussed on the linux-i2c list than lm-sensors. Redirecting there...

I would add it to i2cset. The only problem is that the command line convention of i2cset is a little tricky and thus hard to extend. But I see no reason why allowing multiple VALUEs followed by MODE s (for SMBus block) or i (for I2C block) wouldn't work. I suggest i and s because this is what i2cdump is using.

Signature · 1 line
Jean Delvare

A message that is not in the archive (sent off-list or not yet received): [email protected]

#5 lore raw
Quoted 5 lines · Jean Delvare

Hi Guenter,

Quoted 2 lines · Guenter Roeck

I need the capability to write a SMBus block command with i2ctools.

Quoted 8 lines
Quoted 4 lines

Not an issue writing it myself. Question is if to expand i2cset to include that capability, or to write a new command (i2csetblock ?) to do it.

Thoughts ?

This would probably be best discussed on the linux-i2c list than lm-sensors. Redirecting there...

Yes, I noticed that after I sent my e-mail. Sorry for that.

Quoted 6 lines

I would add it to i2cset. The only problem is that the command line convention of i2cset is a little tricky and thus hard to extend. But I see no reason why allowing multiple VALUEs followed by MODE s (for SMBus block) or i (for I2C block) wouldn't work. I suggest i and s because this is what i2cdump is using.

Ok, that is pretty much along the line of what I was thinking. I'll implement that and submit it for review.

Thanks,
Guenter

#6 lore raw

PATCH 1/1 i2ctools: Add capability to write block command

In reply to a message not in the archive

This patch adds support to write block data to i2cset.

I tried to limit the changes as much as possible. Detecting new write modes is a bit tricky since the command supports an undocumented parameter (mask) after the mode. So I decided to handle block data first and bypass the rest of the parameter handling code.

Guenter

--

i2cset.c +63 -5
Index: tools/i2cset.c
===================================================================
@@ -35,13 +35,15 @@
 static void help(void)
 {
 	fprintf(stderr,
-	        "Usage: i2cset [-f] [-y] [-m MASK] I2CBUS CHIP-ADDRESS DATA-ADDRESS [VALUE] [MODE]\n"
+	        "Usage: i2cset [-f] [-y] [-m MASK] I2CBUS CHIP-ADDRESS DATA-ADDRESS [VALUE] ... [MODE]\n"
 		"  I2CBUS is an integer or an I2C bus name\n"
 		"  ADDRESS is an integer (0x03 - 0x77)\n"
 		"  MODE is one of:\n"
 		"    c (byte, no value)\n"
 		"    b (byte data, default)\n"
 		"    w (word data)\n"
+		"    i (I2C block data)\n"
+		"    s (SMBus block data)\n"
 		"    Append p for SMBus PEC\n");
 	exit(1);
 }

@@ -78,6 +80,19 @@
 			return -1;
 		}
 		break;
+
+	case I2C_SMBUS_BLOCK_DATA:
+		if (!(funcs & I2C_FUNC_SMBUS_READ_BLOCK_DATA)) {
+			fprintf(stderr, MISSING_FUNC_FMT, "SMBus block read");
+			return -1;
+		}
+		break;
+	case I2C_SMBUS_I2C_BLOCK_DATA:
+		if (!(funcs & I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
+			fprintf(stderr, MISSING_FUNC_FMT, "I2C block read");
+			return -1;
+		}
+		break;
 	}
 
 	if (pec

@@ -90,7 +105,7 @@
 }
 
 static int confirm(const char *filename, int address, int size, int daddress,
-		   int value, int vmask, int pec)
+		   int value, int vmask, unsigned char *block, int len, int pec)
 {
 	int dont = 0;
 

@@ -109,7 +124,16 @@
 		"0x%02x, data address\n0x%02x, ", filename, address, daddress);
 	if (size == I2C_SMBUS_BYTE)
 		fprintf(stderr, "no data.\n");
-	else
+	else if (size == I2C_SMBUS_BLOCK_DATA ||
+		 size == I2C_SMBUS_I2C_BLOCK_DATA) {
+		int i;
+
+		fprintf(stderr, "data");
+		for (i = 0; i < len; i++)
+			fprintf(stderr, " 0x%02x", block[i]);
+		fprintf(stderr, ", mode %s.\n", size == I2C_SMBUS_BLOCK_DATA
+			? "smbus block" : "i2c block");
+	} else
 		fprintf(stderr, "data 0x%02x%s, mode %s.\n", value,
 			vmask ? " (masked)" : "",
 			size == I2C_SMBUS_BYTE_DATA ? "byte" : "word");

@@ -136,6 +160,8 @@
 	int pec = 0;
 	int flags = 0;
 	int force = 0, yes = 0, version = 0, readback = 0;
+	unsigned char block[32];
+	int len;
 
 	/* handle (optional) flags first */
 	while (1+flags < argc && argv[1+flags][0] == '-') {

@@ -180,6 +206,30 @@
 		help();
 	}
 
+	/* check for block data */
+	len = 0;
+	if (argc > flags + 5) {
+		switch (argv[argc-1][0]) {
+		case 's': size = I2C_SMBUS_BLOCK_DATA; break;
+		case 'i': size = I2C_SMBUS_I2C_BLOCK_DATA; break;
+		default:
+			size = 0;
+			break;
+		}
+		if (size == I2C_SMBUS_BLOCK_DATA || size == I2C_SMBUS_I2C_BLOCK_DATA) {
+			pec = argv[argc-1][1] == 'p';
+			for (len = 0; len < (int)sizeof(block) && len + flags + 5 < argc; len++) {
+				value = strtol(argv[flags + len + 4], &end, 0);
+				if (*end || value < 0 || value > 0xff) {
+                                	fprintf(stderr, "Error: Block data value invalid!\n");
+                                	help();
+                        	}
+				block[len] = value;
+			}
+			goto dofile;
+		}
+	}
+
 	if (argc > flags + 4) {
 		if (!strcmp(argv[flags+4], "c")
 		 || !strcmp(argv[flags+4], "cp")) {

@@ -236,6 +286,7 @@
 		help();
 	}
 
+dofile:
 	file = open_i2c_dev(i2cbus, filename, sizeof(filename), 0);
 	if (file < 0
 	 || check_funcs(file, size, pec)

@@ -243,7 +294,7 @@
 		exit(1);
 
 	if (!yes && !confirm(filename, address, size, daddress,
-			     value, vmask, pec))
+			     value, vmask, block, len, pec))
 		exit(0);
 
 	if (vmask) {

@@ -299,11 +350,18 @@
 	case I2C_SMBUS_WORD_DATA:
 		res = i2c_smbus_write_word_data(file, daddress, value);
 		break;
+	case I2C_SMBUS_BLOCK_DATA:
+		res = i2c_smbus_write_block_data(file, daddress, len, block);
+		break;
+	case I2C_SMBUS_I2C_BLOCK_DATA:
+		res = i2c_smbus_write_i2c_block_data(file, daddress, len, block);
+		break;
 	default: /* I2C_SMBUS_BYTE_DATA */
 		res = i2c_smbus_write_byte_data(file, daddress, value);
+		break;
 	}
 	if (res < 0) {
-		fprintf(stderr, "Error: Write failed\n");
+		perror("Error: Write failed");
 		close(file);
 		exit(1);
 	}

A message that is not in the archive (sent off-list or not yet received): 20110127160058.GA16853-IzeFyvvaP7pWk0Htik3J/[email protected]

#8 lore raw

Re: [PATCH] i2ctools: Add capability to write block command

Hi Guenter,

Quoted 5 lines · Guenter Roeck

This patch adds support to write block data to i2cset.

I tried to limit the changes as much as possible. Detecting new write modes is a bit tricky since the command supports an undocumented parameter (mask) after the mode.

This can go away if it bothers you. This was the old way to pass the mask value. I have implemented -m meanwhile, and this is what people should be using by now. I added it 2 years ago, so I think it's acceptable to stop supporting the legacy way.

Quoted 34 lines

So I decided to handle block data first and bypass the rest of the parameter handling code.

Guenter

--
Index: tools/i2cset.c
===================================================================

i2cset.c +6 -1
@@ -35,13 +35,15 @@
 static void help(void)
 {
 	fprintf(stderr,
-	        "Usage: i2cset [-f] [-y] [-m MASK] I2CBUS CHIP-ADDRESS DATA-ADDRESS [VALUE] [MODE]\n"
+	        "Usage: i2cset [-f] [-y] [-m MASK] I2CBUS CHIP-ADDRESS DATA-ADDRESS [VALUE] ... [MODE]\n"
 		"  I2CBUS is an integer or an I2C bus name\n"
 		"  ADDRESS is an integer (0x03 - 0x77)\n"
 		"  MODE is one of:\n"
 		"    c (byte, no value)\n"
 		"    b (byte data, default)\n"
 		"    w (word data)\n"
+		"    i (I2C block data)\n"
+		"    s (SMBus block data)\n"
 		"    Append p for SMBus PEC\n");
 	exit(1);
 }

@@ -78,6 +80,19 @@
 			return -1;
 		}
 		break;
+
+	case I2C_SMBUS_BLOCK_DATA:
+		if (!(funcs & I2C_FUNC_SMBUS_READ_BLOCK_DATA)) {

i2cset.c line 95

Quoted 10 lines
i2cset.c +10 -0
+			fprintf(stderr, MISSING_FUNC_FMT, "SMBus block read");
+			return -1;
+		}
+		break;
+	case I2C_SMBUS_I2C_BLOCK_DATA:
+		if (!(funcs & I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
+			fprintf(stderr, MISSING_FUNC_FMT, "I2C block read");
+			return -1;
+		}
+		break;

Why are you testing READ functionalities when what you want to do is WRITE?

i2cset.c line 108

Quoted 9 lines
i2cset.c +1 -1
 	}
 
 	if (pec

@@ -90,7 +105,7 @@
 }
 
 static int confirm(const char *filename, int address, int size, int daddress,
-		   int value, int vmask, int pec)
+		   int value, int vmask, unsigned char *block, int len, int pec)

The block pointer could be const.

Quoted 22 lines
i2cset.c +10 -1
 {
 	int dont = 0;
 

@@ -109,7 +124,16 @@
 		"0x%02x, data address\n0x%02x, ", filename, address, daddress);
 	if (size == I2C_SMBUS_BYTE)
 		fprintf(stderr, "no data.\n");
-	else
+	else if (size == I2C_SMBUS_BLOCK_DATA ||
+		 size == I2C_SMBUS_I2C_BLOCK_DATA) {
+		int i;
+
+		fprintf(stderr, "data");
+		for (i = 0; i < len; i++)
+			fprintf(stderr, " 0x%02x", block[i]);
+		fprintf(stderr, ", mode %s.\n", size == I2C_SMBUS_BLOCK_DATA
+			? "smbus block" : "i2c block");
+	} else
 		fprintf(stderr, "data 0x%02x%s, mode %s.\n", value,
 			vmask ? " (masked)" : "",
 			size == I2C_SMBUS_BYTE_DATA ? "byte" : "word");

i2cset.c line 163

Quoted 4 lines
i2cset.c +1 -0
@@ -136,6 +160,8 @@
 	int pec = 0;
 	int flags = 0;
 	int force = 0, yes = 0, version = 0, readback = 0;
+	unsigned char block[32];

It might make sense to use I2C_SMBUS_BLOCK_MAX instead of hard-coding 32?

i2cset.c line 211

Quoted 11 lines
i2cset.c +4 -0
+	int len;
 
 	/* handle (optional) flags first */
 	while (1+flags < argc && argv[1+flags][0] == '-') {

@@ -180,6 +206,30 @@
 		help();
 	}
 
+	/* check for block data */
+	len = 0;
+	if (argc > flags + 5) {

This makes it impossible to write 1-byte blocks, right? This is bad.

i2cset.c line 220

Quoted 9 lines
i2cset.c +9 -0
+		switch (argv[argc-1][0]) {
+		case 's': size = I2C_SMBUS_BLOCK_DATA; break;
+		case 'i': size = I2C_SMBUS_I2C_BLOCK_DATA; break;
+		default:
+			size = 0;
+			break;
+		}
+		if (size == I2C_SMBUS_BLOCK_DATA || size == I2C_SMBUS_I2C_BLOCK_DATA) {
+			pec = argv[argc-1][1] == 'p';

ip isn't a valid mode. PEC is not defined for non-SMBus transactions (and despite its name, I2C_SMBUS_I2C_BLOCK_DATA read and writes are not SMBus transactions.

i2cset.c line 221

Quoted 1 line
i2cset.c +1 -0
+			for (len = 0; len < (int)sizeof(block) && len + flags + 5 < argc; len++) {

Do you actually need the cast?

Quoted 38 lines
i2cset.c +16 -1
+				value = strtol(argv[flags + len + 4], &end, 0);
+				if (*end || value < 0 || value > 0xff) {
+                                	fprintf(stderr, "Error: Block data value invalid!\n");
+                                	help();
+                        	}
+				block[len] = value;
+			}
+			goto dofile;
+		}
+	}
+
 	if (argc > flags + 4) {
 		if (!strcmp(argv[flags+4], "c")
 		 || !strcmp(argv[flags+4], "cp")) {

@@ -236,6 +286,7 @@
 		help();
 	}
 
+dofile:
 	file = open_i2c_dev(i2cbus, filename, sizeof(filename), 0);
 	if (file < 0
 	 || check_funcs(file, size, pec)

@@ -243,7 +294,7 @@
 		exit(1);
 
 	if (!yes && !confirm(filename, address, size, daddress,
-			     value, vmask, pec))
+			     value, vmask, block, len, pec))
 		exit(0);
 
 	if (vmask) {

@@ -299,11 +350,18 @@
 	case I2C_SMBUS_WORD_DATA:
 		res = i2c_smbus_write_word_data(file, daddress, value);
 		break;
+	case I2C_SMBUS_BLOCK_DATA:
+		res = i2c_smbus_write_block_data(file, daddress, len, block);
+		break;

i2cset.c line 364

Quoted 10 lines
i2cset.c +5 -1
+	case I2C_SMBUS_I2C_BLOCK_DATA:
+		res = i2c_smbus_write_i2c_block_data(file, daddress, len, block);
+		break;
 	default: /* I2C_SMBUS_BYTE_DATA */
 		res = i2c_smbus_write_byte_data(file, daddress, value);
+		break;
 	}
 	if (res < 0) {
-		fprintf(stderr, "Error: Write failed\n");
+		perror("Error: Write failed");

Hmm, do i2c_smbus_()* calls actually set errno? I didn't expect them to. Either way, if this change is wanted, it doesn't belong to this patch.

i2cset.c line 367

Quoted 3 lines
i2cset.c +0 -0
 		close(file);
 		exit(1);
 	}

I tested your patch for the I2C block write case, it worked OK.

Please also update the manual page.

Signature · 1 line
Jean Delvare

A message that is not in the archive (sent off-list or not yet received): [email protected]

#10 lore raw

Re: [PATCH] i2ctools: Add capability to write block command

Quoted 5 lines · Jean Delvare

Hi Guenter,

Quoted 2 lines · Guenter Roeck

This patch adds support to write block data to i2cset.

Quoted 9 lines
Quoted 3 lines

I tried to limit the changes as much as possible. Detecting new write modes is a bit tricky since the command supports an undocumented parameter (mask) after the mode.

This can go away if it bothers you. This was the old way to pass the mask value. I have implemented -m meanwhile, and this is what people should be using by now. I added it 2 years ago, so I think it's acceptable to stop supporting the legacy way.

Would be a separate patch. Also, the current code accepts nonsense parameters (such as comments) after the mode parameter. Not sure if I want to change that.

Quoted 39 lines
Quoted 39 lines

So I decided to handle block data first and bypass the rest of the parameter handling code.

Guenter

--
Index: tools/i2cset.c
===================================================================

i2cset.c +11 -1
@@ -35,13 +35,15 @@
 static void help(void)
 {
 	fprintf(stderr,
-	        "Usage: i2cset [-f] [-y] [-m MASK] I2CBUS CHIP-ADDRESS DATA-ADDRESS [VALUE] [MODE]\n"
+	        "Usage: i2cset [-f] [-y] [-m MASK] I2CBUS CHIP-ADDRESS DATA-ADDRESS [VALUE] ... [MODE]\n"
 		"  I2CBUS is an integer or an I2C bus name\n"
 		"  ADDRESS is an integer (0x03 - 0x77)\n"
 		"  MODE is one of:\n"
 		"    c (byte, no value)\n"
 		"    b (byte data, default)\n"
 		"    w (word data)\n"
+		"    i (I2C block data)\n"
+		"    s (SMBus block data)\n"
 		"    Append p for SMBus PEC\n");
 	exit(1);
 }

@@ -78,6 +80,19 @@
 			return -1;
 		}
 		break;
+
+	case I2C_SMBUS_BLOCK_DATA:
+		if (!(funcs & I2C_FUNC_SMBUS_READ_BLOCK_DATA)) {
+			fprintf(stderr, MISSING_FUNC_FMT, "SMBus block read");
+			return -1;
+		}
+		break;
+	case I2C_SMBUS_I2C_BLOCK_DATA:
Quoted 9 lines

i2cset.c line 95

Quoted 5 lines
i2cset.c +5 -0
+		if (!(funcs & I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
+			fprintf(stderr, MISSING_FUNC_FMT, "I2C block read");
+			return -1;
+		}
+		break;

Why are you testing READ functionalities when what you want to do is WRITE?

Typical cut-and-paste error.

Quoted 6 lines
Quoted 6 lines
i2cset.c +0 -0
 	}
 
 	if (pec

@@ -90,7 +105,7 @@
 }
Quoted 6 lines

i2cset.c line 108

Quoted 3 lines
i2cset.c +1 -1
 static int confirm(const char *filename, int address, int size, int daddress,
-		   int value, int vmask, int pec)
+		   int value, int vmask, unsigned char *block, int len, int pec)

The block pointer could be const.

Ok.

Quoted 21 lines
Quoted 21 lines
i2cset.c +10 -1
 {
 	int dont = 0;
 

@@ -109,7 +124,16 @@
 		"0x%02x, data address\n0x%02x, ", filename, address, daddress);
 	if (size == I2C_SMBUS_BYTE)
 		fprintf(stderr, "no data.\n");
-	else
+	else if (size == I2C_SMBUS_BLOCK_DATA ||
+		 size == I2C_SMBUS_I2C_BLOCK_DATA) {
+		int i;
+
+		fprintf(stderr, "data");
+		for (i = 0; i < len; i++)
+			fprintf(stderr, " 0x%02x", block[i]);
+		fprintf(stderr, ", mode %s.\n", size == I2C_SMBUS_BLOCK_DATA
+			? "smbus block" : "i2c block");
+	} else
 		fprintf(stderr, "data 0x%02x%s, mode %s.\n", value,
 			vmask ? " (masked)" : "",
 			size == I2C_SMBUS_BYTE_DATA ? "byte" : "word");
Quoted 9 lines

i2cset.c line 163

Quoted 5 lines
i2cset.c +1 -0
@@ -136,6 +160,8 @@
 	int pec = 0;
 	int flags = 0;
 	int force = 0, yes = 0, version = 0, readback = 0;
+	unsigned char block[32];

It might make sense to use I2C_SMBUS_BLOCK_MAX instead of hard-coding 32?

Ok.

Quoted 8 lines
Quoted 8 lines
i2cset.c +1 -0
+	int len;
 
 	/* handle (optional) flags first */
 	while (1+flags < argc && argv[1+flags][0] == '-') {

@@ -180,6 +206,30 @@
 		help();
 	}
Quoted 6 lines

i2cset.c line 211

Quoted 3 lines
i2cset.c +3 -0
+	/* check for block data */
+	len = 0;
+	if (argc > flags + 5) {

This makes it impossible to write 1-byte blocks, right? This is bad.

No, it is the same check used for 'b' and 'w'. I tested it, and it works.

Quoted 5 lines
Quoted 5 lines
i2cset.c +5 -0
+		switch (argv[argc-1][0]) {
+		case 's': size = I2C_SMBUS_BLOCK_DATA; break;
+		case 'i': size = I2C_SMBUS_I2C_BLOCK_DATA; break;
+		default:
+			size = 0;
Quoted 9 lines

i2cset.c line 220

Quoted 4 lines
i2cset.c +4 -0
+			break;
+		}
+		if (size == I2C_SMBUS_BLOCK_DATA || size == I2C_SMBUS_I2C_BLOCK_DATA) {
+			pec = argv[argc-1][1] == 'p';

ip isn't a valid mode. PEC is not defined for non-SMBus transactions (and despite its name, I2C_SMBUS_I2C_BLOCK_DATA read and writes are not SMBus transactions.

Ok, I added an error check.

Quoted 4 lines

i2cset.c line 221

Quoted 1 line
i2cset.c +1 -0
+			for (len = 0; len < (int)sizeof(block) && len + flags + 5 < argc; len++) {

Do you actually need the cast?

Yes, at least with recent versions of gcc. sizeof() is an unsigned and len is an int, so without the cast gcc warns about a signed-unsigned comparison.

I could change len to be unsigned, but then it starts complaining about comparisons of len against other variables, so I'd rather stick with the typecast.

Quoted 44 lines
Quoted 44 lines
i2cset.c +20 -1
+				value = strtol(argv[flags + len + 4], &end, 0);
+				if (*end || value < 0 || value > 0xff) {
+                                	fprintf(stderr, "Error: Block data value invalid!\n");
+                                	help();
+                        	}
+				block[len] = value;
+			}
+			goto dofile;
+		}
+	}
+
 	if (argc > flags + 4) {
 		if (!strcmp(argv[flags+4], "c")
 		 || !strcmp(argv[flags+4], "cp")) {

@@ -236,6 +286,7 @@
 		help();
 	}
 
+dofile:
 	file = open_i2c_dev(i2cbus, filename, sizeof(filename), 0);
 	if (file < 0
 	 || check_funcs(file, size, pec)

@@ -243,7 +294,7 @@
 		exit(1);
 
 	if (!yes && !confirm(filename, address, size, daddress,
-			     value, vmask, pec))
+			     value, vmask, block, len, pec))
 		exit(0);
 
 	if (vmask) {

@@ -299,11 +350,18 @@
 	case I2C_SMBUS_WORD_DATA:
 		res = i2c_smbus_write_word_data(file, daddress, value);
 		break;
+	case I2C_SMBUS_BLOCK_DATA:
+		res = i2c_smbus_write_block_data(file, daddress, len, block);
+		break;
+	case I2C_SMBUS_I2C_BLOCK_DATA:
+		res = i2c_smbus_write_i2c_block_data(file, daddress, len, block);
+		break;
 	default: /* I2C_SMBUS_BYTE_DATA */
 		res = i2c_smbus_write_byte_data(file, daddress, value);
+		break;
Quoted 9 lines

i2cset.c line 364

Quoted 4 lines
i2cset.c +1 -1
 	}
 	if (res < 0) {
-		fprintf(stderr, "Error: Write failed\n");
+		perror("Error: Write failed");

Hmm, do i2c_smbus_()* calls actually set errno? I didn't expect them to. Either way, if this change is wanted, it doesn't belong to this patch.

Yes, they do. But you are right, this change doesn't belong into this patch. Leftover from testing anyway, so I removed it.

Quoted 8 lines

i2cset.c line 367

Quoted 3 lines
i2cset.c +0 -0
 		close(file);
 		exit(1);
 	}

I tested your patch for the I2C block write case, it worked OK.

Please also update the manual page.

Ok, will do.

Thanks,
Guenter

#11 lore raw

PATCH v2 1/1 i2ctools: Add capability to write block command

In reply to a message not in the archive

Add support to write SMBus and I2C block data to i2cset.

Rev 2: Included review feedback

--

i2cset.8 +14 -7
Index: tools/i2cset.8
===================================================================
@@ -12,6 +12,7 @@
 .I chip-address
 .I data-address
 .RI [ value ]
+.RI ...
 .RI [ mode ]
 .br
 .B i2cset

@@ -62,18 +63,24 @@
 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 omited, then a short write is
+location on the chip. If this parameter is omitted, then a short write is
 issued. For most chips, it simply sets an internal pointer to the target
 location, but doesn't actually write to that location. For a few chips
 though, in particular simple ones with a single register, this short write
-is an actual write.
+is an actual write. If the mode parameter is \fBs\fP or \fBi\fP, multiple
+values can be specified.
 .PP
-The \fImode\fR parameter, if specified, is one of the letters \fBb\fP or
-\fBw\fP, corresponding to a write size of a single byte or a 16-bit word,
-respectively. A \fBp\fP can also be appended to the \fImode\fR parameter to
-enable PEC. If the \fImode\fR parameter is omitted, i2cset defaults to byte
+The \fImode\fR parameter, if specified, is one of the letters \fBb\fP,
+\fBw\fP, \fBs\fP, or \fBi\fP, corresponding to a write size of a single byte,
+a 16-bit word, a SMBus block write, or an I2C block write, respectively.
+For SMBus and I2C block writes, the write size is determined by the number
+of \fIvalue\fR parameters.
+Except for I2C block writes, a \fBp\fP can also be appended to the \fImode\fR
+parameter to enable PEC.
+If the \fImode\fR parameter is omitted, i2cset defaults to byte
 mode without PEC. The \fIvalue\fR provided must be within range for the
-specified data type (0x00-0xFF for bytes, 0x0000-0xFFFF for words).
+specified data type (0x00-0xFF for byte and block writes, 0x0000-0xFFFF
+for words).
 Another possible mode is \fBc\fP, which doesn't write any value (so-called
 short write). You usually don't have to specify this mode, as it is the
 default when no value is provided, unless you also want to enable PEC.
i2cset.c +67 -4

133 lines not on this page · raw message

CHANGES +1 -0
Index: CHANGES
===================================================================
@@ -3,6 +3,7 @@
 
 SVN
   i2c-dev.h: Make value arrays const for block write functions
+  i2cset: Add support for SMBus and I2C block writes
 
 3.0.3 (2010-12-12)
   Makefile: Let the environment set CC and CFLAGS

A message that is not in the archive (sent off-list or not yet received): 20110127195842.GA17775-IzeFyvvaP7pWk0Htik3J/[email protected]

#13 lore raw

Re: [PATCH v2] i2ctools: Add capability to write block command

Hi Guenter,

Quoted 3 lines · Guenter Roeck

Add support to write SMBus and I2C block data to i2cset.

Rev 2: Included review feedback

Two remaining minor issues:

i2cset.c line 86

Quoted 8 lines
i2cset.c +4 -0
@@ -78,6 +80,19 @@
 			return -1;
 		}
 		break;
+
+	case I2C_SMBUS_BLOCK_DATA:
+		if (!(funcs & I2C_FUNC_SMBUS_WRITE_BLOCK_DATA)) {
+			fprintf(stderr, MISSING_FUNC_FMT, "SMBus block read");

"SMBus block write"

i2cset.c line 92

Quoted 6 lines
i2cset.c +6 -0
+			return -1;
+		}
+		break;
+	case I2C_SMBUS_I2C_BLOCK_DATA:
+		if (!(funcs & I2C_FUNC_SMBUS_WRITE_I2C_BLOCK)) {
+			fprintf(stderr, MISSING_FUNC_FMT, "I2C block read");

"I2C block write"

i2cset.c line 98

Quoted 6 lines
i2cset.c +3 -0
+			return -1;
+		}
+		break;
 	}
 
 	if (pec

Other than that, it looks alright, feel free to commit (I've just added you to the list of committers - same credentials as lm-sensors repository.)

Next, I think we can remove the old method for setting the mask. Then we'll see if some code cleanup or reorganization makes sense.

Signature · 1 line
Jean Delvare

A message that is not in the archive (sent off-list or not yet received): [email protected]

#15 lore raw

Re: [PATCH v2] i2ctools: Add capability to write block command

Hi Jean,

On Sat, Jan 29, 2011 at 11:43:54AM -0500, Jean Delvare wrote:
[ ...]

Quoted 5 lines

Other than that, it looks alright, feel free to commit (I've just added you to the list of committers - same credentials as lm-sensors repository.)

Thanks, and done.

Quoted 3 lines

Next, I think we can remove the old method for setting the mask. Then we'll see if some code cleanup or reorganization makes sense.

I'll see what I can do.

Guenter