Devicetree
 help / color / mirror / Atom feed
* [PATCH 0/2] iio: adc: add LTC2387 and ADAQ2387x support
@ 2026-10-27 13:44 Nicolae-Daniel Deaconescu
  2026-10-27 13:44 ` [PATCH 1/2] dt-bindings: iio: adc: add lltc,ltc2387 Nicolae-Daniel Deaconescu
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Nicolae-Daniel Deaconescu @ 2026-10-27 13:44 UTC (permalink / raw)
  To: Nicolae-Daniel Deaconescu, Sergiu Cuciurean, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König
  Cc: linux-iio, devicetree, linux-kernel, linux-pwm

This series adds a driver for the LTC2387-16/18, 15 Msps SAR ADCs with
an LVDS data interface, and for the ADAQ23875, ADAQ23876 and ADAQ23878
uModule ADCs, which pair an LTC2387 die with a fully differential
amplifier front end.

Conversion start (CNV) and the gated LVDS data clock are generated by
two channels of an AXI PWM generator. Sample data is captured by the
AXI ADC IP core and reached through the IIO backend framework. The
front-end gain of the ADAQ parts is described with adi,gain-milli
(fixed at 2 on the ADAQ23875, pin-strapped on the ADAQ23876/78) and
applied to in_voltage_scale.

Tested on an EV-ADAQ23875FMCZ on a ZedBoard running the adaq2387x HDL
project in two-lane, 16-bit mode: probe, in_voltage_scale,
sampling_frequency (writes, rounding and range checks), buffered
capture up to 1M samples, and unbind/rebind. The bare LTC2387
variants, one-lane mode and the -x4 variants were not tested on
hardware.

Datasheets:
  https://www.analog.com/LTC2387-16
  https://www.analog.com/LTC2387-18
  https://www.analog.com/ADAQ23875
  https://www.analog.com/ADAQ23876
  https://www.analog.com/ADAQ23878

Evaluation board:
  https://www.analog.com/EVAL-ADAQ23875

HDL reference design and IP cores:
  https://analogdevicesinc.github.io/hdl/projects/adaq23875/index.html
  https://analogdevicesinc.github.io/hdl/library/axi_ltc2387/index.html
  https://analogdevicesinc.github.io/hdl/library/axi_pwm_gen/index.html

Sergiu Cuciurean (2):
  dt-bindings: iio: adc: add lltc,ltc2387
  iio: adc: add ltc2387 driver

 .../bindings/iio/adc/lltc,ltc2387.yaml        | 138 +++++
 MAINTAINERS                                   |   9 +
 drivers/iio/adc/Kconfig                       |  11 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/ltc2387.c                     | 482 ++++++++++++++++++
 5 files changed, 641 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/lltc,ltc2387.yaml
 create mode 100644 drivers/iio/adc/ltc2387.c


base-commit: 43ffda3c7f8456bbe641ae8249a077a361b97880
-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] dt-bindings: iio: adc: add lltc,ltc2387
  2026-10-27 13:44 [PATCH 0/2] iio: adc: add LTC2387 and ADAQ2387x support Nicolae-Daniel Deaconescu
@ 2026-10-27 13:44 ` Nicolae-Daniel Deaconescu
  2026-10-27 13:59   ` sashiko-bot
  2026-10-27 18:28   ` Conor Dooley
  2026-10-27 13:44 ` [PATCH 2/2] iio: adc: add ltc2387 driver Nicolae-Daniel Deaconescu
  2026-10-27 15:56 ` [PATCH 0/2] iio: adc: add LTC2387 and ADAQ2387x support Andy Shevchenko
  2 siblings, 2 replies; 9+ messages in thread
From: Nicolae-Daniel Deaconescu @ 2026-10-27 13:44 UTC (permalink / raw)
  To: Nicolae-Daniel Deaconescu, Sergiu Cuciurean, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König
  Cc: linux-iio, devicetree, linux-kernel, linux-pwm

From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>

Add bindings for the LTC2387-16 and LTC2387-18, 15 Msps SAR ADCs with
an LVDS data interface, and for the ADAQ23875, ADAQ23876 and ADAQ23878
uModule ADCs, which pair an LTC2387 die with a fully differential
amplifier front end.

Conversion start (CNV) and the gated LVDS clock are driven by two PWM
channels. Sample data is captured by an AXI ADC IP core, referenced
through io-backends. adi,use-two-lanes selects two-lane output, and
adi,gain-milli gives the front-end gain: fixed at 2 on the ADAQ23875,
set by pin strapping on the ADAQ23876 and ADAQ23878.

Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
Co-developed-by: Nicolae-Daniel Deaconescu <Nicolae-daniel.Deaconescu@analog.com>
Signed-off-by: Nicolae-Daniel Deaconescu <Nicolae-daniel.Deaconescu@analog.com>
Assisted-by: Claude:claude-opus-5-5
---
 .../bindings/iio/adc/lltc,ltc2387.yaml        | 138 ++++++++++++++++++
 MAINTAINERS                                   |   8 +
 2 files changed, 146 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/lltc,ltc2387.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/lltc,ltc2387.yaml b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2387.yaml
