| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250607174521.6dee54fb@jic23-huawei>
Date: Sat, 7 Jun 2025 17:45:21 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Marcelo Schmitt <marcelo.schmitt@...log.com>
Cc: <linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-gpio@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<lars@...afoo.de>, <Michael.Hennerich@...log.com>, <dlechner@...libre.com>,
<nuno.sa@...log.com>, <andy@...nel.org>, <robh@...nel.org>,
<krzk+dt@...nel.org>, <conor+dt@...nel.org>, <linus.walleij@...aro.org>,
<brgl@...ev.pl>, <marcelo.schmitt1@...il.com>
Subject: Re: [PATCH v4 01/11] dt-bindings: iio: adc: Add AD4170
On Mon, 2 Jun 2025 08:36:24 -0300
Marcelo Schmitt <marcelo.schmitt@...log.com> wrote:
> Add device tree documentation for AD4170 and similar sigma-delta ADCs.
> The AD4170 is a 24-bit, multichannel, sigma-delta ADC.
>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@...log.com>
> ---
> Change log v3 -> v4
> - Dropped sensor-node and most of defs.
> - Updated external sensor props to have similar name and type of adi,ad4130 ones.
> - Added constraints to properties related to external bridge sensor excitation.
>
> Some explanation about the constraints to weigh scale sensor types.
>
> The predefined ACX1, ACX1 negated, ACX2, and ACX2 negated signals are used to AC
> excite external bridge circuits and are output on GPIO2, GPIO0, GPIO3, and
> GPIO1, respectively. If only two pins are specified for AC excitation, only ACX1
> and ACX2 (GPIO2 and GPIO3) are used. Because of that, if AC excitation is
> specified/requested, then those specific GPIO pins must be used with the bridge.
> Otherwise, the bridge won't get properly excited and we also cannot guarantee to
> avoid short-circuit conditions since the level set to GPIOs to DC excite the
> bridge depends on the GPIO number. See AD4170 datasheet Figure 113 Weigh Scale
> (AC Excitation) for the reference circuit diagram.
> Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4170-4.pdf#unique_149_Connect_42_ID10354
>
> .../bindings/iio/adc/adi,ad4170.yaml | 543 ++++++++++++++++++
> MAINTAINERS | 7 +
> 2 files changed, 550 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
> new file mode 100644
> index 000000000000..87b6e821fdb8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
> @@ -0,0 +1,543 @@
> +
> +$defs:
> + reference-buffer:
> + description: |
> + Enable precharge buffer, full buffer, or skip reference buffering of
> + the positive/negative voltage reference. Because the output impedance
> + of the source driving the voltage reference inputs may be dynamic, RC
RC?
> + combinations of those inputs can cause DC gain errors if the reference
> + inputs go unbuffered into the ADC. Enable reference buffering if the
> + provided reference source has dynamic high impedance output. Note the
> + absolute voltage allowed on REFINn+ and REFINn- inputs is from
> + AVSS - 50 mV to AVDD + 50 mV when the reference buffers are disabled
> + but narrows to AVSS to AVDD when reference buffering is enabled or in
> + precharge mode. The valid options for this property are:
> + 0: Reference precharge buffer.
> + 1: Full reference buffering.
> + 2: Bypass reference buffers (buffering disabled).
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1, 2]
> + default: 1
> +
> + interrupts:
> + description:
> + Interrupt for signaling the completion of conversion results. The data
> + ready signal (RDY) used as interrupt is by default provided on the SDO
> + pin. Alternatively, it can be provided on the DIG_AUX1 pin in which case
> + the chip disables the RDY function on SDO. Thus, there can be only one
> + data ready interrupt enabled at a time.
> + maxItems: 1
> +
> + interrupt-names:
> + description:
> + Specify which pin should be configured as Data Ready interrupt.
> + enum:
> + - sdo
> + - dig_aux1
> + maxItems: 1
> + default: sdo
I think I'd prefer to see it always provided as there is no strong
reason to prefer one as the default.
>
> + adi,excitation-pin-0:
> + $ref: '#/$defs/excitation-pin'
> +
> + adi,excitation-pin-1:
> + $ref: '#/$defs/excitation-pin'
> +
> + adi,excitation-pin-2:
> + $ref: '#/$defs/excitation-pin'
> +
> + adi,excitation-pin-3:
> + $ref: '#/$defs/excitation-pin'
> +
> + adi,excitation-current-0-microamp:
> + description:
> + Excitation current in microamperes to be applied to pin specified in
> + adi,excitation-pin-0 while this channel is active.
> + enum: [0, 10, 50, 100, 250, 500, 1000, 1500]
What motivated mix of using $ref and here where there is a lot of repetition?
I don't mind which approach is used, but a mix seems the worst option.
> + default: 0
> +
> + adi,excitation-current-1-microamp:
> + description:
> + Excitation current in microamperes to be applied to pin specified in
> + adi,excitation-pin-1 while this channel is active.
> + enum: [0, 10, 50, 100, 250, 500, 1000, 1500]
> + default: 0
> +
> + adi,excitation-current-2-microamp:
> + description:
> + Excitation current in microamperes to be applied to pin specified in
> + adi,excitation-pin-2 while this channel is active.
> + enum: [0, 10, 50, 100, 250, 500, 1000, 1500]
> + default: 0
> +
> + adi,excitation-current-3-microamp:
> + description:
> + Excitation current in microamperes to be applied to pin specified in
> + adi,excitation-pin-3 while this channel is active.
> + enum: [0, 10, 50, 100, 250, 500, 1000, 1500]
> + default: 0
> +
> + adi,excitation-ac:
> + type: boolean
> + description:
> + Whether the external sensor has to be AC or DC excited. When omitted,
> + it is DC excited.
> +required:
> + - compatible
> + - reg
> + - avdd-supply
> + - iovdd-supply
> + - spi-cpol
> + - spi-cpha
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@0 {
> + compatible = "adi,ad4170";
> + reg = <0>;
> + spi-max-frequency = <20000000>;
> + spi-cpol;
> + spi-cpha;
> + avdd-supply = <&avdd>;
> + iovdd-supply = <&iovdd>;
> + clocks = <&clk>;
> + clock-names = "xtal";
> + interrupt-parent = <&gpio_in>;
In examples I don't think we tend to specify interrupt-parent.
That's covered fine by the interrupt bindings.
As above, I'd prefer to always see interrupt-names as well so
we have it really visible which one this is.
> + interrupts = <0 IRQ_TYPE_EDGE_FALLING>;
> + adi,vbias-pins = <7>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + };
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@0 {
> + compatible = "adi,ad4170";
> + reg = <0>;
> + spi-max-frequency = <20000000>;
> + spi-cpol;
> + spi-cpha;
> + avdd-supply = <&avdd>;
> + iovdd-supply = <&iovdd>;
> + #clock-cells = <0>;
> + clock-output-names = "ad4170-clk16mhz";
> + interrupt-parent = <&gpio_in>;
> + interrupts = <0 IRQ_TYPE_EDGE_FALLING>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + // Sample AIN0 with respect to AIN1 throughout AVDD/AVSS input range
> + // Differential bipolar. If AVSS < 0V, differential true bipolar
> + channel@0 {
> + reg = <0>;
> + bipolar;
> + diff-channels = <0 1>;
> + adi,reference-select = <3>;
> + };
> + // Sample AIN2 with respect to DGND throughout AVDD/DGND input range
> + // Pseudo-differential unipolar
> + channel@1 {
> + reg = <1>;
> + single-channel = <2>;
> + common-mode-channel = <24>;
> + adi,reference-select = <3>;
> + };
> + // Sample AIN3 with respect to 2.5V throughout AVDD/AVSS input range
> + // Pseudo-differential bipolar
> + channel@2 {
> + reg = <2>;
> + bipolar;
> + single-channel = <3>;
> + common-mode-channel = <29>;
> + adi,reference-select = <3>;
> + };
> + // Sample AIN4 with respect to DGND throughout AVDD/AVSS input range
> + // Pseudo-differential bipolar
> + channel@3 {
> + reg = <3>;
> + bipolar;
> + single-channel = <4>;
> + common-mode-channel = <24>;
> + adi,reference-select = <3>;
> + };
> + // Sample AIN5 with respect to 2.5V throughout AVDD/AVSS input range
> + // Pseudo-differential unipolar (AD4170 datasheet page 46 example)
> + channel@4 {
> + reg = <4>;
> + single-channel = <5>;
> + common-mode-channel = <29>;
> + adi,reference-select = <3>;
> + };
> + // Sample AIN6 with respect to 2.5V throughout REFIN+/REFIN- input range
> + // Pseudo-differential bipolar
> + channel@5 {
> + reg = <5>;
> + bipolar;
> + single-channel = <6>;
> + common-mode-channel = <29>;
> + adi,reference-select = <0>;
> + };
> + // Weigh scale sensor
> + channel@6 {
> + reg = <6>;
> + bipolar;
> + diff-channels = <7 8>;
> + adi,reference-select = <0>;
> + adi,sensor-type = "weighscale";
> + adi,excitation-pin-0 = <17>;
> + adi,excitation-pin-1 = <18>;
> + adi,excitation-pin-2 = <19>;
> + adi,excitation-pin-3 = <20>;
> + adi,excitation-ac;
> + };
> + };
> + };
> +...
> +
Powered by blists - more mailing lists