linux-kernel - Re: [PATCH v3 4/4] iio: adc: ad9467: check for backend capabilities

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260114124527.00000344@huawei.com>
Date: Wed, 14 Jan 2026 12:45:27 +0000
From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: Tomas Melin <tomas.melin@...sala.com>
CC: Michael Hennerich <Michael.Hennerich@...log.com>, Nuno Sa
	<nuno.sa@...log.com>, Lars-Peter Clausen <lars@...afoo.de>, "Jonathan
 Cameron" <jic23@...nel.org>, David Lechner <dlechner@...libre.com>, "Andy
 Shevchenko" <andy@...nel.org>, Olivier Moysan <olivier.moysan@...s.st.com>,
	<linux-iio@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 4/4] iio: adc: ad9467: check for backend capabilities

On Wed, 14 Jan 2026 10:45:53 +0000
Tomas Melin <tomas.melin@...sala.com> wrote:

> Add capability checks for operation with backends that do not support
> full set of features, but are otherwise compatible with the device.
> 
> Signed-off-by: Tomas Melin <tomas.melin@...sala.com>
Hi Tomas,

A few comments inline around when we should or should not return errors.

> ---
>  drivers/iio/adc/ad9467.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
> index 9cfe66425d4e91e215cccc40e24a92c5e99e9b87..349779a049ad68b4c9f72abfc40154b4a3f2e095 100644
> --- a/drivers/iio/adc/ad9467.c
> +++ b/drivers/iio/adc/ad9467.c
> @@ -645,6 +645,9 @@ static int ad9467_backend_testmode_on(struct ad9467_state *st,
>  	};
>  	int ret;
>  
> +	if (!iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION))
> +		return 0;

If it isn't supported then isn't any attempt to turn test_mode on an error?
I would return errors from all these calls and only decide it doesn't
matter right at the top of the call stack.

In this particular case that's either calibration mode stuff (covered by check
below) or ad9467_chan_test_mod_write()  I'd just add a top level check there
on ability to do calibration. Alternative would be to not register the debugfs
at all.  Challenge is that is a small ABI break.  Do we care?  If we had
been doing this from the start we would have just hidden the pointless
interface.
(Key is we can change userspace interfaces, as long no one notices!)

> +
>  	ret = iio_backend_data_format_set(st->back, chan, &data);
>  	if (ret)
>  		return ret;
> @@ -665,6 +668,9 @@ static int ad9467_backend_testmode_off(struct ad9467_state *st,
 	};
>  	int ret;
>  
> +	if (!iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION))
> +		return 0;
> +
>  	ret = iio_backend_chan_disable(st->back, chan);
>  	if (ret)
>  		return ret;
> @@ -807,6 +813,9 @@ static int ad9467_calibrate(struct ad9467_state *st)
>  	bool invert = false, stat;
>  	int ret;
>  
> +	if (!iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION))
> +		return 0;

So this the idea of a stub calibration bothers me a little, but I think
it's more acceptable to do it for this top level call. So keep this one, but
all the others look to me like they should be returning errors.

> +
>  	/* all points invalid */
>  	bitmap_fill(st->calib_map, st->calib_map_size);
>  
> @@ -1357,7 +1366,7 @@ static int ad9467_probe(struct spi_device *spi)
>  		return ret;
>  
>  	ret = devm_iio_backend_request_buffer(&spi->dev, st->back, indio_dev);
> -	if (ret)
> +	if (ret && ret != -EOPNOTSUPP)

I don't see any modification of devm_iio_backend_request_buffer() in this
set so why why is it now acceptable to return not supported?
Given the capabilities stuff added, should this not be another capability
we check before trying to request the buffer?

>  		return ret;
>  
>  	ret = devm_iio_backend_enable(&spi->dev, st->back);
>