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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.