diff mbox series

iio: adc: sun20i-gpadc: support non-contiguous channel lookups

Message ID 20260513-fix-sunxi-gpadc-sparse-channels-v1-1-6c21e290bcee@mmpsystems.pl (mailing list archive)
State New
Headers
Series iio: adc: sun20i-gpadc: support non-contiguous channel lookups |

Commit Message

Michal Piekos May 13, 2026, 9:51 a.m. UTC 
Using consumer driver like iio-hwmon which resolve channels thorugh
io-channels phandles will fail for sparse channels because IIO core
threats phandle argument as index into channel array.
        eg. <&gpadc 1> will fail if there is only channel@1 specified

Add fwnode_xlate which maps DT phandle to the registered channel whose
chan->channel matches the hardware channel number. It allows sparse
channel maps to be consumed by drivers like iio-hwmon.

Tested on Radxa Cubie A5E.

Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
---
 drivers/iio/adc/sun20i-gpadc-iio.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)


---
base-commit: 1d5dcaa3bd65f2e8c9baa14a393d3a2dc5db7524
change-id: 20260513-fix-sunxi-gpadc-sparse-channels-2b6b2063bd49

Best regards,
--  
Michal Piekos <michal.piekos@mmpsystems.pl>

Comments

Andy Shevchenko May 13, 2026, 10:53 a.m. UTC | #1 
On Wed, May 13, 2026 at 11:51:31AM +0200, Michal Piekos wrote:
> Using consumer driver like iio-hwmon which resolve channels thorugh
> io-channels phandles will fail for sparse channels because IIO core
> threats phandle argument as index into channel array.
>         eg. <&gpadc 1> will fail if there is only channel@1 specified
> 
> Add fwnode_xlate which maps DT phandle to the registered channel whose

.fwnode_xlate()

> chan->channel matches the hardware channel number. It allows sparse
> channel maps to be consumed by drivers like iio-hwmon.

...

> +static int sun20i_gpadc_fwnode_xlate(struct iio_dev *indio_dev,
> +		const struct fwnode_reference_args *iiospec)

Broken indentation (I understand the motivation to shift left, but I leave it
to Jonathan on how to proceed with this).

> +{
> +	int i;
> +
> +	for (i = 0; i < indio_dev->num_channels; i++)

	for (unsigned int i = 0; i < indio_dev->num_channels; i++)

> +		if (indio_dev->channels[i].channel == iiospec->args[0])
> +			return i;
> +
> +	return -EINVAL;
> +}
Jonathan Cameron May 13, 2026, 2:10 p.m. UTC | #2 
On Wed, 13 May 2026 11:51:31 +0200
Michal Piekos <michal.piekos@mmpsystems.pl> wrote:

> Using consumer driver like iio-hwmon which resolve channels thorugh
> io-channels phandles will fail for sparse channels because IIO core

Can we say the "IIO core by default" does this. Otherwise implication
is we don't provide an alternative - whereas the fwnode_xlate() callback
is there to cover this sort of thing.  Otherwise this sounds like we should
be modifying the IIO core rather than the driver.

Otherwise nothing from me to add to what Andy's already covered.

