> On Jul 28, 2020, at 2:40 AM, Wolfram Sang <wsa@xxxxxxxxxx> wrote: > > Hi Daniel, > > wow, that was fast! Thanks for the prototype. > >> * I suggest to just settle on '3' for new macro and type names >> (I2C_SMBUS3_*, i2c_smbus3_*) > > Yes, I agree. > >> >> * Block size definitions maintain I2C_SMBUS_BLOCK_MAX (32). Only adds >> I2C_SMBUS3_BLOCK_MAX (255) >> >> - Means that drivers in drivers/i2c/busses/ default to their safe >> 32B block limit without refactoring. > > This is totally fine for this patch. However, I still think I will do > the renaming to I2C_SMBUS2_BLOCK_MAX in kernel space later. Just so > people will understand by looking at the code that this is an old limit > which can be removed if there is interest. > >> - __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? Yep, exactly. It just made for a nice drop-in replacement for me to focus on i2c-dev.c first. A lot of clients will stack-allocate these. I suppose i2-tools doesn’t load more than one at a time, but haven’t looked. Retrospectively - i2c_smbus_ioctl_data.data shouldn’t have been a pointer type, but is. - i2c_smbus_data.block should have been pointer, but isn’t And then the kernel would pass around an 32-bit-only i2c_smbus_data union -- by value. Which would have been much leaner, and leave the right buffer choice entirely to the client. One could explore this in kernel space. Let me know how you’d like to experiment. But in userspace that means we’re looking at a new call number. >:) Cheers, Daniel
