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

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

[Cong Nguyen]

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;
 
-- 
2.25.1

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

[David Lechner]

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>

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

[Jonathan Cameron]

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

> 

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

[Matteo Martelli]

On Sun,  2 Aug 2026 14:12:46 +0700, Cong Nguyen <congnt264@gmail.com> 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;
>  
> -- 
> 2.25.1
> 

This looks correct to me. I guess I didn't test it properly with a subset of
enabled channels when I wrote this.
Thanks for the fix!

Acked-by: Matteo Martelli <matteomartelli3@gmail.com>