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

[Conor Dooley <conor@kernel.org>]

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

On Mon, Nov 02, 2026 at 10:11:10AM +0200, Nicolae-Daniel Deaconescu wrote:
> 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.

Then modify the PWM API to get the information that you need, you should
not be constructing fake relationships in your devicetree to work around
limitations of some subsystem's interface. If you need the information
from a PWM provider for what the minimum period or duty cycle step it can
provide is so that you can use the waveform API correctly, then add that.
I took a quick look at the code you've added and it looks like it'd just
fall over as soon as someone used a PWM provider with a mandatory
divider?

Trevor got lucky that I didn't ask these questions about the ad7625, you
did not.

> > 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.

Dunno, don't care. How the drivers are written has nothing to with me :)

Cheers,
Conor.

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