Add capability checks for operation with backends that do not necessarily
support full set of features, but are otherwise compatible with the device.
This ensures a fully functional device, but with limited capabilities.
Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>
---
drivers/iio/adc/ad9467.c | 67 +++++++++++++++++++++++++++++-------------------
1 file changed, 41 insertions(+), 26 deletions(-)
diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
index 59c3fa3bcc9b..7a3db36121b0 100644
--- a/drivers/iio/adc/ad9467.c
+++ b/drivers/iio/adc/ad9467.c
@@ -913,7 +913,9 @@ static int __ad9467_update_clock(struct ad9467_state *st, long r_clk)
return ret;
guard(mutex)(&st->lock);
- return ad9467_calibrate(st);
+ if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION))
+ return ad9467_calibrate(st);
+ return 0;
}
static int ad9467_write_raw(struct iio_dev *indio_dev,
@@ -1119,12 +1121,15 @@ static ssize_t ad9467_chan_test_mode_read(struct file *file,
len = scnprintf(buf, sizeof(buf), "Running \"%s\" Test:\n\t",
ad9467_test_modes[chan->mode]);
- ret = iio_backend_debugfs_print_chan_status(st->back, chan->idx,
- buf + len,
- sizeof(buf) - len);
- if (ret < 0)
- return ret;
- len += ret;
+ if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION)) {
+ ret = iio_backend_debugfs_print_chan_status(st->back,
+ chan->idx,
+ buf + len,
+ sizeof(buf) - len);
+ if (ret < 0)
+ return ret;
+ len += ret;
+ }
} else if (chan->mode == AN877_ADC_TESTMODE_OFF) {
len = scnprintf(buf, sizeof(buf), "No test Running...\n");
} else {
@@ -1188,16 +1193,18 @@ static ssize_t ad9467_chan_test_mode_write(struct file *file,
return ret;
/* some patterns have a backend matching monitoring block */
- if (mode == AN877_ADC_TESTMODE_PN9_SEQ) {
- ret = ad9467_backend_testmode_on(st, chan->idx,
+ if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION)) {
+ if (mode == AN877_ADC_TESTMODE_PN9_SEQ) {
+ ret = ad9467_backend_testmode_on(st, chan->idx,
IIO_BACKEND_ADI_PRBS_9A);
- if (ret)
- return ret;
- } else if (mode == AN877_ADC_TESTMODE_PN23_SEQ) {
- ret = ad9467_backend_testmode_on(st, chan->idx,
+ if (ret)
+ return ret;
+ } else if (mode == AN877_ADC_TESTMODE_PN23_SEQ) {
+ ret = ad9467_backend_testmode_on(st, chan->idx,
IIO_BACKEND_ADI_PRBS_23A);
- if (ret)
- return ret;
+ if (ret)
+ return ret;
+ }
}
}
@@ -1263,8 +1270,9 @@ static void ad9467_debugfs_init(struct iio_dev *indio_dev)
if (!st->chan_test)
return;
- debugfs_create_file("calibration_table_dump", 0400, d, st,
- &ad9467_calib_table_fops);
+ if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION))
+ debugfs_create_file("calibration_table_dump", 0400, d, st,
+ &ad9467_calib_table_fops);
for (chan = 0; chan < st->info->num_channels; chan++) {
snprintf(attr_name, sizeof(attr_name), "in_voltage%u_test_mode",
@@ -1339,17 +1347,24 @@ static int ad9467_probe(struct spi_device *spi)
if (ret)
return ret;
- ret = devm_iio_backend_request_buffer(&spi->dev, st->back, indio_dev);
- if (ret)
- return ret;
+ if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_BUFFER)) {
+ ret = devm_iio_backend_request_buffer(&spi->dev, st->back,
+ indio_dev);
+ if (ret)
+ return ret;
+ }
- ret = devm_iio_backend_enable(&spi->dev, st->back);
- if (ret)
- return ret;
+ if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_ENABLE)) {
+ ret = devm_iio_backend_enable(&spi->dev, st->back);
+ if (ret)
+ return ret;
+ }
- ret = ad9467_calibrate(st);
- if (ret)
- return ret;
+ if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION)) {
+ ret = ad9467_calibrate(st);
+ if (ret)
+ return ret;
+ }
ret = devm_iio_device_register(&spi->dev, indio_dev);
if (ret)
--
2.47.3On Thu, 05 Feb 2026 12:24:11 +0000
Tomas Melin <tomas.melin@vaisala.com> wrote:
> Add capability checks for operation with backends that do not necessarily
> support full set of features, but are otherwise compatible with the device.
> This ensures a fully functional device, but with limited capabilities.
>
> Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>
I've mentioned before that I've started messing around with Chris Mason's
set of Claude Code prompts and today I was trying out the new stuff mentioned in:
https://lore.kernel.org/all/b187e0c1-1df8-4529-bfe4-0a1d65221adc@meta.com/
Given this patch set is pretty much ready to merge I asked it to take a look
and it came up with something that certainly merited me taking a look.
Issue: In ad9467_chan_test_mode_write(), the testmode_on path is guarded
by IIO_BACKEND_CAP_CALIBRATION but the matching testmode_off path is not.
On backends without that capability, entering PN test mode succeeds but
exiting fails with -EOPNOTSUPP, leaving the ADC stuck in test mode.
I may be missing some protection path, but I think it is right and if you
were to set up the test modes with a backend that doesn't support the
relevant functionality you would indeed be able to turn them on but never
off again.
So this might be the first bug in IIO that the LLM found and human's, including
me, missed!
Thanks Chris! I won't +CC you every time but wanted to acknowledge that your
hard work taming these tools is very useful to me.
Jonathan
> ---
> drivers/iio/adc/ad9467.c | 67 +++++++++++++++++++++++++++++-------------------
> 1 file changed, 41 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
> index 59c3fa3bcc9b..7a3db36121b0 100644
> --- a/drivers/iio/adc/ad9467.c
> +++ b/drivers/iio/adc/ad9467.c
> @@ -913,7 +913,9 @@ static int __ad9467_update_clock(struct ad9467_state *st, long r_clk)
> return ret;
>
> guard(mutex)(&st->lock);
> - return ad9467_calibrate(st);
> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION))
> + return ad9467_calibrate(st);
> + return 0;
> }
>
> static int ad9467_write_raw(struct iio_dev *indio_dev,
> @@ -1119,12 +1121,15 @@ static ssize_t ad9467_chan_test_mode_read(struct file *file,
> len = scnprintf(buf, sizeof(buf), "Running \"%s\" Test:\n\t",
> ad9467_test_modes[chan->mode]);
>
> - ret = iio_backend_debugfs_print_chan_status(st->back, chan->idx,
> - buf + len,
> - sizeof(buf) - len);
> - if (ret < 0)
> - return ret;
> - len += ret;
> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION)) {
> + ret = iio_backend_debugfs_print_chan_status(st->back,
> + chan->idx,
> + buf + len,
> + sizeof(buf) - len);
> + if (ret < 0)
> + return ret;
> + len += ret;
> + }
> } else if (chan->mode == AN877_ADC_TESTMODE_OFF) {
> len = scnprintf(buf, sizeof(buf), "No test Running...\n");
> } else {
> @@ -1188,16 +1193,18 @@ static ssize_t ad9467_chan_test_mode_write(struct file *file,
> return ret;
>
> /* some patterns have a backend matching monitoring block */
> - if (mode == AN877_ADC_TESTMODE_PN9_SEQ) {
> - ret = ad9467_backend_testmode_on(st, chan->idx,
> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION)) {
> + if (mode == AN877_ADC_TESTMODE_PN9_SEQ) {
> + ret = ad9467_backend_testmode_on(st, chan->idx,
> IIO_BACKEND_ADI_PRBS_9A);
> - if (ret)
> - return ret;
> - } else if (mode == AN877_ADC_TESTMODE_PN23_SEQ) {
> - ret = ad9467_backend_testmode_on(st, chan->idx,
> + if (ret)
> + return ret;
> + } else if (mode == AN877_ADC_TESTMODE_PN23_SEQ) {
> + ret = ad9467_backend_testmode_on(st, chan->idx,
> IIO_BACKEND_ADI_PRBS_23A);
> - if (ret)
> - return ret;
> + if (ret)
> + return ret;
> + }
> }
> }
>
> @@ -1263,8 +1270,9 @@ static void ad9467_debugfs_init(struct iio_dev *indio_dev)
> if (!st->chan_test)
> return;
>
> - debugfs_create_file("calibration_table_dump", 0400, d, st,
> - &ad9467_calib_table_fops);
> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION))
> + debugfs_create_file("calibration_table_dump", 0400, d, st,
> + &ad9467_calib_table_fops);
>
> for (chan = 0; chan < st->info->num_channels; chan++) {
> snprintf(attr_name, sizeof(attr_name), "in_voltage%u_test_mode",
> @@ -1339,17 +1347,24 @@ static int ad9467_probe(struct spi_device *spi)
> if (ret)
> return ret;
>
> - ret = devm_iio_backend_request_buffer(&spi->dev, st->back, indio_dev);
> - if (ret)
> - return ret;
> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_BUFFER)) {
> + ret = devm_iio_backend_request_buffer(&spi->dev, st->back,
> + indio_dev);
> + if (ret)
> + return ret;
> + }
>
> - ret = devm_iio_backend_enable(&spi->dev, st->back);
> - if (ret)
> - return ret;
> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_ENABLE)) {
> + ret = devm_iio_backend_enable(&spi->dev, st->back);
> + if (ret)
> + return ret;
> + }
>
> - ret = ad9467_calibrate(st);
> - if (ret)
> - return ret;
> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION)) {
> + ret = ad9467_calibrate(st);
> + if (ret)
> + return ret;
> + }
>
> ret = devm_iio_device_register(&spi->dev, indio_dev);
> if (ret)
>
Hi,
On 08/02/2026 21:30, Jonathan Cameron wrote:
> On Thu, 05 Feb 2026 12:24:11 +0000
> Tomas Melin <tomas.melin@vaisala.com> wrote:
>
>> Add capability checks for operation with backends that do not necessarily
>> support full set of features, but are otherwise compatible with the device.
>> This ensures a fully functional device, but with limited capabilities.
>>
>> Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>
> I've mentioned before that I've started messing around with Chris Mason's
> set of Claude Code prompts and today I was trying out the new stuff mentioned in:
> https://lore.kernel.org/all/b187e0c1-1df8-4529-bfe4-0a1d65221adc@meta.com/
>
>
> Given this patch set is pretty much ready to merge I asked it to take a look
> and it came up with something that certainly merited me taking a look.
>
> Issue: In ad9467_chan_test_mode_write(), the testmode_on path is guarded
> by IIO_BACKEND_CAP_CALIBRATION but the matching testmode_off path is not.
> On backends without that capability, entering PN test mode succeeds but
> exiting fails with -EOPNOTSUPP, leaving the ADC stuck in test mode.
>
> I may be missing some protection path, but I think it is right and if you
> were to set up the test modes with a backend that doesn't support the
> relevant functionality you would indeed be able to turn them on but never
> off again.
The analysis does look correct. I think this has probably bubbled up in
the whirl of changes to call locations etc. Thanks, I will verify and
fix this.
br,
Tomas
>
> So this might be the first bug in IIO that the LLM found and human's, including
> me, missed!
>
> Thanks Chris! I won't +CC you every time but wanted to acknowledge that your
> hard work taming these tools is very useful to me.
>
> Jonathan
>
>
>
>
>> ---
>> drivers/iio/adc/ad9467.c | 67 +++++++++++++++++++++++++++++-------------------
>> 1 file changed, 41 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
>> index 59c3fa3bcc9b..7a3db36121b0 100644
>> --- a/drivers/iio/adc/ad9467.c
>> +++ b/drivers/iio/adc/ad9467.c
>> @@ -913,7 +913,9 @@ static int __ad9467_update_clock(struct ad9467_state *st, long r_clk)
>> return ret;
>>
>> guard(mutex)(&st->lock);
>> - return ad9467_calibrate(st);
>> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION))
>> + return ad9467_calibrate(st);
>> + return 0;
>> }
>>
>> static int ad9467_write_raw(struct iio_dev *indio_dev,
>> @@ -1119,12 +1121,15 @@ static ssize_t ad9467_chan_test_mode_read(struct file *file,
>> len = scnprintf(buf, sizeof(buf), "Running \"%s\" Test:\n\t",
>> ad9467_test_modes[chan->mode]);
>>
>> - ret = iio_backend_debugfs_print_chan_status(st->back, chan->idx,
>> - buf + len,
>> - sizeof(buf) - len);
>> - if (ret < 0)
>> - return ret;
>> - len += ret;
>> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION)) {
>> + ret = iio_backend_debugfs_print_chan_status(st->back,
>> + chan->idx,
>> + buf + len,
>> + sizeof(buf) - len);
>> + if (ret < 0)
>> + return ret;
>> + len += ret;
>> + }
>> } else if (chan->mode == AN877_ADC_TESTMODE_OFF) {
>> len = scnprintf(buf, sizeof(buf), "No test Running...\n");
>> } else {
>> @@ -1188,16 +1193,18 @@ static ssize_t ad9467_chan_test_mode_write(struct file *file,
>> return ret;
>>
>> /* some patterns have a backend matching monitoring block */
>> - if (mode == AN877_ADC_TESTMODE_PN9_SEQ) {
>> - ret = ad9467_backend_testmode_on(st, chan->idx,
>> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION)) {
>> + if (mode == AN877_ADC_TESTMODE_PN9_SEQ) {
>> + ret = ad9467_backend_testmode_on(st, chan->idx,
>> IIO_BACKEND_ADI_PRBS_9A);
>> - if (ret)
>> - return ret;
>> - } else if (mode == AN877_ADC_TESTMODE_PN23_SEQ) {
>> - ret = ad9467_backend_testmode_on(st, chan->idx,
>> + if (ret)
>> + return ret;
>> + } else if (mode == AN877_ADC_TESTMODE_PN23_SEQ) {
>> + ret = ad9467_backend_testmode_on(st, chan->idx,
>> IIO_BACKEND_ADI_PRBS_23A);
>> - if (ret)
>> - return ret;
>> + if (ret)
>> + return ret;
>> + }
>> }
>> }
>>
>> @@ -1263,8 +1270,9 @@ static void ad9467_debugfs_init(struct iio_dev *indio_dev)
>> if (!st->chan_test)
>> return;
>>
>> - debugfs_create_file("calibration_table_dump", 0400, d, st,
>> - &ad9467_calib_table_fops);
>> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION))
>> + debugfs_create_file("calibration_table_dump", 0400, d, st,
>> + &ad9467_calib_table_fops);
>>
>> for (chan = 0; chan < st->info->num_channels; chan++) {
>> snprintf(attr_name, sizeof(attr_name), "in_voltage%u_test_mode",
>> @@ -1339,17 +1347,24 @@ static int ad9467_probe(struct spi_device *spi)
>> if (ret)
>> return ret;
>>
>> - ret = devm_iio_backend_request_buffer(&spi->dev, st->back, indio_dev);
>> - if (ret)
>> - return ret;
>> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_BUFFER)) {
>> + ret = devm_iio_backend_request_buffer(&spi->dev, st->back,
>> + indio_dev);
>> + if (ret)
>> + return ret;
>> + }
>>
>> - ret = devm_iio_backend_enable(&spi->dev, st->back);
>> - if (ret)
>> - return ret;
>> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_ENABLE)) {
>> + ret = devm_iio_backend_enable(&spi->dev, st->back);
>> + if (ret)
>> + return ret;
>> + }
>>
>> - ret = ad9467_calibrate(st);
>> - if (ret)
>> - return ret;
>> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION)) {
>> + ret = ad9467_calibrate(st);
>> + if (ret)
>> + return ret;
>> + }
>>
>> ret = devm_iio_device_register(&spi->dev, indio_dev);
>> if (ret)
>>
>
On 2/5/26 6:24 AM, Tomas Melin wrote: > Add capability checks for operation with backends that do not necessarily > support full set of features, but are otherwise compatible with the device. > This ensures a fully functional device, but with limited capabilities. > > Signed-off-by: Tomas Melin <tomas.melin@vaisala.com> > --- > drivers/iio/adc/ad9467.c | 67 +++++++++++++++++++++++++++++------------------- > 1 file changed, 41 insertions(+), 26 deletions(-) > > diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c > index 59c3fa3bcc9b..7a3db36121b0 100644 > --- a/drivers/iio/adc/ad9467.c > +++ b/drivers/iio/adc/ad9467.c > @@ -913,7 +913,9 @@ static int __ad9467_update_clock(struct ad9467_state *st, long r_clk) > return ret; > > guard(mutex)(&st->lock); > - return ad9467_calibrate(st); > + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_CALIBRATION)) > + return ad9467_calibrate(st); > + return 0; > } > I would appreciate a blank line before and after the if statement if we end up needing a v7.
On Thu, Feb 05, 2026 at 12:24:11PM +0000, Tomas Melin wrote:
> Add capability checks for operation with backends that do not necessarily
> support full set of features, but are otherwise compatible with the device.
> This ensures a fully functional device, but with limited capabilities.
...
> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_BUFFER)) {
> + ret = devm_iio_backend_request_buffer(&spi->dev, st->back,
> + indio_dev);
With
struct device *dev = &spi->dev;
at the top, this one becomes exactly a 80 character line.
And in general it will help cleaning up the rest afterwards.
> + if (ret)
> + return ret;
> + }
--
With Best Regards,
Andy Shevchenko
Hi,
On 05/02/2026 18:34, Andy Shevchenko wrote:
> On Thu, Feb 05, 2026 at 12:24:11PM +0000, Tomas Melin wrote:
>> Add capability checks for operation with backends that do not necessarily
>> support full set of features, but are otherwise compatible with the device.
>> This ensures a fully functional device, but with limited capabilities.
>
> ...
>
>> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_BUFFER)) {
>> + ret = devm_iio_backend_request_buffer(&spi->dev, st->back,
>> + indio_dev);
>
> With
>
> struct device *dev = &spi->dev;
>
> at the top, this one becomes exactly a 80 character line.
> And in general it will help cleaning up the rest afterwards.
Given it seems it would still be 81 characters long I would like to
leave this change as it also creates a bit of noise.
Thanks,
Tomas
>
>> + if (ret)
>> + return ret;
>> + }
>
On Mon, Feb 09, 2026 at 09:24:09AM +0200, Tomas Melin wrote:
> On 05/02/2026 18:34, Andy Shevchenko wrote:
> > On Thu, Feb 05, 2026 at 12:24:11PM +0000, Tomas Melin wrote:
...
> >> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_BUFFER)) {
> >> + ret = devm_iio_backend_request_buffer(&spi->dev, st->back,
> >> + indio_dev);
> >
> > With
> >
> > struct device *dev = &spi->dev;
> >
> > at the top, this one becomes exactly a 80 character line.
> > And in general it will help cleaning up the rest afterwards.
>
> Given it seems it would still be 81 characters long I would like to
> leave this change as it also creates a bit of noise.
ret = devm_iio_backend_request_buffer(dev, st->back, indio_dev);
We have different arithmetics for sure, I have it 80.
"There are *two* problems in programming: naming, cache invalidation,
and off-by-one error."
> >> + if (ret)
> >> + return ret;
> >> + }
--
With Best Regards,
Andy Shevchenko
On 09/02/2026 10:41, Andy Shevchenko wrote:
> On Mon, Feb 09, 2026 at 09:24:09AM +0200, Tomas Melin wrote:
>> On 05/02/2026 18:34, Andy Shevchenko wrote:
>>> On Thu, Feb 05, 2026 at 12:24:11PM +0000, Tomas Melin wrote:
>
> ...
>
>>>> + if (iio_backend_has_caps(st->back, IIO_BACKEND_CAP_BUFFER)) {
>>>> + ret = devm_iio_backend_request_buffer(&spi->dev, st->back,
>>>> + indio_dev);
>>>
>>> With
>>>
>>> struct device *dev = &spi->dev;
>>>
>>> at the top, this one becomes exactly a 80 character line.
>>> And in general it will help cleaning up the rest afterwards.
>>
>> Given it seems it would still be 81 characters long I would like to
>> leave this change as it also creates a bit of noise.
>
> ret = devm_iio_backend_request_buffer(dev, st->back, indio_dev);
>
> We have different arithmetics for sure, I have it 80.
Indeed this seems to be the case, different editors count this
differently :D
-T
>
> "There are *two* problems in programming: naming, cache invalidation,
> and off-by-one error."
>
>>>> + if (ret)
>>>> + return ret;
>>>> + }
>
On Mon, Feb 09, 2026 at 12:01:07PM +0200, Tomas Melin wrote: > On 09/02/2026 10:41, Andy Shevchenko wrote: > > On Mon, Feb 09, 2026 at 09:24:09AM +0200, Tomas Melin wrote: > >> On 05/02/2026 18:34, Andy Shevchenko wrote: > >>> On Thu, Feb 05, 2026 at 12:24:11PM +0000, Tomas Melin wrote: ... > >>>> + ret = devm_iio_backend_request_buffer(&spi->dev, st->back, > >>>> + indio_dev); > >>> > >>> With > >>> > >>> struct device *dev = &spi->dev; > >>> > >>> at the top, this one becomes exactly a 80 character line. > >>> And in general it will help cleaning up the rest afterwards. > >> > >> Given it seems it would still be 81 characters long I would like to > >> leave this change as it also creates a bit of noise. > > > > ret = devm_iio_backend_request_buffer(dev, st->back, indio_dev); > > > > We have different arithmetics for sure, I have it 80. > > Indeed this seems to be the case, different editors count this > differently :D It depends if your cursor is under the last character or after, and depending on the meaning of the value: position of the cursor versus the line length it may be off-by-one. In VIM in the default mode it shows the position under the cursor and with starting column number equal to 1 the line length is calculated as position of the cursor - 1 + 1 I believe I have in the bar the length of the line in bytes and columns (these numbers are different due to tabs versus spaces) > > "There are *two* problems in programming: naming, cache invalidation, > > and off-by-one error." -- With Best Regards, Andy Shevchenko
© 2016 - 2026 Red Hat, Inc.