new file mode 100644
index 000000000000..f912260c8f79
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2387.yaml
@@ -0,0 +1,138 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2022 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/lltc,ltc2387.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Linear Technology / Analog Devices LTC2387 ADC
+
+maintainers:
+  - Sergiu Cuciurean <sergiu.cuciurean@analog.com>
+  - Nicolae-Daniel Deaconescu <Nicolae-daniel.Deaconescu@analog.com>
+
+description: |
+  The LTC2387-18 is a low noise, high speed, 18-bit 15Msps
+  successive approximation register (SAR) ADC ideally
+  suited for a wide range of applications.
+
+  https://www.analog.com/media/en/technical-documentation/data-sheets/238718fa.pdf
+  https://www.analog.com/media/en/technical-documentation/data-sheets/238716f.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,adaq23875
+      - adi,adaq23876
+      - adi,adaq23878
+      - lltc,ltc2387-16
+      - lltc,ltc2387-16-x4
+      - lltc,ltc2387-18
+      - lltc,ltc2387-18-x4
+
+  clocks:
+    description: Sampling clock
+    maxItems: 1
+
+  pwms:
+    maxItems: 2
+    description:
+      PWM signals used to synchronize the sampling procedure
+
+  pwm-names:
+    items:
+      - const: cnv
+      - const: clk_gate
+
+  io-backends:
+    description:
+      The AXI ADC IP block (axi_ltc2387) that captures the LVDS data stream.
+    maxItems: 1
+
+  vref-supply:
+    description:
+      External voltage reference. When absent the device's internal 4.096 V
+      reference is assumed.
+
+  adi,use-two-lanes:
+    description: The device uses two lanes for data transfer
+    type: boolean
+
+  adi,gain-milli:
+    description:
+      Gain of the fully-differential amplifier that the ADAQ2387x modules
+      integrate ahead of the LTC2387 die, expressed as gain * 1000. It is
+      fixed on the ADAQ23875 and set by pin strapping on the ADAQ23876 and
+      ADAQ23878, so it must match the board. The bare ltc2387-* parts have
+      no amplifier.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [370, 730, 870, 1380, 2000, 2250]
+
+required:
+  - compatible
+  - clocks
+  - io-backends
+  - pwms
+  - pwm-names
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - lltc,ltc2387-16
+              - lltc,ltc2387-16-x4
+              - lltc,ltc2387-18
+              - lltc,ltc2387-18-x4
+    then:
+      properties:
+        adi,gain-milli: false
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: adi,adaq23875
+    then:
+      properties:
+        adi,gain-milli:
+          const: 2000
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,adaq23876
+              - adi,adaq23878
+    then:
+      properties:
+        adi,gain-milli:
+          enum: [370, 730, 870, 1380, 2250]
+      required:
+        - adi,gain-milli
+
+additionalProperties: false
+
+examples:
+  - |
+    adc {
+            compatible = "lltc,ltc2387-16";
+            clocks = <&clk 0>;
+            io-backends = <&axi_adc>;
+            pwms = <&pwm 0 0>, <&pwm 1 0>;
+            pwm-names = "cnv", "clk_gate";
+            vref-supply = <&vref>;
+    };
+  - |
+    adc {
+            compatible = "adi,adaq23878";
+            clocks = <&clk 0>;
+            io-backends = <&axi_adc>;
+            pwms = <&pwm 0 0>, <&pwm 1 0>;
+            pwm-names = "cnv", "clk_gate";
+            vref-supply = <&vref>;
+            adi,use-two-lanes;
+            adi,gain-milli = <870>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 52024397dc78..b871d7f25d76 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15529,6 +15529,14 @@ W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml
 F:	drivers/iio/adc/ltc2378.c
 
+LTC2387 IIO ADC DRIVER
+M:	Nicolae-Daniel Deaconescu <Nicolae-daniel.Deaconescu@analog.com>
+M:	Sergiu Cuciurean <sergiu.cuciurean@analog.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/iio/adc/lltc,ltc2387.yaml
+
 LTC2664 IIO DAC DRIVER
 M:	Michael Hennerich <michael.hennerich@analog.com>
 M:	Kim Seer Paller <kimseer.paller@analog.com>
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] iio: adc: add ltc2387 driver
  2026-10-27 13:44 [PATCH 0/2] iio: adc: add LTC2387 and ADAQ2387x support Nicolae-Daniel Deaconescu
  2026-10-27 13:44 ` [PATCH 1/2] dt-bindings: iio: adc: add lltc,ltc2387 Nicolae-Daniel Deaconescu
@ 2026-10-27 13:44 ` Nicolae-Daniel Deaconescu
  2026-10-27 14:06   ` sashiko-bot
                     ` (2 more replies)
  2026-10-27 15:56 ` [PATCH 0/2] iio: adc: add LTC2387 and ADAQ2387x support Andy Shevchenko
  2 siblings, 3 replies; 9+ messages in thread
From: Nicolae-Daniel Deaconescu @ 2026-10-27 13:44 UTC (permalink / raw)
  To: Nicolae-Daniel Deaconescu, Sergiu Cuciurean, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König
  Cc: linux-iio, devicetree, linux-kernel, linux-pwm

From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>

Add a driver for the LTC2387-16/18 and the ADAQ23875/23876/23878.

The driver generates CNV and the gated data clock with two PWM
channels, timed against the reference clock to meet t_FIRSTCLK, and
gets sample data through the IIO backend framework from the AXI ADC
core. It supports one-lane and two-lane output and reports the
sampling frequency the PWMs produce after rounding to whole reference
clock cycles. in_voltage_scale is referred to the analog input, so on
the ADAQ parts it is divided by the front-end gain.

Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
Co-developed-by: Nicolae-Daniel Deaconescu <Nicolae-daniel.Deaconescu@analog.com>
Signed-off-by: Nicolae-Daniel Deaconescu <Nicolae-daniel.Deaconescu@analog.com>
Assisted-by: Claude:claude-opus-5-5
---
 MAINTAINERS               |   1 +
 drivers/iio/adc/Kconfig   |  11 +
 drivers/iio/adc/Makefile  |   1 +
 drivers/iio/adc/ltc2387.c | 482 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 495 insertions(+)
 create mode 100644 drivers/iio/adc/ltc2387.c

diff --git a/MAINTAINERS b/MAINTAINERS
index b871d7f25d76..6e242a955e09 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15536,6 +15536,7 @@ L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/adc/lltc,ltc2387.yaml
+F:	drivers/iio/adc/ltc2387.c
 
 LTC2664 IIO DAC DRIVER
 M:	Michael Hennerich <michael.hennerich@analog.com>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 02de4809ae31..c543f066dbee 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -994,6 +994,17 @@ config LTC2378
 	  This driver can also be built as a module. If so, the module will
 	  be called ltc2378.
 
+config LTC2387
+	tristate "Linear Technology LTC2387 ADC driver"
+	depends on PWM
+	select IIO_BACKEND
+	help
+	  Say yes here to build support for Linear Technology LTC2387 18/16
+	  bit 15Msps SAR ADC.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called ltc2387.
+
 config LTC2471
 	tristate "Linear Technology LTC2471 and LTC2473 ADC driver"
 	depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 9e82c21dbd37..4032276f55f1 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -84,6 +84,7 @@ 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_LTC2387) += ltc2387.o
 obj-$(CONFIG_LTC2471) += ltc2471.o
 obj-$(CONFIG_LTC2485) += ltc2485.o
 obj-$(CONFIG_LTC2496) += ltc2496.o ltc2497-core.o
diff --git a/drivers/iio/adc/ltc2387.c b/drivers/iio/adc/ltc2387.c
new file mode 100644
index 000000000000..c236b99baa4a
--- /dev/null
+++ b/drivers/iio/adc/ltc2387.c
@@ -0,0 +1,482 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+/*
+ * Linear Technology LTC2387 ADC driver
+ *
+ * Copyright 2022 - 2026 Analog Devices Inc.
+ */
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/iio/backend.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/kernel.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/pwm.h>
+#include <linux/regulator/consumer.h>
+#include <linux/mod_devicetable.h>
+
+#define LTC2387_VREF		4096
+#define LTC2387_T_CNVH		8
+
+/*
+ * Minimal value for t_{FIRSTCLK} according to
+ * https://www.analog.com/media/en/technical-documentation/data-sheets/238718fa.pdf
+ * is 65 ns. Add some slack because there is some rounding involved in the PWM
+ * driver. With the PWM driver rounding to the nearest possible value, targeting
+ * 70 ns works for input clk rates >= 100 MHz.
+ */
+#define LTC2387_T_FIRSTCLK_NS	70
+
+#define KHz 1000
+#define MHz (1000 * KHz)
+
+#define LTC2378_MULTIPLE_CHAN(_idx, _storagebits, _realbits, _shift)	\
+	{								\
+		.type = IIO_VOLTAGE,					\
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
+		.indexed = 1,						\
+		.channel = _idx,					\
+		.scan_index = _idx,					\
+		.scan_type = {						\
+			.sign = 's',					\
+			.storagebits = _storagebits,			\
+			.realbits = _realbits,				\
+			.shift = _shift,				\
+		},							\
+	}
+
+#define LTC2378_CHAN(_realbits, _storagebits)				\
+	{								\
+		.type = IIO_VOLTAGE,					\
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
+		.scan_type = {						\
+			.sign = 's',					\
+			.storagebits = _storagebits,			\
+			.realbits = _realbits,				\
+		},							\
+	}
+
+enum ltc2387_lane_modes {
+	ONE_LANE = 0,
+	TWO_LANES = 1
+};
+
+enum ltc2387_id {
+	ID_LTC2387_16,
+	ID_LTC2387_16_X4,
+	ID_LTC2387_18,
+	ID_LTC2387_18_X4,
+	ID_ADAQ23875,
+	ID_ADAQ23876,
+	ID_ADAQ23878,
+};
+
+struct ltc2387_info {
+	const char *name;
+	struct iio_chan_spec channels[4];
+	unsigned int test_pattern[2];
+	int num_channels;
+	int resolution;
+	/* Gain * 1000 when adi,gain-milli is absent; unity for the bare parts. */
+	u32 default_gain_milli;
+};
+
+#define LTC2387_TEST_PATTERN_16 {					\
+	[ONE_LANE]  = 0b1010000001111111,				\
+	[TWO_LANES] = 0b1100110000111111,				\
+}
+
+#define LTC2387_TEST_PATTERN_18 {					\
+	[ONE_LANE]  = 0b101000000111111100,				\
+	[TWO_LANES] = 0b110011000011111100,				\
+}
+
+static const struct ltc2387_info ltc2387_infos[] = {
+	[ID_LTC2387_16] = {
+		.name = "ltc2387-16",
+		.default_gain_milli = 1000,
+		.resolution = 16,
+		.test_pattern = LTC2387_TEST_PATTERN_16,
+		.channels = { LTC2378_CHAN(16, 16) },
+		.num_channels = 1,
+	},
+	[ID_LTC2387_16_X4] = {
+		.name = "ltc2387-16-x4",
+		.default_gain_milli = 1000,
+		.resolution = 16,
+		.test_pattern = LTC2387_TEST_PATTERN_16,
+		.channels = {
+			LTC2378_MULTIPLE_CHAN(0, 64, 16, 0),
+			LTC2378_MULTIPLE_CHAN(1, 64, 16, 16),
+			LTC2378_MULTIPLE_CHAN(2, 64, 16, 32),
+			LTC2378_MULTIPLE_CHAN(3, 64, 16, 48),
+		},
+		.num_channels = 4,
+	},
+	[ID_LTC2387_18] = {
+		.name = "ltc2387-18",
+		.default_gain_milli = 1000,
+		.resolution = 18,
+		.test_pattern = LTC2387_TEST_PATTERN_18,
+		.channels = { LTC2378_CHAN(18, 32) },
+		.num_channels = 1,
+	},
+	[ID_LTC2387_18_X4] = {
+		.name = "ltc2387-18-x4",
+		.default_gain_milli = 1000,
+		.resolution = 18,
+		.test_pattern = LTC2387_TEST_PATTERN_18,
+		.channels = {
+			LTC2378_MULTIPLE_CHAN(0, 128, 18, 0),
+			LTC2378_MULTIPLE_CHAN(1, 128, 18, 32),
+			LTC2378_MULTIPLE_CHAN(2, 128, 18, 64),
+			LTC2378_MULTIPLE_CHAN(3, 128, 18, 96),
+		},
+		.num_channels = 4,
+	},
+	[ID_ADAQ23875] = {
+		.name = "adaq23875",
+		.default_gain_milli = 2000,
+		.resolution = 16,
+		.test_pattern = LTC2387_TEST_PATTERN_16,
+		.channels = { LTC2378_CHAN(16, 16) },
+		.num_channels = 1,
+	},
+	[ID_ADAQ23876] = {
+		.name = "adaq23876",
+		.default_gain_milli = 0,
+		.resolution = 16,
+		.test_pattern = LTC2387_TEST_PATTERN_16,
+		.channels = { LTC2378_CHAN(16, 16) },
+		.num_channels = 1,
+	},
+	[ID_ADAQ23878] = {
+		.name = "adaq23878",
+		.default_gain_milli = 0,
+		.resolution = 18,
+		.test_pattern = LTC2387_TEST_PATTERN_18,
+		.channels = { LTC2378_CHAN(18, 32) },
+		.num_channels = 1,
+	},
+};
+
+struct ltc2387_dev {
+	const struct ltc2387_info *device_info;
+	enum ltc2387_lane_modes lane_mode;
+	unsigned long ref_clk_rate;
+	struct iio_backend *back;
+	struct pwm_device *clk_gate;
+	struct regulator *vref;
+	struct pwm_device *cnv;
+	struct clk *ref_clk;
+
+	unsigned int vref_mv;
+	u32 sampling_freq;
+	u32 gain_milli;
+};
+
+static int ltc2387_set_sampling_freq(struct ltc2387_dev *ltc, int freq)
+{
+	unsigned long long ref_clk_period_ns;
+	struct pwm_waveform clk_gate_wf = { }, cnv_wf = { };
+	unsigned long long period_cycles;
+	int ret, clk_en_time;
+	u32 rem;
+
+	if (freq <= 0 || freq > ltc->ref_clk_rate)
+		return -EINVAL;
+
+	ref_clk_period_ns = DIV_ROUND_UP(NSEC_PER_SEC, ltc->ref_clk_rate);
+
+	cnv_wf.duty_length_ns = ref_clk_period_ns;
+
+	/*
+	 * The goal here is that the PWM is configured with a minimal period not
+	 * less than 1 / freq (with freq measured in Hz).
+	 *
+	 * When a period P (measured in ns) is passed to pwm_apply_state(), the
+	 * actually implemented period is:
+	 *
+	 *      round_down(P * R / NSEC_PER_SEC) / R
+	 *
+	 * (measured in s) with R = ltc->ref_clk_rate. So we have:
+	 *
+	 *        round_down(P * R / NSEC_PER_SEC) / R ≥ 1 / freq
+	 *      ⟺ round_down(P * R / NSEC_PER_SEC) ≥ R / freq
+	 *
+	 * With the LHS being integer this is equivalent to:
+	 *
+	 *        round_down(P * R / NSEC_PER_SEC) ≥ round_up(R / freq)
+	 *      ⟺ P * R / NSEC_PER_SEC ≥ round_up(R / freq)
+	 *      ⟺ P ≥ round_up(R / freq) * NSEC_PER_SEC / R
+	 */
+
+	cnv_wf.period_length_ns =
+		div_u64_rem((u64)DIV_ROUND_UP(ltc->ref_clk_rate, freq) * NSEC_PER_SEC,
+			    ltc->ref_clk_rate, &rem);
+	if (rem)
+		cnv_wf.period_length_ns += 1;
+
+	/* Gate the active period of the clock (see page 10-13 for both LTC's) */
+	if (ltc->lane_mode == TWO_LANES)
+		clk_en_time = DIV_ROUND_UP_ULL(ltc->device_info->resolution, 4);
+	else
+		clk_en_time = DIV_ROUND_UP_ULL(ltc->device_info->resolution, 2);
+
+	clk_gate_wf.duty_length_ns = ref_clk_period_ns * clk_en_time;
+	clk_gate_wf.duty_offset_ns = LTC2387_T_FIRSTCLK_NS;
+
+	if (cnv_wf.period_length_ns < clk_gate_wf.duty_length_ns)
+		cnv_wf.period_length_ns = clk_gate_wf.duty_length_ns + ref_clk_period_ns;
+
+	clk_gate_wf.period_length_ns = cnv_wf.period_length_ns;
+
+	if (clk_gate_wf.duty_offset_ns >= clk_gate_wf.period_length_ns)
+		div64_u64_rem(clk_gate_wf.duty_offset_ns,
+			      clk_gate_wf.period_length_ns,
+			      &clk_gate_wf.duty_offset_ns);
+
+	ret = pwm_set_waveform_might_sleep(ltc->cnv, &cnv_wf, false);
+	if (ret < 0)
+		return ret;
+
+	ret = pwm_set_waveform_might_sleep(ltc->clk_gate, &clk_gate_wf, false);
+	if (ret < 0)
+		return ret;
+
+	period_cycles = mul_u64_u32_div(cnv_wf.period_length_ns,
+					ltc->ref_clk_rate, NSEC_PER_SEC);
+	if (!period_cycles)
+		return -EINVAL;
+
+	ltc->sampling_freq = DIV_ROUND_CLOSEST_ULL(ltc->ref_clk_rate,
+						   period_cycles);
+
+	return 0;
+}
+
+static int ltc2387_setup(struct iio_dev *indio_dev)
+{
+	struct ltc2387_dev *ltc = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
+
+	if (device_property_present(dev, "adi,use-two-lanes"))
+		ltc->lane_mode = TWO_LANES;
+
+	return ltc2387_set_sampling_freq(ltc, 15 * MHz);
+}
+
+static int ltc2387_read_raw(struct iio_dev *indio_dev,
+			    const struct iio_chan_spec *chan,
+			    int *val, int *val2, long info)
+{
+	struct ltc2387_dev *ltc = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = ltc->sampling_freq;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		/* Referred to the SMA input, so divided by the front-end gain. */
+		*val = ltc->vref_mv * 2 * 1000;
+		*val2 = ltc->gain_milli << chan->scan_type.realbits;
+		return IIO_VAL_FRACTIONAL;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ltc2387_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long info)
+{
+	struct ltc2387_dev *ltc = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return ltc2387_set_sampling_freq(ltc, val);
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static void ltc2387_pwm_disable(void *data)
+{
+	pwm_disable(data);
+}
+
+static void ltc2387_regulator_disable(void *data)
+{
+	regulator_disable(data);
+}
+
+static void ltc2387_clk_disable(void *data)
+{
+	clk_disable_unprepare(data);
+}
+
+static const struct iio_info ltc2387_info = {
+	.read_raw = ltc2387_read_raw,
+	.write_raw = ltc2387_write_raw,
+};
+
+static const struct of_device_id ltc2387_of_match[] = {
+	{
+		.compatible = "lltc,ltc2387-16",
+		.data = &ltc2387_infos[ID_LTC2387_16]
+	}, {
+		.compatible = "lltc,ltc2387-16-x4",
+		.data = &ltc2387_infos[ID_LTC2387_16_X4]
+	}, {
+		.compatible = "lltc,ltc2387-18",
+		.data = &ltc2387_infos[ID_LTC2387_18]
+	}, {
+		.compatible = "lltc,ltc2387-18-x4",
+		.data = &ltc2387_infos[ID_LTC2387_18_X4]
+	}, {
+		.compatible = "adi,adaq23875",
+		.data = &ltc2387_infos[ID_ADAQ23875]
+	}, {
+		.compatible = "adi,adaq23876",
+		.data = &ltc2387_infos[ID_ADAQ23876]
+	}, {
+		.compatible = "adi,adaq23878",
+		.data = &ltc2387_infos[ID_ADAQ23878]
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, ltc2387_of_match);
+
+static int ltc2387_probe(struct platform_device *pdev)
+{
+	struct iio_dev			*indio_dev;
+	struct ltc2387_dev		*ltc;
+	int				ret;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*ltc));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	ltc = iio_priv(indio_dev);
+
+	ltc->vref = devm_regulator_get_optional(&pdev->dev, "vref");
+	if (!IS_ERR(ltc->vref)) {
+		ret = regulator_enable(ltc->vref);
+		if (ret) {
+			dev_err(&pdev->dev, "Can't enable vref regulator\n");
+			return ret;
+		}
+		ret = regulator_get_voltage(ltc->vref);
+		if (ret < 0)
+			return ret;
+
+		ltc->vref_mv = ret / 1000;
+		ret = devm_add_action_or_reset(&pdev->dev,
+					       ltc2387_regulator_disable,
+					       ltc->vref);
+		if (ret)
+			return ret;
+	} else {
+		if (PTR_ERR(ltc->vref) != -ENODEV)
+			return PTR_ERR(ltc->vref);
+
+		ltc->vref_mv = LTC2387_VREF; /* Internal vref is used */
+	}
+
+	ltc->ref_clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(ltc->ref_clk))
+		return PTR_ERR(ltc->ref_clk);
+
+	ret = clk_prepare_enable(ltc->ref_clk);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(&pdev->dev, ltc2387_clk_disable,
+				       ltc->ref_clk);
+	if (ret)
+		return ret;
+	ltc->ref_clk_rate = clk_get_rate(ltc->ref_clk);
+
+	ltc->clk_gate = devm_pwm_get(&pdev->dev, "clk_gate");
+	if (IS_ERR(ltc->clk_gate))
+		return PTR_ERR(ltc->clk_gate);
+
+	ret = devm_add_action_or_reset(&pdev->dev, ltc2387_pwm_disable,
+				       ltc->clk_gate);
+	if (ret)
+		return ret;
+
+	ltc->cnv = devm_pwm_get(&pdev->dev, "cnv");
+	if (IS_ERR(ltc->cnv))
+		return PTR_ERR(ltc->cnv);
+
+	ret = devm_add_action_or_reset(&pdev->dev, ltc2387_pwm_disable,
+				       ltc->cnv);
+
+	if (ret)
+		return ret;
+
+	ltc->device_info = device_get_match_data(&pdev->dev);
+	if (!ltc->device_info)
+		return -EINVAL;
+
+	ltc->gain_milli = ltc->device_info->default_gain_milli;
+	device_property_read_u32(&pdev->dev, "adi,gain-milli", &ltc->gain_milli);
+	if (!ltc->gain_milli)
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "adi,gain-milli must be nonzero\n");
+
+	indio_dev->channels = ltc->device_info->channels;
+	indio_dev->num_channels = ltc->device_info->num_channels;
+	indio_dev->dev.parent = &pdev->dev;
+	indio_dev->name = ltc->device_info->name;
+	indio_dev->info = &ltc2387_info;
+
+	ltc->back = devm_iio_backend_get(&pdev->dev, NULL);
+	if (IS_ERR(ltc->back))
+		return dev_err_probe(&pdev->dev, PTR_ERR(ltc->back),
+				     "failed to get IIO backend\n");
+
+	ret = devm_iio_backend_request_buffer(&pdev->dev, ltc->back, indio_dev);
+	if (ret)
+		return ret;
+
+	ret = devm_iio_backend_enable(&pdev->dev, ltc->back);
+	if (ret)
+		return ret;
+
+	/* axi_ltc2387 has a single capture channel carrying every scan element. */
+	ret = iio_backend_chan_enable(ltc->back, 0);
+	if (ret)
+		return ret;
+
+	ret = ltc2387_setup(indio_dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "\nltc2387 setup failed\n");
+		return ret;
+	}
+
+	return devm_iio_device_register(&pdev->dev, indio_dev);
+}
+
+static struct platform_driver ltc2387_driver = {
+	.probe          = ltc2387_probe,
+	.driver         = {
+		.name   = "ltc2387",
+		.of_match_table = ltc2387_of_match,
+	},
+};
+module_platform_driver(ltc2387_driver);
+
+MODULE_AUTHOR("Sergiu Cuciurean <sergiu.cuciurean@analog.com>");
+MODULE_DESCRIPTION("Linear Technology LTC2387 ADC");
+MODULE_LICENSE("Dual BSD/GPL");
+MODULE_IMPORT_NS("IIO_BACKEND");
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dt-bindings: iio: adc: add lltc,ltc2387
  2026-10-27 13:44 ` [PATCH 1/2] dt-bindings: iio: adc: add lltc,ltc2387 Nicolae-Daniel Deaconescu
@ 2026-10-27 13:59   ` sashiko-bot
  2026-10-27 18:28   ` Conor Dooley
  1 sibling, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-10-27 13:59 UTC (permalink / raw)
  To: Nicolae-Daniel Deaconescu; +Cc: devicetree, robh, conor+dt

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] dt-bindings: iio: adc: conflicting uint32 type for adi,gain-milli
- [Medium] dt-bindings: iio: adc: lltc,ltc2387: undocumented synthetic -x4 compatibles
- [Low] dt-bindings: iio: adc: lltc,ltc2387: example indentation and property order
--

commit 3bc833c852704e956c19734cf54a047dca662012
Author: Sergiu Cuciurean <sergiu.cuciurean@analog.com>

dt-bindings: iio: adc: add lltc,ltc2387

This commit adds devicetree bindings for the Linear Technology LTC2387-16
and LTC2387-18 SAR ADCs as well as the Analog Devices ADAQ23875, ADAQ23876,
and ADAQ23878 uModule ADCs.

> diff --git a/Documentation/devicetree/bindings/iio/adc/lltc,ltc2387.yaml b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2387.yaml
> new file mode 100644
> index 0000000000000..f912260c8f790
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2387.yaml
> @@ -0,0 +1,138 @@

[ ... ]

> +properties:
> +  compatible:
> +    enum:
> +      - adi,adaq23875
> +      - adi,adaq23876
> +      - adi,adaq23878
> +      - lltc,ltc2387-16
> +      - lltc,ltc2387-16-x4
> +      - lltc,ltc2387-18
> +      - lltc,ltc2387-18-x4

[Severity: Medium]
Are lltc,ltc2387-16-x4 and lltc,ltc2387-18-x4 actual IC part numbers, or do
they represent a board/FPGA topology where four discrete LTC2387 chips share
conversion and clock signals and have their samples packed together by an AXI
ADC core?

The LTC2387-16 and LTC2387-18 are single-channel SAR ADCs, and neither the
commit message nor the binding description documents these -x4 compatible
strings. Should multi-chip board or FPGA topologies be described using
standard devicetree properties rather than encoding a chip count into
synthetic compatible strings?

[ ... ]

