Re: i2c block reads > 32 bytes

[Wolfram Sang <wsa@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3150 bytes --]

Hi Daniel,

> Publicly available PMBus revisions appear to be based on SMBus 2.0,
> but with relaxed constraints regarding block read/write length: 255
> bytes, not 32. [1]

I missed that detail for PMBus...

> Contrasting I2C_SMBUS_BLOCK_MAX=32, obviously. Similarly, Smbus 3.1 appears to have removed the 32-byte limit 
> for block read, block write, and block-write block-read process call. [2]

... but I am aware of this change since SMBus 3.0 specs came out. As you
already discovered, you are the first one to ask for support for it.

> There are workarounds in place, but I don’t find them very attractive,
> compared to a more supportive I2C_RDWR ioctl. I’m fully aware that
> I2C_SMBUS_BLOCK_MAX=32 is basically set in stone. But I could imagine
> I2C_RDWR growing to support newer Smbus protocols. My question would
> be whether this has been considered already.

I neither had hardware nor time for actually hacking on an
implementation. However, I thought about how one could do it and I don't
think the old limit is "set in stone". My idea for kernel space:

#define I2C_SMBUS20_BLOCK_MAX 32
/* Next one only for a transition period */
#define I2C_SMBUS_BLOCK_MAX I2C_SMBUS20_BLOCK_MAX

#define I2C_SMBUS30_BLOCK_MAX 255
#define I2C_SMBUS_LATEST_BLOCK_MAX I2C_SMBUS30_BLOCK_MAX

And with that:

1) Convert I2C_SMBUS_BLOCK_MAX to I2C_SMBUS20_BLOCK_MAX for all current
   users in-tree to avoid regressions

2) Update the I2C core to handle I2C_SMBUS_LATEST_BLOCK_MAX

3) People can convert client drivers to I2C_SMBUS30_BLOCK_MAX
   individually. So we ensure it has been properly tested.

I haven't fully dived into it but I'd think something similar can be
done for userspace, too.

> Recap: the problem with the current i2c-core is that i2cdev_ioctl_rdwr
> is passing msg[i].len in a way which makes it impossible for adapters
> to execute block reads greater 32: kernel msg[i].len isn’t user
> msg[i].len, but set to the number of extra bytes initially, so the
> adapter driver is left with assurance that 32 bytes buffer space
> available, not how much, if more. I suppose this is intentional.

I'd think we can extend that guarantee to 255 bytes when changing to the
new max. Needs definately some more discussion with Jean, though. But
also, if everything is correctly updated, you should be able to use the
native i2c_smbus_* functions again. No need for rdwr detour.

> Also, I suspect I’m not tellying anyone in this forum anything new.
> Bear with me, I’ve made an attempt to find older discussions. But
> didn’t see anything later than the exchange leading to the current
> handling of I2C_M_RECV_LEN.

As said above, there was none. However, your timing is not bad. I am
currently devleoping a "testunit" driver for IP cores which can also be
an I2C client themselves. It is already on the todo-list to add test
cases for I2C_M_RECV_LEN there, too, so it can be tested independently
of specific hardware. Of course, it is very good if there are people to
validate the results with devices actually needing that!

Kind regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]