[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6faec58e-f8e3-4a16-b056-9aec1555537d@vaisala.com>
Date: Mon, 9 Feb 2026 09:17:00 +0200
From: Tomas Melin <tomas.melin@...sala.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Michael Hennerich <Michael.Hennerich@...log.com>,
Nuno Sa <nuno.sa@...log.com>, Lars-Peter Clausen <lars@...afoo.de>,
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, Chris Mason <clm@...a.com>
Subject: Re: [PATCH v6 5/5] iio: adc: ad9467: check for backend capabilities
Hi,
On 08/02/2026 21:30, Jonathan Cameron wrote:
> On Thu, 05 Feb 2026 12:24:11 +0000
> Tomas Melin <tomas.melin@...sala.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@...sala.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)
>>
>
Powered by blists - more mailing lists