> threats phandle argument as index into channel array.
>         eg. <&gpadc 1> will fail if there is only channel@1 specified
> 
> Add fwnode_xlate which maps DT phandle to the registered channel whose
> chan->channel matches the hardware channel number. It allows sparse
> channel maps to be consumed by drivers like iio-hwmon.
> 
> Tested on Radxa Cubie A5E.
> 
> Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
> ---
>  drivers/iio/adc/sun20i-gpadc-iio.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/iio/adc/sun20i-gpadc-iio.c b/drivers/iio/adc/sun20i-gpadc-iio.c
> index 861c14da75ad..95f34a352158 100644
> --- a/drivers/iio/adc/sun20i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun20i-gpadc-iio.c
> @@ -139,8 +139,21 @@ static irqreturn_t sun20i_gpadc_irq_handler(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> +static int sun20i_gpadc_fwnode_xlate(struct iio_dev *indio_dev,
> +		const struct fwnode_reference_args *iiospec)
> +{
> +	int i;
> +
> +	for (i = 0; i < indio_dev->num_channels; i++)
> +		if (indio_dev->channels[i].channel == iiospec->args[0])
> +			return i;
> +
> +	return -EINVAL;
> +}
> +
>  static const struct iio_info sun20i_gpadc_iio_info = {
>  	.read_raw = sun20i_gpadc_read_raw,
> +	.fwnode_xlate = sun20i_gpadc_fwnode_xlate,
>  };
>  
>  static void sun20i_gpadc_reset_assert(void *data)
> 
> ---
> base-commit: 1d5dcaa3bd65f2e8c9baa14a393d3a2dc5db7524
> change-id: 20260513-fix-sunxi-gpadc-sparse-channels-2b6b2063bd49
> 
> Best regards,
> --  
> Michal Piekos <michal.piekos@mmpsystems.pl>
>
sashiko-bot@kernel.org May 14, 2026, 3:54 a.m. UTC | #3 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing bounds check on `iiospec->nargs` leads to uninitialized stack memory read
--

commit b20d529d5a2f9f0071b7ae62b25da6250c765297
Author: Michal Piekos <michal.piekos@mmpsystems.pl>

iio: adc: sun20i-gpadc: support non-contiguous channel lookups

This commit adds a fwnode_xlate callback to map a device tree phandle
argument to the registered channel. This allows the driver to support
non-contiguous channel lookups for consumers like iio-hwmon.

> diff --git a/drivers/iio/adc/sun20i-gpadc-iio.c b/drivers/iio/adc/sun20i-gpadc-iio.c
> index 861c14da75add..95f34a3521583 100644
> --- a/drivers/iio/adc/sun20i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun20i-gpadc-iio.c
[ ... ]
> +static int sun20i_gpadc_fwnode_xlate(struct iio_dev *indio_dev,
> +		const struct fwnode_reference_args *iiospec)
> +{
> +	int i;
> +
> +	for (i = 0; i < indio_dev->num_channels; i++)
> +		if (indio_dev->channels[i].channel == iiospec->args[0])

Could this read uninitialized stack memory if the consumer requests a channel
with 0 arguments?

In the IIO core, __fwnode_iio_channel_get() allocates the iiospec structure
on the stack without zero-initializing it. While the Device Tree backend
zero-initializes unused arguments, other backends like Software Nodes
might not.

If iiospec->nargs is 0, this comparison could use a garbage value from the
stack, leading to unpredictable channel selection or probe failure.

Should we verify if (!iiospec->nargs) before accessing iiospec->args[0]?

> +			return i;
> +
> +	return -EINVAL;
> +}
diff mbox series

Patch

diff --git a/drivers/iio/adc/sun20i-gpadc-iio.c b/drivers/iio/adc/sun20i-gpadc-iio.c
index 861c14da75ad..95f34a352158 100644
--- a/drivers/iio/adc/sun20i-gpadc-iio.c
+++ b/drivers/iio/adc/sun20i-gpadc-iio.c
@@ -139,8 +139,21 @@  static irqreturn_t sun20i_gpadc_irq_handler(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static int sun20i_gpadc_fwnode_xlate(struct iio_dev *indio_dev,
+		const struct fwnode_reference_args *iiospec)
+{
+	int i;
+
+	for (i = 0; i < indio_dev->num_channels; i++)
+		if (indio_dev->channels[i].channel == iiospec->args[0])
+			return i;
+
+	return -EINVAL;
+}
+
 static const struct iio_info sun20i_gpadc_iio_info = {
 	.read_raw = sun20i_gpadc_read_raw,
+	.fwnode_xlate = sun20i_gpadc_fwnode_xlate,
 };
 
 static void sun20i_gpadc_reset_assert(void *data)