> +  adi,gain-milli:
> +    description:
> +      Gain of the fully-differential amplifier that the ADAQ2387x modules
> +      integrate ahead of the LTC2387 die, expressed as gain * 1000. It is
> +      fixed on the ADAQ23875 and set by pin strapping on the ADAQ23876 and
> +      ADAQ23878, so it must match the board. The bare ltc2387-* parts have
> +      no amplifier.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [370, 730, 870, 1380, 2000, 2250]

[Severity: Medium]
Does defining adi,gain-milli with $ref: /schemas/types.yaml#/definitions/uint32
cause a property type conflict with existing Analog Devices ADC bindings?

In Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml and
Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml, adi,gain-milli is
already defined as uint16:

  adi,gain-milli:
    ...
    $ref: /schemas/types.yaml#/definitions/uint16

Because vendor properties share a global type namespace across schemas in
dtschema, shouldn't adi,gain-milli use /schemas/types.yaml#/definitions/uint16
here (and /bits/ 16 <870> in the example below) to avoid a dt_binding_check
conflict and keep the devicetree ABI consistent across ADI ADC bindings?

[ ... ]

> +examples:
> +  - |
> +    adc {
> +            compatible = "lltc,ltc2387-16";
> +            clocks = <&clk 0>;
> +            io-backends = <&axi_adc>;
> +            pwms = <&pwm 0 0>, <&pwm 1 0>;
> +            pwm-names = "cnv", "clk_gate";
> +            vref-supply = <&vref>;
> +    };
> +  - |
> +    adc {
> +            compatible = "adi,adaq23878";
> +            clocks = <&clk 0>;
> +            io-backends = <&axi_adc>;
> +            pwms = <&pwm 0 0>, <&pwm 1 0>;
> +            pwm-names = "cnv", "clk_gate";
> +            vref-supply = <&vref>;
> +            adi,use-two-lanes;
> +            adi,gain-milli = <870>;
> +    };

[Severity: Low]
This isn't a bug, but both examples indent the properties inside the adc node
by 8 spaces instead of 4 spaces, and the second example places
adi,use-two-lanes before adi,gain-milli instead of in alphabetical order
within the vendor-specific property group, which triggers warnings from
scripts/dtc/dt-check-style --mode=strict.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027134409.76824-1-Nicolae-daniel.Deaconescu@analog.com?part=1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] iio: adc: add ltc2387 driver
  2026-10-27 13:44 ` [PATCH 2/2] iio: adc: add ltc2387 driver Nicolae-Daniel Deaconescu
@ 2026-10-27 14:06   ` sashiko-bot
  2026-10-27 14:37   ` Joshua Crofts
  2026-10-28  2:39   ` Jonathan Cameron
  2 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-10-27 14:06 UTC (permalink / raw)
  To: Nicolae-Daniel Deaconescu; +Cc: devicetree, robh, conor+dt

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] iio: adc: ltc2387: invalid scan_type and missing scan_masks on x4 parts
- [Medium] iio: adc: ltc2387: regulator enable leak in ltc2387_probe() on error
- [Medium] iio: adc: ltc2387: unsynchronized PWM updates in ltc2387_write_raw()
- [Medium] iio: adc: ltc2387: wrong clk_gate duty and period clamp math
- [Low] iio: adc: ltc2387: unused LTC2387_T_CNVH macro and test_pattern field
--

commit 1dc8bcd329957722e234d172e41e35994a926ade
Author: Sergiu Cuciurean <sergiu.cuciurean@analog.com>

iio: adc: add ltc2387 driver

This commit adds an IIO ADC driver for the Linear Technology LTC2387-16/18
and Analog Devices ADAQ23875/23876/23878 SAR ADCs using two PWM channels and
the IIO backend framework.

> diff --git a/drivers/iio/adc/ltc2387.c b/drivers/iio/adc/ltc2387.c
> new file mode 100644
> index 0000000000000..c236b99baa4ab
> --- /dev/null
> +++ b/drivers/iio/adc/ltc2387.c
> @@ -0,0 +1,482 @@

[ ... ]

> +#define LTC2387_VREF		4096
> +#define LTC2387_T_CNVH		8

[ ... ]

> +struct ltc2387_info {
> +	const char *name;
> +	struct iio_chan_spec channels[4];
> +	unsigned int test_pattern[2];
> +	int num_channels;
> +	int resolution;
> +	/* Gain * 1000 when adi,gain-milli is absent; unity for the bare parts. */
> +	u32 default_gain_milli;
> +};
> +
> +#define LTC2387_TEST_PATTERN_16 {					\
> +	[ONE_LANE]  = 0b1010000001111111,				\
> +	[TWO_LANES] = 0b1100110000111111,				\
> +}
> +
> +#define LTC2387_TEST_PATTERN_18 {					\
> +	[ONE_LANE]  = 0b101000000111111100,				\
> +	[TWO_LANES] = 0b110011000011111100,				\
> +}

[Severity: Low]
Are LTC2387_T_CNVH and the test_pattern field in struct ltc2387_info (along
with LTC2387_TEST_PATTERN_16 and LTC2387_TEST_PATTERN_18) left over from an
earlier version? They are populated across ltc2387_infos[] in
drivers/iio/adc/ltc2387.c, but never read or referenced anywhere in the
driver.

> +
> +static const struct ltc2387_info ltc2387_infos[] = {
> +	[ID_LTC2387_16] = {
> +		.name = "ltc2387-16",
> +		.default_gain_milli = 1000,
> +		.resolution = 16,
> +		.test_pattern = LTC2387_TEST_PATTERN_16,
> +		.channels = { LTC2378_CHAN(16, 16) },
> +		.num_channels = 1,
> +	},
> +	[ID_LTC2387_16_X4] = {
> +		.name = "ltc2387-16-x4",
> +		.default_gain_milli = 1000,
> +		.resolution = 16,
> +		.test_pattern = LTC2387_TEST_PATTERN_16,
> +		.channels = {
> +			LTC2378_MULTIPLE_CHAN(0, 64, 16, 0),
> +			LTC2378_MULTIPLE_CHAN(1, 64, 16, 16),
> +			LTC2378_MULTIPLE_CHAN(2, 64, 16, 32),
> +			LTC2378_MULTIPLE_CHAN(3, 64, 16, 48),
> +		},
> +		.num_channels = 4,
> +	},
> +	[ID_LTC2387_18] = {
> +		.name = "ltc2387-18",
> +		.default_gain_milli = 1000,
> +		.resolution = 18,
> +		.test_pattern = LTC2387_TEST_PATTERN_18,
> +		.channels = { LTC2378_CHAN(18, 32) },
> +		.num_channels = 1,
> +	},
> +	[ID_LTC2387_18_X4] = {
> +		.name = "ltc2387-18-x4",
> +		.default_gain_milli = 1000,
> +		.resolution = 18,
> +		.test_pattern = LTC2387_TEST_PATTERN_18,
> +		.channels = {
> +			LTC2378_MULTIPLE_CHAN(0, 128, 18, 0),
> +			LTC2378_MULTIPLE_CHAN(1, 128, 18, 32),
> +			LTC2378_MULTIPLE_CHAN(2, 128, 18, 64),
> +			LTC2378_MULTIPLE_CHAN(3, 128, 18, 96),
> +		},
> +		.num_channels = 4,
> +	},

[Severity: High]
How does buffered capture work for the 4-channel entries (ID_LTC2387_16_X4
and ID_LTC2387_18_X4) in ltc2387_infos[]?

In the IIO buffer core (iio_compute_scan_bytes() ->
iio_storage_bytes_for_si()), each enabled scan_index is treated as a separate
sequential scan element whose storagebits / 8 bytes are summed into the total
scan size.

With each of the 4 channels specifying storagebits of 64 (or 128) and bit
shifts of 0/16/32/48 (or 0/32/64/96), enabling all 4 channels causes
iio_compute_scan_bytes() to compute 32 bytes (4 x 8) or 64 bytes (4 x 16) per
scan instead of 8 or 16 bytes. Standard IIO userspace consumers step forward
by storagebits / 8 bytes per channel, so they will read each channel from a
different conversion sample and discard three quarters of the data (and
128-bit storage words with shifts up to 96 are not supported by standard IIO
tooling).

Also, since ltc2387_probe() enables only a single backend capture channel
(iio_backend_chan_enable(ltc->back, 0)) that transfers all 4 scan elements
together in hardware DMA mode without per-channel hardware enablement or
software demuxing, shouldn't the channels use per-channel storagebits (16 and
32 with shift 0) and set indio_dev->available_scan_masks to require all 4
channels to be enabled together?

[ ... ]

> +static int ltc2387_set_sampling_freq(struct ltc2387_dev *ltc, int freq)
> +{
> +	unsigned long long ref_clk_period_ns;
> +	struct pwm_waveform clk_gate_wf = { }, cnv_wf = { };
> +	unsigned long long period_cycles;
> +	int ret, clk_en_time;
> +	u32 rem;
> +
> +	if (freq <= 0 || freq > ltc->ref_clk_rate)
> +		return -EINVAL;
> +
> +	ref_clk_period_ns = DIV_ROUND_UP(NSEC_PER_SEC, ltc->ref_clk_rate);

[ ... ]

> +	/* Gate the active period of the clock (see page 10-13 for both LTC's) */
> +	if (ltc->lane_mode == TWO_LANES)
> +		clk_en_time = DIV_ROUND_UP_ULL(ltc->device_info->resolution, 4);
> +	else
> +		clk_en_time = DIV_ROUND_UP_ULL(ltc->device_info->resolution, 2);
> +
> +	clk_gate_wf.duty_length_ns = ref_clk_period_ns * clk_en_time;
> +	clk_gate_wf.duty_offset_ns = LTC2387_T_FIRSTCLK_NS;
> +
> +	if (cnv_wf.period_length_ns < clk_gate_wf.duty_length_ns)
> +		cnv_wf.period_length_ns = clk_gate_wf.duty_length_ns + ref_clk_period_ns;

[Severity: Medium]
Can the rounding and period clamp in ltc2387_set_sampling_freq() produce the
wrong number of gated clock cycles or a 100% duty cycle clock?

First, ref_clk_period_ns rounds a single reference clock cycle up to integer
nanoseconds via DIV_ROUND_UP(NSEC_PER_SEC, ltc->ref_clk_rate), and
clk_gate_wf.duty_length_ns multiplies that pre-rounded value by clk_en_time
(up to 9 cycles). When the PWM driver converts nanoseconds back to clock
cycles via floor(ns * R / NSEC_PER_SEC), the accumulated upward rounding
error across clk_en_time cycles can add an extra clock cycle. For example, on
an 18-bit part in ONE_LANE mode (clk_en_time = 9) at ref_clk_rate = 160 MHz
(6.25 ns period), ref_clk_period_ns is 7 ns and duty_length_ns becomes 63 ns,
which rounds to floor(63 * 160e6 / 1e9) = 10 cycles instead of 9. Similarly,
on a 16-bit part in ONE_LANE mode (clk_en_time = 8) at ref_clk_rate = 120 MHz
and freq = 15 MHz, cnv_wf.period_length_ns is 67 ns (8 cycles) while
duty_length_ns is 72 ns, unexpectedly triggering the period clamp to 81 ns
(9 cycles, 13.33 MHz).

Second, the clamp condition checks if cnv_wf.period_length_ns <
clk_gate_wf.duty_length_ns, whereas the body sets cnv_wf.period_length_ns to
clk_gate_wf.duty_length_ns + ref_clk_period_ns. If cnv_wf.period_length_ns ==
clk_gate_wf.duty_length_ns (for example, at ref_clk_rate = 100 MHz on an
18-bit part in ONE_LANE mode when requesting 12 MHz, where both are 90 ns),
the clamp is skipped, leaving clk_gate_wf with a 100% duty cycle and a
shorter period at 12 MHz (90 ns) than at 15 MHz (clamped to 100 ns).

> +
> +	clk_gate_wf.period_length_ns = cnv_wf.period_length_ns;
> +
> +	if (clk_gate_wf.duty_offset_ns >= clk_gate_wf.period_length_ns)
> +		div64_u64_rem(clk_gate_wf.duty_offset_ns,
> +			      clk_gate_wf.period_length_ns,
> +			      &clk_gate_wf.duty_offset_ns);
> +
> +	ret = pwm_set_waveform_might_sleep(ltc->cnv, &cnv_wf, false);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = pwm_set_waveform_might_sleep(ltc->clk_gate, &clk_gate_wf, false);
> +	if (ret < 0)
> +		return ret;
> +
> +	period_cycles = mul_u64_u32_div(cnv_wf.period_length_ns,
> +					ltc->ref_clk_rate, NSEC_PER_SEC);
> +	if (!period_cycles)
> +		return -EINVAL;
> +
> +	ltc->sampling_freq = DIV_ROUND_CLOSEST_ULL(ltc->ref_clk_rate,
> +						   period_cycles);
> +
> +	return 0;
> +}

