Discussion:
i2ctools: Need capability to write SMBus block command
Guenter Roeck
2011-01-25 16:43:11 UTC
Permalink
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
Jean Delvare
2011-01-25 16:49:22 UTC
Permalink
Hi Guenter,
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.
--
Jean Delvare
Guenter Roeck
2011-01-25 17:14:11 UTC
Permalink
Post by Jean Delvare
Hi Guenter,
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...
Yes, I noticed that after I sent my e-mail. Sorry for that.
Post by Jean Delvare
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
Guenter Roeck
2011-01-27 16:00:58 UTC
Permalink
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

--
Index: tools/i2cset.c
===================================================================
--- tools/i2cset.c (revision 5909)
+++ tools/i2cset.c (working copy)
@@ -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);
}
Jean Delvare
2011-01-27 16:59:08 UTC
Permalink
Hi Guenter,
Post by 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.
Post by Guenter Roeck
So I decided to handle block data first and bypass the rest
of the parameter handling code.
Guenter
--
Index: tools/i2cset.c
===================================================================
--- tools/i2cset.c (revision 5909)
+++ tools/i2cset.c (working copy)
@@ -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;
+
+ if (!(funcs & I2C_FUNC_SMBUS_READ_BLOCK_DATA)) {
+ fprintf(stderr, MISSING_FUNC_FMT, "SMBus block read");
+ return -1;
+ }
+ break;
+ 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?
Post by Guenter Roeck
}
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.
Post by Guenter Roeck
{
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];
It might make sense to use I2C_SMBUS_BLOCK_MAX instead of hard-coding
32?
Post by Guenter Roeck
+ 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.
Post by Guenter Roeck
+ switch (argv[argc-1][0]) {
+ case 's': size = I2C_SMBUS_BLOCK_DATA; break;
+ case 'i': size = I2C_SMBUS_I2C_BLOCK_DATA; break;
+ 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.
Post by Guenter Roeck
+ for (len = 0; len < (int)sizeof(block) && len + flags + 5 < argc; len++) {
Do you actually need the cast?
Post by Guenter Roeck
+ 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();
}
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 @@
res = i2c_smbus_write_word_data(file, daddress, value);
break;
+ res = i2c_smbus_write_block_data(file, daddress, len, block);
+ break;
+ 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.
Post by Guenter Roeck
close(file);
exit(1);
}
I tested your patch for the I2C block write case, it worked OK.

Please also update the manual page.
--
Jean Delvare
Guenter Roeck
2011-01-27 18:36:30 UTC
Permalink
Post by Jean Delvare
Hi Guenter,
Post by 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.
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.
Post by Jean Delvare
Post by Guenter Roeck
So I decided to handle block data first and bypass the rest
of the parameter handling code.
Guenter
--
Index: tools/i2cset.c
===================================================================
--- tools/i2cset.c (revision 5909)
+++ tools/i2cset.c (working copy)
@@ -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;
+
+ if (!(funcs & I2C_FUNC_SMBUS_READ_BLOCK_DATA)) {
+ fprintf(stderr, MISSING_FUNC_FMT, "SMBus block read");
+ return -1;
+ }
+ break;
+ 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.
Post by Jean Delvare
Post by Guenter Roeck
}
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.
Ok.
Post by Jean Delvare
Post by Guenter Roeck
{
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];
It might make sense to use I2C_SMBUS_BLOCK_MAX instead of hard-coding
32?
Ok.
Post by Jean Delvare
Post by Guenter Roeck
+ 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.
No, it is the same check used for 'b' and 'w'. I tested it, and it works.
Post by Jean Delvare
Post by Guenter Roeck
+ switch (argv[argc-1][0]) {
+ case 's': size = I2C_SMBUS_BLOCK_DATA; break;
+ case 'i': size = I2C_SMBUS_I2C_BLOCK_DATA; break;
+ 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.
Ok, I added an error check.
Post by Jean Delvare
Post by Guenter Roeck
+ 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.
Post by Jean Delvare
Post by Guenter Roeck
+ 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();
}
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 @@
res = i2c_smbus_write_word_data(file, daddress, value);
break;
+ res = i2c_smbus_write_block_data(file, daddress, len, block);
+ break;
+ 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.
Yes, they do. But you are right, this change doesn't belong into this patch.
Leftover from testing anyway, so I removed it.
Post by Jean Delvare
Post by Guenter Roeck
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
Guenter Roeck
2011-01-27 19:58:42 UTC
Permalink
Add support to write SMBus and I2C block data to i2cset.

Rev 2: Included review feedback

--
Index: tools/i2cset.8
===================================================================
--- tools/i2cset.8 (revision 5909)
+++ tools/i2cset.8 (working copy)
@@ -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.
Index: tools/i2cset.c
===================================================================
--- tools/i2cset.c (revision 5909)
+++ tools/i2cset.c (working copy)
@@ -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_WRITE_BLOCK_DATA)) {
+ fprintf(stderr, MISSING_FUNC_FMT, "SMBus block read");
+ 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");
+ return -1;
+ }
+ break;
}

if (pec
@@ -90,7 +105,8 @@
}

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

@@ -109,7 +125,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 +161,8 @@
int pec = 0;
int flags = 0;
int force = 0, yes = 0, version = 0, readback = 0;
+ unsigned char block[I2C_SMBUS_BLOCK_MAX];
+ int len;

/* handle (optional) flags first */
while (1+flags < argc && argv[1+flags][0] == '-') {
@@ -180,6 +207,34 @@
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';
+ if (pec && size == I2C_SMBUS_I2C_BLOCK_DATA) {
+ fprintf(stderr, "Error: PEC not supported for I2C block writes!\n");
+ help();
+ }
+ 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 +291,7 @@
help();
}

+dofile:
file = open_i2c_dev(i2cbus, filename, sizeof(filename), 0);
if (file < 0
|| check_funcs(file, size, pec)
@@ -243,7 +299,7 @@
exit(1);

if (!yes && !confirm(filename, address, size, daddress,
- value, vmask, pec))
+ value, vmask, block, len, pec))
exit(0);

if (vmask) {
@@ -299,8 +355,15 @@
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");
Index: CHANGES
===================================================================
--- CHANGES (revision 5909)
+++ CHANGES (working copy)
@@ -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
Jean Delvare
2011-01-29 16:43:54 UTC
Permalink
Hi Guenter,
Post by Guenter Roeck
Add support to write SMBus and I2C block data to i2cset.
Rev 2: Included review feedback
@@ -78,6 +80,19 @@
return -1;
}
break;
+
+ if (!(funcs & I2C_FUNC_SMBUS_WRITE_BLOCK_DATA)) {
+ fprintf(stderr, MISSING_FUNC_FMT, "SMBus block read");
"SMBus block write"
Post by Guenter Roeck
+ return -1;
+ }
+ break;
+ if (!(funcs & I2C_FUNC_SMBUS_WRITE_I2C_BLOCK)) {
+ fprintf(stderr, MISSING_FUNC_FMT, "I2C block read");
"I2C block write"
Post by Guenter Roeck
+ 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.
--
Jean Delvare
Guenter Roeck
2011-01-29 17:39:03 UTC
Permalink
Hi Jean,

On Sat, Jan 29, 2011 at 11:43:54AM -0500, Jean Delvare wrote:
[ ...]
Post by Jean Delvare
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.
Post by Jean Delvare
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

Loading...