[lkml]   [2006]   [Jun]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    SubjectRe: [PATCH] I2C block read
    Hi Alexander,

    Adding Jordan Crouse in the loop, he's working on the scx200_acb driver
    these days.

    > I use SMBus block read with scx200_acb as a bus it uses the lenght
    > which is as i say uninitialized and tries to read a number of random bytes
    > from the board i use. And it doesn't return the lenght as a first byte,
    > so the drivers reads until it gets an oops. Which in turn makes me wonder
    > why with this patch i correcly receive the number of bytes i pass,
    > looking at the driver i can not see that it gets the lenght from the read data.
    > I don't know well SMBus/I2C specs but read with buffer and no lenght
    > doesn't look sane to me. So how should this be fixed?

    There _is_ a length, it's just decided by the target chip rather than
    the host. It makes some sense when you think about it, even if it is a
    bit susprising at first, I agree. The length is limited to 32 bytes as
    per the SMBus specification, so it's safe. I invite you to read the
    SMBus 2.0 specification [1] for more details if you're interested.

    As for the problem you encounter, it looks like a bug in the scx200_acb
    driver to me. It advertises the I2C_FUNC_SMBUS_BLOCK_DATA
    functionality, but due to its implementation it can only support SMBus
    block writes and not SMBus block reads. I see no reason why the chip
    itself couldn't do it, but right now the driver can't. The state
    machine the driver is based on would need some rework before the
    functionality can be added. In the meantime, the quickest fix is to
    have the driver only advertise the functionalities it actually supports:

    The scx200_acb advertises SMBus block transactions as supported. In
    fact, it supports SMBus block writes, but not SMBus block reads which
    are more complex to implement.

    Signed-off-by: Jean Delvare <>
    drivers/i2c/busses/scx200_acb.c | 8 +++++++-
    1 file changed, 7 insertions(+), 1 deletion(-)
    --- linux-2.6.17-rc6.orig/drivers/i2c/busses/scx200_acb.c 2006-06-07 18:13:53.000000000 +0200
    +++ linux-2.6.17-rc6/drivers/i2c/busses/scx200_acb.c 2006-06-07 20:29:27.000000000 +0200
    @@ -308,6 +308,12 @@

    + /* Sanity check */
    + if (rw == I2C_SMBUS_READ) {
    + dev_warn(&adapter->dev, "SMBus block read is not "
    + "supported!\n");
    + return -EINVAL;
    + }
    len = data->block[0];
    buffer = &data->block[1];
    @@ -372,7 +378,7 @@

    /* For now, we only handle combined mode (smbus) */

    May I ask for what slave SMBus chip you need the SMBus block read
    transaction? It's only rarely used, so it could be that it's not what
    you need after all.

    The scx200_acb code would probably benefit from a general review. It
    looks to me like quick command with data bit == 1 would fail, for
    example. It's not a very popular transaction either, but i2c bus
    drivers should handle all transactions they advertise as supported.


    Jean Delvare
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2006-06-07 20:53    [from the cache]
    ©2003-2014 Jasper Spaans. hosted at Digital Ocean