| 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
| ||
|
Message-ID: <68855a28-02f9-4241-ad1c-d523a9e55928@baylibre.com>
Date: Thu, 29 Jan 2026 10:39:16 -0600
From: David Lechner <dlechner@...libre.com>
To: Antoniu Miclaus <antoniu.miclaus@...log.com>,
Lars-Peter Clausen <lars@...afoo.de>,
Michael Hennerich <Michael.Hennerich@...log.com>,
Jonathan Cameron <jic23@...nel.org>, Nuno Sá
<nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, linux-iio@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] iio: adc: ad4080: add support for AD4880 dual-channel
ADC
On 1/29/26 9:27 AM, Antoniu Miclaus wrote:
> Add support for the AD4880, a dual-channel 20-bit 40MSPS SAR ADC with
> integrated fully differential amplifiers (FDA).
>
> The AD4880 has two independent ADC channels, each with its own SPI
> configuration interface. The driver uses spi_new_ancillary_device() to
> create an additional SPI device for the second channel, allowing both
> channels to share the same SPI bus with different chip selects.
>
...
> +static int ad4080_update_scan_mode(struct iio_dev *indio_dev,
> + const unsigned long *scan_mask)
> +{
> + struct ad4080_state *st = iio_priv(indio_dev);
> + unsigned int ch;
> + int ret;
> +
> + for (ch = 0; ch < st->info->num_channels; ch++) {
> + /*
> + * Each backend has a single channel (channel 0 from the
> + * backend's perspective), so always use channel index 0.
> + */
> + if (test_bit(ch, scan_mask))
> + ret = iio_backend_chan_enable(st->back[ch], 0);
> + else
> + ret = iio_backend_chan_disable(st->back[ch], 0);
> + if (ret)
> + return ret;
Previously, the single-channel chips didn't call backend channel
enable/disable. It this going to cause problems for that? I.e. the
function isn't implemented in the backend?
> + }
> +
> + return 0;
> +}
> +
> static const struct iio_info ad4080_iio_info = {
> .debugfs_reg_access = ad4080_reg_access,
> .read_raw = ad4080_read_raw,
> .write_raw = ad4080_write_raw,
> .read_avail = ad4080_read_avail,
> + .update_scan_mode = ad4080_update_scan_mode,
> };
>
> static const struct iio_enum ad4080_filter_type_enum = {
> @@ -414,23 +451,23 @@ static const struct iio_enum ad4080_filter_type_enum = {
> };
>
> static struct iio_chan_spec_ext_info ad4080_ext_info[] = {
> - IIO_ENUM("filter_type", IIO_SHARED_BY_ALL, &ad4080_filter_type_enum),
> - IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_ALL,
> + IIO_ENUM("filter_type", IIO_SEPARATE, &ad4080_filter_type_enum),
> + IIO_ENUM_AVAILABLE("filter_type", IIO_SEPARATE,
> &ad4080_filter_type_enum),
This is a breaking ABI change, so could be problamatic. We probably need to
keep the old info[] for the single channle chips and make a new info[] for
the multi-channels chips to avoid breaking things.
> { }
> };
>
...
> @@ -617,13 +694,35 @@ static int ad4080_probe(struct spi_device *spi)
> return dev_err_probe(dev, ret,
> "failed to get and enable supplies\n");
>
> - st->regmap = devm_regmap_init_spi(spi, &ad4080_regmap_config);
> - if (IS_ERR(st->regmap))
> - return PTR_ERR(st->regmap);
> + /* Setup primary SPI device (channel 0) */
> + st->spi[0] = spi;
> + st->regmap[0] = devm_regmap_init_spi(spi, &ad4080_regmap_config);
> + if (IS_ERR(st->regmap[0]))
> + return PTR_ERR(st->regmap[0]);
>
> - st->info = spi_get_device_match_data(spi);
> - if (!st->info)
> - return -ENODEV;
> + /* Setup ancillary SPI device for additional channel (AD4880) */
> + if (st->info->num_channels > 1) {
> + u32 aux_cs;
> +
> + ret = device_property_read_u32(dev, "adi,aux-spi-cs", &aux_cs);
As in the DT bindings comment, this sould be looking up the value at index 1
of the reg property rather than a custom property.
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "missing adi,aux-spi-cs for multi-channel device\n");
> +
> + st->spi[1] = spi_new_ancillary_device(spi, aux_cs);
> + if (IS_ERR(st->spi[1]))
> + return PTR_ERR(st->spi[1]);
> +
> + ret = devm_add_action_or_reset(dev, ad4080_unregister_ancillary,
> + st->spi[1]);
> + if (ret)
> + return ret;
> +
> + st->regmap[1] = devm_regmap_init_spi(st->spi[1],
> + &ad4080_regmap_config);
> + if (IS_ERR(st->regmap[1]))
> + return PTR_ERR(st->regmap[1]);
> + }
>
> ret = devm_mutex_init(dev, &st->lock);
> if (ret)
> @@ -644,15 +743,22 @@ static int ad4080_probe(struct spi_device *spi)
>
> st->clk_rate = clk_get_rate(clk);
>
> - st->back = devm_iio_backend_get(dev, NULL);
> - if (IS_ERR(st->back))
> - return PTR_ERR(st->back);
> + /* Get backends for all channels */
> + for (ch = 0; ch < st->info->num_channels; ch++) {
> + if (st->info->num_channels > 1)
> + st->back[ch] = devm_iio_backend_get(dev, backend_names[ch]);
It looks like we missed io-backend-names from the DT bindings. In this case
though, going by index instead of name would make more sense. So we could add
a devm_iio_backend_get_by_index() function instead of modifying the DT bindings.
Then we wouldn't need the if statement here.
> + else
> + st->back[ch] = devm_iio_backend_get(dev, NULL);
>
> - ret = devm_iio_backend_request_buffer(dev, st->back, indio_dev);
> - if (ret)
> - return ret;
> + if (IS_ERR(st->back[ch]))
> + return PTR_ERR(st->back[ch]);
> +
> + ret = devm_iio_backend_enable(dev, st->back[ch]);
> + if (ret)
> + return ret;
> + }
>
> - ret = devm_iio_backend_enable(dev, st->back);
> + ret = devm_iio_backend_request_buffer(dev, st->back[0], indio_dev);
If this is correct, it could use a comment explaining why only one
backend has the buffer even if there are two backends.
> if (ret)
> return ret;
>
> @@ -670,6 +776,7 @@ static const struct spi_device_id ad4080_id[] = {
> { "ad4084", (kernel_ulong_t)&ad4084_chip_info },
> { "ad4086", (kernel_ulong_t)&ad4086_chip_info },
> { "ad4087", (kernel_ulong_t)&ad4087_chip_info },
> + { "ad4880", (kernel_ulong_t)&ad4880_chip_info },
> { }
> };
> MODULE_DEVICE_TABLE(spi, ad4080_id);
> @@ -681,6 +788,7 @@ static const struct of_device_id ad4080_of_match[] = {
> { .compatible = "adi,ad4084", &ad4084_chip_info },
> { .compatible = "adi,ad4086", &ad4086_chip_info },
> { .compatible = "adi,ad4087", &ad4087_chip_info },
> + { .compatible = "adi,ad4880", &ad4880_chip_info },
> { }
> };
> MODULE_DEVICE_TABLE(of, ad4080_of_match);
Powered by blists - more mailing lists