Hi Jean,
On Tue, Mar 2, 2010 at 11:22 PM, Jean Delvare <khali@xxxxxxxxxxxx> wrote:
>
> Hi Felix,
>
> On Wed, 24 Feb 2010 14:01:56 +0200, Felix Rubinstein wrote:
> > Here is my code:
> >
> > ------------
> > #include <stdio.h>
> > #include <unistd.h>
> > #include <sys/types.h>
> > #include <sys/stat.h>
> > #include <fcntl.h>
> > #include <sys/ioctl.h>
> > #include <stdlib.h>
> >
> > #include "i2c-dev.h"
> > #include "i2cbusses.h"
> > #include "util.h"
> >
> > /* actually smbus allows up to 32 and i2c even more */
> > #define I2CRWL_MAX_PARAMS 10
> > #define I2CRWL_PARAMS_SHIFT 3
> >
> > static int i2c_writel(int fd, int datac, char *datav[])
> > {
> > int i;
> > unsigned char buf[I2CRWL_MAX_PARAMS];
> > unsigned int data;
> >
> > for (i = 0; i < datac && i < I2CRWL_MAX_PARAMS; i++) {
> > sscanf(datav[i], "%x", &data);
> > buf[i] = (unsigned char)data;
> > }
> >
> > if (i2c_smbus_write_i2c_block_data(fd, buf[0], datac - 1,
> > &buf[1]) < 0) {
> > perror("\n");
>
> You'd rather use:
>
> perror("i2c_smbus_write_i2c_block_data");
>
> so that error messages are clearer.
>
> > return 1;
> > }
> >
> > return 0;
> > }
> >
> >
> > static void help(const char *progname)
> > {
> > fprintf(stderr,
> > "Usage: %s I2CBUS CHIP-ADDRESS DATA0 [DATA1
> > ... DATAn]\n"
> > " I2CBUS is an integer or an I2C bus name\n"
> > " CHIP-ADDRESS is an integer (0x03 - 0x77)\n"
> > " DATAx is data to be written to the chip,
> > where 0 <= x <= n\n\n", progname);
> > exit(1);
> > }
> >
> > int main(int argc, char *argv[])
> > {
> > int fd, i2cbus, addr, ret = 0;
> > char filename[20];
> >
> > if ((argc < I2CRWL_PARAMS_SHIFT + 1) || (I2CRWL_MAX_PARAMS +
> > I2CRWL_PARAMS_SHIFT < argc))
> > help(argv[0]);
> >
> > i2cbus = lookup_i2c_bus(argv[1]);
> > if (i2cbus < 0)
> > help(argv[0]);
> >
> > addr = parse_i2c_address(argv[2]);
> > if (addr < 0)
> > help(argv[0]);
> >
> > fd = open_i2c_dev(i2cbus, filename, 0);
> > if (fd < 0)
> > exit(1);
> >
> > if (ioctl(fd, I2C_SLAVE, addr) < 0) {
> > ret = 1;
> > perror("");
>
> Same here, perror("ioctl(I2C_SLAVE)") or similar.
>
> > goto out;
> > }
> >
> >
> > if (i2c_writel(fd, argc - I2CRWL_PARAMS_SHIFT,
> > &argv[I2CRWL_PARAMS_SHIFT])) {
> > ret = 1;
> > goto out;
> > }
> >
> >
> > out:
> > close(fd);
> >
> > return ret;
> > }
> > ------------
> >
> > BTW, I've disabled the FEATURE_BLOCK_BUFFER
> >
> > --- i2c-i801.c 2010-02-24 10:50:50.060209638 +0200
> > +++ i2c-i801.c.orig 2010-02-24 13:55:29.664070673 +0200
> > @@ -603,7 +603,6 @@
> > /* fall through */
> > case PCI_DEVICE_ID_INTEL_82801DB_3:
> > i801_features |= FEATURE_SMBUS_PEC;
> > - i801_features |= FEATURE_BLOCK_BUFFER;
> > break;
> > }
> >
> > and now everything works smoothly. I2C write transaction of arbitrary
> > length are seen even by scope :)
> >
> > In case if I don't, here is what I get:
> >
> > $ dmesg | tail
> > Transaction timeout
> > Terminating the current operation
> > Failed terminating the transaction
> > Failed clearing status flags at end of transaction ...
>
> Obviously, if disabling the block buffer makes the same transaction
> work, then it has to be a bug in the driver. And the good news is: I
> was able to reproduce the bug using your test program, on an ICH5
> running kernel 2.6.27.45. On the same system, I can get SMBus block
> reads to work with or without the block buffer, so block buffer support
> is not entirely broken (if it was, we'd certainly have noticed earlier.)
>
> Now ideally we need to figure out whether SMBus block writes are
> affected as well. We already know that SMBus block reads are not, and
> I2C block writes are. As I2C block reads are excluded (the block buffer
> can not be used for them according to the datasheet, and the driver
> already does the right thing), checking whether SMBus block writes are
> affected will tell us whether all block writes are affected, or if I2C
> block writes only are affected. This should help us find out where and
> what the bug could be.
Ok, here is what I did.
I run the code with i2c_smbus_write_block_data with E32B enabled,
i.e. the original version of the driver and SMBus multi-byte write
transactions were successful (I also verified it against the scope
analyzer).
But what I run the code with i2c_smbus_write_i2c_block_data, here is what I got:
# ./i2c_wl 0 0x28 0x1 0xff 0xff
i2c_smbus_write_i2c_block_data: Operation not permitted
l# ./i2c_wl 0 0x28 0x1 0xff 0xff 0xff 0xff
Success ...
# dmesg | tail
[ 430.580131] i801_smbus 0000:00:1f.3: Transaction (post): CNT=14,
CMD=01, ADD=50, DAT0=02, DAT1=00
[ 430.580312] i2c-adapter i2c-0: ioctl, cmd=0x708, arg=0x01
[ 464.280155] i2c-adapter i2c-0: ioctl, cmd=0x703, arg=0x28
[ 464.280155] i2c-adapter i2c-0: ioctl, cmd=0x708, arg=0x00
[ 464.280155] i2c-adapter i2c-0: ioctl, cmd=0x720, arg=0xbfdaa63c
[ 464.280155] i801_smbus 0000:00:1f.3: Transaction (pre): CNT=14,
CMD=01, ADD=50, DAT0=04, DAT1=00
[ 464.280155] i801_smbus 0000:00:1f.3: SMBus busy (14). Resetting...
[ 464.280155] i801_smbus 0000:00:1f.3: Successful!
[ 464.287033] i801_smbus 0000:00:1f.3: Transaction (post): CNT=14,
CMD=01, ADD=50, DAT0=04, DAT1=00
[ 464.287080] i2c-adapter i2c-0: ioctl, cmd=0x708, arg=0x01
In addition, the scope analyzer showed me that (when I run this
command ./i2c_wl 0 0x28 0x1 0xff 0xff) the last byte 0xff wasn't sent
on the bus. In contrary, every byte was on the bus with
i2c_smbus_write_block_data.
Thanks,
Felix R.
>
> --
> Jean Delvare
> http://khali.linux-fr.org/wishlist.html
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
