Hi Cosmin, thanks for your patch! On Fri, Jul 15, 2022 at 6:50 AM Cosmin Tanislav <demonsingur@xxxxxxxxx> wrote: > AD4130-8 is an ultra-low power, high precision, measurement solution for > low bandwidth battery operated applications. > > The fully integrated AFE (Analog Front-End) includes a multiplexer for up > to 16 single-ended or 8 differential inputs, PGA (Programmable Gain > Amplifier), 24-bit Sigma-Delta ADC, on-chip reference and oscillator, > selectable filter options, smart sequencer, sensor biasing and excitation > options, diagnostics, and a FIFO buffer. > > Signed-off-by: Cosmin Tanislav <cosmin.tanislav@xxxxxxxxxx> > Reviewed-by: Rob Herring <robh@xxxxxxxxxx> (...) This caught my eye: > + adi,int-clk-out: > + description: Specify if the internal clock should be exposed on the CLK pin. > + type: boolean Okay, but would it not make more sense to just imply this if the clock on the CLK pin has any consumers? Like update this setting in hardware when the consumer does clk_prepare() or so on that externally routed clock? > + adi,ext-clk-freq-hz: > + description: Specify the frequency of the external clock. > + enum: [76800, 153600] > + default: 76800 This looks like cheating, i.e just outputting a clock on that pin and ignoring to model the consumer. Shouldn't this rather be a clkout subnode with 2 #clock-cells and the fequency set in a cell in a consumer phandle? Like how I did in commit 7335631fcd5eecfa84555bd57433e6446d06ad21 "dt-bindings: clock: u8500: Add clkout clock bindings" Usually it is the consumer that requests a specific clock and then the producer will respond. Certainly whatever is consuming this clock needs to be in the device tree as well, and then this is the right pattern. (In Linux you will then use the clk framework to manage the clock and callbacks but that is irrelevant for the DT bindings.) > + adi,bipolar: > + description: Specify if the device should be used in bipolar mode. > + type: boolean Can you explain what this means? I don't understand what it would mean for an analog device / AFE to be in bipolar mode. Other than that it looks very nice! Yours, Linus Walleij
