iio: adc: Add support for LTC2378 and similar ADCs
Subsystems (13)
Marcelo Schmitt
maintainerv3 cover iio: adc: Add support for LTC2378 and similar ADCs
This patch series adds support for LTC2378 and similar low noise, low power, high speed, successive approximation register (SAR) ADCs. These ADCs are similar among each other, varying mainly on the amount of precision bits, maximum sample rate, and input configuration (either fully differential or pseudo-differential).
The first patch adds device tree documentation for LTC2378.
The second patch enables single-shot sample read with a GPIO connected to the LTC2378 CNV pin.
The third patch extends IIO DMAengine buffer interface to make sampling_frequency and sampling_frequency_available buffer attributes.
The fourth patch enables high-speed data captures with SPI offloading. The setup is similar to AD4030, with a specialized PWM generator being used both for SPI offload triggering and conversion start signaling.
The last support patch enables running buffered data captures without SPI offloading.
Even though these parts are somewhat similar to AD4000, the wiring configuration for LTC parts is different as well as the available HDL for high speed sample rate mode. Because of that, I propose creating a new device driver for supporting LTC2378-like devices.
Specifications can be found at:
Some structures and variables are introduced earlier to reduce diff in latter patches.
The initial version of the LTC2378 driver was developed by Ioan-Daniel. Though, despite of the many changes I made to the code, I've kept him as module author to provide credit for his work.
Previous submissions:
v2: https://lore.kernel.org/linux-iio/[email protected]/
v1: https://lore.kernel.org/linux-iio/[email protected]/
Change log v2 -> v3:
[DT]
- Re-added device tree fallback compatibles for LTC2378 chips, now with options to provide a single compatible string or a pair of single compatible string plus a fallback string to a slower sample rate spec in case a driver for the specific part is not found.
[IIO]
- Restricted LTC2378 dependency list to require GPIO
- Use datasheet terminology to indicate polarity/output code.
- Added missing block scope to IIO_DEV_ACQUIRE_DIRECT_MODE.
- Added missing static inline modifier to function stubs declared in header file.
- Fixed the evaluation loop conditions for CNV PWM and SPI Engine trigger PWM, avoiding potential infinite loop if and CPU stall.
- Added comment to about initial PWM disable.
- Adjusted SPI offload setup initialization to not print error on a valid condition.
- Fully initialize IIO channel scan_type.
- Reworked to make offload support not imply all dependencies to be built in.
- Made sampling_frequency a buffer attribute.
- Made offload support not require DMA and other features to be built in.
- Now using same scan_type configuration for all use cases.
With best regards,
Marcelo
Marcelo Schmitt (5):
dt-bindings: iio: adc: Add ltc2378
iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs
iio: buffer: Extend DMAengine buffer interfaces to take extra sysfs
attributes
iio: adc: ltc2378: Enable high-speed data capture
iio: adc: ltc2378: Enable triggered buffer data capture
.../bindings/iio/adc/adi,ltc2378.yaml | 160 ++++++++ MAINTAINERS | 8 + drivers/iio/adc/Kconfig | 30 ++ drivers/iio/adc/Makefile | 7 + drivers/iio/adc/ad4000.c | 3 +- drivers/iio/adc/ad4030.c | 3 +- drivers/iio/adc/ad4691.c | 3 +- drivers/iio/adc/ad4695.c | 2 +- drivers/iio/adc/ad7380.c | 2 +- drivers/iio/adc/ad7606_spi.c | 2 +- drivers/iio/adc/ad7768-1.c | 3 +- drivers/iio/adc/ad7944.c | 2 +- drivers/iio/adc/ad_sigma_delta.c | 2 +- drivers/iio/adc/ltc2378-lib-core.c | 50 +++ drivers/iio/adc/ltc2378-offload-buffer.c | 305 ++++++++++++++ drivers/iio/adc/ltc2378-triggered-buffer.c | 49 +++ drivers/iio/adc/ltc2378.c | 384 ++++++++++++++++++ drivers/iio/adc/ltc2378.h | 120 ++++++ .../buffer/industrialio-buffer-dmaengine.c | 19 +- drivers/iio/dac/ad5791.c | 2 +- drivers/iio/dac/ad8460.c | 2 +- drivers/iio/dac/adi-axi-dac.c | 2 +- include/linux/iio/buffer-dmaengine.h | 16 +- 23 files changed, 1151 insertions(+), 25 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml create mode 100644 drivers/iio/adc/ltc2378-lib-core.c create mode 100644 drivers/iio/adc/ltc2378-offload-buffer.c create mode 100644 drivers/iio/adc/ltc2378-triggered-buffer.c create mode 100644 drivers/iio/adc/ltc2378.c create mode 100644 drivers/iio/adc/ltc2378.h
base-commit: a50909aa46dec46de3c73235fc15a7d6f763d996
Signature · 1 line
2.53.0
Marcelo Schmitt
maintainerPATCH v3 1/5 dt-bindings: iio: adc: Add ltc2378
Document how to describe LTC2378-20 and similar ADCs in device tree.
---
Change log v2 -> v3:
- Re-added device tree fallback compatibles for LTC2378 chips, now with options to provide a single compatible string or a pair of single compatible string plus a fallback string to a slower sample rate spec in case a driver for the specific part is not found.
.../bindings/iio/adc/adi,ltc2378.yaml | 160 ++++++++++++++++++ MAINTAINERS | 7 + 2 files changed, 167 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +160 -0
@@ -0,0 +1,160 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/adi,ltc2378.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices LTC2378 and similar Analog to Digital Converters + +maintainers: + - Marcelo Schmitt <[email protected]> + +description: | + Analog Devices LTC2378 series of ADCs. + Specifications can be found at: + https://www.analog.com/media/en/technical-documentation/data-sheets/233818fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236416fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236418f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236716fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236718f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236816f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236818f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236918fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237016fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237616fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237618fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237620fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237716fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237718fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237720fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237816fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237818fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237820fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237918fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/238016fb.pdf + +$ref: /schemas/spi/spi-peripheral-props.yaml# + +properties: + compatible: + oneOf: + # Single compatible string match. + - enum: + - adi,ltc2338-18 + - adi,ltc2364-16 + - adi,ltc2364-18 + - adi,ltc2367-16 + - adi,ltc2367-18 + - adi,ltc2368-16 + - adi,ltc2368-18 + - adi,ltc2369-18 + - adi,ltc2370-16 + - adi,ltc2376-16 + - adi,ltc2376-18 + - adi,ltc2376-20 + - adi,ltc2377-16 + - adi,ltc2377-18 + - adi,ltc2377-20 + - adi,ltc2378-16 + - adi,ltc2378-18 + - adi,ltc2378-20 + - adi,ltc2379-18 + - adi,ltc2380-16 + + # Low sample rate fallback for 16-bit unipolar sensors. + - items: + - enum: + - adi,ltc2370-16 # 2 MSPS + - adi,ltc2368-16 # 1 MSPS + - adi,ltc2367-16 # 500 kSPS + - const: adi,ltc2364-16 # fallback (250 kSPS) + + # Low sample rate fallback for 18-bit unipolar sensors. + - items: + - enum: + - adi,ltc2369-18 # 1.6 MSPS + - adi,ltc2368-18 # 1 MSPS + - adi,ltc2367-18 # 500 kSPS + - const: adi,ltc2364-18 # fallback (250 kSPS) + + # Low sample rate fallback for 16-bit bipolar sensors. + - items: + - enum: + - adi,ltc2380-16 # 2 MSPS + - adi,ltc2378-16 # 1 MSPS + - adi,ltc2377-16 # 500 kSPS + - const: adi,ltc2376-16 # fallback (250 kSPS) + + # Low sample rate fallback for 18-bit bipolar sensors. + - items: + - enum: + - adi,ltc2379-18 # 1.6 MSPS + - adi,ltc2338-18 # 1 MSPS + - adi,ltc2378-18 # 1 MSPS + - adi,ltc2377-18 # 500 kSPS + - const: adi,ltc2376-18 # fallback (250 kSPS) + + # Low sample rate fallback for 20-bit bipolar sensors. + - items: + - enum: + - adi,ltc2378-20 # 1 MSPS + - adi,ltc2377-20 # 500 kSPS + - const: adi,ltc2376-20 # fallback (250 kSPS) + + reg: + maxItems: 1 + + spi-max-frequency: + maximum: 100000000 + + vdd-supply: + description: A 2.5V supply that powers the chip (VDD). + + ovdd-supply: + description: + A 1.71V to 5.25V supply that sets the logic level for digital interface. + + ref-supply: + description: + A 2.5V to 5.1V supply for the reference input (REF). + + cnv-gpios: + description: + When provided, this property indicates the GPIO that is connected to the + CNV pin. + maxItems: 1 + + interrupts: + description: + Interrupt for signaling the completion of conversion results. The active + low signal provided on the BUSY pin asserts when ADC conversions finish. + maxItems: 1 + +required: + - compatible + - reg + - vdd-supply + - ovdd-supply + - ref-supply + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/interrupt-controller/irq.h> + spi { + #address-cells = <1>; + #size-cells = <0>; + adc@0 { + compatible = "adi,ltc2378-20", "adi,ltc2376-20"; + reg = <0>; + spi-max-frequency = <71000000>; + vdd-supply = <&supply_2_5V>; + ovdd-supply = <&supply_3_3V>; + ref-supply = <&supply_5V>; + cnv-gpios = <&gpio0 88 GPIO_ACTIVE_HIGH>; + interrupts = <7 IRQ_TYPE_EDGE_FALLING>; + interrupt-parent = <&gpio>; + }; + };
MAINTAINERS +7 -0
@@ -15220,6 +15220,13 @@ S: Maintained F: Documentation/devicetree/bindings/iio/dac/lltc,ltc1660.yaml F: drivers/iio/dac/ltc1660.c +LTC2378 IIO ADC DRIVER +M: Marcelo Schmitt <[email protected]> +L: [email protected] +S: Supported +W: https://ez.analog.com/linux-software-drivers +F: Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml + LTC2664 IIO DAC DRIVER M: Michael Hennerich <[email protected]> M: Kim Seer Paller <[email protected]>
Signature · 1 line
2.53.0
Re: [PATCH v3 1/5] dt-bindings: iio: adc: Add ltc2378
Quoted 81 lines · Marcelo Schmitt
Document how to describe LTC2378-20 and similar ADCs in device tree.
Signed-off-by Marcelo Schmitt <[email protected]>---
Change log v2 -> v3:
- Re-added device tree fallback compatibles for LTC2378 chips, now with options to provide a single compatible string or a pair of single compatible string plus a fallback string to a slower sample rate spec in case a driver for the specific part is not found..../bindings/iio/adc/adi,ltc2378.yaml | 160 ++++++++++++++++++ MAINTAINERS | 7 + 2 files changed, 167 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yamlDocumentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +60
-0@@ -0,0 +1,160 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/adi,ltc2378.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices LTC2378 and similar Analog to Digital Converters + +maintainers: + - Marcelo Schmitt <[email protected]> + +description: | + Analog Devices LTC2378 series of ADCs. + Specifications can be found at: + https://www.analog.com/media/en/technical-documentation/data-sheets/233818fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236416fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236418f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236716fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236718f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236816f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236818f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236918fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237016fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237616fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237618fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237620fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237716fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237718fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237720fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237816fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237818fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237820fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237918fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/238016fb.pdf + +$ref: /schemas/spi/spi-peripheral-props.yaml# + +properties: + compatible: + oneOf: + # Single compatible string match. + - enum: + - adi,ltc2338-18 + - adi,ltc2364-16 + - adi,ltc2364-18 + - adi,ltc2367-16 + - adi,ltc2367-18 + - adi,ltc2368-16 + - adi,ltc2368-18 + - adi,ltc2369-18 + - adi,ltc2370-16 + - adi,ltc2376-16 + - adi,ltc2376-18 + - adi,ltc2376-20 + - adi,ltc2377-16 + - adi,ltc2377-18 + - adi,ltc2377-20 + - adi,ltc2378-16 + - adi,ltc2378-18 + - adi,ltc2378-20
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml line 70
Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +10
-0+ - adi,ltc2379-18 + - adi,ltc2380-16 + + # Low sample rate fallback for 16-bit unipolar sensors. + - items: + - enum: + - adi,ltc2370-16 # 2 MSPS + - adi,ltc2368-16 # 1 MSPS + - adi,ltc2367-16 # 500 kSPS + - const: adi,ltc2364-16 # fallback (250 kSPS)
Your driver still matches on ltc2370-16, which makes me question the value of these fallbacks. That said, the chip info struct contains no information about sampling rate. What actually is the impact of the sample rate on the programming model?
Is there actually a benefit to matching on ltc2370-16, or can you just match on the fallback?
+static const struct ltc2378_chip_info ltc2370_16_chip_info = {
+ .name = "ltc2370-16",
+ .resolution = 16,
+ .bipolar = false,
+};
+static const struct ltc2378_chip_info ltc2368_16_chip_info = {
+ .name = "ltc2368-16",
+ .resolution = 16,
+ .bipolar = false,
+};
+static const struct ltc2378_chip_info ltc2367_16_chip_info = {
+ .name = "ltc2367-16",
+ .resolution = 16,
+ .bipolar = false,
+};
+static const struct ltc2378_chip_info ltc2364_16_chip_info = {
+ .name = "ltc2364-16",
+ .resolution = 16,
+ .bipolar = false,
+};
All the devices have the same match data, other than the name, as the fallback.
Quoted 22 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +22
-0+ + # Low sample rate fallback for 18-bit unipolar sensors. + - items: + - enum: + - adi,ltc2369-18 # 1.6 MSPS + - adi,ltc2368-18 # 1 MSPS + - adi,ltc2367-18 # 500 kSPS + - const: adi,ltc2364-18 # fallback (250 kSPS) + + # Low sample rate fallback for 16-bit bipolar sensors. + - items: + - enum: + - adi,ltc2380-16 # 2 MSPS + - adi,ltc2378-16 # 1 MSPS + - adi,ltc2377-16 # 500 kSPS + - const: adi,ltc2376-16 # fallback (250 kSPS) + + # Low sample rate fallback for 18-bit bipolar sensors. + - items: + - enum: + - adi,ltc2379-18 # 1.6 MSPS + - adi,ltc2338-18 # 1 MSPS
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml line 102
Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +10
-0+ - adi,ltc2378-18 # 1 MSPS + - adi,ltc2377-18 # 500 kSPS + - const: adi,ltc2376-18 # fallback (250 kSPS) + + # Low sample rate fallback for 20-bit bipolar sensors. + - items: + - enum: + - adi,ltc2378-20 # 1 MSPS + - adi,ltc2377-20 # 500 kSPS + - const: adi,ltc2376-20 # fallback (250 kSPS)
I didn't check these, but I assume they are the same.
Re: [PATCH v3 1/5] dt-bindings: iio: adc: Add ltc2378
Quoted 88 lines · Conor Dooley
Quoted 87 lines · Marcelo Schmitt
Document how to describe LTC2378-20 and similar ADCs in device tree.
Signed-off-by Marcelo Schmitt <[email protected]>---
Change log v2 -> v3:
- Re-added device tree fallback compatibles for LTC2378 chips, now with options to provide a single compatible string or a pair of single compatible string plus a fallback string to a slower sample rate spec in case a driver for the specific part is not found..../bindings/iio/adc/adi,ltc2378.yaml | 160 ++++++++++++++++++ MAINTAINERS | 7 + 2 files changed, 167 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yamlDocumentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +66
-0@@ -0,0 +1,160 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/adi,ltc2378.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices LTC2378 and similar Analog to Digital Converters + +maintainers: + - Marcelo Schmitt <[email protected]> + +description: | + Analog Devices LTC2378 series of ADCs. + Specifications can be found at: + https://www.analog.com/media/en/technical-documentation/data-sheets/233818fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236416fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236418f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236716fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236718f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236816f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236818f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236918fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237016fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237616fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237618fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237620fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237716fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237718fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237720fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237816fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237818fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237820fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237918fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/238016fb.pdf + +$ref: /schemas/spi/spi-peripheral-props.yaml# + +properties: + compatible: + oneOf: + # Single compatible string match. + - enum: + - adi,ltc2338-18 + - adi,ltc2364-16 + - adi,ltc2364-18 + - adi,ltc2367-16 + - adi,ltc2367-18 + - adi,ltc2368-16 + - adi,ltc2368-18 + - adi,ltc2369-18 + - adi,ltc2370-16 + - adi,ltc2376-16 + - adi,ltc2376-18 + - adi,ltc2376-20 + - adi,ltc2377-16 + - adi,ltc2377-18 + - adi,ltc2377-20 + - adi,ltc2378-16 + - adi,ltc2378-18 + - adi,ltc2378-20 + - adi,ltc2379-18 + - adi,ltc2380-16 + + # Low sample rate fallback for 16-bit unipolar sensors. + - items: + - enum:
Quoted 8 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml line 70
Quoted 4 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +4
-0+ - adi,ltc2370-16 # 2 MSPS + - adi,ltc2368-16 # 1 MSPS + - adi,ltc2367-16 # 500 kSPS + - const: adi,ltc2364-16 # fallback (250 kSPS)Your driver still matches on ltc2370-16, which makes me question the value of these fallbacks. That said, the chip info struct contains no information about sampling rate.
The info about sample rate is added in patch 4.
Quoted 1 line
What actually is the impact of the sample rate on the programming model?
If the user tries to set a sample rate beyond the maximum supported, software can throw an error to indicate that.
Quoted 2 lines
Is there actually a benefit to matching on ltc2370-16, or can you just match on the fallback?
The benefit is telling software that it is safe to go up to 2 MSPS. It is also okay to match on the fallback, but software may restrain operation to a slower sample rate.
With the code being proposed in this patch set, there is no benefit in matching the ltc2364-16 fallback if there is a faster ltc2370-16 device connected because device driver supports that. Though, other operating systems and/or platforms might not support all device variants and so the fallback might be useful.
Review
Quoted 3 lines
(quoted diff) +3
-0+static const struct ltc2378_chip_info ltc2370_16_chip_info = { + .name = "ltc2370-16", + .resolution = 16,
In a later patch ...
+ .max_sample_rate_hz = 2 * HZ_PER_MHZ,
+ .tconv_ns = 322,
Review
Quoted 6 lines
(quoted diff) +5
-0+ .bipolar = false, +}; +static const struct ltc2378_chip_info ltc2368_16_chip_info = { + .name = "ltc2368-16", + .resolution = 16,
+ .max_sample_rate_hz = HZ_PER_MHZ,
+ .tconv_ns = 527
Review
Quoted 6 lines
(quoted diff) +5
-0+ .bipolar = false, +}; +static const struct ltc2378_chip_info ltc2367_16_chip_info = { + .name = "ltc2367-16", + .resolution = 16,
+ .max_sample_rate_hz = 500 * HZ_PER_KHZ,
+ .tconv_ns = 1500,
Review
Quoted 6 lines
(quoted diff) +5
-0+ .bipolar = false, +}; +static const struct ltc2378_chip_info ltc2364_16_chip_info = { + .name = "ltc2364-16", + .resolution = 16,
+ .max_sample_rate_hz = 250 * HZ_PER_KHZ,
+ .tconv_ns = 3000,
Quoted 3 lines
(quoted diff) +2
-0+ .bipolar = false, +};
...
Quoted 8 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml line 102
Quoted 6 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +6
-0+ # Low sample rate fallback for 20-bit bipolar sensors. + - items: + - enum: + - adi,ltc2378-20 # 1 MSPS + - adi,ltc2377-20 # 500 kSPS + - const: adi,ltc2376-20 # fallback (250 kSPS)I didn't check these, but I assume they are the same.
Yes, except for small variations on input (unipolar/bipolar) and precision bits, these devices are pretty much equal.
Re: [PATCH v3 1/5] dt-bindings: iio: adc: Add ltc2378
Quoted 97 lines · Marcelo Schmitt
Quoted 88 lines · Conor Dooley
Quoted 87 lines · Marcelo Schmitt
Document how to describe LTC2378-20 and similar ADCs in device tree.
Signed-off-by Marcelo Schmitt <[email protected]>---
Change log v2 -> v3:
- Re-added device tree fallback compatibles for LTC2378 chips, now with options to provide a single compatible string or a pair of single compatible string plus a fallback string to a slower sample rate spec in case a driver for the specific part is not found..../bindings/iio/adc/adi,ltc2378.yaml | 160 ++++++++++++++++++ MAINTAINERS | 7 + 2 files changed, 167 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yamlDocumentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +66
-0@@ -0,0 +1,160 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/adi,ltc2378.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices LTC2378 and similar Analog to Digital Converters + +maintainers: + - Marcelo Schmitt <[email protected]> + +description: | + Analog Devices LTC2378 series of ADCs. + Specifications can be found at: + https://www.analog.com/media/en/technical-documentation/data-sheets/233818fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236416fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236418f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236716fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236718f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236816f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236818f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236918fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237016fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237616fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237618fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237620fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237716fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237718fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237720fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237816fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237818fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237820fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237918fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/238016fb.pdf + +$ref: /schemas/spi/spi-peripheral-props.yaml# + +properties: + compatible: + oneOf: + # Single compatible string match. + - enum: + - adi,ltc2338-18 + - adi,ltc2364-16 + - adi,ltc2364-18 + - adi,ltc2367-16 + - adi,ltc2367-18 + - adi,ltc2368-16 + - adi,ltc2368-18 + - adi,ltc2369-18 + - adi,ltc2370-16 + - adi,ltc2376-16 + - adi,ltc2376-18 + - adi,ltc2376-20 + - adi,ltc2377-16 + - adi,ltc2377-18 + - adi,ltc2377-20 + - adi,ltc2378-16 + - adi,ltc2378-18 + - adi,ltc2378-20 + - adi,ltc2379-18 + - adi,ltc2380-16 + + # Low sample rate fallback for 16-bit unipolar sensors. + - items: + - enum:Quoted 8 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml line 70
Quoted 4 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +4
-0+ - adi,ltc2370-16 # 2 MSPS + - adi,ltc2368-16 # 1 MSPS + - adi,ltc2367-16 # 500 kSPS + - const: adi,ltc2364-16 # fallback (250 kSPS)Your driver still matches on ltc2370-16, which makes me question the value of these fallbacks. That said, the chip info struct contains no information about sampling rate.
Quoted 1 line
The info about sample rate is added in patch 4.
Ah, I missed that.
Quoted 3 lines
Quoted 1 line
What actually is the impact of the sample rate on the programming model?
If the user tries to set a sample rate beyond the maximum supported, software can throw an error to indicate that.
I think you have misunderstood this. I was asking how the driver interacts with the hardware. Do all devices come out of reset with the minimum sampling rate? Or a per-device default rate? Do the faster devices support all slower rates? If they do, do the registers have the same meaning and the same value sets 2370-16 device to 1 MSPS as 2368-16.
Quoted 5 lines
Quoted 2 lines
Is there actually a benefit to matching on ltc2370-16, or can you just match on the fallback?
The benefit is telling software that it is safe to go up to 2 MSPS. It is also okay to match on the fallback, but software may restrain operation to a slower sample rate.
My whole review here is shit cos I didn't see patch 4, so the use of fallbacks seems appropriate (provided your answers to my programming model questions are sane!).
Quoted 47 lines
With the code being proposed in this patch set, there is no benefit in matching the ltc2364-16 fallback if there is a faster ltc2370-16 device connected because device driver supports that. Though, other operating systems and/or platforms might not support all device variants and so the fallback might be useful.
Review
Quoted 3 lines
(quoted diff) +3
-0+static const struct ltc2378_chip_info ltc2370_16_chip_info = { + .name = "ltc2370-16", + .resolution = 16,In a later patch ...
(quoted diff) +2
-0+ .max_sample_rate_hz = 2 * HZ_PER_MHZ, + .tconv_ns = 322,Quoted 6 lines
(quoted diff) +5
-0+ .bipolar = false, +}; +static const struct ltc2378_chip_info ltc2368_16_chip_info = { + .name = "ltc2368-16", + .resolution = 16,(quoted diff) +2
-0+ .max_sample_rate_hz = HZ_PER_MHZ, + .tconv_ns = 527Quoted 6 lines
(quoted diff) +5
-0+ .bipolar = false, +}; +static const struct ltc2378_chip_info ltc2367_16_chip_info = { + .name = "ltc2367-16", + .resolution = 16,(quoted diff) +2
-0+ .max_sample_rate_hz = 500 * HZ_PER_KHZ, + .tconv_ns = 1500,Quoted 6 lines
(quoted diff) +5
-0+ .bipolar = false, +}; +static const struct ltc2378_chip_info ltc2364_16_chip_info = { + .name = "ltc2364-16", + .resolution = 16,(quoted diff) +2
-0+ .max_sample_rate_hz = 250 * HZ_PER_KHZ, + .tconv_ns = 3000,Quoted 3 lines
(quoted diff) +2
-0+ .bipolar = false, +};...
Quoted 7 lines
Quoted 6 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +6
-0+ # Low sample rate fallback for 20-bit bipolar sensors. + - items: + - enum: + - adi,ltc2378-20 # 1 MSPS + - adi,ltc2377-20 # 500 kSPS + - const: adi,ltc2376-20 # fallback (250 kSPS)
Quoted 4 lines
Quoted 1 line
I didn't check these, but I assume they are the same.
Yes, except for small variations on input (unipolar/bipolar) and precision bits, these devices are pretty much equal.
What I meant here was that 2378-20, 2377-20 and 2376-20 would follow the same pattern of having identical match data (pre patch 4).
Re: [PATCH v3 1/5] dt-bindings: iio: adc: Add ltc2378
Quoted 95 lines · Conor Dooley
Quoted 94 lines · Marcelo Schmitt
Quoted 93 lines · Conor Dooley
Quoted 91 lines · Marcelo Schmitt
Document how to describe LTC2378-20 and similar ADCs in device tree.
Signed-off-by Marcelo Schmitt <[email protected]>---
Change log v2 -> v3:
- Re-added device tree fallback compatibles for LTC2378 chips, now with options to provide a single compatible string or a pair of single compatible string plus a fallback string to a slower sample rate spec in case a driver for the specific part is not found..../bindings/iio/adc/adi,ltc2378.yaml | 160 ++++++++++++++++++ MAINTAINERS | 7 + 2 files changed, 167 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yamlDocumentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +70
-0@@ -0,0 +1,160 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/adi,ltc2378.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices LTC2378 and similar Analog to Digital Converters + +maintainers: + - Marcelo Schmitt <[email protected]> + +description: | + Analog Devices LTC2378 series of ADCs. + Specifications can be found at: + https://www.analog.com/media/en/technical-documentation/data-sheets/233818fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236416fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236418f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236716fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236718f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236816f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236818f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236918fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237016fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237616fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237618fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237620fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237716fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237718fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237720fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237816fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237818fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237820fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237918fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/238016fb.pdf + +$ref: /schemas/spi/spi-peripheral-props.yaml# + +properties: + compatible: + oneOf: + # Single compatible string match. + - enum: + - adi,ltc2338-18 + - adi,ltc2364-16 + - adi,ltc2364-18 + - adi,ltc2367-16 + - adi,ltc2367-18 + - adi,ltc2368-16 + - adi,ltc2368-18 + - adi,ltc2369-18 + - adi,ltc2370-16 + - adi,ltc2376-16 + - adi,ltc2376-18 + - adi,ltc2376-20 + - adi,ltc2377-16 + - adi,ltc2377-18 + - adi,ltc2377-20 + - adi,ltc2378-16 + - adi,ltc2378-18 + - adi,ltc2378-20 + - adi,ltc2379-18 + - adi,ltc2380-16 + + # Low sample rate fallback for 16-bit unipolar sensors. + - items: + - enum: + - adi,ltc2370-16 # 2 MSPS + - adi,ltc2368-16 # 1 MSPS + - adi,ltc2367-16 # 500 kSPS + - const: adi,ltc2364-16 # fallback (250 kSPS)
...
Quoted 10 lines
Quoted 3 lines
Quoted 1 line
What actually is the impact of the sample rate on the programming model?
If the user tries to set a sample rate beyond the maximum supported, software can throw an error to indicate that.
I think you have misunderstood this. I was asking how the driver interacts with the hardware. Do all devices come out of reset with the minimum sampling rate? Or a per-device default rate? Do the faster devices support all slower rates? If they do, do the registers have the same meaning and the same value sets 2370-16 device to 1 MSPS as 2368-16.
Ah, the sampling rate is indirectly dictated both by how fast the CNV pin is toggled and how fast SPI transfers run. Each rising edge of CNV starts a new ADC conversion. Software may take spi-max-frequency as default but it will also need to set a pace for CNV, and there is no default for that. The CNV pin may also toggled on demand (e.g. single-shot read) and so I'd say all devices support all slower sample rates. What software ought to do is orchestrate CNV and SPI transfers to meet the timing requirements. No configuration registers to be set.
Quoted 47 lines
Quoted 47 lines
With the code being proposed in this patch set, there is no benefit in matching the ltc2364-16 fallback if there is a faster ltc2370-16 device connected because device driver supports that. Though, other operating systems and/or platforms might not support all device variants and so the fallback might be useful.
Review
Quoted 3 lines
(quoted diff) +3
-0+static const struct ltc2378_chip_info ltc2370_16_chip_info = { + .name = "ltc2370-16", + .resolution = 16,In a later patch ...
(quoted diff) +2
-0+ .max_sample_rate_hz = 2 * HZ_PER_MHZ, + .tconv_ns = 322,Quoted 6 lines
(quoted diff) +5
-0+ .bipolar = false, +}; +static const struct ltc2378_chip_info ltc2368_16_chip_info = { + .name = "ltc2368-16", + .resolution = 16,(quoted diff) +2
-0+ .max_sample_rate_hz = HZ_PER_MHZ, + .tconv_ns = 527Quoted 6 lines
(quoted diff) +5
-0+ .bipolar = false, +}; +static const struct ltc2378_chip_info ltc2367_16_chip_info = { + .name = "ltc2367-16", + .resolution = 16,(quoted diff) +2
-0+ .max_sample_rate_hz = 500 * HZ_PER_KHZ, + .tconv_ns = 1500,Quoted 6 lines
(quoted diff) +5
-0+ .bipolar = false, +}; +static const struct ltc2378_chip_info ltc2364_16_chip_info = { + .name = "ltc2364-16", + .resolution = 16,(quoted diff) +2
-0+ .max_sample_rate_hz = 250 * HZ_PER_KHZ, + .tconv_ns = 3000,Quoted 3 lines
(quoted diff) +2
-0+ .bipolar = false, +};...
Quoted 7 lines
Quoted 6 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +6
-0+ # Low sample rate fallback for 20-bit bipolar sensors. + - items: + - enum: + - adi,ltc2378-20 # 1 MSPS + - adi,ltc2377-20 # 500 kSPS + - const: adi,ltc2376-20 # fallback (250 kSPS)
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
I didn't check these, but I assume they are the same.
Yes, except for small variations on input (unipolar/bipolar) and precision bits, these devices are pretty much equal.
What I meant here was that 2378-20, 2377-20 and 2376-20 would follow the same pattern of having identical match data (pre patch 4).
Yes, devices that have same input type and precision bits have the same match data configuration (except for name). That's because the initial support patch only supports single-shot data read.
Conor Dooley
Reviewed-byRe: [PATCH v3 1/5] dt-bindings: iio: adc: Add ltc2378
Quoted 108 lines · Marcelo Schmitt
Quoted 95 lines · Conor Dooley
Quoted 94 lines · Marcelo Schmitt
Quoted 93 lines · Conor Dooley
Quoted 91 lines · Marcelo Schmitt
Document how to describe LTC2378-20 and similar ADCs in device tree.
Signed-off-by Marcelo Schmitt <[email protected]>---
Change log v2 -> v3:
- Re-added device tree fallback compatibles for LTC2378 chips, now with options to provide a single compatible string or a pair of single compatible string plus a fallback string to a slower sample rate spec in case a driver for the specific part is not found..../bindings/iio/adc/adi,ltc2378.yaml | 160 ++++++++++++++++++ MAINTAINERS | 7 + 2 files changed, 167 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yamlDocumentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +70
-0@@ -0,0 +1,160 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/adi,ltc2378.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices LTC2378 and similar Analog to Digital Converters + +maintainers: + - Marcelo Schmitt <[email protected]> + +description: | + Analog Devices LTC2378 series of ADCs. + Specifications can be found at: + https://www.analog.com/media/en/technical-documentation/data-sheets/233818fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236416fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236418f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236716fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236718f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236816f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236818f.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/236918fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237016fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237616fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237618fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237620fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237716fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237718fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237720fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237816fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237818fa.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237820fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/237918fb.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/238016fb.pdf + +$ref: /schemas/spi/spi-peripheral-props.yaml# + +properties: + compatible: + oneOf: + # Single compatible string match. + - enum: + - adi,ltc2338-18 + - adi,ltc2364-16 + - adi,ltc2364-18 + - adi,ltc2367-16 + - adi,ltc2367-18 + - adi,ltc2368-16 + - adi,ltc2368-18 + - adi,ltc2369-18 + - adi,ltc2370-16 + - adi,ltc2376-16 + - adi,ltc2376-18 + - adi,ltc2376-20 + - adi,ltc2377-16 + - adi,ltc2377-18 + - adi,ltc2377-20 + - adi,ltc2378-16 + - adi,ltc2378-18 + - adi,ltc2378-20 + - adi,ltc2379-18 + - adi,ltc2380-16 + + # Low sample rate fallback for 16-bit unipolar sensors. + - items: + - enum: + - adi,ltc2370-16 # 2 MSPS + - adi,ltc2368-16 # 1 MSPS + - adi,ltc2367-16 # 500 kSPS + - const: adi,ltc2364-16 # fallback (250 kSPS)...
Quoted 10 lines
Quoted 3 lines
Quoted 1 line
What actually is the impact of the sample rate on the programming model?
If the user tries to set a sample rate beyond the maximum supported, software can throw an error to indicate that.
I think you have misunderstood this. I was asking how the driver interacts with the hardware. Do all devices come out of reset with the minimum sampling rate? Or a per-device default rate? Do the faster devices support all slower rates? If they do, do the registers have the same meaning and the same value sets 2370-16 device to 1 MSPS as 2368-16.
Quoted 8 lines
Ah, the sampling rate is indirectly dictated both by how fast the CNV pin is toggled and how fast SPI transfers run. Each rising edge of CNV starts a new ADC conversion. Software may take spi-max-frequency as default but it will also need to set a pace for CNV, and there is no default for that. The CNV pin may also toggled on demand (e.g. single-shot read) and so I'd say all devices support all slower sample rates. What software ought to do is orchestrate CNV and SPI transfers to meet the timing requirements. No configuration registers to be set.
I forget if I asked for other changes, but the compatible setup then is
David Lechner
reviewerRe: [PATCH v3 1/5] dt-bindings: iio: adc: Add ltc2378
In reply to Marcelo SchmittQuoted 2 lines · Marcelo Schmitt
Document how to describe LTC2378-20 and similar ADCs in device tree.
...
Quoted 6 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +6
-0+ vdd-supply: + description: A 2.5V supply that powers the chip (VDD). + + ovdd-supply: + description: + A 1.71V to 5.25V supply that sets the logic level for digital interface.
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml line 125
Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +10
-0+ + ref-supply: + description: + A 2.5V to 5.1V supply for the reference input (REF). + + cnv-gpios: + description: + When provided, this property indicates the GPIO that is connected to the + CNV pin. + maxItems: 1
Missing pwms property for when CNV pin is connected to PWM.
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml line 138
Quoted 13 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +13
-0+ + interrupts: + description: + Interrupt for signaling the completion of conversion results. The active + low signal provided on the BUSY pin asserts when ADC conversions finish. + maxItems: 1 + +required: + - compatible + - reg + - vdd-supply + - ovdd-supply + - ref-supply
I only looked at LTC2338-18, but it has a REFIN, which is optional. Nothing named REF that is required.
Re: [PATCH v3 1/5] dt-bindings: iio: adc: Add ltc2378
Quoted 16 lines · David Lechner
Quoted 2 lines · Marcelo Schmitt
Document how to describe LTC2378-20 and similar ADCs in device tree.
...
Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +10
-0+ vdd-supply: + description: A 2.5V supply that powers the chip (VDD). + + ovdd-supply: + description: + A 1.71V to 5.25V supply that sets the logic level for digital interface. + + ref-supply: + description: + A 2.5V to 5.1V supply for the reference input (REF).
Quoted 8 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml line 125
Quoted 6 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +6
-0+ + cnv-gpios: + description: + When provided, this property indicates the GPIO that is connected to the + CNV pin. + maxItems: 1Missing pwms property for when CNV pin is connected to PWM.
Ah, thanks for pointing that out. Will add that in v4.
Quoted 18 lines
Quoted 4 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +3
-0+ + interrupts: + description:Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml line 138
Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml new file +10
-0+ Interrupt for signaling the completion of conversion results. The active + low signal provided on the BUSY pin asserts when ADC conversions finish. + maxItems: 1 + +required: + - compatible + - reg + - vdd-supply + - ovdd-supply + - ref-supplyI only looked at LTC2338-18, but it has a REFIN, which is optional. Nothing named REF that is required.
Hmm, I think that's the only device that has it differently. Will add allOf/if properties to differentiate these.
Thanks,
Marcelo
Marcelo Schmitt
maintainerPATCH v3 2/5 iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs
In reply to Marcelo SchmittSupport for LTC2378-20 and similar analog-to-digital converters.
---
Change log v2 -> v3:
- Restricted LTC2378 dependency list to require GPIO
- Use datasheet terminology to indicate polarity/output code.
- Added missing block scope to IIO_DEV_ACQUIRE_DIRECT_MODE.
MAINTAINERS | 1 + drivers/iio/adc/Kconfig | 11 ++ drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ltc2378.c | 338 ++++++++++++++++++++++++++++++++++++++ drivers/iio/adc/ltc2378.h | 63 +++++++ 5 files changed, 414 insertions(+) create mode 100644 drivers/iio/adc/ltc2378.c create mode 100644 drivers/iio/adc/ltc2378.h
MAINTAINERS +1 -0
@@ -15226,6 +15226,7 @@ L: [email protected] S: Supported W: https://ez.analog.com/linux-software-drivers F: Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml +F: drivers/iio/adc/ltc2378* LTC2664 IIO DAC DRIVER M: Michael Hennerich <[email protected]>
drivers/iio/adc/Kconfig +11 -0
@@ -940,6 +940,17 @@ config LTC2309 This driver can also be built as a module. If so, the module will be called ltc2309. +config LTC2378 + tristate "Analog Devices LTC2378 ADC driver" + depends on SPI + depends on GPIOLIB + help + Say yes here to build support for Analog Devices LTC2378-20 and + similar analog to digital converters. + + This driver can also be built as a module. If so, the module will + be called ltc2378. + config LTC2471 tristate "Linear Technology LTC2471 and LTC2473 ADC driver" depends on I2C
drivers/iio/adc/Makefile +1 -0
@@ -81,6 +81,7 @@ obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o obj-$(CONFIG_LTC2309) += ltc2309.o +obj-$(CONFIG_LTC2378) += ltc2378.o obj-$(CONFIG_LTC2471) += ltc2471.o obj-$(CONFIG_LTC2485) += ltc2485.o obj-$(CONFIG_LTC2496) += ltc2496.o ltc2497-core.o
drivers/iio/adc/ltc2378.c new file +338 -0
338 lines not on this page · raw message
drivers/iio/adc/ltc2378.h new file +63 -0
63 lines not on this page · raw message
Signature · 1 line
2.53.0
[email protected]
AI reviewRe: [PATCH v3 2/5] iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider: - [High] Unsupported SPI bits_per_word values for 18-bit and 20-bit ADCs. - [High] Logic Error: Keeping CNV…
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Unsupported SPI bits_per_word values for 18-bit and 20-bit ADCs.
- [High] Logic Error: Keeping CNV high during SPI transfer outputs BUSY flags instead of data.
- [High] Data Corruption: Buffered reads for 16-bit parts always return 0 on big-endian architectures.
- [Medium] ABI Violation: Non-standard sysfs attribute name without channel index.
- [Medium] Missing Kconfig dependency on REGULATOR causes unconditional probe failure. --
commit 30af41465781d1df468bbb782a984208c0789b78
Author: Marcelo Schmitt <[email protected]>
iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs
Support for LTC2378-20 and similar analog-to-digital converters. Co-developed by Ioan-Daniel Pop.
Quoted 4 lines
drivers/iio/adc/Kconfig +0
-0
[ ... ]
drivers/iio/adc/Kconfig config LTC2309 · line 946
Quoted 8 lines
drivers/iio/adc/Kconfig +4
-0@@ -940,6 +940,17 @@ config LTC2309 This driver can also be built as a module. If so, the module will be called ltc2309. +config LTC2378 + tristate "Analog Devices LTC2378 ADC driver" + depends on SPI + depends on GPIOLIB
[Severity: Medium]
Should this Kconfig entry also depend on REGULATOR?
The probe function unconditionally calls devm_regulator_get_enable_read_voltage() and fails if it returns an error. If the kernel is built with CONFIG_REGULATOR disabled, the regulator API is stubbed out to return -ENODEV, making it structurally impossible for the driver to successfully probe.
Quoted 5 lines
drivers/iio/adc/ltc2378.c new file +0
-0
[ ... ]
Quoted 2 lines
drivers/iio/adc/ltc2378.c new file +2
-0+static int ltc2378_probe (struct spi_device *spi) +{
[ ... ]
drivers/iio/adc/ltc2378.c line 250
Quoted 5 lines
drivers/iio/adc/ltc2378.c new file +5
-0+ st->chans[st->num_iio_chans++] = (struct iio_chan_spec ) { + .type =IIO_VOLTAGE , + .info_mask_separate =BIT (IIO_CHAN_INFO_RAW ) | +BIT (IIO_CHAN_INFO_SCALE ), + .scan_index =0 ,
[Severity: Medium]
Does this channel specification need .indexed = 1 and .channel = 0?
Without the index, the IIO core exposes the sysfs attribute as in_voltage_raw instead of in_voltage0_raw. The IIO ABI documentation strictly mandates that the number must always be specified and unique to allow association with event codes, so this non-standard naming breaks compatibility with generic userspace clients.
drivers/iio/adc/ltc2378.c line 262
Quoted 12 lines
drivers/iio/adc/ltc2378.c new file +12
-0+ .scan_type = { + .format = st->info->bipolar ?IIO_SCAN_FORMAT_SIGNED_INT : + IIO_SCAN_FORMAT_UNSIGNED_INT, + .realbits = st->info->resolution, +/* +* Buffer elements could be 16-bit for low precision +* parts. Though, using more storage bits allows keeping +* the same scan_type configuration for both types of +* buffer support. +*/ + .storagebits = 32, + },
[Severity: High]
Will this configuration cause buffered reads for 16-bit parts to return 0 on big-endian architectures?
For 16-bit parts, the SPI transfer writes 2 bytes to the 32-bit union. On big-endian architectures, these 2 bytes are written to the most significant bytes of the 32-bit space. However, setting storagebits = 32 with an implicit shift of 0 causes the IIO core to extract the lower 16 bits (which remain 0). This causes the IIO core to persistently push 0 to the buffers.
drivers/iio/adc/ltc2378.c line 267
Quoted 5 lines
drivers/iio/adc/ltc2378.c new file +5
-0+ }; + + st->xfer.rx_buf = &st->scan.data; + st->xfer.len = st->info->resolution >16 ?4 :2 ; + st->xfer.bits_per_word = st->info->resolution;
[Severity: High]
Is it safe to set bits_per_word directly to 18 or 20?
The SPI core validates bits_per_word against the controller's bits_per_word_mask during transfer. Since the vast majority of SPI controllers only support 8, 16, or 32 bits per word, setting this to 18 or 20 will cause spi_sync_transfer() to immediately fail with -EINVAL, breaking data capture on standard hardware.
Quoted 5 lines
drivers/iio/adc/ltc2378.h new file +0
-0
[ ... ]
drivers/iio/adc/ltc2378.h line 61
Quoted 12 lines
drivers/iio/adc/ltc2378.h new file +12
-0+static inline int ltc2378_convert_and_acquire (struct ltc2378_state *st) +{ +int ret; + +/* Cause a rising edge of CNV to initiate a new ADC conversion */ +gpiod_set_value_cansleep (st->cnv_gpio,1 ); +fsleep (4 ); + ret =spi_sync_transfer (st->spi, &st->xfer,1 ); +gpiod_set_value_cansleep (st->cnv_gpio,0 ); + +return ret; +}
[Severity: High]
Does keeping the CNV pin high during the SPI transfer cause the ADC to output BUSY flags instead of conversion data?
For the LTC2378 series, bringing the Chip Select low while CNV is still high places the ADC into a BUSY indicator mode instead of shifting out the sampled data. Should CNV be brought low before initiating the SPI transfer?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
David Lechner
reviewerRe: [PATCH v3 2/5] iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs
In reply to Marcelo SchmittQuoted 2 lines · Marcelo Schmitt
Support for LTC2378-20 and similar analog-to-digital converters.
...
Quoted 9 lines
drivers/iio/adc/ltc2378.c new file +9
-0+static int ltc2378_probe (struct spi_device *spi) +{ +struct device *dev = &spi->dev; +struct iio_dev *indio_dev; +struct ltc2378_state *st; +int ret; + + indio_dev =devm_iio_device_alloc (&spi->dev,sizeof (*st)); +if (!indio_dev)
drivers/iio/adc/ltc2378.c line 231
Quoted 10 lines
drivers/iio/adc/ltc2378.c new file +10
-0+return -ENOMEM ; + + st =iio_priv (indio_dev); + st->spi = spi; + + ret =devm_regulator_get_enable_read_voltage (dev,"ref" ); +if (ret <0 ) +return dev_err_probe (dev, ret,"failed to read ref regulator\n" ); + + st->ref_uV = ret;
add blank line here
Quoted 5 lines
drivers/iio/adc/ltc2378.c new file +5
-0+ st->info =spi_get_device_match_data (spi); +if (!st->info) +return -EINVAL ; + + indio_dev->name = st->info->name;
drivers/iio/adc/ltc2378.c line 246
Quoted 10 lines
drivers/iio/adc/ltc2378.c new file +10
-0+ indio_dev->info = <c2378_iio_info; + indio_dev->modes =INDIO_DIRECT_MODE ; + + st->cnv_gpio =devm_gpiod_get (dev,"cnv" ,GPIOD_OUT_LOW ); +if (IS_ERR (st->cnv_gpio)) +return dev_err_probe (dev,PTR_ERR (st->cnv_gpio), +"failed to get CNV GPIO" ); + + st->num_iio_chans =0 ; + st->chans[st->num_iio_chans++] = (struct iio_chan_spec ) {
Why can't we make this static const (part of chip info) like we do in most drivers?
Quoted 4 lines
drivers/iio/adc/ltc2378.c new file +4
-0+ .type = IIO_VOLTAGE, + .info_mask_separate =BIT (IIO_CHAN_INFO_RAW ) | + BIT(IIO_CHAN_INFO_SCALE), + .scan_index = 0,
drivers/iio/adc/ltc2378.c line 260
Quoted 10 lines
drivers/iio/adc/ltc2378.c new file +10
-0+ .scan_type = { + .format = st->info->bipolar ?IIO_SCAN_FORMAT_SIGNED_INT : + IIO_SCAN_FORMAT_UNSIGNED_INT, + .realbits = st->info->resolution, +/* +* Buffer elements could be 16-bit for low precision +* parts. Though, using more storage bits allows keeping +* the same scan_type configuration for both types of +* buffer support. +*/
Won't this make non-SPI offload buffered reads more complicated later? I.e. have to shift the value or not before pushing to buffers depending on CPU endianness.
drivers/iio/adc/ltc2378.c line 266
Quoted 6 lines
drivers/iio/adc/ltc2378.c new file +6
-0+ .storagebits = 32, + }, + }; + + st->xfer.rx_buf = &st->scan.data; + st->xfer.len = st->info->resolution >16 ?4 :2 ;
Can use spi_bpw_to_bytes() here.
Quoted 8 lines
drivers/iio/adc/ltc2378.c new file +8
-0+ st->xfer.bits_per_word = st->info->resolution; + + indio_dev->channels = st->chans; + indio_dev->num_channels = st->num_iio_chans; + +return devm_iio_device_register (&spi->dev, indio_dev); +} +
...
Quoted 59 lines
drivers/iio/adc/ltc2378.h new file +53
-0@@ -0,0 +1,63 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* Analog Devices LTC2378 and similar ADCs common definitions and properties +* Copyright (C) 2026 Analog Devices, Inc. +* Author: Marcelo Schmitt <[email protected]> +*/ + +#ifndef __DRIVERS_IIO_ADC_LTC2378_H__ +#define __DRIVERS_IIO_ADC_LTC2378_H__ + +#include <linux/iio/iio.h> +#include <linux/gpio/consumer.h> +#include <linux/delay.h> +#include <linux/spi/spi.h> +#include <linux/types.h> +#include <linux/units.h> + +#define LTC2378_TDSDOBUSYL_NS 5 +#define LTC2378_TBUSYLH_NS 13 +#define LTC2378_TCNV_HIGH_NS 20 + +struct ltc2378_chip_info { +const char *name; +int resolution; +bool bipolar; +}; + +struct ltc2378_state { +const struct ltc2378_chip_info *info; +struct gpio_desc *cnv_gpio; +struct spi_device *spi; +struct spi_transfer xfer; +unsigned int num_iio_chans; +struct iio_chan_spec chans[2 ];/* 1 physical chan + 1 timestamp chan */ +int ref_uV; + +/* +* DMA (thus cache coherency maintenance) requires the +* transfer buffers to live in their own cache lines. +*/ +struct { +union { +u16 sample_buf16; +u32 sample_buf32; + } data; +aligned_s64 timestamp; + } scan__aligned(IIO_DMA_MINALIGN) ; +}; + +static inline int ltc2378_convert_and_acquire (struct ltc2378_state *st) +{ +int ret; +
drivers/iio/adc/ltc2378.h line 63
Quoted 10 lines
drivers/iio/adc/ltc2378.h new file +10
-0+/* Cause a rising edge of CNV to initiate a new ADC conversion */ +gpiod_set_value_cansleep (st->cnv_gpio,1 ); +fsleep (4 ); + ret =spi_sync_transfer (st->spi, &st->xfer,1 ); +gpiod_set_value_cansleep (st->cnv_gpio,0 ); + + return ret; +} + +#endif /* __DRIVERS_IIO_ADC_LTC2378_H__ */
Why do we need a header for this stuff? If there is a good reason the commit message should explain it. Otherwise, it makes the driver harder to read.
Marcelo Schmitt
maintainerRFC PATCH v3 3/5 iio: buffer: Extend DMAengine buffer interfaces to take extra sysfs attributes
In reply to Marcelo SchmittSome devices using DMAengine buffers are connected to extra hardware that allows setting how fast data is transferred to/from the buffer. However, those extra pieces of harwdware are external to the sensor chip such that supporting the transfer speed as a sensor property is a bit of an inaccuracy. Expand IIO DMAengine buffer interfaces to take arguments for extra sysfs attributes, enabling the transfer speed to be configured through the buffer interface.
---
New patch.
Now that I've come to this buffer "solution", I have pretty much convinced myself it would be better to instead have some sort of IIO trigger to control the signal source connected to SPI offloading trigger module.
drivers/iio/adc/ad4000.c | 3 ++- drivers/iio/adc/ad4030.c | 3 ++- drivers/iio/adc/ad4691.c | 3 ++- drivers/iio/adc/ad4695.c | 2 +- drivers/iio/adc/ad7380.c | 2 +- drivers/iio/adc/ad7606_spi.c | 2 +- drivers/iio/adc/ad7768-1.c | 3 ++- drivers/iio/adc/ad7944.c | 2 +- drivers/iio/adc/ad_sigma_delta.c | 2 +- .../buffer/industrialio-buffer-dmaengine.c | 19 ++++++++++++------- drivers/iio/dac/ad5791.c | 2 +- drivers/iio/dac/ad8460.c | 2 +- drivers/iio/dac/adi-axi-dac.c | 2 +- include/linux/iio/buffer-dmaengine.h | 16 ++++++++++------ 14 files changed, 38 insertions(+), 25 deletions(-)
drivers/iio/adc/ad4000.c +2 -1
@@ -871,7 +871,8 @@ static int ad4000_spi_offload_setup(struct iio_dev *indio_dev, "Failed to get offload RX DMA\n"); ret =devm_iio_dmaengine_buffer_setup_with_handle (dev, indio_dev, rx_dma, -IIO_BUFFER_DIRECTION_IN ); +IIO_BUFFER_DIRECTION_IN , +NULL );if (ret)return dev_err_probe (dev, ret,"Failed to setup DMA buffer\n" );
drivers/iio/adc/ad4030.c +2 -1
@@ -1400,7 +1400,8 @@ static int ad4030_spi_offload_setup(struct iio_dev *indio_dev, "failed to get offload RX DMA\n");return devm_iio_dmaengine_buffer_setup_with_handle (dev, indio_dev, rx_dma, -IIO_BUFFER_DIRECTION_IN ); +IIO_BUFFER_DIRECTION_IN , +NULL ); } static int ad4030_setup_pga(struct device *dev, struct iio_dev *indio_dev,
drivers/iio/adc/ad4691.c +2 -1
@@ -1980,7 +1980,8 @@ static int ad4691_setup_offload(struct iio_dev *indio_dev, indio_dev->setup_ops = &ad4691_cnv_burst_offload_buffer_setup_ops; ret =devm_iio_dmaengine_buffer_setup_with_handle (dev, indio_dev, rx_dma, -IIO_BUFFER_DIRECTION_IN ); +IIO_BUFFER_DIRECTION_IN , +NULL );if (ret)return ret;
drivers/iio/adc/ad4695.c +1 -1
@@ -1779,7 +1779,7 @@ static int ad4695_probe_spi_offload(struct iio_dev *indio_dev, }return devm_iio_dmaengine_buffer_setup_with_handle (dev, indio_dev, - rx_dma,IIO_BUFFER_DIRECTION_IN ); + rx_dma,IIO_BUFFER_DIRECTION_IN ,NULL ); }static const struct spi_offload_config ad4695_spi_offload_config = {
drivers/iio/adc/ad7380.c +1 -1
@@ -1874,7 +1874,7 @@ static int ad7380_probe_spi_offload(struct iio_dev *indio_dev, "failed to get offload RX DMA\n"); ret =devm_iio_dmaengine_buffer_setup_with_handle (dev, indio_dev, - rx_dma,IIO_BUFFER_DIRECTION_IN ); + rx_dma,IIO_BUFFER_DIRECTION_IN ,NULL );if (ret)return dev_err_probe (dev, ret,"cannot setup dma buffer\n" );
drivers/iio/adc/ad7606_spi.c +1 -1
@@ -317,7 +317,7 @@ static int ad7606_spi_offload_probe(struct device *dev, "failed to get offload RX DMA\n"); ret =devm_iio_dmaengine_buffer_setup_with_handle (dev, indio_dev, - rx_dma,IIO_BUFFER_DIRECTION_IN ); + rx_dma,IIO_BUFFER_DIRECTION_IN ,NULL );if (ret)return dev_err_probe (dev, ret,"failed to setup offload RX DMA\n" );
drivers/iio/adc/ad7768-1.c +2 -1
@@ -1792,7 +1792,8 @@ static int ad7768_spi_offload_probe(struct iio_dev *indio_dev,return dev_err_probe (dev,PTR_ERR (rx_dma),"failed to get offload RX DMA\n" ); ret =devm_iio_dmaengine_buffer_setup_with_handle (dev, indio_dev, rx_dma, -IIO_BUFFER_DIRECTION_IN ); +IIO_BUFFER_DIRECTION_IN , +NULL );if (ret)return dev_err_probe (dev, ret,"failed to setup offload RX DMA\n" );
drivers/iio/adc/ad7944.c +1 -1
@@ -843,7 +843,7 @@ static int ad7944_probe(struct spi_device *spi)*/ ret =devm_iio_dmaengine_buffer_setup_with_handle (dev, - indio_dev, rx_dma,IIO_BUFFER_DIRECTION_IN ); + indio_dev, rx_dma,IIO_BUFFER_DIRECTION_IN ,NULL );if (ret)return ret;
drivers/iio/adc/ad_sigma_delta.c +1 -1
@@ -820,7 +820,7 @@ int devm_ad_sd_setup_buffer_and_trigger(struct device *dev, struct iio_dev *indi "Failed to get RX DMA channel\n"); ret =devm_iio_dmaengine_buffer_setup_with_handle (dev, indio_dev, - rx_dma,IIO_BUFFER_DIRECTION_IN ); + rx_dma,IIO_BUFFER_DIRECTION_IN ,NULL );if (ret)return dev_err_probe (dev, ret,"Cannot setup DMA buffer\n" );
drivers/iio/buffer/industrialio-buffer-dmaengine.c +12 -7
@@ -289,7 +289,8 @@ EXPORT_SYMBOL_NS_GPL(iio_dmaengine_buffer_teardown, "IIO_DMAENGINE_BUFFER");static struct iio_buffer *__iio_dmaengine_buffer_setup_ext (struct iio_dev *indio_dev,struct dma_chan *chan, -enum iio_buffer_direction dir) +enum iio_buffer_direction dir, +const struct iio_dev_attr **buffer_attrs) {struct iio_buffer *buffer;int ret;
@@ -301,6 +302,7 @@ static struct iio_buffer indio_dev->modes |=INDIO_BUFFER_HARDWARE ; buffer->direction = dir; + buffer->attrs = buffer_attrs; ret =iio_device_attach_buffer (indio_dev, buffer);if (ret) {
@@ -329,7 +331,8 @@ static struct iio_bufferstruct iio_buffer *iio_dmaengine_buffer_setup_ext (struct device *dev,struct iio_dev *indio_dev,const char *channel, -enum iio_buffer_direction dir) +enum iio_buffer_direction dir, +const struct iio_dev_attr **buffer_attrs) {struct dma_chan *chan;struct iio_buffer *buffer;
@@ -338,7 +341,7 @@ struct iio_buffer *iio_dmaengine_buffer_setup_ext(struct device *dev,if (IS_ERR (chan))return ERR_CAST (chan); - buffer =__iio_dmaengine_buffer_setup_ext (indio_dev, chan, dir); + buffer =__iio_dmaengine_buffer_setup_ext (indio_dev, chan, dir, buffer_attrs);if (IS_ERR (buffer))dma_release_channel (chan);
@@ -366,11 +369,12 @@ static void devm_iio_dmaengine_buffer_teardown(void *buffer)int devm_iio_dmaengine_buffer_setup_ext (struct device *dev,struct iio_dev *indio_dev,const char *channel, -enum iio_buffer_direction dir) +enum iio_buffer_direction dir, +const struct iio_dev_attr **buffer_attrs) {struct iio_buffer *buffer; - buffer =iio_dmaengine_buffer_setup_ext (dev, indio_dev, channel, dir); + buffer =iio_dmaengine_buffer_setup_ext (dev, indio_dev, channel, dir, buffer_attrs);if (IS_ERR (buffer))return PTR_ERR (buffer);
@@ -403,11 +407,12 @@ static void devm_iio_dmaengine_buffer_free(void *buffer)int devm_iio_dmaengine_buffer_setup_with_handle (struct device *dev,struct iio_dev *indio_dev,struct dma_chan *chan, -enum iio_buffer_direction dir) +enum iio_buffer_direction dir, +const struct iio_dev_attr **buffer_attrs) {struct iio_buffer *buffer; - buffer =__iio_dmaengine_buffer_setup_ext (indio_dev, chan, dir); + buffer =__iio_dmaengine_buffer_setup_ext (indio_dev, chan, dir, buffer_attrs);if (IS_ERR (buffer))return PTR_ERR (buffer);
drivers/iio/dac/ad5791.c +1 -1
@@ -448,7 +448,7 @@ static int ad5791_offload_setup(struct iio_dev *indio_dev) "failed to get offload TX DMA\n"); ret =devm_iio_dmaengine_buffer_setup_with_handle (&spi->dev, - indio_dev, tx_dma,IIO_BUFFER_DIRECTION_OUT ); + indio_dev, tx_dma,IIO_BUFFER_DIRECTION_OUT ,NULL );if (ret)return ret;
drivers/iio/dac/ad8460.c +1 -1
@@ -921,7 +921,7 @@ static int ad8460_probe(struct spi_device *spi) indio_dev->setup_ops = &ad8460_buffer_setup_ops; ret =devm_iio_dmaengine_buffer_setup_ext (dev, indio_dev,"tx" , -IIO_BUFFER_DIRECTION_OUT ); +IIO_BUFFER_DIRECTION_OUT ,NULL );if (ret)return dev_err_probe (dev, ret,"Failed to get DMA buffer\n" );
drivers/iio/dac/adi-axi-dac.c +1 -1
@@ -162,7 +162,7 @@ static struct iio_buffer *axi_dac_request_buffer(struct iio_backend *back, dma_name ="tx" ;return iio_dmaengine_buffer_setup_ext (st->dev, indio_dev, dma_name, -IIO_BUFFER_DIRECTION_OUT ); +IIO_BUFFER_DIRECTION_OUT ,NULL ); } static void axi_dac_free_buffer(struct iio_backend *back,
include/linux/iio/buffer-dmaengine.h +10 -6
@@ -10,6 +10,7 @@#include <linux/iio/buffer.h> struct iio_dev ; +struct iio_dev_attr ;struct device ;struct dma_chan ;
@@ -17,23 +18,26 @@ void iio_dmaengine_buffer_teardown(struct iio_buffer *buffer);struct iio_buffer *iio_dmaengine_buffer_setup_ext (struct device *dev,struct iio_dev *indio_dev,const char *channel, -enum iio_buffer_direction dir); +enum iio_buffer_direction dir, +const struct iio_dev_attr **buffer_attrs);#define iio_dmaengine_buffer_setup (dev, indio_dev, channel) \ - iio_dmaengine_buffer_setup_ext(dev, indio_dev, channel, \ - IIO_BUFFER_DIRECTION_IN) + iio_dmaengine_buffer_setup_ext(dev, indio_dev, channel, \ + IIO_BUFFER_DIRECTION_IN, NULL)int devm_iio_dmaengine_buffer_setup_ext (struct device *dev,struct iio_dev *indio_dev,const char *channel, -enum iio_buffer_direction dir); +enum iio_buffer_direction dir, +const struct iio_dev_attr **buffer_attrs);int devm_iio_dmaengine_buffer_setup_with_handle (struct device *dev,struct iio_dev *indio_dev,struct dma_chan *chan, -enum iio_buffer_direction dir); +enum iio_buffer_direction dir, +const struct iio_dev_attr **buffer_attrs);#define devm_iio_dmaengine_buffer_setup (dev, indio_dev, channel) \ devm_iio_dmaengine_buffer_setup_ext(dev, indio_dev, channel, \ - IIO_BUFFER_DIRECTION_IN) + IIO_BUFFER_DIRECTION_IN, NULL)#endif
Signature · 1 line
2.53.0
[email protected]
AI reviewRe: [RFC PATCH v3 3/5] iio: buffer: Extend DMAengine buffer interfaces to take extra sysfs attributes
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Critical] Unconditional assignment of buffer->attrs overwrites the default DMAengine buffer attributes, removing length_align_bytes from sysfs and breaking the userspace ABI.
Pre-existing issues:
- [High] The ad4691 driver manually overwrites the IIO buffer's attrs field after setup, clobbering the generic DMAengine buffer attributes. --
commit e15805cbfeb85d6ab2a3f8819279738d4402da62
Author: Marcelo Schmitt <[email protected]>
iio: buffer: Extend DMAengine buffer interfaces to take extra sysfs attributes
This patch extends the IIO DMAengine buffer setup interfaces to accept an additional parameter for extra sysfs attributes. It updates all existing callers to pass NULL for this new parameter.
drivers/iio/adc/ad4691.c static int ad4691_setup_offload(struct iio_dev *indio_dev, · line 1984
Quoted 10 lines
drivers/iio/adc/ad4691.c +2
-1@@ -1980,7 +1980,8 @@ static int ad4691_setup_offload(struct iio_dev *indio_dev, indio_dev->setup_ops = &ad4691_cnv_burst_offload_buffer_setup_ops; ret =devm_iio_dmaengine_buffer_setup_with_handle (dev, indio_dev, rx_dma, -IIO_BUFFER_DIRECTION_IN ); +IIO_BUFFER_DIRECTION_IN , +NULL );
[Severity: High]
This is a pre-existing issue, but does manually assigning the buffer attributes further down in this function clobber the generic DMAengine buffer attributes?
Following this setup call in ad4691_setup_offload(), the driver manually assigns:
indio_dev->buffer->attrs = ad4691_buffer_attrs;
Could this overwrite the default DMAengine buffer attributes like length_align_bytes? Since the new API allows injecting custom attributes, should this driver be updated to pass ad4691_buffer_attrs into devm_iio_dmaengine_buffer_setup_with_handle() rather than passing NULL and manually overwriting it afterwards?
Quoted 3 lines
drivers/iio/adc/ad4691.c +0
-0if (ret)return ret;
[ ... ]
Quoted 14 lines
drivers/iio/buffer/industrialio-buffer-dmaengine.c +2
-1@@ -289,7 +289,8 @@ EXPORT_SYMBOL_NS_GPL(iio_dmaengine_buffer_teardown, "IIO_DMAENGINE_BUFFER");static struct iio_buffer *__iio_dmaengine_buffer_setup_ext (struct iio_dev *indio_dev,struct dma_chan *chan, -enum iio_buffer_direction dir) +enum iio_buffer_direction dir, +const struct iio_dev_attr **buffer_attrs) {struct iio_buffer *buffer;int ret;
drivers/iio/buffer/industrialio-buffer-dmaengine.c static struct iio_buffer · line 305
Quoted 4 lines
drivers/iio/buffer/industrialio-buffer-dmaengine.c +1
-0@@ -301,6 +302,7 @@ static struct iio_buffer indio_dev->modes |=INDIO_BUFFER_HARDWARE ; buffer->direction = dir; + buffer->attrs = buffer_attrs;
[Severity: Critical]
Does unconditionally overwriting buffer->attrs here remove the default DMAengine buffer attributes?
Since iio_dmaengine_buffer_alloc() sets up the default attributes array (which exposes length_align_bytes to userspace), and all current callers updated in this patch pass NULL for buffer_attrs, it appears this will silently delete length_align_bytes from sysfs for all IIO DMAengine buffers.
Could this break the userspace ABI, as applications might rely on length_align_bytes to determine the required memory alignment for DMA buffers?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
David Lechner
reviewerRe: [RFC PATCH v3 3/5] iio: buffer: Extend DMAengine buffer interfaces to take extra sysfs attributes
In reply to Marcelo SchmittQuoted 7 lines · Marcelo Schmitt
Some devices using DMAengine buffers are connected to extra hardware that allows setting how fast data is transferred to/from the buffer. However, those extra pieces of harwdware are external to the sensor chip such that supporting the transfer speed as a sensor property is a bit of an inaccuracy. Expand IIO DMAengine buffer interfaces to take arguments for extra sysfs attributes, enabling the transfer speed to be configured through the buffer interface.
This message is a bit confusing. It sounds like it is attempting to control something about the DMA controller itself. But based on the later patches, it looks like this is just so we can add arbitrary sysfs attributes to the bufferX directory. And in this specific case, a sampling_frequency attribute.
Quoted 9 lines
Signed-off-by Marcelo Schmitt <[email protected]>---
New patch.Now that I've come to this buffer "solution", I have pretty much convinced myself it would be better to instead have some sort of IIO trigger to control the signal source connected to SPI offloading trigger module.
In the other chips with SPI offload we've done already, we just used IIO_CHAN_INFO_SAMP_FREQ to control the SPI offload trigger rate. Any reason why we can't do that here? In the original SPI offload discussions, IIRC the general consensus was that adding a trigger just to control that was overkill when I suggested the same.
Re: [RFC PATCH v3 3/5] iio: buffer: Extend DMAengine buffer interfaces to take extra sysfs attributes
Quoted 9 lines · David Lechner
Quoted 7 lines · Marcelo Schmitt
Some devices using DMAengine buffers are connected to extra hardware that allows setting how fast data is transferred to/from the buffer. However, those extra pieces of harwdware are external to the sensor chip such that supporting the transfer speed as a sensor property is a bit of an inaccuracy. Expand IIO DMAengine buffer interfaces to take arguments for extra sysfs attributes, enabling the transfer speed to be configured through the buffer interface.
Quoted 5 lines
This message is a bit confusing. It sounds like it is attempting to control something about the DMA controller itself. But based on the later patches, it looks like this is just so we can add arbitrary sysfs attributes to the bufferX directory. And in this specific case, a
sampling_frequencyattribute.
Agreed. Seems like rate control comes from the buffer.
Quoted 10 lines
Quoted 10 lines
Signed-off-by Marcelo Schmitt <[email protected]>---
New patch.Now that I've come to this buffer "solution", I have pretty much convinced myself it would be better to instead have some sort of IIO trigger to control the signal source connected to SPI offloading trigger module.
Quoted 6 lines
In the other chips with SPI offload we've done already, we just used
IIO_CHAN_INFO_SAMP_FREQto control the SPI offload trigger rate. Any reason why we can't do that here? In the original SPI offload discussions, IIRC the general consensus was that adding a trigger just to control that was overkill when I suggested the same.
I tend to agree with David. Even if we come to a conclusion that we can't use IIO_CHAN_INFO_SAMP_FREQ to control the trigger rate, I'm not really convinced sysfs interfaces on the buffer itself are the place for this.
- Nuno Sá
Re: [RFC PATCH v3 3/5] iio: buffer: Extend DMAengine buffer interfaces to take extra sysfs attributes
Quoted 10 lines · Nuno Sá
Quoted 9 lines · David Lechner
Quoted 7 lines · Marcelo Schmitt
Some devices using DMAengine buffers are connected to extra hardware that allows setting how fast data is transferred to/from the buffer. However, those extra pieces of harwdware are external to the sensor chip such that supporting the transfer speed as a sensor property is a bit of an inaccuracy. Expand IIO DMAengine buffer interfaces to take arguments for extra sysfs attributes, enabling the transfer speed to be configured through the buffer interface.
Quoted 7 lines
Quoted 5 lines
This message is a bit confusing. It sounds like it is attempting to control something about the DMA controller itself. But based on the later patches, it looks like this is just so we can add arbitrary sysfs attributes to the bufferX directory. And in this specific case, a
sampling_frequencyattribute.Agreed. Seems like rate control comes from the buffer.
For this series, that was the actual result. Since we already have extra sysfs attributes for kfifo and for triggered buffers, I thought of trying it wish DMAengine buffers as well. The intent is to decouple the sampling frequency sysfs attributes (which depend on SPI offload and PWM availability) from the ADC IIO device. And the reason for that is to avoid making the device driver directly select SPI_OFFLOAD and/or depend on PWM because these devices can be used without offloading.
Quoted 18 lines
Quoted 11 lines
Quoted 11 lines
Signed-off-by Marcelo Schmitt <[email protected]>---
New patch.Now that I've come to this buffer "solution", I have pretty much convinced myself it would be better to instead have some sort of IIO trigger to control the signal source connected to SPI offloading trigger module.
Quoted 6 lines
In the other chips with SPI offload we've done already, we just used
IIO_CHAN_INFO_SAMP_FREQto control the SPI offload trigger rate. Any reason why we can't do that here? In the original SPI offload discussions, IIRC the general consensus was that adding a trigger just to control that was overkill when I suggested the same.
Quoted 4 lines
I tend to agree with David. Even if we come to a conclusion that we can't use
IIO_CHAN_INFO_SAMP_FREQto control the trigger rate, I'm not really convinced sysfs interfaces on the buffer itself are the place for this.
I understand it might be odd and confusing to have the sampling freq attributes in the buffer. Using IIO_CHAN_INFO_SAMP_FREQ shall also work for this device so I'll probably switch to that. Though, an interesting point Jonathan made on v2 was that some devices have sampling frequency controls natively (e.g. ad7173). I wonder if having a trigger device would help with the SPI offload dependency separation. We might avoid attribute name clashing for some devices as bonus.
Thanks,
Marcelo
David Lechner
reviewerRe: [RFC PATCH v3 3/5] iio: buffer: Extend DMAengine buffer interfaces to take extra sysfs attributes
Quoted 19 lines · Marcelo Schmitt
Quoted 10 lines · Nuno Sá
Quoted 9 lines · David Lechner
Quoted 7 lines · Marcelo Schmitt
Some devices using DMAengine buffers are connected to extra hardware that allows setting how fast data is transferred to/from the buffer. However, those extra pieces of harwdware are external to the sensor chip such that supporting the transfer speed as a sensor property is a bit of an inaccuracy. Expand IIO DMAengine buffer interfaces to take arguments for extra sysfs attributes, enabling the transfer speed to be configured through the buffer interface.
Quoted 7 lines
Quoted 5 lines
This message is a bit confusing. It sounds like it is attempting to control something about the DMA controller itself. But based on the later patches, it looks like this is just so we can add arbitrary sysfs attributes to the bufferX directory. And in this specific case, a
sampling_frequencyattribute.Agreed. Seems like rate control comes from the buffer.
Quoted 8 lines
For this series, that was the actual result. Since we already have extra sysfs attributes for kfifo and for triggered buffers, I thought of trying it wish DMAengine buffers as well. The intent is to decouple the sampling frequency sysfs attributes (which depend on SPI offload and PWM availability) from the ADC IIO device. And the reason for that is to avoid making the device driver directly select
SPI_OFFLOADand/or depend on PWM because these devices can be used without offloading.
Selecting SPI_OFFLOAD when it isn't needed isn't a big deal IMHO, it just isn't very much code at all. And PWM is quite common for fans and backlights, so it seems like it would be rare to have a system that didn't have PWM enabled for something else already. So it just seems like extra work to me to try to make things optional when it may never be needed in practice.
Marcelo Schmitt
maintainerPATCH v3 4/5 iio: adc: ltc2378: Enable high-speed data capture
In reply to Marcelo SchmittMake use of SPI transfer offloading to speed up data capture, enabling data acquisition at faster sample rates (up to 2 MSPS).
---
Change log v2 -> v3:
- Fixed the evaluation loop conditions for CNV PWM and SPI Engine trigger PWM, avoiding potential infinite loop if and CPU stall.
- Added comment to about initial PWM disable.
- Adjusted SPI offload setup initialization to not print error on a valid condition.
- Fully initialize IIO channel scan_type.
- Reworked to make offload support not imply all dependencies to be built in.
- Made sampling_frequency a buffer attribute.
- Made offload support not require DMA and other features to be built in.
- Now using same scan_type configuration for all use cases.
drivers/iio/adc/Kconfig | 19 ++ drivers/iio/adc/Makefile | 6 + drivers/iio/adc/ltc2378-lib-core.c | 35 +++ drivers/iio/adc/ltc2378-offload-buffer.c | 305 +++++++++++++++++++++++ drivers/iio/adc/ltc2378.c | 46 ++++ drivers/iio/adc/ltc2378.h | 42 ++++ 6 files changed, 453 insertions(+) create mode 100644 drivers/iio/adc/ltc2378-lib-core.c create mode 100644 drivers/iio/adc/ltc2378-offload-buffer.c
drivers/iio/adc/Kconfig +19 -0
@@ -944,6 +944,9 @@ config LTC2378 tristate "Analog Devices LTC2378 ADC driver" depends on SPI depends on GPIOLIB + select LTC2378_LIB + select LTC2378_LIB_OFFLOAD_BUFFER if SPI_OFFLOAD && PWM && SPI_OFFLOAD_TRIGGER_PWM && IIO_BUFFER && IIO_BUFFER_DMAENGINE + select LTC2378_LIB_TRIGGERED_BUFFER if IIO_BUFFER help Say yes here to build support for Analog Devices LTC2378-20 and similar analog to digital converters.
@@ -2027,3 +2030,19 @@ config XILINX_AMS xilinx-ams. endmenu + +config LTC2378_LIB + tristate + help + Say yes here to build support for buffered data capture with LTC2378 + +config LTC2378_LIB_OFFLOAD_BUFFER + bool + help + Say yes here to build support for high speed data capture with LTC2378 + +config LTC2378_LIB_TRIGGERED_BUFFER + bool + select IIO_TRIGGERED_BUFFER + help + Say yes here to build support for buffered data capture with LTC2378
drivers/iio/adc/Makefile +6 -0
@@ -82,6 +82,12 @@ obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o obj-$(CONFIG_LTC2309) += ltc2309.o obj-$(CONFIG_LTC2378) += ltc2378.o + +ltc2378_lib-y += ltc2378-lib-core.o +ltc2378_lib-$(CONFIG_LTC2378_LIB_OFFLOAD_BUFFER) += ltc2378-offload-buffer.o +ltc2378_lib-$(CONFIG_LTC2378_LIB_TRIGGERED_BUFFER) += ltc2378-triggered-buffer.o +obj-$(CONFIG_LTC2378_LIB) += ltc2378_lib.o + obj-$(CONFIG_LTC2471) += ltc2471.o obj-$(CONFIG_LTC2485) += ltc2485.o obj-$(CONFIG_LTC2496) += ltc2496.o ltc2497-core.o
drivers/iio/adc/ltc2378-lib-core.c new file +35 -0
@@ -0,0 +1,35 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* +* Analog Devices LTC2378 ADC series driver +* +* Copyright (C) 2026 Analog Devices Inc. +* Author: Marcelo Schmitt <[email protected]> +*/ + +#include <linux/err.h> +#include <linux/iio/iio.h> + +#include "ltc2378.h" + +int ltc2378_lib_buffer_setup (struct iio_dev *indio_dev,struct ltc2378_state *st) +{ +struct device *dev = &st->spi->dev; +int ret; + + ret =__ltc2378_set_offload_ops (st); +if (ret == -EOPNOTSUPP ) +return 0 ;/* Let device setup complete without buffer support */ + +if (!ret) + ret = st->ops->buffer_setup (indio_dev, st); + +if (ret) +return dev_err_probe (dev, ret,"error on SPI offload setup\n" ); + +return 0 ; +} +EXPORT_SYMBOL_NS_GPL (ltc2378_lib_buffer_setup,"IIO_LTC2378" ); + +MODULE_AUTHOR ("Marcelo Schmitt <[email protected]>" ); +MODULE_DESCRIPTION ("Analog Devices LTC2378 ADC series driver" ); +MODULE_LICENSE ("GPL" );
drivers/iio/adc/ltc2378-offload-buffer.c new file +305 -0
305 lines not on this page · raw message
drivers/iio/adc/ltc2378.c +46 -0
181 lines not on this page · raw message
drivers/iio/adc/ltc2378.h +42 -0
74 lines not on this page · raw message
Signature · 1 line
2.53.0
[email protected]
AI reviewRe: [PATCH v3 4/5] iio: adc: ltc2378: Enable high-speed data capture
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider: - [High] Unconditional propagation of -ENODEV breaks driver probe on systems without SPI offload - [High]…
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Unconditional propagation of -ENODEV breaks driver probe on systems without SPI offload
- [High] Silent failure in SPI offload trigger offset validation loop risks data corruption
- [Medium] Incomplete state rollback on failed conversion rate validation corrupts driver state
- [Low] Missing source file causes build failure
- [Low] Accumulative drift of PWM period due to reusing modified waveform structure --
commit 0bb089e96af6d65eeaa13775a0321f6c17a80b75
Author: Marcelo Schmitt <[email protected]>
iio: adc: ltc2378: Enable high-speed data capture
Make use of SPI transfer offloading to speed up data capture, enabling data acquisition at faster sample rates (up to 2 MSPS).
Quoted 11 lines
drivers/iio/adc/Kconfig +3
-0@@ -944,6 +944,9 @@ config LTC2378 tristate "Analog Devices LTC2378 ADC driver" depends on SPI depends on GPIOLIB + select LTC2378_LIB + select LTC2378_LIB_OFFLOAD_BUFFER if SPI_OFFLOAD && PWM && SPI_OFFLOAD_TRIGGER_PWM && IIO_BUFFER && IIO_BUFFER_DMAENGINE + select LTC2378_LIB_TRIGGERED_BUFFER if IIO_BUFFER
[ ... ]
drivers/iio/adc/Makefile obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o · line 88
Quoted 12 lines
drivers/iio/adc/Makefile +4
-0@@ -82,6 +82,12 @@ obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o obj-$(CONFIG_LTC2309) += ltc2309.o obj-$(CONFIG_LTC2378) += ltc2378.o + +ltc2378_lib-y += ltc2378-lib-core.o +ltc2378_lib-$(CONFIG_LTC2378_LIB_OFFLOAD_BUFFER) += ltc2378-offload-buffer.o +ltc2378_lib-$(CONFIG_LTC2378_LIB_TRIGGERED_BUFFER) += ltc2378-triggered-buffer.o
[Severity: Low]
Will this break the build since ltc2378-triggered-buffer.c is not included in this patch?
The patch adds CONFIG_LTC2378_LIB_TRIGGERED_BUFFER and references ltc2378-triggered-buffer.o in the Makefile, which is automatically selected if IIO_BUFFER is enabled. However, the corresponding ltc2378-triggered-buffer.c file is missing.
This regression was fixed later in the series in commit 975300646e8809a19fe7c482edd698d128476317 ("iio: adc: ltc2378: Enable triggered buffer data capture"), but shouldn't each patch in the series compile independently to preserve bisectability?
[ ... ]
Quoted 36 lines
drivers/iio/adc/ltc2378-lib-core.c new file +30
-0@@ -0,0 +1,35 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* +* Analog Devices LTC2378 ADC series driver +* +* Copyright (C) 2026 Analog Devices Inc. +* Author: Marcelo Schmitt <[email protected]> +*/ + +#include <linux/err.h> +#include <linux/iio/iio.h> + +#include "ltc2378.h" + +int ltc2378_lib_buffer_setup (struct iio_dev *indio_dev,struct ltc2378_state *st) +{ +struct device *dev = &st->spi->dev; +int ret; + + ret =__ltc2378_set_offload_ops (st); +if (ret == -EOPNOTSUPP ) +return 0 ;/* Let device setup complete without buffer support */ + +if (!ret) + ret = st->ops->buffer_setup (indio_dev, st); + +if (ret) +return dev_err_probe (dev, ret,"error on SPI offload setup\n" ); + +return 0 ; +}
[ ... ]
Quoted 6 lines
drivers/iio/adc/ltc2378-offload-buffer.c new file +0
-0@@ -0,0 +1,305 @@
[ ... ]
drivers/iio/adc/ltc2378-offload-buffer.c line 267
Quoted 11 lines
drivers/iio/adc/ltc2378-offload-buffer.c new file +11
-0+static int ltc2378_offload_buffer_setup (struct iio_dev *indio_dev, +struct ltc2378_state *st) +{ +struct spi_device *spi = st->spi; +struct device *dev = &spi->dev; +int ret; + + st->offload =devm_spi_offload_get (dev, spi, <c2378_offload_config); + ret =PTR_ERR_OR_ZERO (st->offload); +if (ret) +return ret;
[Severity: High]
Does this unconditionally propagate -ENODEV and break the driver probe on systems without SPI offload?
When CONFIG_LTC2378_LIB_OFFLOAD_BUFFER is enabled, the driver attempts to set up the SPI offload buffer. If the system hardware does not support SPI offload, devm_spi_offload_get() returns -ENODEV.
Because ltc2378_offload_buffer_setup() returns this error, ltc2378_lib_buffer_setup() will pass it to dev_err_probe() and fail the entire ADC probe. Should the driver treat -ENODEV as a signal to bypass offload setup and gracefully fall back to triggered buffer or direct mode?
[ ... ]
Quoted 21 lines
drivers/iio/adc/ltc2378-offload-buffer.c new file +21
-0+static int ltc2378_update_conversion_rate (struct ltc2378_state *st,int freq_Hz) +{ +struct spi_offload_trigger_config *config = &st->offload_trigger_config; +unsigned int min_read_offset, offload_period_ns; +struct pwm_waveform cnv_wf = { }; +u64 target =LTC2378_TCNV_HIGH_NS ; +unsigned int count =0 ; +u64 offload_offset_ns; +int ret; + +if (freq_Hz ==0 ) +return -EINVAL ; + +if (freq_Hz <1 || freq_Hz > st->info->max_sample_rate_hz) +return -ERANGE ; + +/* Configure CNV PWM waveform */ + cnv_wf.period_length_ns =DIV_ROUND_CLOSEST (NSEC_PER_SEC , freq_Hz); + +/* +* Ensure CNV high time meets minimum requirement (20ns). The PWM
drivers/iio/adc/ltc2378-offload-buffer.c line 75
Quoted 10 lines
drivers/iio/adc/ltc2378-offload-buffer.c new file +10
-0+* hardware may round the duty cycle, so iterate until we get at least +* the minimum required high time. +*/ +do { + cnv_wf.duty_length_ns = target; + ret =pwm_round_waveform_might_sleep (st->cnv_trigger, &cnv_wf); +if (ret) +return ret; + target +=10 ;/* Increment by PWM duty cycle period */ + }while (count++ <100 && cnv_wf.duty_length_ns <LTC2378_TCNV_HIGH_NS );
[Severity: Low]
Could this cause an accumulative drift of the PWM period?
In this loop, pwm_round_waveform_might_sleep(&cnv_wf) can modify fields in cnv_wf based on hardware rounding capabilities. Because cnv_wf.period_length_ns is not reset to the original target period at the beginning of each iteration, any rounding applied to the period in one iteration is passed into the next, which might drift the PWM period away from the requested frequency.
Quoted 13 lines
drivers/iio/adc/ltc2378-offload-buffer.c new file +13
-0+ +/* Double check the minimum CNV high time is met */ +if (cnv_wf.duty_length_ns <LTC2378_TCNV_HIGH_NS ) +return -EIO ; + +/* +* Configure SPI offload PWM trigger. +* The trigger should fire after tBUSYLH + tCONV + tDSDOBUSYL. +* Minimum time needed: TBUSYLH (13ns) + TCONV (part-specific) + TDSDOBUSYL (5ns) +* +* Use the same period as CNV PWM to avoid timing issues. +* Convert back from period to frequency for the SPI offload API. +*/
drivers/iio/adc/ltc2378-offload-buffer.c line 98
Quoted 10 lines
drivers/iio/adc/ltc2378-offload-buffer.c new file +10
-0+ offload_period_ns = cnv_wf.period_length_ns; + config->periodic.frequency_hz =DIV_ROUND_UP (HZ_PER_GHZ , offload_period_ns); + min_read_offset =LTC2378_TBUSYLH_NS + st->info->tconv_ns +LTC2378_TDSDOBUSYL_NS ; + offload_offset_ns = min_read_offset; + count =0 ; + do { + config->periodic.offset_ns = offload_offset_ns; + ret = spi_offload_trigger_validate(st->offload_trigger, config); + if (ret) + return ret;
[Severity: Medium]
Does returning here leave the driver in an inconsistent state?
The pointer config directly points to the device state st->offload_trigger_config. If validation fails, returning an error immediately leaves st->offload_trigger_config containing the rejected values, while st->cnv_wf and st->cnv_Hz retain their old values.
Subsequent buffer enable operations might try to pair the old PWM waveform with the corrupted offload trigger config. Shouldn't the state be modified on a local copy and only committed upon success?
drivers/iio/adc/ltc2378-offload-buffer.c line 105
Quoted 7 lines
drivers/iio/adc/ltc2378-offload-buffer.c new file +7
-0+ offload_offset_ns +=10 ; + }while (count++ <100 && config->periodic.offset_ns < min_read_offset); + + st->cnv_wf = cnv_wf; + st->cnv_Hz =DIV_ROUND_CLOSEST_ULL (HZ_PER_GHZ , cnv_wf.period_length_ns); + +return 0 ;
[Severity: High]
Is it possible for this to silently return 0 on a failed loop condition?
The loop iterates up to 100 times to find an SPI offload trigger offset >= min_read_offset. If the underlying hardware driver continually rounds the offset down such that it never reaches min_read_offset, the loop drops out after 100 iterations.
The function then unconditionally returns 0 (success). This would configure the SPI offload to trigger an SPI read before the ADC finishes its conversion, leading to corrupted data. Should this check if config->periodic.offset_ns >= min_read_offset after the loop and return an error if it fails?
Quoted 1 line
+}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
Re: [PATCH v3 4/5] iio: adc: ltc2378: Enable high-speed data capture
In reply to Marcelo SchmittHi Marcelo,
drivers/iio/adc/Kconfig config LTC2378 · line 948
Quoted 1 line · Marcelo Schmitt
drivers/iio/adc/Kconfig +1
-0+ select LTC2378_LIB_OFFLOAD_BUFFER if SPI_OFFLOAD && PWM && SPI_OFFLOAD_TRIGGER_PWM && IIO_BUFFER && IIO_BUFFER_DMAENGINE
The PWM in this expression is unnecessary since you're && it with SPI_OFFLOAD_TRIGGER_PWM.
- Julian Braha
David Lechner
reviewerRe: [PATCH v3 4/5] iio: adc: ltc2378: Enable high-speed data capture
In reply to Marcelo SchmittQuoted 49 lines · Marcelo Schmitt
Make use of SPI transfer offloading to speed up data capture, enabling data acquisition at faster sample rates (up to 2 MSPS).
Signed-off-by Marcelo Schmitt <[email protected]>---
Change log v2 -> v3:
- Fixed the evaluation loop conditions for CNV PWM and SPI Engine trigger PWM, avoiding potential infinite loop if and CPU stall.
- Added comment to about initial PWM disable.
- Adjusted SPI offload setup initialization to not print error on a valid condition.
- Fully initialize IIO channelscan_type.
- Reworked to make offload support not imply all dependencies to be built in.
- Madesampling_frequencya buffer attribute.
- Made offload support not require DMA and other features to be built in.
- Now using samescan_typeconfiguration for all use cases.drivers/iio/adc/Kconfig | 19 ++ drivers/iio/adc/Makefile | 6 + drivers/iio/adc/ltc2378-lib-core.c | 35 +++ drivers/iio/adc/ltc2378-offload-buffer.c | 305 +++++++++++++++++++++++ drivers/iio/adc/ltc2378.c | 46 ++++ drivers/iio/adc/ltc2378.h | 42 ++++ 6 files changed, 453 insertions(+) create mode 100644 drivers/iio/adc/ltc2378-lib-core.c create mode 100644 drivers/iio/adc/ltc2378-offload-buffer.cdrivers/iio/adc/Kconfig +8
-0@@ -944,6 +944,9 @@ config LTC2378 tristate "Analog Devices LTC2378 ADC driver" depends on SPI depends on GPIOLIB + select LTC2378_LIB + select LTC2378_LIB_OFFLOAD_BUFFER if SPI_OFFLOAD && PWM && SPI_OFFLOAD_TRIGGER_PWM && IIO_BUFFER && IIO_BUFFER_DMAENGINE + select LTC2378_LIB_TRIGGERED_BUFFER if IIO_BUFFER help Say yes here to build support for Analog Devices LTC2378-20 and similar analog to digital converters.@@ -2027,3 +2030,19 @@ config XILINX_AMS xilinx-ams. endmenu + +config LTC2378_LIB + tristate + help + Say yes here to build support for buffered data capture with LTC2378
drivers/iio/adc/Kconfig config XILINX_AMS · line 2047
Quoted 10 lines
drivers/iio/adc/Kconfig +10
-0+ +config LTC2378_LIB_OFFLOAD_BUFFER + bool + help + Say yes here to build support for high speed data capture with LTC2378 + +config LTC2378_LIB_TRIGGERED_BUFFER + bool + select IIO_TRIGGERED_BUFFER + help
Why do these need to be compile-time options to only select one or the other? In all other SPI offload ADCs we've done so far, they always support both and gets selected at runtime based on devicetree config.
Quoted 6 lines
drivers/iio/adc/Kconfig +1
-0+ Say yes here to build support for buffered data capture with LTC2378drivers/iio/adc/Makefile +0
-0
drivers/iio/adc/Makefile obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o · line 89
Quoted 8 lines
drivers/iio/adc/Makefile +5
-0@@ -82,6 +82,12 @@ obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o obj-$(CONFIG_LTC2309) += ltc2309.o obj-$(CONFIG_LTC2378) += ltc2378.o + +ltc2378_lib-y += ltc2378-lib-core.o +ltc2378_lib-$(CONFIG_LTC2378_LIB_OFFLOAD_BUFFER) += ltc2378-offload-buffer.o +ltc2378_lib-$(CONFIG_LTC2378_LIB_TRIGGERED_BUFFER) += ltc2378-triggered-buffer.o +obj-$(CONFIG_LTC2378_LIB) += ltc2378_lib.o
Why do these need to be split into separate files? The driver isn't that long, so seems better to just do it all in one file to make it easier to read.
Quoted 27 lines
drivers/iio/adc/Makefile +1
-0+ obj-$(CONFIG_LTC2471) += ltc2471.o obj-$(CONFIG_LTC2485) += ltc2485.o obj-$(CONFIG_LTC2496) += ltc2496.o ltc2497-core.odrivers/iio/adc/ltc2378-lib-core.c new file +17
-0@@ -0,0 +1,35 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* +* Analog Devices LTC2378 ADC series driver +* +* Copyright (C) 2026 Analog Devices Inc. +* Author: Marcelo Schmitt <[email protected]> +*/ + +#include <linux/err.h> +#include <linux/iio/iio.h> + +#include "ltc2378.h" + +int ltc2378_lib_buffer_setup (struct iio_dev *indio_dev,struct ltc2378_state *st) +{ +struct device *dev = &st->spi->dev; +int ret;
drivers/iio/adc/ltc2378-lib-core.c line 27
Quoted 10 lines
drivers/iio/adc/ltc2378-lib-core.c new file +10
-0+ + ret =__ltc2378_set_offload_ops (st); +if (ret == -EOPNOTSUPP ) +return 0 ;/* Let device setup complete without buffer support */ + +if (!ret) + ret = st->ops->buffer_setup (indio_dev, st); + +if (ret) +return dev_err_probe (dev, ret,"error on SPI offload setup\n" );
Would be better to just return early instead of doing the !ret check.
Quoted 5 lines
drivers/iio/adc/ltc2378-lib-core.c new file +5
-0+ +return 0 ; +} +EXPORT_SYMBOL_NS_GPL (ltc2378_lib_buffer_setup,"IIO_LTC2378" ); +
...
Quoted 13 lines
drivers/iio/adc/ltc2378.c +1
-0@@ -17,6 +17,7 @@#include <linux/regulator/consumer.h> #include <linux/spi/spi.h> #include <linux/types.h> +#include <linux/units.h> #include <linux/iio/iio.h> #include <linux/iio/types.h>
drivers/iio/adc/ltc2378.c line 30
Quoted 4 lines
drivers/iio/adc/ltc2378.c +1
-0@@ -26,120 +27,160 @@static const struct ltc2378_chip_info ltc2338_18_chip_info = { .name ="ltc2338-18" , .resolution =18 , + .max_sample_rate_hz =HZ_PER_MHZ ,
1 * HZ_PER_MHZ would make a bit more sense
Quoted 4 lines
drivers/iio/adc/ltc2378.c +1
-0+ .tconv_ns = 527, .bipolar = true, };
Marcelo Schmitt
maintainerPATCH v3 5/5 iio: adc: ltc2378: Enable triggered buffer data capture
In reply to Marcelo SchmittEnable users to run triggered data captures with LTC2378 and similar ADCs.
---
Change log v2 -> v3:
- Extracted from main driver file into a separate buffer-specific containment.
drivers/iio/adc/ltc2378-lib-core.c | 17 +++++++- drivers/iio/adc/ltc2378-triggered-buffer.c | 49 ++++++++++++++++++++++ drivers/iio/adc/ltc2378.h | 15 +++++++ 3 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 drivers/iio/adc/ltc2378-triggered-buffer.c
drivers/iio/adc/ltc2378-lib-core.c +16 -1
@@ -18,7 +18,7 @@ int ltc2378_lib_buffer_setup(struct iio_dev *indio_dev, struct ltc2378_state *st ret =__ltc2378_set_offload_ops (st);if (ret == -EOPNOTSUPP ) -return 0 ;/* Let device setup complete without buffer support */ + goto trigger_buf_setup;if (!ret) ret = st->ops->buffer_setup (indio_dev, st);
@@ -27,6 +27,21 @@ int ltc2378_lib_buffer_setup(struct iio_dev *indio_dev, struct ltc2378_state *streturn dev_err_probe (dev, ret,"error on SPI offload setup\n" );return 0 ; + +trigger_buf_setup: + ret =__ltc2378_set_triggered_buf_ops (st); +if (ret == -EOPNOTSUPP ) +return 0 ;/* Let device setup complete without buffer support */ + +if (!ret) + ret = st->ops->buffer_setup (indio_dev, st); + +if (ret) +return dev_err_probe (dev, ret,"error on buffer setup\n" ); + + st->chans[st->num_iio_chans++] =IIO_CHAN_SOFT_TIMESTAMP (1 ); + +return 0 ; }EXPORT_SYMBOL_NS_GPL (ltc2378_lib_buffer_setup,"IIO_LTC2378" );
drivers/iio/adc/ltc2378-triggered-buffer.c new file +49 -0
49 lines not on this page · raw message
drivers/iio/adc/ltc2378.h +15 -0
@@ -89,6 +89,8 @@ int ltc2378_lib_buffer_setup(struct iio_dev *indio_dev, struct ltc2378_state *st#define __ltc2378_set_offload_ops (st) ltc2378_set_offload_ops((st)) +#define __ltc2378_set_triggered_buf_ops (st) ltc2378_set_triggered_buf_ops((st)) +#ifdef CONFIG_LTC2378_LIB_OFFLOAD_BUFFER int ltc2378_set_offload_ops (struct ltc2378_state *st);
@@ -102,4 +104,17 @@ static inline int ltc2378_set_offload_ops(struct ltc2378_state *st)#endif /* CONFIG_LTC2378_LIB_OFFLOAD_BUFFER */ +#ifdef CONFIG_LTC2378_LIB_TRIGGERED_BUFFER + +int ltc2378_set_triggered_buf_ops (struct ltc2378_state *st); + +#else + +static inline int ltc2378_set_triggered_buf_ops (struct ltc2378_state *st) +{ +return -EOPNOTSUPP ; +} + +#endif /* CONFIG_LTC2378_LIB_TRIGGERED_BUFFER */ +#endif /* __DRIVERS_IIO_ADC_LTC2378_H__ */
Signature · 1 line
2.53.0
[email protected]
AI reviewRe: [PATCH v3 5/5] iio: adc: ltc2378: Enable triggered buffer data capture
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Triggered buffer setup fallback is unreachable when SPI offload support is compiled in, causing…
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Triggered buffer setup fallback is unreachable when SPI offload support is compiled in, causing driver probe to fail on systems without SPI offload hardware.
- [High] The triggered buffer pushes misaligned data on big-endian architectures, yielding constant zero readings for 16-bit parts.
- [Low] The newly added file ltc2378-triggered-buffer.c uses #include <ltc2378.h> instead of #include "ltc2378.h", causing a compile error. --
commit 975300646e8809a19fe7c482edd698d128476317
Author: Marcelo Schmitt <[email protected]>
iio: adc: ltc2378: Enable triggered buffer data capture
Enable users to run triggered data captures with LTC2378 and similar ADCs.
Quoted 14 lines
drivers/iio/adc/ltc2378-lib-core.c +1
-1@@ -18,7 +18,7 @@ int ltc2378_lib_buffer_setup(struct iio_dev *indio_dev, struct ltc2378_state *st ret =__ltc2378_set_offload_ops (st);if (ret == -EOPNOTSUPP ) -return 0 ;/* Let device setup complete without buffer support */ + goto trigger_buf_setup;if (!ret) ret = st->ops->buffer_setup (indio_dev, st);
drivers/iio/adc/ltc2378-lib-core.c
Quoted 2 lines
drivers/iio/adc/ltc2378-lib-core.c +0
-0if (ret)return dev_err_probe (dev, ret,"error on SPI offload setup\n" );
[Severity: High]
If CONFIG_LTC2378_LIB_OFFLOAD_BUFFER is enabled, does __ltc2378_set_offload_ops() unconditionally return 0?
If so, the fallback condition if (ret == -EOPNOTSUPP) is never met. The driver then calls st->ops->buffer_setup(), which maps to ltc2378_offload_buffer_setup.
If the device lacks an SPI offload engine in the device tree, devm_spi_offload_get() will return -ENODEV, failing the probe entirely.
Does this code prevent the driver from gracefully falling back to triggered buffers on non-offload setups?
[ ... ]
Quoted 7 lines
drivers/iio/adc/ltc2378-triggered-buffer.c new file +1
-0@@ -0,0 +1,49 @@ +// SPDX-License-Identifier: GPL-2.0+
drivers/iio/adc/ltc2378-triggered-buffer.c line 11
Quoted 10 lines
drivers/iio/adc/ltc2378-triggered-buffer.c new file +10
-0+/* +* Copyright (C) 2026 Analog Devices, Inc. +* Author: Marcelo Schmitt <[email protected]> +*/ + +#include <linux/iio/buffer.h> +#include <linux/iio/triggered_buffer.h> +#include <linux/iio/trigger_consumer.h> + +#include <ltc2378.h>
[Severity: Low]
Does this include directive cause a compile error because it uses angle brackets?
In C, #include <...> instructs the preprocessor to search standard system directories. Because ltc2378.h resides in the local subsystem directory drivers/iio/adc/ and the IIO Makefile does not add -I$(src), compilation fails with a missing file error.
drivers/iio/adc/ltc2378-triggered-buffer.c line 25
Quoted 13 lines
drivers/iio/adc/ltc2378-triggered-buffer.c new file +13
-0+static irqreturn_t ltc2378_trigger_handler (int irq,void *p) +{ +struct iio_poll_func *pf = p; +struct iio_dev *indio_dev = pf->indio_dev; +struct ltc2378_state *st =iio_priv (indio_dev); +int ret; + + ret =ltc2378_convert_and_acquire (st); +if (ret <0 ) + goto err_out; + +iio_push_to_buffers_with_ts (indio_dev, &st->scan,sizeof (st->scan), + pf->timestamp);
[Severity: High]
Does this code push misaligned data on big-endian architectures, resulting in constant zero readings for 16-bit parts?
The IIO channel specifies storagebits = 32 and shift = 0, so consumers expect a 32-bit native-endian value with data in the lowest 16 bits.
During ltc2378_trigger_handler(), spi_sync_transfer() with bits_per_word = 16 stores the 16-bit sample in CPU byte order at the start of the st->scan.data union (sample_buf16).
On little-endian systems, this natively aligns with the lower 16 bits of the 32-bit container. However, on big-endian systems, the 16-bit sample occupies the most significant bytes of the 32-bit word.
When the IIO core processes this as a 32-bit word, the data sits in the upper 16 bits while the lower 16 bits remain zero. Since scan_type.shift is 0, masking the lower 16 bits results in the consumer incorrectly receiving zero for every sample.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
David Lechner
reviewerRe: [PATCH v3 5/5] iio: adc: ltc2378: Enable triggered buffer data capture
In reply to Marcelo SchmittQuoted 31 lines · Marcelo Schmitt
Enable users to run triggered data captures with LTC2378 and similar ADCs.
Signed-off-by Marcelo Schmitt <[email protected]>---
Change log v2 -> v3:
- Extracted from main driver file into a separate buffer-specific containment.drivers/iio/adc/ltc2378-lib-core.c | 17 +++++++- drivers/iio/adc/ltc2378-triggered-buffer.c | 49 ++++++++++++++++++++++ drivers/iio/adc/ltc2378.h | 15 +++++++ 3 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 drivers/iio/adc/ltc2378-triggered-buffer.cdrivers/iio/adc/ltc2378-lib-core.c +2
-1@@ -18,7 +18,7 @@ int ltc2378_lib_buffer_setup(struct iio_dev *indio_dev, struct ltc2378_state *st ret =__ltc2378_set_offload_ops (st);if (ret == -EOPNOTSUPP ) -return 0 ;/* Let device setup complete without buffer support */ + goto trigger_buf_setup;if (!ret) ret = st->ops->buffer_setup (indio_dev, st);@@ -27,6 +27,21 @@ int ltc2378_lib_buffer_setup(struct iio_dev *indio_dev, struct ltc2378_state *streturn dev_err_probe (dev, ret,"error on SPI offload setup\n" );return 0 ; +
drivers/iio/adc/ltc2378-lib-core.c int ltc2378_lib_buffer_setup(struct iio_dev *indio_dev, struct ltc2378_state *st · line 40
Quoted 10 lines
drivers/iio/adc/ltc2378-lib-core.c +10
-0+trigger_buf_setup: + ret =__ltc2378_set_triggered_buf_ops (st); +if (ret == -EOPNOTSUPP ) +return 0 ;/* Let device setup complete without buffer support */ + +if (!ret) + ret = st->ops->buffer_setup (indio_dev, st); + +if (ret) +return dev_err_probe (dev, ret,"error on buffer setup\n" );
This is repeating the code above. Seems like it would be better without the goto.
drivers/iio/adc/ltc2378-lib-core.c int ltc2378_lib_buffer_setup(struct iio_dev *indio_dev, struct ltc2378_state *st · line 42
Quoted 2 lines
drivers/iio/adc/ltc2378-lib-core.c +2
-0+ + st->chans[st->num_iio_chans++] =IIO_CHAN_SOFT_TIMESTAMP (1 );
Adding a channel here seems messy. I still think static const channel data would be better. But at least would be better if this was moved to the same function as the other channel setup.
Quoted 23 lines
drivers/iio/adc/ltc2378-lib-core.c +2
-0+ +return 0 ; }EXPORT_SYMBOL_NS_GPL (ltc2378_lib_buffer_setup,"IIO_LTC2378" );drivers/iio/adc/ltc2378-triggered-buffer.c new file +12
-0@@ -0,0 +1,49 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* +* Copyright (C) 2026 Analog Devices, Inc. +* Author: Marcelo Schmitt <[email protected]> +*/ + +#include <linux/iio/buffer.h> +#include <linux/iio/triggered_buffer.h> +#include <linux/iio/trigger_consumer.h> + +#include <ltc2378.h> +
drivers/iio/adc/ltc2378-triggered-buffer.c line 22
Quoted 10 lines
drivers/iio/adc/ltc2378-triggered-buffer.c new file +10
-0+static irqreturn_t ltc2378_trigger_handler (int irq,void *p) +{ +struct iio_poll_func *pf = p; +struct iio_dev *indio_dev = pf->indio_dev; +struct ltc2378_state *st =iio_priv (indio_dev); +int ret; + + ret =ltc2378_convert_and_acquire (st); +if (ret <0 ) + goto err_out;
As mentioned elsewhere, SPI xfer size may be 2 or 4 bytes, but we are always pushing 4 bytes, so this only works on little-endian architecture.
I think best would be to have storagesize = 16 when appropriate to avoid having to manually handle this.
Quoted 9 lines
drivers/iio/adc/ltc2378-triggered-buffer.c new file +9
-0+ +iio_push_to_buffers_with_ts (indio_dev, &st->scan,sizeof (st->scan), + pf->timestamp); + +err_out: +iio_trigger_notify_done (indio_dev->trig); +return IRQ_HANDLED ; +} +
Re: [PATCH v3 5/5] iio: adc: ltc2378: Enable triggered buffer data capture
Quoted 5 lines · David Lechner
Quoted 4 lines · Marcelo Schmitt
Enable users to run triggered data captures with LTC2378 and similar ADCs.
Signed-off-by Marcelo Schmitt <[email protected]>---
...
Quoted 18 lines
Quoted 5 lines
drivers/iio/adc/ltc2378-lib-core.c +1
-0@@ -27,6 +27,21 @@ int ltc2378_lib_buffer_setup(struct iio_dev *indio_dev, struct ltc2378_state *streturn dev_err_probe (dev, ret,"error on SPI offload setup\n" );return 0 ; +drivers/iio/adc/ltc2378-lib-core.c int ltc2378_lib_buffer_setup(struct iio_dev *indio_dev, struct ltc2378_state *st · line 40
Quoted 10 lines
drivers/iio/adc/ltc2378-lib-core.c +10
-0+trigger_buf_setup: + ret =__ltc2378_set_triggered_buf_ops (st); +if (ret == -EOPNOTSUPP ) +return 0 ;/* Let device setup complete without buffer support */ + +if (!ret) + ret = st->ops->buffer_setup (indio_dev, st); + +if (ret) +return dev_err_probe (dev, ret,"error on buffer setup\n" );This is repeating the code above. Seems like it would be better without the goto.
Ack
Quoted 8 lines
Review int ltc2378_lib_buffer_setup(struct iio_dev *indio_dev, struct ltc2378_state *st · line 42
Quoted 3 lines
(quoted diff) +2
-0+ + st->chans[st->num_iio_chans++] = IIO_CHAN_SOFT_TIMESTAMP(1);Adding a channel here seems messy. I still think static const channel data would be better. But at least would be better if this was moved to the same function as the other channel setup.
Agree, will think of a better place to init the timestamp channel while switching to static const channel.
Quoted 3 lines
Quoted 3 lines
drivers/iio/adc/ltc2378-lib-core.c +2
-0+ +return 0 ; }
...
Quoted 15 lines
drivers/iio/adc/ltc2378-triggered-buffer.c line 22
Quoted 10 lines
drivers/iio/adc/ltc2378-triggered-buffer.c new file +10
-0+static irqreturn_t ltc2378_trigger_handler (int irq,void *p) +{ +struct iio_poll_func *pf = p; +struct iio_dev *indio_dev = pf->indio_dev; +struct ltc2378_state *st =iio_priv (indio_dev); +int ret; + + ret =ltc2378_convert_and_acquire (st); +if (ret <0 ) + goto err_out;As mentioned elsewhere, SPI xfer size may be 2 or 4 bytes, but we are always pushing 4 bytes, so this only works on little-endian architecture.
Quoted 3 lines
I think best would be to have storagesize = 16 when appropriate to avoid having to manually handle this.
Okay, it will probably be easier to do that with static const channels. Am checking out other sashiko comments as well.
Quoted 10 lines
drivers/iio/adc/ltc2378-triggered-buffer.c line 31
Quoted 9 lines
drivers/iio/adc/ltc2378-triggered-buffer.c new file +9
-0+ +iio_push_to_buffers_with_ts (indio_dev, &st->scan,sizeof (st->scan), + pf->timestamp); + +err_out: +iio_trigger_notify_done (indio_dev->trig); +return IRQ_HANDLED ; +} +
Thanks,
Marcelo
David Lechner
reviewerQuoted 37 lines · Marcelo Schmitt
This patch series adds support for LTC2378 and similar low noise, low power, high speed, successive approximation register (SAR) ADCs. These ADCs are similar among each other, varying mainly on the amount of precision bits, maximum sample rate, and input configuration (either fully differential or pseudo-differential).
The first patch adds device tree documentation for LTC2378.
The second patch enables single-shot sample read with a GPIO connected to the LTC2378 CNV pin.
The third patch extends IIO DMAengine buffer interface to make
sampling_frequencyandsampling_frequency_availablebuffer attributes.The fourth patch enables high-speed data captures with SPI offloading. The setup is similar to AD4030, with a specialized PWM generator being used both for SPI offload triggering and conversion start signaling.
The last support patch enables running buffered data captures without SPI offloading.
Even though these parts are somewhat similar to AD4000, the wiring configuration for LTC parts is different as well as the available HDL for high speed sample rate mode. Because of that, I propose creating a new device driver for supporting LTC2378-like devices.
Specifications can be found at:
Quoted 8 lines
Putting Link: tags like this in the cover letter will cause b4 to apply all of these to all patches in the series, so we don't want to do that. I think it is enough that they are already in the code, we don't need to list them again here.