Re: [PATCH v8 1/2] dt-bindings: iio: adc: add AD4130

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 7/18/22 16:14, Linus Walleij wrote:
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?


You're right, this is indeed fit for being implemented using the clock
framework.

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

You got this wrong.

The chip has 4 operating modes regarding clocking.

Internal 76.8kHz clock (clkout can be used as an interrupt pin).
Internal 76.8kHz clock, available externally on the clkout pin (clkout
becomes an output).
External 76.8kHz clock (clkout is an input).
External 153.6kHz clock, internally divided by two (clkout is an input).

This property is used to choose between what frequency to set the
external clock up with. Indeed, if the external clock is not present,
then exposing the 76.8kHz clock using the clock framework would be fine.

Maybe you have a better suggestion about what to do with this?
How do I tell the chip what frequency the external clock is, but also
tell the clock what frequency to use? It's a bit of a conundrum for me.


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.


Range becomes [-VRef, VRef], as opposed to [0, VRef], resolution is
halved.

Other than that it looks very nice!

Yours,
Linus Walleij



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux