Re: [PATCH] iio: adc: pac1921: fix wrong channel used in trigger handler read

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 2 Aug 2026 10:48:26 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 8/2/26 2:12 AM, Cong Nguyen wrote:
> > pac1921_trigger_handler() walks the enabled channels with
> > iio_for_each_active_channel(), which yields the scan index (bit) of each
> > active channel, while ch is a separate counter used to pack the samples
> > contiguously into the scan buffer.
> > 
> > The register to read was looked up with the packing counter instead of
> > the scan index:
> > 
> > 	ret = pac1921_read_res(priv, idev->channels[ch].address, &val);
> > 
> > pac1921_channels[] is ordered by scan index, so channels[bit] is the
> > channel that is actually enabled, whereas channels[ch] is merely the
> > ch-th array entry. These coincide only when the enabled channels form a
> > contiguous prefix (e.g. all channels enabled). With a sparse scan mask -
> > for example when only the power channel (scan index 3) is enabled - the
> > handler reads the wrong register (VBUS instead of VPOWER) and pushes it
> > to userspace as the enabled channel's data.
> > 
> > Index the channel array by the scan index (bit) to read the correct
> > register, keeping ch only for contiguous packing into the scan buffer.
> > 
> > Fixes: 371f778b83cd ("iio: adc: add support for pac1921")
> > Cc: stable@vger.kernel.org
> > Assisted-by: Claude:claude-opus-4
> > Signed-off-by: Cong Nguyen <congnt264@gmail.com>
> > ---
> >  drivers/iio/adc/pac1921.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/adc/pac1921.c b/drivers/iio/adc/pac1921.c
> > index bce7185953ec..0037509503ed 100644
> > --- a/drivers/iio/adc/pac1921.c
> > +++ b/drivers/iio/adc/pac1921.c
> > @@ -1037,7 +1037,7 @@ static irqreturn_t pac1921_trigger_handler(int irq, void *p)
> >  	iio_for_each_active_channel(idev, bit) {
> >  		u16 val;
> >  
> > -		ret = pac1921_read_res(priv, idev->channels[ch].address, &val);
> > +		ret = pac1921_read_res(priv, idev->channels[bit].address, &val);
> >  		if (ret)
> >  			goto done;
> >    
> 
> Looks correct.
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>

+CC Ariana who is dealing with similar parts and might be able to sanity
check this.

FWIW looks correct to me too.

Jonathan

>