Re: [PATCH 1/2] dt-bindings: iio: adc: add lltc,ltc2387

[Nicolae-Daniel Deaconescu]

On Fri, 30 Oct 2026 20:50:11 +0000, Conor Dooley wrote:
> On Fri, Oct 30, 2026 at 02:02:05PM +0200, Nicolae-Daniel Deaconescu wrote:
> > On Tue, Oct 27, 2026 at 18:28:34 +0000, Conor Dooley wrote:
> > > Why are we getting new devices with linear vendor prefixes?
> >
> > I'll fix this in v2. The binding file will be adi,ltc2387.yaml.
> > The -x4 compatibles will also be dropped, because they described
> > four devices sharing one AXI ADC.
>
> Please retain more context than this, it's not enough to just have what
> you're replying to. I see so many patches that I will have completely
> forgotten what you're doing by the time I get to the next day.

My bad, I'll keep more relevant info in my responses from now on.

> > > 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?
> >
> > The LTC2387-16 datasheet has these pins: CNV+/CNV- (pins 27/28), which
> > is the conversion-start input; CLK+/CLK- (pins 23/24), which is an
> > externally applied clock that shifts out the result; and DCO+/DCO-,
> > the echoed clock. There is no clk_gate pin.
>
> Then there should not be a clk_gate pwm for this device.
>
> > The part needs a burst of 8 CLK pulses per conversion in one-lane mode
> > (4 in two-lane), starting t_FIRSTCLK = 65 ns after CNV and ending
> > within t_LASTCLK = 49 ns of the next CNV. So the CLK input cannot run
> > freely: the clk_gate PWM gates the host clock into that burst. [...]
> > The clocks property is the reference clock the PWM generator runs
> > from, and the driver needs its rate to compute the waveforms.
>
> Then you've got a "layering" violation of some kind here. If this clock
> is consumed by the PWM generator, it should be an input to the pwm
> generator not this adc.

It IS an input to the PWM generator, The ADC node references the same
clock only because the driver has to know its rate to compute the CNV
and CLK waveforms in clock cycles. adi,ad7625.yaml does the same.

> The second pwm should then just be called "clk", because that's the
> input(s) that it represents.

I'll rename it to "clk" in v2 and reword the pwms description. ad7625
uses the name clk_gate for the same signal.

> I want to see what the dts you have for a complete setup of this device
> looks like, including the backend and pwm generator.

This is an ADAQ23878 on a Zedboard, two-lane, with the AXI ADC backend,
the DMA it uses, and the AXI PWM generator. The 120 MHz ext_clk is what
the HDL provides to the PWM generator. Supplies are fixed regulators here;
VS- is 0 V.

/ {
	vdd_5v: regulator-vdd-5v {
		compatible = "regulator-fixed";
		regulator-name = "vdd-5v";
		regulator-min-microvolt = <5000000>;
		regulator-max-microvolt = <5000000>;
		regulator-always-on;
	};

	vio_2v5: regulator-vio-2v5 {
		compatible = "regulator-fixed";
		regulator-name = "vio-2v5";
		regulator-min-microvolt = <2500000>;
		regulator-max-microvolt = <2500000>;
		regulator-always-on;
	};

	vs_p: regulator-vs-p {
		compatible = "regulator-fixed";
		regulator-name = "vs-p";
		regulator-min-microvolt = <5000000>;
		regulator-max-microvolt = <5000000>;
		regulator-always-on;
	};

	vs_n: regulator-vs-n {
		compatible = "regulator-fixed";
		regulator-name = "vs-n";
		regulator-min-microvolt = <0>;
		regulator-max-microvolt = <0>;
		regulator-always-on;
	};

	vref: regulator-vref {
		compatible = "regulator-fixed";
		regulator-name = "vref-4v096";
		regulator-min-microvolt = <4096000>;
		regulator-max-microvolt = <4096000>;
		regulator-always-on;
	};

	clocks {
		ext_clk: clock@0 {
			#clock-cells = <0>;
			compatible = "fixed-clock";
			clock-frequency = <120000000>;
		};
	};
};

