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

Cong Nguyen posted 1 patch 2 months ago
drivers/iio/adc/pac1921.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] iio: adc: pac1921: fix wrong channel used in trigger handler read
Posted by Cong Nguyen 2 months ago
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
Posted by Matteo Martelli 1 month, 4 weeks ago
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>
Re: [PATCH] iio: adc: pac1921: fix wrong channel used in trigger handler read
Posted by Jonathan Cameron 1 month, 4 weeks ago
On Tue, 04 Aug 2026 08:49:25 +0200
Matteo Martelli <matteomartelli3@gmail.com> wrote:

> 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>
Applied to the fixes-togreg branch of iio.git

Thanks,

Jonathan
Re: [PATCH] iio: adc: pac1921: fix wrong channel used in trigger handler read
Posted by David Lechner 2 months ago
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
Posted by Jonathan Cameron 2 months ago
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

>