Re: [PATCH RFT] iio: Fix core buffer demux failure to account for unwanted channels at tail

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Sun, 3 Aug 2025 21:46:50 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sat, Aug 2, 2025 at 7:15 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > The IIO channel demultiplexer code is there to deal with a mismatch between
> > the channels captured and those requested by user space either due to
> > driver supporting only particular channel combinations
> > (available_scan_masks) or due to multiple concurrent consumers (e.g.
> > userspace IIO buffered interfaces and an inkernel consumer such as a
> > touch screen).
> >
> > Whilst this code is exercised by many drivers, a corner case has been
> > hiding there all along.
> >
> > Consider an input of (postfix is the channel size)  
> 
> size in bits
> 
> > a_32, b_32, c_32, d_32, ts_64
> >
> > and desired output of
> >
> > a32, b_32, ts_64  
> 
> You missed underscore here and below.
> 
> > the current code ends up with
> >
> > a32, b_32, c_32, d_32
> >
> > because of a failure to iterate over the tail of unwanted channels
> > (here c_32 and d_32).
> >
> > Fix this by adding the code to walk the channels in the gap.
> >
> > Reported-by: Jianping Shen <Jianping.Shen@de.bosch.com>
> > Closes: https://lore.kernel.org/all/AM8PR10MB4721FB1A78F25B204BE3A26ACD5FA@AM8PR10MB4721.EURPRD10.PROD.OUTLOOK.COM/
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>  
> 
> Fixes?
> 
> ...
> 
> > Whilst this is being tested I'll try to figure out a Fixes tag.
> > There is some code movement so needs more digging that I have time for
> > today.  
> 
> Ah, okay.
> 
> 
> > +       /* Walk remaining bits of active_scan_mask */
> > +       in_ind = find_next_bit(indio_dev->active_scan_mask, masklength,
> > +                              in_ind + 1);
> > +       while (in_ind != masklength) {  
> 
> for_each_set_bit_from() ?

Would work but then takes the style away from the code above that this
effectively duplicates.  I'd need to have a closer look to see if we
can potentially convert that as well.  I don't want two very different
looking bits of code effectively doing the same thing under subtly
different constraints.

Jonathan

> 
> > +               ret = iio_storage_bytes_for_si(indio_dev, in_ind);
> > +               if (ret < 0)
> > +                       goto error_clear_mux_table;
> > +
> > +               length = ret;
> > +               /* Make sure we are aligned */
> > +               in_loc = roundup(in_loc, length) + length;
> > +               in_ind = find_next_bit(indio_dev->active_scan_mask,
> > +                                      masklength, in_ind + 1);
> > +       }  
> 
>