[ ... ]

> +static int ltc2387_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int val, int val2, long info)
> +{
> +	struct ltc2387_dev *ltc = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return ltc2387_set_sampling_freq(ltc, val);
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}

[Severity: Medium]
What prevents concurrent or mid-capture updates to the PWM channels in
ltc2387_write_raw() -> ltc2387_set_sampling_freq()?

Because iio_write_channel_info() does not hold an IIO device lock and
ltc2387_write_raw() neither claims direct mode (e.g., via
iio_device_claim_direct()) nor acquires a driver mutex:

1. Userspace can write to sampling_frequency while buffered capture is
   active, reprogramming ltc->cnv and ltc->clk_gate non-atomically mid-stream.
2. Concurrent writes across separate file descriptors can interleave the two
   sleeping pwm_set_waveform_might_sleep() calls and the update to
   ltc->sampling_freq.
3. If pwm_set_waveform_might_sleep(ltc->cnv, ...) succeeds and
   pwm_set_waveform_might_sleep(ltc->clk_gate, ...) fails, ltc->cnv is left
   running at the new period while ltc->clk_gate and ltc->sampling_freq
   remain at the old settings.

[ ... ]

> +static int ltc2387_probe(struct platform_device *pdev)
> +{
> +	struct iio_dev			*indio_dev;
> +	struct ltc2387_dev		*ltc;
> +	int				ret;
> +
> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*ltc));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	ltc = iio_priv(indio_dev);
> +
> +	ltc->vref = devm_regulator_get_optional(&pdev->dev, "vref");
> +	if (!IS_ERR(ltc->vref)) {
> +		ret = regulator_enable(ltc->vref);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Can't enable vref regulator\n");
> +			return ret;
> +		}
> +		ret = regulator_get_voltage(ltc->vref);
> +		if (ret < 0)
> +			return ret;
> +
> +		ltc->vref_mv = ret / 1000;
> +		ret = devm_add_action_or_reset(&pdev->dev,
> +					       ltc2387_regulator_disable,
> +					       ltc->vref);
> +		if (ret)
> +			return ret;

[Severity: Medium]
Does ltc2387_probe() leak the regulator enable count on ltc->vref if
regulator_get_voltage() fails?

In ltc2387_probe(), regulator_enable(ltc->vref) is called before
regulator_get_voltage(ltc->vref), while the devm_add_action_or_reset()
registration for ltc2387_regulator_disable() happens after
regulator_get_voltage(). If regulator_get_voltage() returns a negative error,
ltc2387_probe() returns early without disabling ltc->vref (or should this use
devm_regulator_get_enable_read_voltage()?).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027134409.76824-1-Nicolae-daniel.Deaconescu@analog.com?part=2

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] iio: adc: add ltc2387 driver
  2026-10-27 13:44 ` [PATCH 2/2] iio: adc: add ltc2387 driver Nicolae-Daniel Deaconescu
  2026-10-27 14:06   ` sashiko-bot
@ 2026-10-27 14:37   ` Joshua Crofts
  2026-10-28  2:39   ` Jonathan Cameron
  2 siblings, 0 replies; 9+ messages in thread
From: Joshua Crofts @ 2026-10-27 14:37 UTC (permalink / raw)
  To: Nicolae-Daniel Deaconescu
  Cc: Sergiu Cuciurean, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Uwe Kleine-König, linux-iio, devicetree, linux-kernel,
	linux-pwm

On Tue, 27 Oct 2026 15:44:09 +0200
Nicolae-Daniel Deaconescu <Nicolae-daniel.Deaconescu@analog.com> wrote:

> From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> 
> Add a driver for the LTC2387-16/18 and the ADAQ23875/23876/23878.

Hi, some comments from me. Additionally, please check out what
Sashiko says:

https://sashiko.dev/#/patchset/20261027134409.76824-1-Nicolae-daniel.Deaconescu%40analog.com


> The driver generates CNV and the gated data clock with two PWM
> channels, timed against the reference clock to meet t_FIRSTCLK, and
> gets sample data through the IIO backend framework from the AXI ADC
> core. It supports one-lane and two-lane output and reports the
> sampling frequency the PWMs produce after rounding to whole reference
> clock cycles. in_voltage_scale is referred to the analog input, so on
> the ADAQ parts it is divided by the front-end gain.
> 
> Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> Co-developed-by: Nicolae-Daniel Deaconescu <Nicolae-daniel.Deaconescu@analog.com>
> Signed-off-by: Nicolae-Daniel Deaconescu <Nicolae-daniel.Deaconescu@analog.com>
> Assisted-by: Claude:claude-opus-5-5

Drop this tag, it has been deprecated.

> diff --git a/drivers/iio/adc/ltc2387.c b/drivers/iio/adc/ltc2387.c
> new file mode 100644
> index 000000000000..c236b99baa4a
> --- /dev/null
> +++ b/drivers/iio/adc/ltc2387.c
> @@ -0,0 +1,482 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
> +/*
> + * Linear Technology LTC2387 ADC driver
> + *
> + * Copyright 2022 - 2026 Analog Devices Inc.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/iio/backend.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>

Better to have the iio specific headers separately after the generic
linux headers.

> +#include <linux/kernel.h>

No, do not use this header for new drivers. Use the IWYU principle when
deciding what to include.

> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/pwm.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/mod_devicetable.h>

Drop this header, it's included by platform_device.h

+ you're missing additional headers, e.g. types.h

> +
> +#define LTC2387_VREF		4096
> +#define LTC2387_T_CNVH		8
> +
> +/*
> + * Minimal value for t_{FIRSTCLK} according to
> + * https://www.analog.com/media/en/technical-documentation/data-sheets/238718fa.pdf
> + * is 65 ns. Add some slack because there is some rounding involved in the PWM
> + * driver. With the PWM driver rounding to the nearest possible value, targeting
> + * 70 ns works for input clk rates >= 100 MHz.
> + */
> +#define LTC2387_T_FIRSTCLK_NS	70
> +
> +#define KHz 1000

We have units.h for this.

> +#define MHz (1000 * KHz)
> +
> +#define LTC2378_MULTIPLE_CHAN(_idx, _storagebits, _realbits, _shift)	\
> +	{								\
> +		.type = IIO_VOLTAGE,					\
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
> +		.indexed = 1,						\
> +		.channel = _idx,					\
> +		.scan_index = _idx,					\
> +		.scan_type = {						\
> +			.sign = 's',					\
> +			.storagebits = _storagebits,			\
> +			.realbits = _realbits,				\
> +			.shift = _shift,				\
> +		},							\
> +	}
> +

...

