Re: [RESEND PATCH 00/22] IIO: Clean out superfluous I2C checks for single transfers
Andy Shevchenko <[email protected]>
| Newsgroups | org.kernel.vger.linux-i2c,org.kernel.vger.linux-iio |
|---|---|
| Organization | Intel Finland Oy - BIC 0357606-4 - c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo |
| Message-ID | <[email protected]> |
On Thu, Jul 16, 2026 at 04:20:53PM -0700, Jonathan Cameron wrote: > On Thu, 16 Jul 2026 07:55:32 -0700 > Lars-Peter Clausen <[email protected]> wrote: > > > On 7/15/26 5:57 PM, Jonathan Cameron wrote: > > > Resend to include linux-iio. Sorry to all who get it twice. > > > > > > This came up in a recent review and I realised I have been giving incorrect > > > review feedback for years. Both i2_master_send() and i2c_master_recv() plus > > > the dma safe variants only ever return either an error or the full buffer > > > size. Short accesses do not occur. > > There is actually one interesting corner case. If > > i2c_transfer_buffer_flags() returns 0 i2c_master_{send,recv}() will also > > return 0, in which case the updated drivers will now read uninitialized > > memory. Maybe i2c_master_{send,recv}() needs to be updated to translate > > that to -EIO. > > That's indeed curious (and I'd missed it entirely!). > > So ends up being either the master_xfer_atomic() or master_xfer() callbacks > returning 0. master_xfer_atomic() is novel, it's a union with xfer_atomic() > and no one ever sets it via that union element but given the types are the same > all calls will actually be to xfer_atomic(). > > Could remove that complexity whilst here but there a bunch of related callbacks > (e.g. master_xfer() in union with xfer() where drivers are still using the old > naming and I'm not sure I want to chase that through various subsystem trees > right now (though for fun - and maybe profit) I had claude work out a series. > Was fun watching it write a semantic patch and spend a while debugging it. > Glad it has as much trouble with coccinelle scripts as the rest of us. > There are still a few hundred instances and I haven't touched all the callbacks, > just this pair. > > Anyhow, back to the question of whether they do return 0. Yup they do :( > > https://elixir.bootlin.com/linux/v7.1.3/source/drivers/i2c/busses/i2c-cadence.c#L1153 > > So any defense against that case would require hardening further up the stack. > > +CC Andi and the i2c list. Would anyone mind a patch to do something like: I am fine with the approach but return value conversion needs a good comprehensive comment on top. > static inline int i2c_master_recv(const struct i2c_client *client, > char *buf, int count) > { > - return i2c_transfer_buffer_flags(client, buf, count, I2C_M_RD); > + int ret; > + > + ret = i2c_transfer_buffer_flags(client, buf, count, I2C_M_RD); > + if (ret == 0) > + return -EIO; > + > + return ret; > }; > > Just to avoid the need for special case checks on ever call from every driver > so that they return ret if negative or else have to check for non 0 return > value and return -EIO or similar. Maybe somewhere out there is someone using > that 0 to mean something magic, but I doubt it. > > > This series (also my first upstream series prepared by poking claude) > > > removes the redundant checks from all IIO drivers. -- With Best Regards, Andy Shevchenko