&fpga_axi {
	rx_dma: dma-controller@44a30000 {
		compatible = "adi,axi-dmac-1.00.a";
		reg = <0x44a30000 0x1000>;
		#dma-cells = <1>;
		interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
		clocks = <&clkc 15>;
	};

	axi_adc: adc@44a00000 {
		compatible = "adi,axi-adc-10.0.a";
		reg = <0x44a00000 0x10000>;
		dmas = <&rx_dma 0>;
		dma-names = "rx";
		clocks = <&clkc 15>;
		#io-backend-cells = <0>;
	};

	axi_pwm_gen: pwm@44a60000 {
		compatible = "adi,axi-pwmgen-2.00.a";
		reg = <0x44a60000 0x1000>;
		#pwm-cells = <3>;
		clocks = <&clkc 15>, <&ext_clk>;
		clock-names = "axi", "ext";
	};

	adc@0 {
		compatible = "adi,adaq23878";
		clocks = <&ext_clk>;
		io-backends = <&axi_adc>;
		pwms = <&axi_pwm_gen 0 0 0>, <&axi_pwm_gen 1 0 0>;
		pwm-names = "cnv", "clk";
		vdd-supply = <&vdd_5v>;
		vio-supply = <&vio_2v5>;
		vref-supply = <&vref>;
		vs-n-supply = <&vs_n>;
		vs-p-supply = <&vs_p>;

		adi,gain-milli = /bits/ 16 <870>;
		adi,use-two-lanes;
	};
};

One more thing, on the cover letter Andy asked whether this should be
done in an existing driver with per-compatible chip data instead.

On Fri, 30 Oct 2026 16:41:27 +0200, Andy Shevchenko wrote:
> On Fri, Oct 30, 2026 at 12:08:08PM +0200, Nicolae-Daniel Deaconescu wrote:
> > On Tue, Oct 27, 2026 at 17:56:43 +0200, Andy Shevchenko wrote:
> > > Why have you written a brand new driver?
> >
> > ad7625 is the closest existing driver, and it shares the transport:
> > CNV PWM, clk_gate PWM and the AXI ADC backend. The converters differ
> > in a few ways though. For instance, the LTC2387 is DDR on one or two
> > lanes. ad7625 is one bit per clock on one lane. Also, the ad7625
> > clamps the period to 100 ns (10 MSPS). The LTC2387 runs at 15 MSPS.
>
> But these difference doesn't explain why you need a brand new driver.
> Is hard to configure this at run-time by choosing the respective driver
> data based on the compatible string?

On Sat, 31 Oct 2026 10:36:27 +0200, Andy Shevchenko wrote:
> On Fri, Oct 30, 2026 at 05:07:05PM +0200, Nicolae-Daniel Deaconescu wrote:
> > On Fri, 30 Oct 2026 16:41:27 +0200, Andy Shevchenko wrote:
> > > But these difference doesn't explain why you need a brand new driver.
> > > Is hard to configure this at run-time by choosing the respective driver
> > > data based on the compatible string?
> >
> > Looking at ad7625.c again, most of it can come from chip data,
> > like the driver already does for max_sample_freq_hz. The lack of
> > en pins could probably also be parametrized.
> >
> > However, doing it this way would risk clashing with the sampling
> > path of the existing driver. Keeping it separate would make testing
> > easier, as well as isolating drivers for unrelated families, which
> > might be out of scope for this.
> >
> > Ultimately I think keeping them separate would be simpler, but
> > I'll integrate it if you believe it's worth it. We might also
> > need to consult with the authors of the original driver, as I
> > haven't worked with the ad7625 before.
>
> If there is no big differences on RTL or programming flow, no need to create
> a brand new driver. Even if there are some differences which are minor, we
> may quirk that in the code. See infamous drivers/gpio/gpio-pca953x.c for how
> many similar, but not fully chips are supported. With your elaboration above
> I believe we may join these drivers to the existing one.

Andy would rather join the drivers. If we port all of this to ad7625, the
LTC2387 and ADAQ2387x compatibles would be added to adi,ad7625.yaml instead
of a new adi,ltc2387.yaml, and the two PWMs would keep one name for all the
parts. Would you prefer that, or a separate binding for these parts?

Thanks,
Daniel