> +struct ltc2387_dev {
> +	const struct ltc2387_info *device_info;
> +	enum ltc2387_lane_modes lane_mode;
> +	unsigned long ref_clk_rate;
> +	struct iio_backend *back;
> +	struct pwm_device *clk_gate;
> +	struct regulator *vref;
> +	struct pwm_device *cnv;
> +	struct clk *ref_clk;
> +
> +	unsigned int vref_mv;

_mV

> +	u32 sampling_freq;
> +	u32 gain_milli;
> +};
> +
> +static int ltc2387_set_sampling_freq(struct ltc2387_dev *ltc, int freq)
> +{
> +	unsigned long long ref_clk_period_ns;
> +	struct pwm_waveform clk_gate_wf = { }, cnv_wf = { };

Reverse xmas tree please.

> +	unsigned long long period_cycles;
> +	int ret, clk_en_time;
> +	u32 rem;
> +
> +	if (freq <= 0 || freq > ltc->ref_clk_rate)
> +		return -EINVAL;
> +
> +	ref_clk_period_ns = DIV_ROUND_UP(NSEC_PER_SEC, ltc->ref_clk_rate);
> +
> +	cnv_wf.duty_length_ns = ref_clk_period_ns;
> +
> +	/*
> +	 * The goal here is that the PWM is configured with a minimal period not
> +	 * less than 1 / freq (with freq measured in Hz).
> +	 *
> +	 * When a period P (measured in ns) is passed to pwm_apply_state(), the
> +	 * actually implemented period is:
> +	 *
> +	 *      round_down(P * R / NSEC_PER_SEC) / R
> +	 *
> +	 * (measured in s) with R = ltc->ref_clk_rate. So we have:
> +	 *
> +	 *        round_down(P * R / NSEC_PER_SEC) / R ≥ 1 / freq
> +	 *      ⟺ round_down(P * R / NSEC_PER_SEC) ≥ R / freq

Ehhh, not really sure about these non-standard characters in the comments.
Had to check lore to see what this renders to. Wouldn't it be easier to use
a trigraph <=>? Same for the digraph stuff you have.

> +	 *
> +	 * With the LHS being integer this is equivalent to:
> +	 *
> +	 *        round_down(P * R / NSEC_PER_SEC) ≥ round_up(R / freq)
> +	 *      ⟺ P * R / NSEC_PER_SEC ≥ round_up(R / freq)
> +	 *      ⟺ P ≥ round_up(R / freq) * NSEC_PER_SEC / R
> +	 */
> +
> +	cnv_wf.period_length_ns =
> +		div_u64_rem((u64)DIV_ROUND_UP(ltc->ref_clk_rate, freq) * NSEC_PER_SEC,
> +			    ltc->ref_clk_rate, &rem);
> +	if (rem)
> +		cnv_wf.period_length_ns += 1;
> +
> +	/* Gate the active period of the clock (see page 10-13 for both LTC's) */
> +	if (ltc->lane_mode == TWO_LANES)
> +		clk_en_time = DIV_ROUND_UP_ULL(ltc->device_info->resolution, 4);
> +	else
> +		clk_en_time = DIV_ROUND_UP_ULL(ltc->device_info->resolution, 2);
> +
> +	clk_gate_wf.duty_length_ns = ref_clk_period_ns * clk_en_time;
> +	clk_gate_wf.duty_offset_ns = LTC2387_T_FIRSTCLK_NS;
> +
> +	if (cnv_wf.period_length_ns < clk_gate_wf.duty_length_ns)
> +		cnv_wf.period_length_ns = clk_gate_wf.duty_length_ns + ref_clk_period_ns;
> +
> +	clk_gate_wf.period_length_ns = cnv_wf.period_length_ns;
> +
> +	if (clk_gate_wf.duty_offset_ns >= clk_gate_wf.period_length_ns)
> +		div64_u64_rem(clk_gate_wf.duty_offset_ns,
> +			      clk_gate_wf.period_length_ns,
> +			      &clk_gate_wf.duty_offset_ns);
> +
> +	ret = pwm_set_waveform_might_sleep(ltc->cnv, &cnv_wf, false);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = pwm_set_waveform_might_sleep(ltc->clk_gate, &clk_gate_wf, false);
> +	if (ret < 0)
> +		return ret;
> +
> +	period_cycles = mul_u64_u32_div(cnv_wf.period_length_ns,
> +					ltc->ref_clk_rate, NSEC_PER_SEC);
> +	if (!period_cycles)
> +		return -EINVAL;
> +
> +	ltc->sampling_freq = DIV_ROUND_CLOSEST_ULL(ltc->ref_clk_rate,
> +						   period_cycles);
> +
> +	return 0;
> +}
> +
> +static int ltc2387_setup(struct iio_dev *indio_dev)
> +{
> +	struct ltc2387_dev *ltc = iio_priv(indio_dev);
> +	struct device *dev = indio_dev->dev.parent;
> +
> +	if (device_property_present(dev, "adi,use-two-lanes"))
> +		ltc->lane_mode = TWO_LANES;
> +
> +	return ltc2387_set_sampling_freq(ltc, 15 * MHz);
> +}
> +
> +static int ltc2387_read_raw(struct iio_dev *indio_dev,
> +			    const struct iio_chan_spec *chan,
> +			    int *val, int *val2, long info)
> +{
> +	struct ltc2387_dev *ltc = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = ltc->sampling_freq;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		/* Referred to the SMA input, so divided by the front-end gain. */
> +		*val = ltc->vref_mv * 2 * 1000;

vref_mv * 2 * MILLI;

> +		*val2 = ltc->gain_milli << chan->scan_type.realbits;
> +		return IIO_VAL_FRACTIONAL;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ltc2387_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int val, int val2, long info)
> +{
> +	struct ltc2387_dev *ltc = iio_priv(indio_dev);
> +

Probably add a mutex here to protect writing, ideally in guard() form.

> +	switch (info) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return ltc2387_set_sampling_freq(ltc, val);
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static void ltc2387_pwm_disable(void *data)
> +{
> +	pwm_disable(data);
> +}
> +
> +static void ltc2387_regulator_disable(void *data)
> +{
> +	regulator_disable(data);
> +}
> +
> +static void ltc2387_clk_disable(void *data)
> +{
> +	clk_disable_unprepare(data);
> +}
> +
> +static const struct iio_info ltc2387_info = {
> +	.read_raw = ltc2387_read_raw,
> +	.write_raw = ltc2387_write_raw,
> +};
> +
> +static const struct of_device_id ltc2387_of_match[] = {
> +	{
> +		.compatible = "lltc,ltc2387-16",
> +		.data = &ltc2387_infos[ID_LTC2387_16]
> +	}, {
> +		.compatible = "lltc,ltc2387-16-x4",
> +		.data = &ltc2387_infos[ID_LTC2387_16_X4]
> +	}, {
> +		.compatible = "lltc,ltc2387-18",
> +		.data = &ltc2387_infos[ID_LTC2387_18]
> +	}, {
> +		.compatible = "lltc,ltc2387-18-x4",
> +		.data = &ltc2387_infos[ID_LTC2387_18_X4]
> +	}, {
> +		.compatible = "adi,adaq23875",
> +		.data = &ltc2387_infos[ID_ADAQ23875]
> +	}, {
> +		.compatible = "adi,adaq23876",
> +		.data = &ltc2387_infos[ID_ADAQ23876]
> +	}, {
> +		.compatible = "adi,adaq23878",
> +		.data = &ltc2387_infos[ID_ADAQ23878]
> +	},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, ltc2387_of_match);

Have the device tables after the probe() function.

> +
> +static int ltc2387_probe(struct platform_device *pdev)
> +{
> +	struct iio_dev			*indio_dev;
> +	struct ltc2387_dev		*ltc;
> +	int				ret;

Maybe unify the style you define variables? (aligned vs. non-aligned).

> +
> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*ltc));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	ltc = iio_priv(indio_dev);
> +
> +	ltc->vref = devm_regulator_get_optional(&pdev->dev, "vref");
> +	if (!IS_ERR(ltc->vref)) {
> +		ret = regulator_enable(ltc->vref);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Can't enable vref regulator\n");

dev_err_probe() instead of dev_err()

> +			return ret;
> +		}
> +		ret = regulator_get_voltage(ltc->vref);
> +		if (ret < 0)
> +			return ret;
> +
> +		ltc->vref_mv = ret / 1000;

ret / (MICRO / MILLI)

> +		ret = devm_add_action_or_reset(&pdev->dev,
> +					       ltc2387_regulator_disable,
> +					       ltc->vref);
> +		if (ret)
> +			return ret;
> +	} else {
> +		if (PTR_ERR(ltc->vref) != -ENODEV)
> +			return PTR_ERR(ltc->vref);
> +
> +		ltc->vref_mv = LTC2387_VREF; /* Internal vref is used */
> +	}
> +
> +	ltc->ref_clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(ltc->ref_clk))
> +		return PTR_ERR(ltc->ref_clk);
> +
> +	ret = clk_prepare_enable(ltc->ref_clk);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(&pdev->dev, ltc2387_clk_disable,
> +				       ltc->ref_clk);
> +	if (ret)
> +		return ret;

+ blank line.

> +	ltc->ref_clk_rate = clk_get_rate(ltc->ref_clk);
> +
> +	ltc->clk_gate = devm_pwm_get(&pdev->dev, "clk_gate");
> +	if (IS_ERR(ltc->clk_gate))
> +		return PTR_ERR(ltc->clk_gate);
> +
> +	ret = devm_add_action_or_reset(&pdev->dev, ltc2387_pwm_disable,
> +				       ltc->clk_gate);
> +	if (ret)
> +		return ret;
> +
> +	ltc->cnv = devm_pwm_get(&pdev->dev, "cnv");
> +	if (IS_ERR(ltc->cnv))
> +		return PTR_ERR(ltc->cnv);
> +
> +	ret = devm_add_action_or_reset(&pdev->dev, ltc2387_pwm_disable,
> +				       ltc->cnv);
> +
> +	if (ret)
> +		return ret;
> +
> +	ltc->device_info = device_get_match_data(&pdev->dev);
> +	if (!ltc->device_info)
> +		return -EINVAL;

return -ENODATA;

> +
> +	ltc->gain_milli = ltc->device_info->default_gain_milli;
> +	device_property_read_u32(&pdev->dev, "adi,gain-milli", &ltc->gain_milli);
> +	if (!ltc->gain_milli)
> +		return dev_err_probe(&pdev->dev, -EINVAL,
> +				     "adi,gain-milli must be nonzero\n");
> +
> +	indio_dev->channels = ltc->device_info->channels;
> +	indio_dev->num_channels = ltc->device_info->num_channels;
> +	indio_dev->dev.parent = &pdev->dev;
> +	indio_dev->name = ltc->device_info->name;
> +	indio_dev->info = &ltc2387_info;
> +
> +	ltc->back = devm_iio_backend_get(&pdev->dev, NULL);
> +	if (IS_ERR(ltc->back))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(ltc->back),
> +				     "failed to get IIO backend\n");
> +
> +	ret = devm_iio_backend_request_buffer(&pdev->dev, ltc->back, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_iio_backend_enable(&pdev->dev, ltc->back);
> +	if (ret)
> +		return ret;
> +
> +	/* axi_ltc2387 has a single capture channel carrying every scan element. */
> +	ret = iio_backend_chan_enable(ltc->back, 0);
> +	if (ret)
> +		return ret;
> +
> +	ret = ltc2387_setup(indio_dev);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "\nltc2387 setup failed\n");

Why the additional \n at the beginning.
> +		return ret;
> +	}
> +
> +	return devm_iio_device_register(&pdev->dev, indio_dev);
> +}
> +
> +static struct platform_driver ltc2387_driver = {
> +	.probe          = ltc2387_probe,
> +	.driver         = {
> +		.name   = "ltc2387",
> +		.of_match_table = ltc2387_of_match,
> +	},
> +};
> +module_platform_driver(ltc2387_driver);
> +
> +MODULE_AUTHOR("Sergiu Cuciurean <sergiu.cuciurean@analog.com>");
> +MODULE_DESCRIPTION("Linear Technology LTC2387 ADC");

The description should be the same as what's in the Kconfig tristate entry.

> +MODULE_LICENSE("Dual BSD/GPL");
> +MODULE_IMPORT_NS("IIO_BACKEND");



-- 
Kind regards,
Joshua Crofts

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] iio: adc: add LTC2387 and ADAQ2387x support
  2026-10-27 13:44 [PATCH 0/2] iio: adc: add LTC2387 and ADAQ2387x support Nicolae-Daniel Deaconescu
  2026-10-27 13:44 ` [PATCH 1/2] dt-bindings: iio: adc: add lltc,ltc2387 Nicolae-Daniel Deaconescu
  2026-10-27 13:44 ` [PATCH 2/2] iio: adc: add ltc2387 driver Nicolae-Daniel Deaconescu
