* [PATCH v6 0/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC
@ 2026-03-13 11:58 Antoniu Miclaus
2026-03-13 11:58 ` [PATCH v6 1/4] iio: backend: use __free(fwnode_handle) for automatic cleanup Antoniu Miclaus
` (4 more replies)
0 siblings, 5 replies; 20+ messages in thread
From: Antoniu Miclaus @ 2026-03-13 11:58 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Antoniu Miclaus,
Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan,
linux-iio, devicetree, linux-kernel
Add support for the AD4880, a dual-channel 20-bit 40MSPS SAR ADC from
the same family as AD4080.
The AD4880 has two independent ADC channels, each with its own SPI
configuration interface and LVDS data output. The driver uses
spi_new_ancillary_device() for the second channel's SPI and requires
two io-backend instances for the data interfaces.
This series includes:
- Use __free(fwnode_handle) for automatic cleanup in iio backend
- Refactored devm_iio_backend_get_by_index() for multi-channel backend lookup
- DT bindings update for AD4880
- Driver support for AD4880
This series depends on the SPI ancillary device patches already picked
up in Mark Brown's SPI tree (https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git).
Added base-commit in the cover letter below.
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4880.pdf
Changes in v6:
- Move fwnode_back declaration back to inline position above
IS_ERR() check (patch 2)
- Rebase on top of broonie/spi for-next and use --base to declare
the SPI dependency (cover letter)
Antoniu Miclaus (4):
iio: backend: use __free(fwnode_handle) for automatic cleanup
iio: backend: add devm_iio_backend_get_by_index()
dt-bindings: iio: adc: ad4080: add AD4880 support
iio: adc: ad4080: add support for AD4880 dual-channel ADC
.../bindings/iio/adc/adi,ad4080.yaml | 53 +++-
drivers/iio/adc/ad4080.c | 230 ++++++++++++++----
drivers/iio/industrialio-backend.c | 62 +++--
include/linux/iio/backend.h | 2 +
4 files changed, 278 insertions(+), 69 deletions(-)
base-commit: 2cd3974b9ae59ac731a4792e1608be32621b6e98
--
2.43.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v6 1/4] iio: backend: use __free(fwnode_handle) for automatic cleanup 2026-03-13 11:58 [PATCH v6 0/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC Antoniu Miclaus @ 2026-03-13 11:58 ` Antoniu Miclaus 2026-03-13 14:16 ` Andy Shevchenko 2026-03-14 10:53 ` Nuno Sá 2026-03-13 11:58 ` [PATCH v6 2/4] iio: backend: add devm_iio_backend_get_by_index() Antoniu Miclaus ` (3 subsequent siblings) 4 siblings, 2 replies; 20+ messages in thread From: Antoniu Miclaus @ 2026-03-13 11:58 UTC (permalink / raw) To: Lars-Peter Clausen, Michael Hennerich, Antoniu Miclaus, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio, devicetree, linux-kernel Convert __devm_iio_backend_fwnode_get() to use the __free(fwnode_handle) cleanup attribute for the fwnode_back variable, removing the need for manual fwnode_handle_put() calls. Move the declaration closer to its first use, narrowing its scope. No functional change. Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> --- Changes in v6: - No changes drivers/iio/industrialio-backend.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c index 447b694d6d5f..58f7e1426095 100644 --- a/drivers/iio/industrialio-backend.c +++ b/drivers/iio/industrialio-backend.c @@ -952,7 +952,6 @@ EXPORT_SYMBOL_NS_GPL(iio_backend_data_transfer_addr, "IIO_BACKEND"); static struct iio_backend *__devm_iio_backend_fwnode_get(struct device *dev, const char *name, struct fwnode_handle *fwnode) { - struct fwnode_handle *fwnode_back; struct iio_backend *back; unsigned int index; int ret; @@ -967,7 +966,8 @@ static struct iio_backend *__devm_iio_backend_fwnode_get(struct device *dev, con index = 0; } - fwnode_back = fwnode_find_reference(fwnode, "io-backends", index); + struct fwnode_handle *fwnode_back __free(fwnode_handle) = + fwnode_find_reference(fwnode, "io-backends", index); if (IS_ERR(fwnode_back)) return dev_err_cast_probe(dev, fwnode_back, "Cannot get Firmware reference\n"); @@ -977,7 +977,6 @@ static struct iio_backend *__devm_iio_backend_fwnode_get(struct device *dev, con if (!device_match_fwnode(back->dev, fwnode_back)) continue; - fwnode_handle_put(fwnode_back); ret = __devm_iio_backend_get(dev, back); if (ret) return ERR_PTR(ret); @@ -988,7 +987,6 @@ static struct iio_backend *__devm_iio_backend_fwnode_get(struct device *dev, con return back; } - fwnode_handle_put(fwnode_back); return ERR_PTR(-EPROBE_DEFER); } -- 2.43.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v6 1/4] iio: backend: use __free(fwnode_handle) for automatic cleanup 2026-03-13 11:58 ` [PATCH v6 1/4] iio: backend: use __free(fwnode_handle) for automatic cleanup Antoniu Miclaus @ 2026-03-13 14:16 ` Andy Shevchenko 2026-03-14 10:53 ` Nuno Sá 1 sibling, 0 replies; 20+ messages in thread From: Andy Shevchenko @ 2026-03-13 14:16 UTC (permalink / raw) To: Antoniu Miclaus Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio, devicetree, linux-kernel On Fri, Mar 13, 2026 at 01:58:50PM +0200, Antoniu Miclaus wrote: > Convert __devm_iio_backend_fwnode_get() to use the __free(fwnode_handle) > cleanup attribute for the fwnode_back variable, removing the need for > manual fwnode_handle_put() calls. Move the declaration closer to its > first use, narrowing its scope. > > No functional change. Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v6 1/4] iio: backend: use __free(fwnode_handle) for automatic cleanup 2026-03-13 11:58 ` [PATCH v6 1/4] iio: backend: use __free(fwnode_handle) for automatic cleanup Antoniu Miclaus 2026-03-13 14:16 ` Andy Shevchenko @ 2026-03-14 10:53 ` Nuno Sá 1 sibling, 0 replies; 20+ messages in thread From: Nuno Sá @ 2026-03-14 10:53 UTC (permalink / raw) To: Antoniu Miclaus, Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio, devicetree, linux-kernel On Fri, 2026-03-13 at 13:58 +0200, Antoniu Miclaus wrote: > Convert __devm_iio_backend_fwnode_get() to use the __free(fwnode_handle) > cleanup attribute for the fwnode_back variable, removing the need for > manual fwnode_handle_put() calls. Move the declaration closer to its > first use, narrowing its scope. > > No functional change. > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> > --- Reviewed-by: Nuno Sá <nuno.sa@analog.com> > Changes in v6: > - No changes > > drivers/iio/industrialio-backend.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio- > backend.c > index 447b694d6d5f..58f7e1426095 100644 > --- a/drivers/iio/industrialio-backend.c > +++ b/drivers/iio/industrialio-backend.c > @@ -952,7 +952,6 @@ EXPORT_SYMBOL_NS_GPL(iio_backend_data_transfer_addr, > "IIO_BACKEND"); > static struct iio_backend *__devm_iio_backend_fwnode_get(struct device *dev, const > char *name, > struct fwnode_handle > *fwnode) > { > - struct fwnode_handle *fwnode_back; > struct iio_backend *back; > unsigned int index; > int ret; > @@ -967,7 +966,8 @@ static struct iio_backend *__devm_iio_backend_fwnode_get(struct > device *dev, con > index = 0; > } > > - fwnode_back = fwnode_find_reference(fwnode, "io-backends", index); > + struct fwnode_handle *fwnode_back __free(fwnode_handle) = > + fwnode_find_reference(fwnode, "io-backends", index); > if (IS_ERR(fwnode_back)) > return dev_err_cast_probe(dev, fwnode_back, > "Cannot get Firmware reference\n"); > @@ -977,7 +977,6 @@ static struct iio_backend *__devm_iio_backend_fwnode_get(struct > device *dev, con > if (!device_match_fwnode(back->dev, fwnode_back)) > continue; > > - fwnode_handle_put(fwnode_back); > ret = __devm_iio_backend_get(dev, back); > if (ret) > return ERR_PTR(ret); > @@ -988,7 +987,6 @@ static struct iio_backend *__devm_iio_backend_fwnode_get(struct > device *dev, con > return back; > } > > - fwnode_handle_put(fwnode_back); > return ERR_PTR(-EPROBE_DEFER); > } > ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v6 2/4] iio: backend: add devm_iio_backend_get_by_index() 2026-03-13 11:58 [PATCH v6 0/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC Antoniu Miclaus 2026-03-13 11:58 ` [PATCH v6 1/4] iio: backend: use __free(fwnode_handle) for automatic cleanup Antoniu Miclaus @ 2026-03-13 11:58 ` Antoniu Miclaus 2026-03-13 14:16 ` Andy Shevchenko 2026-03-13 11:58 ` [PATCH v6 3/4] dt-bindings: iio: adc: ad4080: add AD4880 support Antoniu Miclaus ` (2 subsequent siblings) 4 siblings, 1 reply; 20+ messages in thread From: Antoniu Miclaus @ 2026-03-13 11:58 UTC (permalink / raw) To: Antoniu Miclaus, Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio, devicetree, linux-kernel Add a new function to get an IIO backend by its index in the io-backends device tree property. This is useful for multi-channel devices that have multiple backends, where looking up by index is more straightforward than using named backends. Extract __devm_iio_backend_fwnode_get_by_index() from the existing __devm_iio_backend_fwnode_get(), taking the index directly as a parameter. The new public API devm_iio_backend_get_by_index() uses the index to find the backend reference in the io-backends property, avoiding the need for io-backend-names. Reviewed-by: David Lechner <dlechner@baylibre.com> Reviewed-by: Nuno Sá <nuno.sa@analog.com> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> --- Changes in v6: - Move fwnode_back declaration back to inline position above IS_ERR() check drivers/iio/industrialio-backend.c | 56 ++++++++++++++++++++++-------- include/linux/iio/backend.h | 2 ++ 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c index 58f7e1426095..b5ab93221acd 100644 --- a/drivers/iio/industrialio-backend.c +++ b/drivers/iio/industrialio-backend.c @@ -949,23 +949,14 @@ int iio_backend_data_transfer_addr(struct iio_backend *back, u32 address) } EXPORT_SYMBOL_NS_GPL(iio_backend_data_transfer_addr, "IIO_BACKEND"); -static struct iio_backend *__devm_iio_backend_fwnode_get(struct device *dev, const char *name, - struct fwnode_handle *fwnode) +static struct iio_backend * +__devm_iio_backend_fwnode_get_by_index(struct device *dev, + struct fwnode_handle *fwnode, + unsigned int index) { struct iio_backend *back; - unsigned int index; int ret; - if (name) { - ret = device_property_match_string(dev, "io-backend-names", - name); - if (ret < 0) - return ERR_PTR(ret); - index = ret; - } else { - index = 0; - } - struct fwnode_handle *fwnode_back __free(fwnode_handle) = fwnode_find_reference(fwnode, "io-backends", index); if (IS_ERR(fwnode_back)) @@ -981,8 +972,7 @@ static struct iio_backend *__devm_iio_backend_fwnode_get(struct device *dev, con if (ret) return ERR_PTR(ret); - if (name) - back->idx = index; + back->idx = index; return back; } @@ -990,6 +980,24 @@ static struct iio_backend *__devm_iio_backend_fwnode_get(struct device *dev, con return ERR_PTR(-EPROBE_DEFER); } +static struct iio_backend *__devm_iio_backend_fwnode_get(struct device *dev, const char *name, + struct fwnode_handle *fwnode) +{ + unsigned int index; + int ret; + + if (name) { + ret = device_property_match_string(dev, "io-backend-names", name); + if (ret < 0) + return ERR_PTR(ret); + index = ret; + } else { + index = 0; + } + + return __devm_iio_backend_fwnode_get_by_index(dev, fwnode, index); +} + /** * devm_iio_backend_get - Device managed backend device get * @dev: Consumer device for the backend @@ -1006,6 +1014,24 @@ struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name) } EXPORT_SYMBOL_NS_GPL(devm_iio_backend_get, "IIO_BACKEND"); +/** + * devm_iio_backend_get_by_index - Device managed backend device get by index + * @dev: Consumer device for the backend + * @index: Index of the backend in the io-backends property + * + * Gets the backend at @index associated with @dev. + * + * RETURNS: + * A backend pointer, negative error pointer otherwise. + */ +struct iio_backend *devm_iio_backend_get_by_index(struct device *dev, + unsigned int index) +{ + return __devm_iio_backend_fwnode_get_by_index(dev, dev_fwnode(dev), + index); +} +EXPORT_SYMBOL_NS_GPL(devm_iio_backend_get_by_index, "IIO_BACKEND"); + /** * devm_iio_backend_fwnode_get - Device managed backend firmware node get * @dev: Consumer device for the backend diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h index 7f815f3fed6a..8f18df0ca896 100644 --- a/include/linux/iio/backend.h +++ b/include/linux/iio/backend.h @@ -237,6 +237,8 @@ int iio_backend_extend_chan_spec(struct iio_backend *back, struct iio_chan_spec *chan); void *iio_backend_get_priv(const struct iio_backend *conv); struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name); +struct iio_backend *devm_iio_backend_get_by_index(struct device *dev, + unsigned int index); struct iio_backend *devm_iio_backend_fwnode_get(struct device *dev, const char *name, struct fwnode_handle *fwnode); -- 2.43.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v6 2/4] iio: backend: add devm_iio_backend_get_by_index() 2026-03-13 11:58 ` [PATCH v6 2/4] iio: backend: add devm_iio_backend_get_by_index() Antoniu Miclaus @ 2026-03-13 14:16 ` Andy Shevchenko 0 siblings, 0 replies; 20+ messages in thread From: Andy Shevchenko @ 2026-03-13 14:16 UTC (permalink / raw) To: Antoniu Miclaus Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio, devicetree, linux-kernel On Fri, Mar 13, 2026 at 01:58:51PM +0200, Antoniu Miclaus wrote: > Add a new function to get an IIO backend by its index in the > io-backends device tree property. This is useful for multi-channel > devices that have multiple backends, where looking up by index is > more straightforward than using named backends. > > Extract __devm_iio_backend_fwnode_get_by_index() from the existing > __devm_iio_backend_fwnode_get(), taking the index directly as a > parameter. The new public API devm_iio_backend_get_by_index() uses > the index to find the backend reference in the io-backends property, > avoiding the need for io-backend-names. ... > -static struct iio_backend *__devm_iio_backend_fwnode_get(struct device *dev, const char *name, > - struct fwnode_handle *fwnode) > +static struct iio_backend * > +__devm_iio_backend_fwnode_get_by_index(struct device *dev, > + struct fwnode_handle *fwnode, > + unsigned int index) Why out of a sudden to shorten under 80? The rest seems exercises 100 limit... static struct iio_backend * __devm_iio_backend_fwnode_get_by_index(struct device *dev, struct fwnode_handle *fwnode, unsigned int index) is under 100. ... > +struct iio_backend *devm_iio_backend_get_by_index(struct device *dev, > + unsigned int index) Ditto. > +{ > + return __devm_iio_backend_fwnode_get_by_index(dev, dev_fwnode(dev), > + index); Ditto. > +} > +EXPORT_SYMBOL_NS_GPL(devm_iio_backend_get_by_index, "IIO_BACKEND"); -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v6 3/4] dt-bindings: iio: adc: ad4080: add AD4880 support 2026-03-13 11:58 [PATCH v6 0/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC Antoniu Miclaus 2026-03-13 11:58 ` [PATCH v6 1/4] iio: backend: use __free(fwnode_handle) for automatic cleanup Antoniu Miclaus 2026-03-13 11:58 ` [PATCH v6 2/4] iio: backend: add devm_iio_backend_get_by_index() Antoniu Miclaus @ 2026-03-13 11:58 ` Antoniu Miclaus 2026-03-13 11:58 ` [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC Antoniu Miclaus 2026-03-14 11:40 ` [PATCH v6 0/4] " Jonathan Cameron 4 siblings, 0 replies; 20+ messages in thread From: Antoniu Miclaus @ 2026-03-13 11:58 UTC (permalink / raw) To: Antoniu Miclaus, Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio, devicetree, linux-kernel 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. This requires: - Two entries in reg property for primary and secondary channel chip selects - Two io-backends entries for the two data channels Reviewed-by: David Lechner <dlechner@baylibre.com> Reviewed-by: Rob Herring (Arm) <robh@kernel.org> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> --- Changes in v6: - No changes .../bindings/iio/adc/adi,ad4080.yaml | 53 ++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4080.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4080.yaml index ccd6a0ac1539..0cf86c6f9925 100644 --- a/Documentation/devicetree/bindings/iio/adc/adi,ad4080.yaml +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4080.yaml @@ -18,7 +18,11 @@ description: | service a wide variety of precision, wide bandwidth data acquisition applications. + The AD4880 is a dual-channel variant with two independent ADC channels, + each with its own SPI configuration interface. + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4080.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/ad4880.pdf $ref: /schemas/spi/spi-peripheral-props.yaml# @@ -31,9 +35,15 @@ properties: - adi,ad4084 - adi,ad4086 - adi,ad4087 + - adi,ad4880 reg: - maxItems: 1 + minItems: 1 + maxItems: 2 + description: + SPI chip select(s). For single-channel devices, one chip select. + For multi-channel devices like AD4880, two chip selects are required + as each channel has its own SPI configuration interface. spi-max-frequency: description: Configuration of the SPI bus. @@ -57,7 +67,10 @@ properties: vrefin-supply: true io-backends: - maxItems: 1 + minItems: 1 + items: + - description: Backend for channel A (primary) + - description: Backend for channel B (secondary) adi,lvds-cnv-enable: description: Enable the LVDS signal type on the CNV pin. Default is CMOS. @@ -78,6 +91,25 @@ required: - vdd33-supply - vrefin-supply +allOf: + - if: + properties: + compatible: + contains: + const: adi,ad4880 + then: + properties: + reg: + minItems: 2 + io-backends: + minItems: 2 + else: + properties: + reg: + maxItems: 1 + io-backends: + maxItems: 1 + additionalProperties: false examples: @@ -98,4 +130,21 @@ examples: io-backends = <&iio_backend>; }; }; + - | + spi { + #address-cells = <1>; + #size-cells = <0>; + + adc@0 { + compatible = "adi,ad4880"; + reg = <0>, <1>; + spi-max-frequency = <10000000>; + vdd33-supply = <&vdd33>; + vddldo-supply = <&vddldo>; + vrefin-supply = <&vrefin>; + clocks = <&cnv>; + clock-names = "cnv"; + io-backends = <&iio_backend_cha>, <&iio_backend_chb>; + }; + }; ... -- 2.43.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC 2026-03-13 11:58 [PATCH v6 0/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC Antoniu Miclaus ` (2 preceding siblings ...) 2026-03-13 11:58 ` [PATCH v6 3/4] dt-bindings: iio: adc: ad4080: add AD4880 support Antoniu Miclaus @ 2026-03-13 11:58 ` Antoniu Miclaus 2026-03-13 14:22 ` Andy Shevchenko 2026-03-14 11:40 ` [PATCH v6 0/4] " Jonathan Cameron 4 siblings, 1 reply; 20+ messages in thread From: Antoniu Miclaus @ 2026-03-13 11:58 UTC (permalink / raw) To: Antoniu Miclaus, Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio, devicetree, linux-kernel 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. Reviewed-by: David Lechner <dlechner@baylibre.com> Reviewed-by: Nuno Sá <nuno.sa@analog.com> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> --- Changes in v6: - No changes drivers/iio/adc/ad4080.c | 230 +++++++++++++++++++++++++++++++-------- 1 file changed, 182 insertions(+), 48 deletions(-) diff --git a/drivers/iio/adc/ad4080.c b/drivers/iio/adc/ad4080.c index 7cf3b6ed7940..48bd124cc9d6 100644 --- a/drivers/iio/adc/ad4080.c +++ b/drivers/iio/adc/ad4080.c @@ -16,6 +16,7 @@ #include <linux/mod_devicetable.h> #include <linux/module.h> #include <linux/mutex.h> +#include <linux/property.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> #include <linux/spi/spi.h> @@ -131,6 +132,9 @@ #define AD4084_CHIP_ID 0x0054 #define AD4086_CHIP_ID 0x0056 #define AD4087_CHIP_ID 0x0057 +#define AD4880_CHIP_ID 0x0750 + +#define AD4080_MAX_CHANNELS 2 #define AD4080_LVDS_CNV_CLK_CNT_MAX 7 @@ -176,8 +180,9 @@ struct ad4080_chip_info { }; struct ad4080_state { - struct regmap *regmap; - struct iio_backend *back; + struct spi_device *spi[AD4080_MAX_CHANNELS]; + struct regmap *regmap[AD4080_MAX_CHANNELS]; + struct iio_backend *back[AD4080_MAX_CHANNELS]; const struct ad4080_chip_info *info; /* * Synchronize access to members the of driver state, and ensure @@ -203,10 +208,11 @@ static int ad4080_reg_access(struct iio_dev *indio_dev, unsigned int reg, { struct ad4080_state *st = iio_priv(indio_dev); + /* Use channel 0 regmap for debugfs access */ if (readval) - return regmap_read(st->regmap, reg, readval); + return regmap_read(st->regmap[0], reg, readval); - return regmap_write(st->regmap, reg, writeval); + return regmap_write(st->regmap[0], reg, writeval); } static int ad4080_get_scale(struct ad4080_state *st, int *val, int *val2) @@ -227,8 +233,9 @@ static unsigned int ad4080_get_dec_rate(struct iio_dev *dev, struct ad4080_state *st = iio_priv(dev); int ret; unsigned int data; + unsigned int ch = chan->channel; - ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &data); + ret = regmap_read(st->regmap[ch], AD4080_REG_FILTER_CONFIG, &data); if (ret) return ret; @@ -240,13 +247,14 @@ static int ad4080_set_dec_rate(struct iio_dev *dev, unsigned int mode) { struct ad4080_state *st = iio_priv(dev); + unsigned int ch = chan->channel; guard(mutex)(&st->lock); if ((st->filter_type >= SINC_5 && mode >= 512) || mode < 2) return -EINVAL; - return regmap_update_bits(st->regmap, AD4080_REG_FILTER_CONFIG, + return regmap_update_bits(st->regmap[ch], AD4080_REG_FILTER_CONFIG, AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK, FIELD_PREP(AD4080_FILTER_CONFIG_SINC_DEC_RATE_MSK, (ilog2(mode) - 1))); @@ -304,23 +312,23 @@ static int ad4080_write_raw(struct iio_dev *indio_dev, } } -static int ad4080_lvds_sync_write(struct ad4080_state *st) +static int ad4080_lvds_sync_write(struct ad4080_state *st, unsigned int ch) { - struct device *dev = regmap_get_device(st->regmap); + struct device *dev = regmap_get_device(st->regmap[ch]); int ret; - ret = regmap_set_bits(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A, + ret = regmap_set_bits(st->regmap[ch], AD4080_REG_ADC_DATA_INTF_CONFIG_A, AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN); if (ret) return ret; - ret = iio_backend_interface_data_align(st->back, 10000); + ret = iio_backend_interface_data_align(st->back[ch], 10000); if (ret) return dev_err_probe(dev, ret, "Data alignment process failed\n"); dev_dbg(dev, "Success: Pattern correct and Locked!\n"); - return regmap_clear_bits(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A, + return regmap_clear_bits(st->regmap[ch], AD4080_REG_ADC_DATA_INTF_CONFIG_A, AD4080_ADC_DATA_INTF_CONFIG_A_INTF_CHK_EN); } @@ -329,9 +337,10 @@ static int ad4080_get_filter_type(struct iio_dev *dev, { struct ad4080_state *st = iio_priv(dev); unsigned int data; + unsigned int ch = chan->channel; int ret; - ret = regmap_read(st->regmap, AD4080_REG_FILTER_CONFIG, &data); + ret = regmap_read(st->regmap[ch], AD4080_REG_FILTER_CONFIG, &data); if (ret) return ret; @@ -343,6 +352,7 @@ static int ad4080_set_filter_type(struct iio_dev *dev, unsigned int mode) { struct ad4080_state *st = iio_priv(dev); + unsigned int ch = chan->channel; int dec_rate; int ret; @@ -355,11 +365,11 @@ static int ad4080_set_filter_type(struct iio_dev *dev, if (mode >= SINC_5 && dec_rate >= 512) return -EINVAL; - ret = iio_backend_filter_type_set(st->back, mode); + ret = iio_backend_filter_type_set(st->back[ch], mode); if (ret) return ret; - ret = regmap_update_bits(st->regmap, AD4080_REG_FILTER_CONFIG, + ret = regmap_update_bits(st->regmap[ch], AD4080_REG_FILTER_CONFIG, AD4080_FILTER_CONFIG_FILTER_SEL_MSK, FIELD_PREP(AD4080_FILTER_CONFIG_FILTER_SEL_MSK, mode)); @@ -399,6 +409,28 @@ static int ad4080_read_avail(struct iio_dev *indio_dev, } } +static int ad4880_update_scan_mode(struct iio_dev *indio_dev, + const unsigned long *scan_mask) +{ + struct ad4080_state *st = iio_priv(indio_dev); + int ret; + + for (unsigned int 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; + } + + return 0; +} + static const struct iio_info ad4080_iio_info = { .debugfs_reg_access = ad4080_reg_access, .read_raw = ad4080_read_raw, @@ -406,6 +438,19 @@ static const struct iio_info ad4080_iio_info = { .read_avail = ad4080_read_avail, }; +/* + * AD4880 needs update_scan_mode to enable/disable individual backend channels. + * Single-channel devices don't need this as their backends may not implement + * chan_enable/chan_disable operations. + */ +static const struct iio_info ad4880_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 = ad4880_update_scan_mode, +}; + static const struct iio_enum ad4080_filter_type_enum = { .items = ad4080_filter_type_iio_enum, .num_items = ARRAY_SIZE(ad4080_filter_type_iio_enum), @@ -420,17 +465,51 @@ static struct iio_chan_spec_ext_info ad4080_ext_info[] = { { } }; -#define AD4080_CHANNEL_DEFINE(bits, storage) { \ +/* + * AD4880 needs per-channel filter configuration since each channel has + * its own independent ADC with separate SPI interface. + */ +static struct iio_chan_spec_ext_info ad4880_ext_info[] = { + IIO_ENUM("filter_type", IIO_SEPARATE, &ad4080_filter_type_enum), + IIO_ENUM_AVAILABLE("filter_type", IIO_SEPARATE, + &ad4080_filter_type_enum), + { } +}; + +#define AD4080_CHANNEL_DEFINE(bits, storage, idx) { \ .type = IIO_VOLTAGE, \ .indexed = 1, \ - .channel = 0, \ + .channel = (idx), \ .info_mask_separate = BIT(IIO_CHAN_INFO_SCALE), \ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \ .info_mask_shared_by_all_available = \ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \ .ext_info = ad4080_ext_info, \ - .scan_index = 0, \ + .scan_index = (idx), \ + .scan_type = { \ + .sign = 's', \ + .realbits = (bits), \ + .storagebits = (storage), \ + }, \ +} + +/* + * AD4880 has per-channel attributes (filter_type, oversampling_ratio, + * sampling_frequency) since each channel has its own independent ADC + * with separate SPI configuration interface. + */ +#define AD4880_CHANNEL_DEFINE(bits, storage, idx) { \ + .type = IIO_VOLTAGE, \ + .indexed = 1, \ + .channel = (idx), \ + .info_mask_separate = BIT(IIO_CHAN_INFO_SCALE) | \ + BIT(IIO_CHAN_INFO_SAMP_FREQ) | \ + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \ + .info_mask_separate_available = \ + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \ + .ext_info = ad4880_ext_info, \ + .scan_index = (idx), \ .scan_type = { \ .sign = 's', \ .realbits = (bits), \ @@ -438,17 +517,22 @@ static struct iio_chan_spec_ext_info ad4080_ext_info[] = { }, \ } -static const struct iio_chan_spec ad4080_channel = AD4080_CHANNEL_DEFINE(20, 32); +static const struct iio_chan_spec ad4080_channel = AD4080_CHANNEL_DEFINE(20, 32, 0); -static const struct iio_chan_spec ad4081_channel = AD4080_CHANNEL_DEFINE(20, 32); +static const struct iio_chan_spec ad4081_channel = AD4080_CHANNEL_DEFINE(20, 32, 0); -static const struct iio_chan_spec ad4083_channel = AD4080_CHANNEL_DEFINE(16, 16); +static const struct iio_chan_spec ad4083_channel = AD4080_CHANNEL_DEFINE(16, 16, 0); -static const struct iio_chan_spec ad4084_channel = AD4080_CHANNEL_DEFINE(16, 16); +static const struct iio_chan_spec ad4084_channel = AD4080_CHANNEL_DEFINE(16, 16, 0); -static const struct iio_chan_spec ad4086_channel = AD4080_CHANNEL_DEFINE(14, 16); +static const struct iio_chan_spec ad4086_channel = AD4080_CHANNEL_DEFINE(14, 16, 0); -static const struct iio_chan_spec ad4087_channel = AD4080_CHANNEL_DEFINE(14, 16); +static const struct iio_chan_spec ad4087_channel = AD4080_CHANNEL_DEFINE(14, 16, 0); + +static const struct iio_chan_spec ad4880_channels[] = { + AD4880_CHANNEL_DEFINE(20, 32, 0), + AD4880_CHANNEL_DEFINE(20, 32, 1), +}; static const struct ad4080_chip_info ad4080_chip_info = { .name = "ad4080", @@ -510,25 +594,34 @@ static const struct ad4080_chip_info ad4087_chip_info = { .lvds_cnv_clk_cnt_max = 1, }; -static int ad4080_setup(struct iio_dev *indio_dev) +static const struct ad4080_chip_info ad4880_chip_info = { + .name = "ad4880", + .product_id = AD4880_CHIP_ID, + .scale_table = ad4080_scale_table, + .num_scales = ARRAY_SIZE(ad4080_scale_table), + .num_channels = 2, + .channels = ad4880_channels, + .lvds_cnv_clk_cnt_max = AD4080_LVDS_CNV_CLK_CNT_MAX, +}; + +static int ad4080_setup_channel(struct ad4080_state *st, unsigned int ch) { - struct ad4080_state *st = iio_priv(indio_dev); - struct device *dev = regmap_get_device(st->regmap); + struct device *dev = regmap_get_device(st->regmap[ch]); __le16 id_le; u16 id; int ret; - ret = regmap_write(st->regmap, AD4080_REG_INTERFACE_CONFIG_A, + ret = regmap_write(st->regmap[ch], AD4080_REG_INTERFACE_CONFIG_A, AD4080_INTERFACE_CONFIG_A_SW_RESET); if (ret) return ret; - ret = regmap_write(st->regmap, AD4080_REG_INTERFACE_CONFIG_A, + ret = regmap_write(st->regmap[ch], AD4080_REG_INTERFACE_CONFIG_A, AD4080_INTERFACE_CONFIG_A_SDO_ENABLE); if (ret) return ret; - ret = regmap_bulk_read(st->regmap, AD4080_REG_PRODUCT_ID_L, &id_le, + ret = regmap_bulk_read(st->regmap[ch], AD4080_REG_PRODUCT_ID_L, &id_le, sizeof(id_le)); if (ret) return ret; @@ -537,18 +630,18 @@ static int ad4080_setup(struct iio_dev *indio_dev) if (id != st->info->product_id) dev_info(dev, "Unrecognized CHIP_ID 0x%X\n", id); - ret = regmap_set_bits(st->regmap, AD4080_REG_GPIO_CONFIG_A, + ret = regmap_set_bits(st->regmap[ch], AD4080_REG_GPIO_CONFIG_A, AD4080_GPIO_CONFIG_A_GPO_1_EN); if (ret) return ret; - ret = regmap_write(st->regmap, AD4080_REG_GPIO_CONFIG_B, + ret = regmap_write(st->regmap[ch], AD4080_REG_GPIO_CONFIG_B, FIELD_PREP(AD4080_GPIO_CONFIG_B_GPIO_1_SEL_MSK, AD4080_GPIO_CONFIG_B_GPIO_FILTER_RES_RDY)); if (ret) return ret; - ret = iio_backend_num_lanes_set(st->back, st->num_lanes); + ret = iio_backend_num_lanes_set(st->back[ch], st->num_lanes); if (ret) return ret; @@ -556,7 +649,7 @@ static int ad4080_setup(struct iio_dev *indio_dev) return 0; /* Set maximum LVDS Data Transfer Latency */ - ret = regmap_update_bits(st->regmap, + ret = regmap_update_bits(st->regmap[ch], AD4080_REG_ADC_DATA_INTF_CONFIG_B, AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_CLK_CNT_MSK, FIELD_PREP(AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_CLK_CNT_MSK, @@ -565,24 +658,38 @@ static int ad4080_setup(struct iio_dev *indio_dev) return ret; if (st->num_lanes > 1) { - ret = regmap_set_bits(st->regmap, AD4080_REG_ADC_DATA_INTF_CONFIG_A, + ret = regmap_set_bits(st->regmap[ch], AD4080_REG_ADC_DATA_INTF_CONFIG_A, AD4080_ADC_DATA_INTF_CONFIG_A_SPI_LVDS_LANES); if (ret) return ret; } - ret = regmap_set_bits(st->regmap, + ret = regmap_set_bits(st->regmap[ch], AD4080_REG_ADC_DATA_INTF_CONFIG_B, AD4080_ADC_DATA_INTF_CONFIG_B_LVDS_CNV_EN); if (ret) return ret; - return ad4080_lvds_sync_write(st); + return ad4080_lvds_sync_write(st, ch); +} + +static int ad4080_setup(struct iio_dev *indio_dev) +{ + struct ad4080_state *st = iio_priv(indio_dev); + int ret; + + for (unsigned int ch = 0; ch < st->info->num_channels; ch++) { + ret = ad4080_setup_channel(st, ch); + if (ret) + return ret; + } + + return 0; } static int ad4080_properties_parse(struct ad4080_state *st) { - struct device *dev = regmap_get_device(st->regmap); + struct device *dev = regmap_get_device(st->regmap[0]); st->lvds_cnv_en = device_property_read_bool(dev, "adi,lvds-cnv-enable"); @@ -617,14 +724,30 @@ 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 devices for additional channels */ + for (unsigned int ch = 1; ch < st->info->num_channels; ch++) { + st->spi[ch] = devm_spi_new_ancillary_device(spi, + spi_get_chipselect(spi, ch)); + if (IS_ERR(st->spi[ch])) + return dev_err_probe(dev, PTR_ERR(st->spi[ch]), + "failed to register ancillary device\n"); + + st->regmap[ch] = devm_regmap_init_spi(st->spi[ch], + &ad4080_regmap_config); + if (IS_ERR(st->regmap[ch])) + return PTR_ERR(st->regmap[ch]); + } + ret = devm_mutex_init(dev, &st->lock); if (ret) return ret; @@ -632,7 +755,8 @@ static int ad4080_probe(struct spi_device *spi) indio_dev->name = st->info->name; indio_dev->channels = st->info->channels; indio_dev->num_channels = st->info->num_channels; - indio_dev->info = &ad4080_iio_info; + indio_dev->info = st->info->num_channels > 1 ? + &ad4880_iio_info : &ad4080_iio_info; ret = ad4080_properties_parse(st); if (ret) @@ -644,15 +768,23 @@ 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 (unsigned int ch = 0; ch < st->info->num_channels; ch++) { + st->back[ch] = devm_iio_backend_get_by_index(dev, ch); + if (IS_ERR(st->back[ch])) + return PTR_ERR(st->back[ch]); - ret = devm_iio_backend_request_buffer(dev, st->back, indio_dev); - if (ret) - return ret; + ret = devm_iio_backend_enable(dev, st->back[ch]); + if (ret) + return ret; + } - ret = devm_iio_backend_enable(dev, st->back); + /* + * Request buffer from the first backend only. For multi-channel + * devices (e.g., AD4880), all backends share a single IIO buffer + * as data from all ADC channels is interleaved into one stream. + */ + ret = devm_iio_backend_request_buffer(dev, st->back[0], indio_dev); if (ret) return ret; @@ -670,6 +802,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 +814,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); -- 2.43.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC 2026-03-13 11:58 ` [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC Antoniu Miclaus @ 2026-03-13 14:22 ` Andy Shevchenko 2026-03-14 12:00 ` Jonathan Cameron 0 siblings, 1 reply; 20+ messages in thread From: Andy Shevchenko @ 2026-03-13 14:22 UTC (permalink / raw) To: Antoniu Miclaus Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio, devicetree, linux-kernel On Fri, Mar 13, 2026 at 01:58:53PM +0200, 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. I am still not sure this is the best approach we can have. In any case, I have immediate questions here about regmap usage. - Why do we need to have a separate regmap per channel? - What is special about channel 0? - Is it okay to communicate with different channels simultaneously? Wouldn't be a nasty race with HW IO? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC 2026-03-13 14:22 ` Andy Shevchenko @ 2026-03-14 12:00 ` Jonathan Cameron 2026-03-16 9:56 ` Andy Shevchenko 0 siblings, 1 reply; 20+ messages in thread From: Jonathan Cameron @ 2026-03-14 12:00 UTC (permalink / raw) To: Andy Shevchenko Cc: Antoniu Miclaus, Lars-Peter Clausen, Michael Hennerich, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio, devicetree, linux-kernel On Fri, 13 Mar 2026 16:22:53 +0200 Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > On Fri, Mar 13, 2026 at 01:58:53PM +0200, 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. > > I am still not sure this is the best approach we can have. > In any case, I have immediate questions here about regmap usage. I think we have a fairly fundamental misalignment on what this is. To my understanding (diagram on first page of the datasheet) + the functional block diagram on page 3 it's effectively two almost entirely separate devices in one package (sharing of power etc) and a few common wires for clocks references etc. Pretty close to some of the multi die devices we get for IMUs etc but with tighter coupling that forces one driver (for the IMUs we just register separate drivers). It 'might' use one SPI bus, or 2 or even 4 (if using separate data interfaces). Just to speed things up let me have a go at answering the questions. > > - Why do we need to have a separate regmap per channel? Propose an alternative? It's two independent interfaces, so you could spin a special regmap to handle that, but it's much simpler to just use standard stuff and keep them separate. Not to mention it would either have to do external locking or falsely imply there was any restriction on using both interfaces at once (there isn't) > - What is special about channel 0? Nothing. > - Is it okay to communicate with different channels simultaneously? Yes. They are entirely parallel bits of silicon. Own state machines and everything. The configuration registers section of the datasheet says: "Each channel has it's own independent configuration memory accessible through it's separate configuration SPI interface." > Wouldn't be a nasty race with HW IO? Nope. You are talking to different devices (more or less). Jonathan ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC 2026-03-14 12:00 ` Jonathan Cameron @ 2026-03-16 9:56 ` Andy Shevchenko 2026-03-16 12:31 ` Miclaus, Antoniu 0 siblings, 1 reply; 20+ messages in thread From: Andy Shevchenko @ 2026-03-16 9:56 UTC (permalink / raw) To: Jonathan Cameron Cc: Antoniu Miclaus, Lars-Peter Clausen, Michael Hennerich, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio, devicetree, linux-kernel On Sat, Mar 14, 2026 at 12:00:22PM +0000, Jonathan Cameron wrote: > On Fri, 13 Mar 2026 16:22:53 +0200 > Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > On Fri, Mar 13, 2026 at 01:58:53PM +0200, 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. > > > > I am still not sure this is the best approach we can have. > > In any case, I have immediate questions here about regmap usage. > > I think we have a fairly fundamental misalignment on what this is. > > To my understanding (diagram on first page of the datasheet) > + the functional block diagram on page 3 it's effectively two almost > entirely separate devices in one package (sharing of power etc) and > a few common wires for clocks references etc. Pretty close to some > of the multi die devices we get for IMUs etc but with tighter coupling > that forces one driver (for the IMUs we just register separate drivers). > > It 'might' use one SPI bus, or 2 or even 4 (if using separate data > interfaces). > > Just to speed things up let me have a go at answering the questions. > > > - Why do we need to have a separate regmap per channel? > > Propose an alternative? It's two independent interfaces, so you > could spin a special regmap to handle that, but it's much simpler > to just use standard stuff and keep them separate. Not to mention it > would either have to do external locking or falsely imply > there was any restriction on using both interfaces at once > (there isn't) > > > - What is special about channel 0? > > Nothing. Then why code does explicit access to regmap channel 0? We should have regmap[ch] in all cases in the code. > > - Is it okay to communicate with different channels simultaneously? > > Yes. They are entirely parallel bits of silicon. Own state machines > and everything. > The configuration registers section of the datasheet says: > "Each channel has it's own independent configuration memory > accessible through it's separate configuration SPI interface." > > > Wouldn't be a nasty race with HW IO? > > Nope. You are talking to different devices (more or less). If it's a twins in the package, why do we have a special handling and not just describing two independent devices in the DT/fw? TO me is either something special about channel 0, then we have to synchronise accesses, or there is no point to have this patch at all, just make devices to be the same under the hood and describe as independent pair. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC 2026-03-16 9:56 ` Andy Shevchenko @ 2026-03-16 12:31 ` Miclaus, Antoniu 2026-03-16 14:41 ` Andy Shevchenko 0 siblings, 1 reply; 20+ messages in thread From: Miclaus, Antoniu @ 2026-03-16 12:31 UTC (permalink / raw) To: Andy Shevchenko, Jonathan Cameron Cc: Lars-Peter Clausen, Hennerich, Michael, David Lechner, Sa, Nuno, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org > -----Original Message----- > From: Andy Shevchenko <andriy.shevchenko@intel.com> > Sent: Monday, March 16, 2026 11:57 AM > To: Jonathan Cameron <jic23@kernel.org> > Cc: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>; Lars-Peter Clausen > <lars@metafoo.de>; Hennerich, Michael <Michael.Hennerich@analog.com>; > David Lechner <dlechner@baylibre.com>; Sa, Nuno <Nuno.Sa@analog.com>; > Andy Shevchenko <andy@kernel.org>; Rob Herring <robh@kernel.org>; > Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley > <conor+dt@kernel.org>; Olivier Moysan <olivier.moysan@foss.st.com>; linux- > iio@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual- > channel ADC > > [External] > > On Sat, Mar 14, 2026 at 12:00:22PM +0000, Jonathan Cameron wrote: > > On Fri, 13 Mar 2026 16:22:53 +0200 > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > > On Fri, Mar 13, 2026 at 01:58:53PM +0200, 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. > > > > > > I am still not sure this is the best approach we can have. > > > In any case, I have immediate questions here about regmap usage. > > > > I think we have a fairly fundamental misalignment on what this is. > > > > To my understanding (diagram on first page of the datasheet) > > + the functional block diagram on page 3 it's effectively two almost > > entirely separate devices in one package (sharing of power etc) and > > a few common wires for clocks references etc. Pretty close to some > > of the multi die devices we get for IMUs etc but with tighter coupling > > that forces one driver (for the IMUs we just register separate drivers). > > > > It 'might' use one SPI bus, or 2 or even 4 (if using separate data > > interfaces). > > > > Just to speed things up let me have a go at answering the questions. > > > > > - Why do we need to have a separate regmap per channel? > > > > Propose an alternative? It's two independent interfaces, so you > > could spin a special regmap to handle that, but it's much simpler > > to just use standard stuff and keep them separate. Not to mention it > > would either have to do external locking or falsely imply > > there was any restriction on using both interfaces at once > > (there isn't) > > > > > - What is special about channel 0? > > > > Nothing. > > Then why code does explicit access to regmap channel 0? > We should have regmap[ch] in all cases in the code. > There are three places that use channel 0 explicitly, none of which imply channel 0 is functionally special from a hardware perspective: 1. ad4080_reg_access() - the debugfs reg_access callback has no channel concept, it's a single (reg, val) interface. We have to pick one regmap, channel 0 is the default choice. I can improve the comment to make this clearer. 2. ad4080_properties_parse() - uses regmap_get_device(st->regmap[0]) solely to obtain the struct device * for reading DT properties. The device tree properties live on the parent SPI node, which is channel 0's device. This isn't "channel 0 is special", it's just "DT properties belong to the primary SPI device." 3. devm_iio_backend_request_buffer() - requests the buffer from st->back[0] because all channel data is interleaved into a single stream (there's an inline comment). Only one buffer is needed. All register configuration (setup, filter, decimation) already uses regmap[ch] throughout. > > > - Is it okay to communicate with different channels simultaneously? > > > > Yes. They are entirely parallel bits of silicon. Own state machines > > and everything. > > The configuration registers section of the datasheet says: > > "Each channel has it's own independent configuration memory > > accessible through it's separate configuration SPI interface." > > > > > Wouldn't be a nasty race with HW IO? > > > > Nope. You are talking to different devices (more or less). > > If it's a twins in the package, why do we have a special handling and not just > describing two independent devices in the DT/fw? > Because they are not fully independent - they share: - Power supplies and voltage reference - The CNV clock (conversion trigger) - A single interleaved data output stream Describing them as two independent DT nodes would mean duplicating all the shared resources, and more importantly, the data interface is a single interleaved stream feeding into one IIO buffer. Having two separate IIO devices would make synchronized capture impossible from userspace. This is exactly the use case spi_new_ancillary_device() was designed for - a multi-die device sharing a bus with separate chip selects for configuration but common data/clock/power infrastructure. > TO me is either something special about channel 0, then we have to > synchronise > accesses, or there is no point to have this patch at all, just make devices to > be the same under the hood and describe as independent pair. > -- > With Best Regards, > Andy Shevchenko > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC 2026-03-16 12:31 ` Miclaus, Antoniu @ 2026-03-16 14:41 ` Andy Shevchenko 2026-03-16 15:09 ` Miclaus, Antoniu 0 siblings, 1 reply; 20+ messages in thread From: Andy Shevchenko @ 2026-03-16 14:41 UTC (permalink / raw) To: Miclaus, Antoniu Cc: Jonathan Cameron, Lars-Peter Clausen, Hennerich, Michael, David Lechner, Sa, Nuno, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org On Mon, Mar 16, 2026 at 12:31:09PM +0000, Miclaus, Antoniu wrote: > > From: Andy Shevchenko <andriy.shevchenko@intel.com> > > Sent: Monday, March 16, 2026 11:57 AM > > On Sat, Mar 14, 2026 at 12:00:22PM +0000, Jonathan Cameron wrote: > > > On Fri, 13 Mar 2026 16:22:53 +0200 > > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > > > On Fri, Mar 13, 2026 at 01:58:53PM +0200, Antoniu Miclaus wrote: ... > > > > - What is special about channel 0? > > > > > > Nothing. > > > > Then why code does explicit access to regmap channel 0? > > We should have regmap[ch] in all cases in the code. > > > There are three places that use channel 0 explicitly, none of which > imply channel 0 is functionally special from a hardware perspective: > > 1. ad4080_reg_access() - the debugfs reg_access callback has no > channel concept, it's a single (reg, val) interface. We have to > pick one regmap, channel 0 is the default choice. I can improve > the comment to make this clearer. Then it's simply wrong. You allow only one channel to be printed. The debugfs has to print two channels, no? > 2. ad4080_properties_parse() - uses regmap_get_device(st->regmap[0]) > solely to obtain the struct device * for reading DT properties. > The device tree properties live on the parent SPI node, which is > channel 0's device. This isn't "channel 0 is special", it's just > "DT properties belong to the primary SPI device." Can we simply pass the struct device to that function? > 3. devm_iio_backend_request_buffer() - requests the buffer from > st->back[0] because all channel data is interleaved into a > single stream (there's an inline comment). Only one buffer is needed. But this is not regmap related, is it? > All register configuration (setup, filter, decimation) already uses > regmap[ch] throughout. > > > > > - Is it okay to communicate with different channels simultaneously? > > > > > > Yes. They are entirely parallel bits of silicon. Own state machines > > > and everything. > > > The configuration registers section of the datasheet says: > > > "Each channel has it's own independent configuration memory > > > accessible through it's separate configuration SPI interface." > > > > > > > Wouldn't be a nasty race with HW IO? > > > > > > Nope. You are talking to different devices (more or less). > > > > If it's a twins in the package, why do we have a special handling and not just > > describing two independent devices in the DT/fw? > > Because they are not fully independent - they share: > - Power supplies and voltage reference > - The CNV clock (conversion trigger) Okay, then why not having a core part and a glue driver that registers as many devices as you wish and provides just a common stuff? We have similar (to some extend) cases with SPI/I²C where drivers/platform/x86/serial-multi-instantiate.c services as "MFD" for that type of busses. > - A single interleaved data output stream How does it work in non-racy way? > Describing them as two independent DT nodes would mean duplicating > all the shared resources, and more importantly, the data interface > is a single interleaved stream feeding into one IIO buffer. Having > two separate IIO devices would make synchronized capture impossible > from userspace. > > This is exactly the use case spi_new_ancillary_device() was designed > for - a multi-die device sharing a bus with separate chip selects for > configuration but common data/clock/power infrastructure. See above. > > TO me is either something special about channel 0, then we have to > > synchronise > > accesses, or there is no point to have this patch at all, just make devices to > > be the same under the hood and describe as independent pair. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC 2026-03-16 14:41 ` Andy Shevchenko @ 2026-03-16 15:09 ` Miclaus, Antoniu 2026-03-16 15:21 ` Andy Shevchenko 0 siblings, 1 reply; 20+ messages in thread From: Miclaus, Antoniu @ 2026-03-16 15:09 UTC (permalink / raw) To: Andy Shevchenko Cc: Jonathan Cameron, Lars-Peter Clausen, Hennerich, Michael, David Lechner, Sa, Nuno, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org > -----Original Message----- > From: Andy Shevchenko <andriy.shevchenko@intel.com> > Sent: Monday, March 16, 2026 4:42 PM > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com> > Cc: Jonathan Cameron <jic23@kernel.org>; Lars-Peter Clausen > <lars@metafoo.de>; Hennerich, Michael <Michael.Hennerich@analog.com>; > David Lechner <dlechner@baylibre.com>; Sa, Nuno <Nuno.Sa@analog.com>; > Andy Shevchenko <andy@kernel.org>; Rob Herring <robh@kernel.org>; > Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley > <conor+dt@kernel.org>; Olivier Moysan <olivier.moysan@foss.st.com>; linux- > iio@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual- > channel ADC > > [External] > > On Mon, Mar 16, 2026 at 12:31:09PM +0000, Miclaus, Antoniu wrote: > > > From: Andy Shevchenko <andriy.shevchenko@intel.com> > > > Sent: Monday, March 16, 2026 11:57 AM > > > On Sat, Mar 14, 2026 at 12:00:22PM +0000, Jonathan Cameron wrote: > > > > On Fri, 13 Mar 2026 16:22:53 +0200 > > > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > > > > On Fri, Mar 13, 2026 at 01:58:53PM +0200, Antoniu Miclaus wrote: > > ... > > > > > > - What is special about channel 0? > > > > > > > > Nothing. > > > > > > Then why code does explicit access to regmap channel 0? > > > We should have regmap[ch] in all cases in the code. > > > > > There are three places that use channel 0 explicitly, none of which > > imply channel 0 is functionally special from a hardware perspective: > > > > 1. ad4080_reg_access() - the debugfs reg_access callback has no > > channel concept, it's a single (reg, val) interface. We have to > > pick one regmap, channel 0 is the default choice. I can improve > > the comment to make this clearer. > > Then it's simply wrong. You allow only one channel to be printed. The debugfs > has to print two channels, no? The IIO debugfs_reg_access callback signature is fixed by the framework - it provides (reg, writeval, readval) with no channel parameter. For v7 I can drop debugfs_reg_access. But doesn't hurt if we have at least half access to the debugfs for it. > > > 2. ad4080_properties_parse() - uses regmap_get_device(st->regmap[0]) > > solely to obtain the struct device * for reading DT properties. > > The device tree properties live on the parent SPI node, which is > > channel 0's device. This isn't "channel 0 is special", it's just > > "DT properties belong to the primary SPI device." > > Can we simply pass the struct device to that function? Yes, can do that in v7 if you think it is absolutely necessary. > > > 3. devm_iio_backend_request_buffer() - requests the buffer from > > st->back[0] because all channel data is interleaved into a > > single stream (there's an inline comment). Only one buffer is needed. > > But this is not regmap related, is it? > Correct, it's not - I should not have grouped it with the regmap points. > > All register configuration (setup, filter, decimation) already uses > > regmap[ch] throughout. > > > > > > > - Is it okay to communicate with different channels simultaneously? > > > > > > > > Yes. They are entirely parallel bits of silicon. Own state machines > > > > and everything. > > > > The configuration registers section of the datasheet says: > > > > "Each channel has it's own independent configuration memory > > > > accessible through it's separate configuration SPI interface." > > > > > > > > > Wouldn't be a nasty race with HW IO? > > > > > > > > Nope. You are talking to different devices (more or less). > > > > > > If it's a twins in the package, why do we have a special handling and not > just > > > describing two independent devices in the DT/fw? > > > > Because they are not fully independent - they share: > > - Power supplies and voltage reference > > - The CNV clock (conversion trigger) > > Okay, then why not having a core part and a glue driver that registers as many > devices as you wish and provides just a common stuff? > Because the AD4880 is not two independent ADCs sharing a package - it is a single device with a single interleaved data output. Splitting into separate IIO devices would make synchronized dual-channel capture impossible from userspace, which is the primary use case for this part. The shared resources (supplies, CNV clock, interleaved data stream) are not just "glue" - they define the device's operating model. The per-channel SPI interfaces exist only for register configuration; the actual data path is a single stream handled entirely by the backend. > We have similar (to some extend) cases with SPI/I²C where > drivers/platform/x86/serial-multi-instantiate.c services as "MFD" for that > type of busses. > > > - A single interleaved data output stream > > How does it work in non-racy way? > The data path has no software involvement at runtime. The CNV clock triggers both channels to sample simultaneously, and the device outputs the conversion results as a single interleaved bitstream on the data lane(s). The FPGA backend captures this stream directly - no SPI register reads are involved in the data path. The only SPI traffic is for configuration, and each channel has its own independent SPI interface and regmap, so there is no shared bus contention. > > Describing them as two independent DT nodes would mean duplicating > > all the shared resources, and more importantly, the data interface > > is a single interleaved stream feeding into one IIO buffer. Having > > two separate IIO devices would make synchronized capture impossible > > from userspace. > > > > This is exactly the use case spi_new_ancillary_device() was designed > > for - a multi-die device sharing a bus with separate chip selects for > > configuration but common data/clock/power infrastructure. > > See above. > > > > TO me is either something special about channel 0, then we have to > > > synchronise > > > accesses, or there is no point to have this patch at all, just make devices to > > > be the same under the hood and describe as independent pair. > > -- > With Best Regards, > Andy Shevchenko > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC 2026-03-16 15:09 ` Miclaus, Antoniu @ 2026-03-16 15:21 ` Andy Shevchenko 2026-03-17 9:54 ` Miclaus, Antoniu 0 siblings, 1 reply; 20+ messages in thread From: Andy Shevchenko @ 2026-03-16 15:21 UTC (permalink / raw) To: Miclaus, Antoniu Cc: Jonathan Cameron, Lars-Peter Clausen, Hennerich, Michael, David Lechner, Sa, Nuno, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org On Mon, Mar 16, 2026 at 03:09:18PM +0000, Miclaus, Antoniu wrote: > > From: Andy Shevchenko <andriy.shevchenko@intel.com> > > Sent: Monday, March 16, 2026 4:42 PM > > On Mon, Mar 16, 2026 at 12:31:09PM +0000, Miclaus, Antoniu wrote: > > > > From: Andy Shevchenko <andriy.shevchenko@intel.com> > > > > Sent: Monday, March 16, 2026 11:57 AM > > > > On Sat, Mar 14, 2026 at 12:00:22PM +0000, Jonathan Cameron wrote: > > > > > On Fri, 13 Mar 2026 16:22:53 +0200 > > > > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > > > > > On Fri, Mar 13, 2026 at 01:58:53PM +0200, Antoniu Miclaus wrote: ... > > > > > > - What is special about channel 0? > > > > > > > > > > Nothing. > > > > > > > > Then why code does explicit access to regmap channel 0? > > > > We should have regmap[ch] in all cases in the code. > > > > > > > There are three places that use channel 0 explicitly, none of which > > > imply channel 0 is functionally special from a hardware perspective: > > > > > > 1. ad4080_reg_access() - the debugfs reg_access callback has no > > > channel concept, it's a single (reg, val) interface. We have to > > > pick one regmap, channel 0 is the default choice. I can improve > > > the comment to make this clearer. > > > > Then it's simply wrong. You allow only one channel to be printed. The debugfs > > has to print two channels, no? > > The IIO debugfs_reg_access callback signature is fixed by the framework - > it provides (reg, writeval, readval) with no channel parameter. > > For v7 I can drop debugfs_reg_access. But doesn't hurt if we have at least half > access to the debugfs for it. This will confuse the users. Either do not print or print it all. > > > 2. ad4080_properties_parse() - uses regmap_get_device(st->regmap[0]) > > > solely to obtain the struct device * for reading DT properties. > > > The device tree properties live on the parent SPI node, which is > > > channel 0's device. This isn't "channel 0 is special", it's just > > > "DT properties belong to the primary SPI device." > > > > Can we simply pass the struct device to that function? > Yes, can do that in v7 if you think it is absolutely necessary. Yes, please. ... > > > All register configuration (setup, filter, decimation) already uses > > > regmap[ch] throughout. > > > > > > > > > - Is it okay to communicate with different channels simultaneously? > > > > > > > > > > Yes. They are entirely parallel bits of silicon. Own state machines > > > > > and everything. > > > > > The configuration registers section of the datasheet says: > > > > > "Each channel has it's own independent configuration memory > > > > > accessible through it's separate configuration SPI interface." > > > > > > > > > > > Wouldn't be a nasty race with HW IO? > > > > > > > > > > Nope. You are talking to different devices (more or less). > > > > > > > > If it's a twins in the package, why do we have a special handling and not > > just > > > > describing two independent devices in the DT/fw? > > > > > > Because they are not fully independent - they share: > > > - Power supplies and voltage reference > > > - The CNV clock (conversion trigger) > > > > Okay, then why not having a core part and a glue driver that registers as many > > devices as you wish and provides just a common stuff? > Because the AD4880 is not two independent ADCs sharing a package - it is > a single device with a single interleaved data output. Splitting into > separate IIO devices would make synchronized dual-channel capture > impossible from userspace, which is the primary use case for this part. Sounds to me like you need, probably, a virtual device for that. Maybe even on IIO level. Do we expect more devices like this to be enabled in the future (or maybe already in tree, but lacking this feature)? > The shared resources (supplies, CNV clock, interleaved data stream) are > not just "glue" - they define the device's operating model. Sure, like any other resource for MFD (HW speaking). > The per-channel SPI interfaces exist only for register configuration; the > actual data path is a single stream handled entirely by the backend. > > > We have similar (to some extend) cases with SPI/I²C where > > drivers/platform/x86/serial-multi-instantiate.c services as "MFD" for that > > type of busses. > > > > > - A single interleaved data output stream > > > > How does it work in non-racy way? > > The data path has no software involvement at runtime. The CNV clock > triggers both channels to sample simultaneously, and the device outputs > the conversion results as a single interleaved bitstream on the data > lane(s). The FPGA backend captures this stream directly - no SPI > register reads are involved in the data path. The only SPI traffic is > for configuration, and each channel has its own independent SPI > interface and regmap, so there is no shared bus contention. Okay, so it's in a way more complex (like a camera sensor in terms of data/configuration paths) device. It's now even more looking that the current approach is a quick hack rather than a solution to make this properly fit Linux device model. > > > Describing them as two independent DT nodes would mean duplicating > > > all the shared resources, and more importantly, the data interface > > > is a single interleaved stream feeding into one IIO buffer. Having > > > two separate IIO devices would make synchronized capture impossible > > > from userspace. > > > This is exactly the use case spi_new_ancillary_device() was designed > > > for - a multi-die device sharing a bus with separate chip selects for > > > configuration but common data/clock/power infrastructure. This... It doesn't fit the data path as far as I read from the above. > > See above. > > > > > > TO me is either something special about channel 0, then we have to > > > > synchronise > > > > accesses, or there is no point to have this patch at all, just make devices to > > > > be the same under the hood and describe as independent pair. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC 2026-03-16 15:21 ` Andy Shevchenko @ 2026-03-17 9:54 ` Miclaus, Antoniu 2026-03-17 10:31 ` Andy Shevchenko 0 siblings, 1 reply; 20+ messages in thread From: Miclaus, Antoniu @ 2026-03-17 9:54 UTC (permalink / raw) To: Andy Shevchenko Cc: Jonathan Cameron, Lars-Peter Clausen, Hennerich, Michael, David Lechner, Sa, Nuno, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org > On Mon, Mar 16, 2026 at 03:09:18PM +0000, Miclaus, Antoniu wrote: > > > From: Andy Shevchenko <andriy.shevchenko@intel.com> > > > Sent: Monday, March 16, 2026 4:42 PM > > > On Mon, Mar 16, 2026 at 12:31:09PM +0000, Miclaus, Antoniu wrote: > > > > > From: Andy Shevchenko <andriy.shevchenko@intel.com> > > > > > Sent: Monday, March 16, 2026 11:57 AM > > > > > On Sat, Mar 14, 2026 at 12:00:22PM +0000, Jonathan Cameron > wrote: > > > > > > On Fri, 13 Mar 2026 16:22:53 +0200 > > > > > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > > > > > > On Fri, Mar 13, 2026 at 01:58:53PM +0200, Antoniu Miclaus > wrote: > > ... > > > > > > > > - What is special about channel 0? > > > > > > > > > > > > Nothing. > > > > > > > > > > Then why code does explicit access to regmap channel 0? > > > > > We should have regmap[ch] in all cases in the code. > > > > > > > > > There are three places that use channel 0 explicitly, none of which > > > > imply channel 0 is functionally special from a hardware perspective: > > > > > > > > 1. ad4080_reg_access() - the debugfs reg_access callback has no > > > > channel concept, it's a single (reg, val) interface. We have to > > > > pick one regmap, channel 0 is the default choice. I can improve > > > > the comment to make this clearer. > > > > > > Then it's simply wrong. You allow only one channel to be printed. The > debugfs > > > has to print two channels, no? > > > > The IIO debugfs_reg_access callback signature is fixed by the framework - > > it provides (reg, writeval, readval) with no channel parameter. > > > > For v7 I can drop debugfs_reg_access. But doesn't hurt if we have at least > half > > access to the debugfs for it. > > This will confuse the users. Either do not print or print it all. Agreed, will drop it in v7. > > > > > 2. ad4080_properties_parse() - uses regmap_get_device(st- > >regmap[0]) > > > > solely to obtain the struct device * for reading DT properties. > > > > The device tree properties live on the parent SPI node, which is > > > > channel 0's device. This isn't "channel 0 is special", it's just > > > > "DT properties belong to the primary SPI device." > > > > > > Can we simply pass the struct device to that function? > > Yes, can do that in v7 if you think it is absolutely necessary. > > Yes, please. Will do. > ... > > > > > All register configuration (setup, filter, decimation) already uses > > > > regmap[ch] throughout. > > > > > > > > > > > - Is it okay to communicate with different channels simultaneously? > > > > > > > > > > > > Yes. They are entirely parallel bits of silicon. Own state machines > > > > > > and everything. > > > > > > The configuration registers section of the datasheet says: > > > > > > "Each channel has it's own independent configuration memory > > > > > > accessible through it's separate configuration SPI interface." > > > > > > > > > > > > > Wouldn't be a nasty race with HW IO? > > > > > > > > > > > > Nope. You are talking to different devices (more or less). > > > > > > > > > > If it's a twins in the package, why do we have a special handling and not > > > just > > > > > describing two independent devices in the DT/fw? > > > > > > > > Because they are not fully independent - they share: > > > > - Power supplies and voltage reference > > > > - The CNV clock (conversion trigger) > > > > > > Okay, then why not having a core part and a glue driver that registers as > many > > > devices as you wish and provides just a common stuff? > > > Because the AD4880 is not two independent ADCs sharing a package - it is > > a single device with a single interleaved data output. Splitting into > > separate IIO devices would make synchronized dual-channel capture > > impossible from userspace, which is the primary use case for this part. > > Sounds to me like you need, probably, a virtual device for that. > Maybe even on IIO level. Do we expect more devices like this to > be enabled in the future (or maybe already in tree, but lacking this > feature)? > The AD4880 is a fairly unique part - having separate SPI config interfaces per channel with a shared interleaved data output is not a common pattern, and the chances of another device like this being upstreamed are low. Given that Jonathan has already reviewed and approved a previous version of this series, and the patch has collected multiple Reviewed-by tags, I'd prefer to keep the current approach. > > The shared resources (supplies, CNV clock, interleaved data stream) are > > not just "glue" - they define the device's operating model. > > Sure, like any other resource for MFD (HW speaking). MFD models a single device exposing multiple functionally distinct sub-devices. The AD4880 channels are not distinct sub-devices - they share a single interleaved data stream, and splitting them would break synchronized capture. > > > The per-channel SPI interfaces exist only for register configuration; the > > actual data path is a single stream handled entirely by the backend. > > > > > We have similar (to some extend) cases with SPI/I²C where > > > drivers/platform/x86/serial-multi-instantiate.c services as "MFD" for that > > > type of busses. > > > > > > > - A single interleaved data output stream > > > > > > How does it work in non-racy way? > > > > The data path has no software involvement at runtime. The CNV clock > > triggers both channels to sample simultaneously, and the device outputs > > the conversion results as a single interleaved bitstream on the data > > lane(s). The FPGA backend captures this stream directly - no SPI > > register reads are involved in the data path. The only SPI traffic is > > for configuration, and each channel has its own independent SPI > > interface and regmap, so there is no shared bus contention. > > Okay, so it's in a way more complex (like a camera sensor in terms of > data/configuration paths) device. It's now even more looking that the > current approach is a quick hack rather than a solution to make this > properly fit Linux device model. > The driver uses spi_new_ancillary_device() for the config path and the IIO backend for the data path - both existing kernel infrastructure used as intended. No custom abstractions are added. > > > > Describing them as two independent DT nodes would mean duplicating > > > > all the shared resources, and more importantly, the data interface > > > > is a single interleaved stream feeding into one IIO buffer. Having > > > > two separate IIO devices would make synchronized capture impossible > > > > from userspace. > > > > > This is exactly the use case spi_new_ancillary_device() was designed > > > > for - a multi-die device sharing a bus with separate chip selects for > > > > configuration but common data/clock/power infrastructure. > > This... It doesn't fit the data path as far as I read from the above. Right, spi_new_ancillary_device() covers only the config path. The data path is handled entirely by the IIO backend with no SPI involvement at runtime. > > > > See above. > > > > > > > > TO me is either something special about channel 0, then we have to > > > > > synchronise > > > > > accesses, or there is no point to have this patch at all, just make devices > to > > > > > be the same under the hood and describe as independent pair. > > -- > With Best Regards, > Andy Shevchenko > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC 2026-03-17 9:54 ` Miclaus, Antoniu @ 2026-03-17 10:31 ` Andy Shevchenko 2026-03-21 12:01 ` Jonathan Cameron 0 siblings, 1 reply; 20+ messages in thread From: Andy Shevchenko @ 2026-03-17 10:31 UTC (permalink / raw) To: Miclaus, Antoniu Cc: Jonathan Cameron, Lars-Peter Clausen, Hennerich, Michael, David Lechner, Sa, Nuno, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org On Tue, Mar 17, 2026 at 09:54:13AM +0000, Miclaus, Antoniu wrote: > > On Mon, Mar 16, 2026 at 03:09:18PM +0000, Miclaus, Antoniu wrote: > > > > From: Andy Shevchenko <andriy.shevchenko@intel.com> > > > > Sent: Monday, March 16, 2026 4:42 PM > > > > On Mon, Mar 16, 2026 at 12:31:09PM +0000, Miclaus, Antoniu wrote: > > > > > > From: Andy Shevchenko <andriy.shevchenko@intel.com> > > > > > > Sent: Monday, March 16, 2026 11:57 AM > > > > > > On Sat, Mar 14, 2026 at 12:00:22PM +0000, Jonathan Cameron > > wrote: > > > > > > > On Fri, 13 Mar 2026 16:22:53 +0200 > > > > > > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > > > > > > > On Fri, Mar 13, 2026 at 01:58:53PM +0200, Antoniu Miclaus > > wrote: ... > > > > > > > Nope. You are talking to different devices (more or less). > > > > > > > > > > > > If it's a twins in the package, why do we have a special handling and not > > > > just > > > > > > describing two independent devices in the DT/fw? > > > > > > > > > > Because they are not fully independent - they share: > > > > > - Power supplies and voltage reference > > > > > - The CNV clock (conversion trigger) > > > > > > > > Okay, then why not having a core part and a glue driver that registers as > > many > > > > devices as you wish and provides just a common stuff? > > > > > Because the AD4880 is not two independent ADCs sharing a package - it is > > > a single device with a single interleaved data output. Splitting into > > > separate IIO devices would make synchronized dual-channel capture > > > impossible from userspace, which is the primary use case for this part. > > > > Sounds to me like you need, probably, a virtual device for that. > > Maybe even on IIO level. Do we expect more devices like this to > > be enabled in the future (or maybe already in tree, but lacking this > > feature)? > > The AD4880 is a fairly unique part - having separate SPI config > interfaces per channel with a shared interleaved data output is not > a common pattern, and the chances of another device like this being > upstreamed are low. Given that Jonathan has already reviewed and > approved a previous version of this series, and the patch has > collected multiple Reviewed-by tags, I'd prefer to keep the current > approach. This... > > > The shared resources (supplies, CNV clock, interleaved data stream) are > > > not just "glue" - they define the device's operating model. > > > > Sure, like any other resource for MFD (HW speaking). > > MFD models a single device exposing multiple functionally distinct > sub-devices. The AD4880 channels are not distinct sub-devices - they share a > single interleaved data stream, and splitting them would break > synchronized capture. ...and this are important pieces of information. Can you add a summary to v7 commit message explaining on the chosen approach? > > > The per-channel SPI interfaces exist only for register configuration; the > > > actual data path is a single stream handled entirely by the backend. > > > > > > > We have similar (to some extend) cases with SPI/I²C where > > > > drivers/platform/x86/serial-multi-instantiate.c services as "MFD" for that > > > > type of busses. > > > > > > > > > - A single interleaved data output stream > > > > > > > > How does it work in non-racy way? > > > > > > The data path has no software involvement at runtime. The CNV clock > > > triggers both channels to sample simultaneously, and the device outputs > > > the conversion results as a single interleaved bitstream on the data > > > lane(s). The FPGA backend captures this stream directly - no SPI > > > register reads are involved in the data path. The only SPI traffic is > > > for configuration, and each channel has its own independent SPI > > > interface and regmap, so there is no shared bus contention. > > > > Okay, so it's in a way more complex (like a camera sensor in terms of > > data/configuration paths) device. It's now even more looking that the > > current approach is a quick hack rather than a solution to make this > > properly fit Linux device model. > > The driver uses spi_new_ancillary_device() for the config path and > the IIO backend for the data path - both existing kernel > infrastructure used as intended. No custom abstractions are added. Also would be nice to have something from this. So, I think for v7 we need transform this discussion to a summary that covers all aspects: - why MFD box can't be applied here - why regmaps are independent - how does it look with data and configuration paths - the device is unique and we do not expect more to come (or very little chances) - et cetera (or what I forgot to mention) > > > > > Describing them as two independent DT nodes would mean duplicating > > > > > all the shared resources, and more importantly, the data interface > > > > > is a single interleaved stream feeding into one IIO buffer. Having > > > > > two separate IIO devices would make synchronized capture impossible > > > > > from userspace. > > > > > > > This is exactly the use case spi_new_ancillary_device() was designed > > > > > for - a multi-die device sharing a bus with separate chip selects for > > > > > configuration but common data/clock/power infrastructure. > > > > This... It doesn't fit the data path as far as I read from the above. > > Right, spi_new_ancillary_device() covers only the config path. > The data path is handled entirely by the IIO backend with no SPI > involvement at runtime. > > > > > See above. > > > > > > > > > > TO me is either something special about channel 0, then we have to > > > > > > synchronise > > > > > > accesses, or there is no point to have this patch at all, just make devices > > to > > > > > > be the same under the hood and describe as independent pair. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC 2026-03-17 10:31 ` Andy Shevchenko @ 2026-03-21 12:01 ` Jonathan Cameron 0 siblings, 0 replies; 20+ messages in thread From: Jonathan Cameron @ 2026-03-21 12:01 UTC (permalink / raw) To: Andy Shevchenko Cc: Miclaus, Antoniu, Lars-Peter Clausen, Hennerich, Michael, David Lechner, Sa, Nuno, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org On Tue, 17 Mar 2026 12:31:06 +0200 Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > On Tue, Mar 17, 2026 at 09:54:13AM +0000, Miclaus, Antoniu wrote: > > > On Mon, Mar 16, 2026 at 03:09:18PM +0000, Miclaus, Antoniu wrote: > > > > > From: Andy Shevchenko <andriy.shevchenko@intel.com> > > > > > Sent: Monday, March 16, 2026 4:42 PM > > > > > On Mon, Mar 16, 2026 at 12:31:09PM +0000, Miclaus, Antoniu wrote: > > > > > > > From: Andy Shevchenko <andriy.shevchenko@intel.com> > > > > > > > Sent: Monday, March 16, 2026 11:57 AM > > > > > > > On Sat, Mar 14, 2026 at 12:00:22PM +0000, Jonathan Cameron > > > wrote: > > > > > > > > On Fri, 13 Mar 2026 16:22:53 +0200 > > > > > > > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > > > > > > > > On Fri, Mar 13, 2026 at 01:58:53PM +0200, Antoniu Miclaus > > > wrote: > > ... > > > > > > > > > Nope. You are talking to different devices (more or less). > > > > > > > > > > > > > > If it's a twins in the package, why do we have a special handling and not > > > > > just > > > > > > > describing two independent devices in the DT/fw? > > > > > > > > > > > > Because they are not fully independent - they share: > > > > > > - Power supplies and voltage reference > > > > > > - The CNV clock (conversion trigger) > > > > > > > > > > Okay, then why not having a core part and a glue driver that registers as > > > many > > > > > devices as you wish and provides just a common stuff? > > > > > > > Because the AD4880 is not two independent ADCs sharing a package - it is > > > > a single device with a single interleaved data output. Splitting into > > > > separate IIO devices would make synchronized dual-channel capture > > > > impossible from userspace, which is the primary use case for this part. > > > > > > Sounds to me like you need, probably, a virtual device for that. > > > Maybe even on IIO level. Do we expect more devices like this to > > > be enabled in the future (or maybe already in tree, but lacking this > > > feature)? > > > > The AD4880 is a fairly unique part - having separate SPI config > > interfaces per channel with a shared interleaved data output is not > > a common pattern, and the chances of another device like this being > > upstreamed are low. Given that Jonathan has already reviewed and > > approved a previous version of this series, and the patch has > > collected multiple Reviewed-by tags, I'd prefer to keep the current > > approach. > > This... > > > > > The shared resources (supplies, CNV clock, interleaved data stream) are > > > > not just "glue" - they define the device's operating model. > > > > > > Sure, like any other resource for MFD (HW speaking). > > > > MFD models a single device exposing multiple functionally distinct > > sub-devices. The AD4880 channels are not distinct sub-devices - they share a > > single interleaved data stream, and splitting them would break > > synchronized capture. > > ...and this are important pieces of information. Can you add a summary > to v7 commit message explaining on the chosen approach? > > > > > The per-channel SPI interfaces exist only for register configuration; the > > > > actual data path is a single stream handled entirely by the backend. > > > > > > > > > We have similar (to some extend) cases with SPI/I²C where > > > > > drivers/platform/x86/serial-multi-instantiate.c services as "MFD" for that > > > > > type of busses. > > > > > > > > > > > - A single interleaved data output stream > > > > > > > > > > How does it work in non-racy way? > > > > > > > > The data path has no software involvement at runtime. The CNV clock > > > > triggers both channels to sample simultaneously, and the device outputs > > > > the conversion results as a single interleaved bitstream on the data > > > > lane(s). The FPGA backend captures this stream directly - no SPI > > > > register reads are involved in the data path. The only SPI traffic is > > > > for configuration, and each channel has its own independent SPI > > > > interface and regmap, so there is no shared bus contention. In this whole discussion this is the one element that I think maybe needs some additional clarification. From the datasheet it looks to me like there are actually separate data paths per channel. There are separate SPI / LVDS data lines. Its a decision in the backend design / FPGA to fuse them. That is logical because as you've noted the cnv clock is shared. Or is it? In the initial diagram on the datasheet first page it is shown as shared, but there are actually separate CNV_chA+- and CNV_chB+. It's a draft datasheet, so if you can feedback to the author it would be good to make it more consistent! It would be possible to build a similar setup with multiple discrete ADCs connected to a single backend - I just don't think we've seen that done yet. Have I understood the above correctly? My initial thoughts if we did would be a virtual / multiplexer IIO device front end that was a consumer of the two separate IIO ADC device instances (and probably had a path to their shared backend) + those two instances would use the same backend. Thus the front end device could provide the buffer interfaces. Without some prototyping I'm not sure how we'd pull that together. Note that I'm not suggesting this is the time to work out how to solve that setup! To my mind, this driver is effectively performing role of that front end and the individual ADC instance drivers all in one. I think it's reasonable to assume no one is going to buy this chip to use the channels independently and if they do we can later work out how to make that work without this initial path getting much in the way. > > > > > > Okay, so it's in a way more complex (like a camera sensor in terms of > > > data/configuration paths) device. It's now even more looking that the > > > current approach is a quick hack rather than a solution to make this > > > properly fit Linux device model. > > > > The driver uses spi_new_ancillary_device() for the config path and > > the IIO backend for the data path - both existing kernel > > infrastructure used as intended. No custom abstractions are added. > > Also would be nice to have something from this. > > So, I think for v7 we need transform this discussion to a summary that covers > all aspects: > > - why MFD box can't be applied here > - why regmaps are independent > - how does it look with data and configuration paths > - the device is unique and we do not expect more to come (or very little chances) > - et cetera (or what I forgot to mention) Agreed on this list. thanks, Jonathan > > > > > > > Describing them as two independent DT nodes would mean duplicating > > > > > > all the shared resources, and more importantly, the data interface > > > > > > is a single interleaved stream feeding into one IIO buffer. Having > > > > > > two separate IIO devices would make synchronized capture impossible > > > > > > from userspace. > > > > > > > > > This is exactly the use case spi_new_ancillary_device() was designed > > > > > > for - a multi-die device sharing a bus with separate chip selects for > > > > > > configuration but common data/clock/power infrastructure. > > > > > > This... It doesn't fit the data path as far as I read from the above. > > > > Right, spi_new_ancillary_device() covers only the config path. > > The data path is handled entirely by the IIO backend with no SPI > > involvement at runtime. > > > > > > > See above. > > > > > > > > > > > > TO me is either something special about channel 0, then we have to > > > > > > > synchronise > > > > > > > accesses, or there is no point to have this patch at all, just make devices > > > to > > > > > > > be the same under the hood and describe as independent pair. > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v6 0/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC 2026-03-13 11:58 [PATCH v6 0/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC Antoniu Miclaus ` (3 preceding siblings ...) 2026-03-13 11:58 ` [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC Antoniu Miclaus @ 2026-03-14 11:40 ` Jonathan Cameron 2026-03-16 9:35 ` Miclaus, Antoniu 4 siblings, 1 reply; 20+ messages in thread From: Jonathan Cameron @ 2026-03-14 11:40 UTC (permalink / raw) To: Antoniu Miclaus Cc: Lars-Peter Clausen, Michael Hennerich, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio, devicetree, linux-kernel On Fri, 13 Mar 2026 13:58:49 +0200 Antoniu Miclaus <antoniu.miclaus@analog.com> wrote: > Add support for the AD4880, a dual-channel 20-bit 40MSPS SAR ADC from > the same family as AD4080. > > The AD4880 has two independent ADC channels, each with its own SPI > configuration interface and LVDS data output. The driver uses > spi_new_ancillary_device() for the second channel's SPI and requires > two io-backend instances for the data interfaces. > > This series includes: > - Use __free(fwnode_handle) for automatic cleanup in iio backend > - Refactored devm_iio_backend_get_by_index() for multi-channel backend lookup > - DT bindings update for AD4880 > - Driver support for AD4880 > > This series depends on the SPI ancillary device patches already picked > up in Mark Brown's SPI tree (https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git). > Added base-commit in the cover letter below. > > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4880.pdf > > Changes in v6: > - Move fwnode_back declaration back to inline position above > IS_ERR() check (patch 2) > - Rebase on top of broonie/spi for-next and use --base to declare > the SPI dependency (cover letter) Ok. Given there was nothing in v5 to indicate a need to do a v6 you should have replied to that thread to say you were doing so. The fwnode thing was something I said I'd fix up whilst applying. The SPI dependency was a 'do it better next time' comment. I've dropped the v5 version I applied. Now you have questions to answer from Andy on this one. > > Antoniu Miclaus (4): > iio: backend: use __free(fwnode_handle) for automatic cleanup > iio: backend: add devm_iio_backend_get_by_index() > dt-bindings: iio: adc: ad4080: add AD4880 support > iio: adc: ad4080: add support for AD4880 dual-channel ADC > > .../bindings/iio/adc/adi,ad4080.yaml | 53 +++- > drivers/iio/adc/ad4080.c | 230 ++++++++++++++---- > drivers/iio/industrialio-backend.c | 62 +++-- > include/linux/iio/backend.h | 2 + > 4 files changed, 278 insertions(+), 69 deletions(-) > > > base-commit: 2cd3974b9ae59ac731a4792e1608be32621b6e98 > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v6 0/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC 2026-03-14 11:40 ` [PATCH v6 0/4] " Jonathan Cameron @ 2026-03-16 9:35 ` Miclaus, Antoniu 0 siblings, 0 replies; 20+ messages in thread From: Miclaus, Antoniu @ 2026-03-16 9:35 UTC (permalink / raw) To: Jonathan Cameron Cc: Lars-Peter Clausen, Hennerich, Michael, David Lechner, Sa, Nuno, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org > -----Original Message----- > From: Jonathan Cameron <jic23@kernel.org> > Sent: Saturday, March 14, 2026 1:41 PM > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com> > Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael > <Michael.Hennerich@analog.com>; David Lechner <dlechner@baylibre.com>; > Sa, Nuno <Nuno.Sa@analog.com>; Andy Shevchenko <andy@kernel.org>; > Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; > Conor Dooley <conor+dt@kernel.org>; Olivier Moysan > <olivier.moysan@foss.st.com>; linux-iio@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v6 0/4] iio: adc: ad4080: add support for AD4880 dual- > channel ADC > > [External] > > On Fri, 13 Mar 2026 13:58:49 +0200 > Antoniu Miclaus <antoniu.miclaus@analog.com> wrote: > > > Add support for the AD4880, a dual-channel 20-bit 40MSPS SAR ADC from > > the same family as AD4080. > > > > The AD4880 has two independent ADC channels, each with its own SPI > > configuration interface and LVDS data output. The driver uses > > spi_new_ancillary_device() for the second channel's SPI and requires > > two io-backend instances for the data interfaces. > > > > This series includes: > > - Use __free(fwnode_handle) for automatic cleanup in iio backend > > - Refactored devm_iio_backend_get_by_index() for multi-channel backend > lookup > > - DT bindings update for AD4880 > > - Driver support for AD4880 > > > > This series depends on the SPI ancillary device patches already picked > > up in Mark Brown's SPI tree > (https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/gi > t/broonie/spi.git__;!!A3Ni8CS0y2Y!4Ng6JS3ciBPGwHjV- > Kah0kU9afOLLNPuUyEHkg8963vQV9xN3CFZlfzOXhDLETb0oh-- > OL5ap9VvYgc2MQ$ ). > > Added base-commit in the cover letter below. > > > > Datasheet: https://www.analog.com/media/en/technical- > documentation/data-sheets/ad4880.pdf > > > > Changes in v6: > > - Move fwnode_back declaration back to inline position above > > IS_ERR() check (patch 2) > > - Rebase on top of broonie/spi for-next and use --base to declare > > the SPI dependency (cover letter) > > Ok. Given there was nothing in v5 to indicate a need to do a v6 > you should have replied to that thread to say you were doing so. > The fwnode thing was something I said I'd fix up whilst applying. > The SPI dependency was a 'do it better next time' comment. > > I've dropped the v5 version I applied. Now you have questions > to answer from Andy on this one. I saw you answered already some of them. The rest are some minor format stuff. Should I wait for the v6 patchseries to go from "New" state to "Changes Requested" before sending a new v7? I don't want to make the same mistake as I did for v5->v6 😊. Regards, > > > > > Antoniu Miclaus (4): > > iio: backend: use __free(fwnode_handle) for automatic cleanup > > iio: backend: add devm_iio_backend_get_by_index() > > dt-bindings: iio: adc: ad4080: add AD4880 support > > iio: adc: ad4080: add support for AD4880 dual-channel ADC > > > > .../bindings/iio/adc/adi,ad4080.yaml | 53 +++- > > drivers/iio/adc/ad4080.c | 230 ++++++++++++++---- > > drivers/iio/industrialio-backend.c | 62 +++-- > > include/linux/iio/backend.h | 2 + > > 4 files changed, 278 insertions(+), 69 deletions(-) > > > > > > base-commit: 2cd3974b9ae59ac731a4792e1608be32621b6e98 > > -- > > 2.43.0 > > ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2026-03-21 12:02 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-13 11:58 [PATCH v6 0/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC Antoniu Miclaus 2026-03-13 11:58 ` [PATCH v6 1/4] iio: backend: use __free(fwnode_handle) for automatic cleanup Antoniu Miclaus 2026-03-13 14:16 ` Andy Shevchenko 2026-03-14 10:53 ` Nuno Sá 2026-03-13 11:58 ` [PATCH v6 2/4] iio: backend: add devm_iio_backend_get_by_index() Antoniu Miclaus 2026-03-13 14:16 ` Andy Shevchenko 2026-03-13 11:58 ` [PATCH v6 3/4] dt-bindings: iio: adc: ad4080: add AD4880 support Antoniu Miclaus 2026-03-13 11:58 ` [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC Antoniu Miclaus 2026-03-13 14:22 ` Andy Shevchenko 2026-03-14 12:00 ` Jonathan Cameron 2026-03-16 9:56 ` Andy Shevchenko 2026-03-16 12:31 ` Miclaus, Antoniu 2026-03-16 14:41 ` Andy Shevchenko 2026-03-16 15:09 ` Miclaus, Antoniu 2026-03-16 15:21 ` Andy Shevchenko 2026-03-17 9:54 ` Miclaus, Antoniu 2026-03-17 10:31 ` Andy Shevchenko 2026-03-21 12:01 ` Jonathan Cameron 2026-03-14 11:40 ` [PATCH v6 0/4] " Jonathan Cameron 2026-03-16 9:35 ` Miclaus, Antoniu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox