Hi Wolfram, Daniel, On Tue, 28 Jul 2020 11:40:37 +0200, Wolfram Sang wrote: > > - __u8 block[I2C_SMBUS_BLOCK_MAX + 2]; /* block[0] is used for length */ > > + __u8 block[I2C_SMBUS3_BLOCK_MAX + 2]; /* block[0] is used for length */ > > /* and one more for user-space compatibility */ > > I thought about this, too, and wondered if this isn't a size regression > in userspace with every i2c_smbus_data getting 8 times the size? But > maybe it is worth if backwards compatibility is maintained in an > otherwise not so intrusive manner? Jean, what do you think? In i2c-tools these are always allocated on the stack and one at a time. Size isn't an issue in my opinion. The only thing I'm truly concerned about here is compatibility. You need to ensure that: * User-space binaries that are around today (obviously compiled with the old versions of the kernel uapi headers) will work the same with kernels including your changes. * User-space binaries compiled with the new kernel uapi headers will work with both old and new kernels. For all transfer types. You will have to keep in mind that SMBus-style and I2C-style read block transfers differ in who decides the length. For I2C, it is the caller (I want to read N bytes from this location), for SMBus it is the slave (I want to read a block from this location, tell me how long it is). In both cases you need to ensure you do not write beyond the buffer, no matter what, and return a proper error code if it wouldn't fit. The other compatibility type you need to care about is inside the kernel itself: SMBus 2 and SMBus 3 controllers and devices may be mixed up. Some (I expect most) SMBus 3 devices may be able to work with SMBus 2 controllers in "degraded" mode, in which case it will be the driver's responsibility to check the capabilities of the controller and only use transfer types that are supported. If such "degraded" mode is not possible then the driver would simply refuse to bind. For such checks to be possible, I would expect either one global I2C_FUNC_SMBUS* flag to be added to denote SMBus 3 compliance, or possibly several flags for finer grained control (seems overkill to me but you may have a different opinion - also depends on what else SMBus 3 is introducing, I admit I did not check). However I did not see that in your prototype. Do you believe we can do without it? If so, please explain how. -- Jean Delvare SUSE L3 Support