@ 2026-10-27 15:56 ` Andy Shevchenko
  2 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2026-10-27 15:56 UTC (permalink / raw)
  To: Nicolae-Daniel Deaconescu
  Cc: Sergiu Cuciurean, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Uwe Kleine-König, linux-iio, devicetree, linux-kernel,
	linux-pwm

On Tue, Oct 27, 2026 at 03:44:07PM +0200, Nicolae-Daniel Deaconescu wrote:
> This series adds a driver for the LTC2387-16/18, 15 Msps SAR ADCs with
> an LVDS data interface, and for the ADAQ23875, ADAQ23876 and ADAQ23878
> uModule ADCs, which pair an LTC2387 die with a fully differential
> amplifier front end.

Why have you written a brand new driver?

> Conversion start (CNV) and the gated LVDS data clock are generated by
> two channels of an AXI PWM generator. Sample data is captured by the
> AXI ADC IP core and reached through the IIO backend framework. The
> front-end gain of the ADAQ parts is described with adi,gain-milli
> (fixed at 2 on the ADAQ23875, pin-strapped on the ADAQ23876/78) and
> applied to in_voltage_scale.
> 
> Tested on an EV-ADAQ23875FMCZ on a ZedBoard running the adaq2387x HDL
> project in two-lane, 16-bit mode: probe, in_voltage_scale,
> sampling_frequency (writes, rounding and range checks), buffered
> capture up to 1M samples, and unbind/rebind. The bare LTC2387
> variants, one-lane mode and the -x4 variants were not tested on
> hardware.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dt-bindings: iio: adc: add lltc,ltc2387
  2026-10-27 13:44 ` [PATCH 1/2] dt-bindings: iio: adc: add lltc,ltc2387 Nicolae-Daniel Deaconescu
  2026-10-27 13:59   ` sashiko-bot
@ 2026-10-27 18:28   ` Conor Dooley
  1 sibling, 0 replies; 9+ messages in thread
From: Conor Dooley @ 2026-10-27 18:28 UTC (permalink / raw)
  To: Nicolae-Daniel Deaconescu
  Cc: Sergiu Cuciurean, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Uwe Kleine-König, linux-iio, devicetree, linux-kernel,
	linux-pwm

[-- Attachment #1: Type: text/plain, Size: 7026 bytes --]

On Tue, Oct 27, 2026 at 03:44:08PM +0200, Nicolae-Daniel Deaconescu wrote:
> From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> 
> Add bindings for the LTC2387-16 and LTC2387-18, 15 Msps SAR ADCs with
> an LVDS data interface, and for the ADAQ23875, ADAQ23876 and ADAQ23878
> uModule ADCs, which pair an LTC2387 die with a fully differential
> amplifier front end.
> 
> Conversion start (CNV) and the gated LVDS clock are driven by two PWM
> channels. Sample data is captured by an AXI ADC IP core, referenced
> through io-backends. adi,use-two-lanes selects two-lane output, and
> adi,gain-milli gives the front-end gain: fixed at 2 on the ADAQ23875,
> set by pin strapping on the ADAQ23876 and ADAQ23878.
> 
> Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> Co-developed-by: Nicolae-Daniel Deaconescu <Nicolae-daniel.Deaconescu@analog.com>
> Signed-off-by: Nicolae-Daniel Deaconescu <Nicolae-daniel.Deaconescu@analog.com>
> Assisted-by: Claude:claude-opus-5-5
> ---
>  .../bindings/iio/adc/lltc,ltc2387.yaml        | 138 ++++++++++++++++++
>  MAINTAINERS                                   |   8 +
>  2 files changed, 146 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/lltc,ltc2387.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/lltc,ltc2387.yaml b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2387.yaml
> new file mode 100644
> index 000000000000..f912260c8f79
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2387.yaml
> @@ -0,0 +1,138 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2022 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/lltc,ltc2387.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Linear Technology / Analog Devices LTC2387 ADC
> +
> +maintainers:
> +  - Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> +  - Nicolae-Daniel Deaconescu <Nicolae-daniel.Deaconescu@analog.com>
> +
> +description: |
> +  The LTC2387-18 is a low noise, high speed, 18-bit 15Msps
> +  successive approximation register (SAR) ADC ideally
> +  suited for a wide range of applications.
> +
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/238718fa.pdf
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/238716f.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,adaq23875
> +      - adi,adaq23876
> +      - adi,adaq23878
> +      - lltc,ltc2387-16
> +      - lltc,ltc2387-16-x4
> +      - lltc,ltc2387-18
> +      - lltc,ltc2387-18-x4

Why are we getting new devices with linear vendor prefixes? My
understanding was that new devices from ADI subsidiaries were to use the
adi vendor prefix.


> +
> +  clocks:
> +    description: Sampling clock
> +    maxItems: 1
> +
> +  pwms:
> +    maxItems: 2
> +    description:
> +      PWM signals used to synchronize the sampling procedure
> +
> +  pwm-names:
> +    items:
> +      - const: cnv
> +      - const: clk_gate

Hmm, ltc2387-16 does not appear to have these signals?
Well, it has "cnv" but "cnv" has "sample clock" as the input name so is
the clocks property above and pwm-clock can be used if the provider is a
pwm?

Either there's some mutual exclusion missing here or the modelling in
incorrect.

"clk_gate" doesn't appear in either of the datasheets I looked at, what
does that actually do?

pw-bot: changes-requested

> +
> +  io-backends:
> +    description:
> +      The AXI ADC IP block (axi_ltc2387) that captures the LVDS data stream.
> +    maxItems: 1
> +
> +  vref-supply:
> +    description:
> +      External voltage reference. When absent the device's internal 4.096 V
> +      reference is assumed.
> +
> +  adi,use-two-lanes:
> +    description: The device uses two lanes for data transfer
> +    type: boolean

Please explain how this is not determinable from the compatible.

Cheers,
Conor.

> +
> +  adi,gain-milli:
> +    description:
> +      Gain of the fully-differential amplifier that the ADAQ2387x modules
> +      integrate ahead of the LTC2387 die, expressed as gain * 1000. It is
> +      fixed on the ADAQ23875 and set by pin strapping on the ADAQ23876 and
> +      ADAQ23878, so it must match the board. The bare ltc2387-* parts have
> +      no amplifier.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [370, 730, 870, 1380, 2000, 2250]
> +
> +required:
> +  - compatible
> +  - clocks
> +  - io-backends
> +  - pwms
> +  - pwm-names
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - lltc,ltc2387-16
> +              - lltc,ltc2387-16-x4
> +              - lltc,ltc2387-18
> +              - lltc,ltc2387-18-x4
> +    then:
> +      properties:
> +        adi,gain-milli: false
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: adi,adaq23875
> +    then:
> +      properties:
> +        adi,gain-milli:
> +          const: 2000
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - adi,adaq23876
> +              - adi,adaq23878
> +    then:
> +      properties:
> +        adi,gain-milli:
> +          enum: [370, 730, 870, 1380, 2250]
> +      required:
> +        - adi,gain-milli
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    adc {
> +            compatible = "lltc,ltc2387-16";
> +            clocks = <&clk 0>;
> +            io-backends = <&axi_adc>;
> +            pwms = <&pwm 0 0>, <&pwm 1 0>;
> +            pwm-names = "cnv", "clk_gate";
> +            vref-supply = <&vref>;
> +    };
> +  - |
> +    adc {
> +            compatible = "adi,adaq23878";
> +            clocks = <&clk 0>;
> +            io-backends = <&axi_adc>;
> +            pwms = <&pwm 0 0>, <&pwm 1 0>;
> +            pwm-names = "cnv", "clk_gate";
> +            vref-supply = <&vref>;
> +            adi,use-two-lanes;
> +            adi,gain-milli = <870>;
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 52024397dc78..b871d7f25d76 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15529,6 +15529,14 @@ W:	https://ez.analog.com/linux-software-drivers
>  F:	Documentation/devicetree/bindings/iio/adc/adi,ltc2378.yaml
>  F:	drivers/iio/adc/ltc2378.c
>  
> +LTC2387 IIO ADC DRIVER
> +M:	Nicolae-Daniel Deaconescu <Nicolae-daniel.Deaconescu@analog.com>
> +M:	Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> +L:	linux-iio@vger.kernel.org
> +S:	Supported
> +W:	https://ez.analog.com/linux-software-drivers
> +F:	Documentation/devicetree/bindings/iio/adc/lltc,ltc2387.yaml
> +
>  LTC2664 IIO DAC DRIVER
>  M:	Michael Hennerich <michael.hennerich@analog.com>
>  M:	Kim Seer Paller <kimseer.paller@analog.com>
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] iio: adc: add ltc2387 driver
  2026-10-27 13:44 ` [PATCH 2/2] iio: adc: add ltc2387 driver Nicolae-Daniel Deaconescu
  2026-10-27 14:06   ` sashiko-bot
  2026-10-27 14:37   ` Joshua Crofts
@ 2026-10-28  2:39   ` Jonathan Cameron
  2 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2026-10-28  2:39 UTC (permalink / raw)
  To: Nicolae-Daniel Deaconescu
  Cc: Sergiu Cuciurean, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Uwe Kleine-König, linux-iio, devicetree, linux-kernel,
	linux-pwm

On Tue, 27 Oct 2026 15:44:09 +0200
Nicolae-Daniel Deaconescu <Nicolae-daniel.Deaconescu@analog.com> wrote:

> From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> 
> Add a driver for the LTC2387-16/18 and the ADAQ23875/23876/23878.
> 
> The driver generates CNV and the gated data clock with two PWM
> channels, timed against the reference clock to meet t_FIRSTCLK, and
> gets sample data through the IIO backend framework from the AXI ADC
> core. It supports one-lane and two-lane output and reports the
> sampling frequency the PWMs produce after rounding to whole reference
> clock cycles. in_voltage_scale is referred to the analog input, so on
> the ADAQ parts it is divided by the front-end gain.
> 
> Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> Co-developed-by: Nicolae-Daniel Deaconescu <Nicolae-daniel.Deaconescu@analog.com>
> Signed-off-by: Nicolae-Daniel Deaconescu <Nicolae-daniel.Deaconescu@analog.com>
> Assisted-by: Claude:claude-opus-5-5

Check the docs on this.  We moved away from this form to the LLM
one not naming the model and now convention is to just mention
it in the cover letter or below the ---
Ideally with a bit more info on how you used it.

Various things inline.  Some of the stuff in here
looks like old drivers and the copyright suggests this is
one that is now being upstreamed.  Please take a look
at some more recent ones for preferred style etc.

Jonathan

> diff --git a/drivers/iio/adc/ltc2387.c b/drivers/iio/adc/ltc2387.c
> new file mode 100644
> index 000000000000..c236b99baa4a
> --- /dev/null
> +++ b/drivers/iio/adc/ltc2387.c
> @@ -0,0 +1,482 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
> +/*
> + * Linear Technology LTC2387 ADC driver
> + *
> + * Copyright 2022 - 2026 Analog Devices Inc.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/iio/backend.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/kernel.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/pwm.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/mod_devicetable.h>
> +
> +#define LTC2387_VREF		4096
> +#define LTC2387_T_CNVH		8
> +
> +/*
> + * Minimal value for t_{FIRSTCLK} according to
> + * https://www.analog.com/media/en/technical-documentation/data-sheets/238718fa.pdf
> + * is 65 ns. Add some slack because there is some rounding involved in the PWM
> + * driver. With the PWM driver rounding to the nearest possible value, targeting
> + * 70 ns works for input clk rates >= 100 MHz.
> + */
> +#define LTC2387_T_FIRSTCLK_NS	70
> +
> +#define KHz 1000
There are units for this already...  See units.h
Probably want HZ_PER_KHZ for this one.

> +#define MHz (1000 * KHz)
HZ_PER_MHZ

> +enum ltc2387_lane_modes {
> +	ONE_LANE = 0,
> +	TWO_LANES = 1

Trailing comma.

> +};
> +
> +enum ltc2387_id {
> +	ID_LTC2387_16,
> +	ID_LTC2387_16_X4,
> +	ID_LTC2387_18,
> +	ID_LTC2387_18_X4,
> +	ID_ADAQ23875,
> +	ID_ADAQ23876,
> +	ID_ADAQ23878,

See below. I'd expect this to go away with changes there.

> +};
> +
> +struct ltc2387_info {
> +	const char *name;
> +	struct iio_chan_spec channels[4];
> +	unsigned int test_pattern[2];
> +	int num_channels;
> +	int resolution;
> +	/* Gain * 1000 when adi,gain-milli is absent; unity for the bare parts. */

Gain / 1000?  If its in units of milli.
For the unity parts why 0?

> +	u32 default_gain_milli;
> +};
>
> +
> +static const struct ltc2387_info ltc2387_infos[] = {
> +	[ID_LTC2387_16] = {
> +		.name = "ltc2387-16",
> +		.default_gain_milli = 1000,
> +		.resolution = 16,
> +		.test_pattern = LTC2387_TEST_PATTERN_16,
> +		.channels = { LTC2378_CHAN(16, 16) },
> +		.num_channels = 1,
> +	},
As mentioned below. Split this up into individual named
structures where the name makes it clear what device they are for.

> +
> +static int ltc2387_setup(struct iio_dev *indio_dev)
> +{
> +	struct ltc2387_dev *ltc = iio_priv(indio_dev);
> +	struct device *dev = indio_dev->dev.parent;
> +
> +	if (device_property_present(dev, "adi,use-two-lanes"))
> +		ltc->lane_mode = TWO_LANES;
> +
> +	return ltc2387_set_sampling_freq(ltc, 15 * MHz);
So this (I think) is the call that actually enables the pwms.
The devm unwinding should pair with setup not the pwm_get()
> +}


> +
> +static const struct of_device_id ltc2387_of_match[] = {
> +	{
> +		.compatible = "lltc,ltc2387-16",
> +		.data = &ltc2387_infos[ID_LTC2387_16]
> +	}, {
> +		.compatible = "lltc,ltc2387-16-x4",
> +		.data = &ltc2387_infos[ID_LTC2387_16_X4]
> +	}, {
> +		.compatible = "lltc,ltc2387-18",
> +		.data = &ltc2387_infos[ID_LTC2387_18]
> +	}, {
> +		.compatible = "lltc,ltc2387-18-x4",
> +		.data = &ltc2387_infos[ID_LTC2387_18_X4]
> +	}, {
> +		.compatible = "adi,adaq23875",
> +		.data = &ltc2387_infos[ID_ADAQ23875]
> +	}, {
> +		.compatible = "adi,adaq23876",
> +		.data = &ltc2387_infos[ID_ADAQ23876]
> +	}, {
> +		.compatible = "adi,adaq23878",
> +		.data = &ltc2387_infos[ID_ADAQ23878]

trailing comma.

Also there doesn't seem to be a reason to have these in an array.
Split them up into individually named structures. That tends
to end up easier to read in the longer term and removes
the need for the enum.  We are slowly removing this array
approach from existing drivers.  

> +	},
> +	{}

	{ }

> +};
> +MODULE_DEVICE_TABLE(of, ltc2387_of_match);
> +
> +static int ltc2387_probe(struct platform_device *pdev)
> +{
> +	struct iio_dev			*indio_dev;
> +	struct ltc2387_dev		*ltc;
> +	int				ret;

Don't force indent. It just creates churn in places like this.
Keep it for aligning numeric tables and that sort of thing only.

> +
> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*ltc));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	ltc = iio_priv(indio_dev);
> +
> +	ltc->vref = devm_regulator_get_optional(&pdev->dev, "vref");
> +	if (!IS_ERR(ltc->vref)) {
> +		ret = regulator_enable(ltc->vref);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Can't enable vref regulator\n");
> +			return ret;

return dev_err_probe()

> +		}
> +		ret = regulator_get_voltage(ltc->vref);
> +		if (ret < 0)
> +			return ret;
> +
> +		ltc->vref_mv = ret / 1000;
> +		ret = devm_add_action_or_reset(&pdev->dev,
> +					       ltc2387_regulator_disable,
> +					       ltc->vref);
> +		if (ret)
> +			return ret;
> +	} else {
> +		if (PTR_ERR(ltc->vref) != -ENODEV)
> +			return PTR_ERR(ltc->vref);
> +
> +		ltc->vref_mv = LTC2387_VREF; /* Internal vref is used */
> +	}
> +
> +	ltc->ref_clk = devm_clk_get(&pdev->dev, NULL);

	ltc->ref_clk = devm_clk_get_enabled() looks like it would
work here.

> +	if (IS_ERR(ltc->ref_clk))
> +		return PTR_ERR(ltc->ref_clk);
> +
> +	ret = clk_prepare_enable(ltc->ref_clk);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(&pdev->dev, ltc2387_clk_disable,
> +				       ltc->ref_clk);
> +	if (ret)
> +		return ret;
> +	ltc->ref_clk_rate = clk_get_rate(ltc->ref_clk);
> +
> +	ltc->clk_gate = devm_pwm_get(&pdev->dev, "clk_gate");
> +	if (IS_ERR(ltc->clk_gate))
> +		return PTR_ERR(ltc->clk_gate);
> +
See above. IIRC pwms aren't turned on by the get.  So
as above I think this should be paired with the
setup call not here.

> +	ret = devm_add_action_or_reset(&pdev->dev, ltc2387_pwm_disable,
> +				       ltc->clk_gate);
> +	if (ret)
> +		return ret;
> +
> +	ltc->cnv = devm_pwm_get(&pdev->dev, "cnv");
> +	if (IS_ERR(ltc->cnv))
> +		return PTR_ERR(ltc->cnv);
> +
> +	ret = devm_add_action_or_reset(&pdev->dev, ltc2387_pwm_disable,
> +				       ltc->cnv);
Same as previous. Also no blank line here.
> +
> +	if (ret)
> +		return ret;
> +
> +	ltc->device_info = device_get_match_data(&pdev->dev);
> +	if (!ltc->device_info)
> +		return -EINVAL;

Do this early given it is a trivial check and saves on unwinding all
the above if it fails.

> +
> +	ltc->gain_milli = ltc->device_info->default_gain_milli;
> +	device_property_read_u32(&pdev->dev, "adi,gain-milli", &ltc->gain_milli);
We now do optional properties with error checking but
	if (device_property_present()) {
		ret = device_property_read_u32();
		if (ret)
			..
		if (!ltc->gain_milli)
				...
		}
// or drop this out of the if else if you prefer.
	} else {
		ltc->gain_milli = ...
	}	
> +	if (!ltc->gain_milli)
> +		return dev_err_probe(&pdev->dev, -EINVAL,
> +				     "adi,gain-milli must be nonzero\n");
> +
> +	indio_dev->channels = ltc->device_info->channels;
> +	indio_dev->num_channels = ltc->device_info->num_channels;
> +	indio_dev->dev.parent = &pdev->dev;

That is unlikely to be needed... The IIO core has set parent
up for you for quite a while now.

> +	indio_dev->name = ltc->device_info->name;
> +	indio_dev->info = &ltc2387_info;
> +
> +	ltc->back = devm_iio_backend_get(&pdev->dev, NULL);
> +	if (IS_ERR(ltc->back))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(ltc->back),
> +				     "failed to get IIO backend\n");
> +
> +	ret = devm_iio_backend_request_buffer(&pdev->dev, ltc->back, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_iio_backend_enable(&pdev->dev, ltc->back);
> +	if (ret)
> +		return ret;
> +
> +	/* axi_ltc2387 has a single capture channel carrying every scan element. */
> +	ret = iio_backend_chan_enable(ltc->back, 0);
> +	if (ret)
> +		return ret;
> +
> +	ret = ltc2387_setup(indio_dev);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "\nltc2387 setup failed\n");
> +		return ret;

return dev_err_probe() for any error prints in probe or functions
only called from probe.  It improves the message formatting
even when no chance of deferral.

> +	}
> +
> +	return devm_iio_device_register(&pdev->dev, indio_dev);
> +}
> +
> +static struct platform_driver ltc2387_driver = {
> +	.probe          = ltc2387_probe,
> +	.driver         = {
I'd not bother forcing alignment. It just ends up inconsistent.

> +		.name   = "ltc2387",
> +		.of_match_table = ltc2387_of_match,
> +	},
> +};
> +module_platform_driver(ltc2387_driver);


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-10-28  2:39 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-27 13:44 [PATCH 0/2] iio: adc: add LTC2387 and ADAQ2387x support Nicolae-Daniel Deaconescu
2026-10-27 13:44 ` [PATCH 1/2] dt-bindings: iio: adc: add lltc,ltc2387 Nicolae-Daniel Deaconescu
2026-10-27 13:59   ` sashiko-bot
2026-10-27 18:28   ` Conor Dooley
2026-10-27 13:44 ` [PATCH 2/2] iio: adc: add ltc2387 driver Nicolae-Daniel Deaconescu
2026-10-27 14:06   ` sashiko-bot
2026-10-27 14:37   ` Joshua Crofts
2026-10-28  2:39   ` Jonathan Cameron
2026-10-27 15:56 ` [PATCH 0/2] iio: adc: add LTC2387 and ADAQ2387x support Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox