linux-kernel - Re: [PATCH v4 3/5] dt-bindings: iio: adc: add ade9000

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250820210340.GB986565-robh@kernel.org>
Date: Wed, 20 Aug 2025 16:03:40 -0500
From: Rob Herring <robh@...nel.org>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Antoniu Miclaus <antoniu.miclaus@...log.com>, conor+dt@...nel.org,
	linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org
Subject: Re: [PATCH v4 3/5] dt-bindings: iio: adc: add ade9000

On Sat, Aug 16, 2025 at 05:52:58PM +0100, Jonathan Cameron wrote:
> On Fri, 15 Aug 2025 09:56:36 +0000
> Antoniu Miclaus <antoniu.miclaus@...log.com> wrote:
> 
> > Add devicetree bindings support for ade9000.
> > 
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@...log.com>
> 
> Hi Antoniu,
> Sorry I missed v3 last week. Garage door crisis ate up my review time!
> 
> A few minor comments inline.
> 
> Jonathan
> 
> > ---
> > changes in v4:
> >  - improve description formatting (remove unnecessary pipe symbols)
> >  - move $ref to end and remove allOf section for cleaner structure
> >  .../bindings/iio/adc/adi,ade9000.yaml         | 108 ++++++++++++++++++
> >  1 file changed, 108 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ade9000.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ade9000.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ade9000.yaml
> > new file mode 100644
> > index 000000000000..bd374c0d57d4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ade9000.yaml
> > @@ -0,0 +1,108 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright 2025 Analog Devices Inc.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/adc/adi,ade9000.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices ADE9000 High Performance, Polyphase Energy Metering driver
> > +
> > +maintainers:
> > +  - Antoniu Miclaus <antoniu.miclaus@...log.com>
> > +
> > +description: |
> > +  The ADE9000 s a highly accurate, fully integrated, multiphase energy and power
> 
> is a 
> 
> > +  quality monitoring device. Superior analog performance and a digital signal
> > +  processing (DSP) core enable accurate energy monitoring over a wide dynamic
> > +  range. An integrated high end reference ensures low drift over temperature
> > +  with a combined drift of less than ±25 ppm/°C maximum for the entire channel
> > +  including a programmable gain amplifier (PGA) and an analog-to- digital
> analog-to-digital
> 
> > +  converter (ADC).
> > +
> > +  https://www.analog.com/media/en/technical-documentation/data-sheets/ADE9000.pdf
> > +
> > +$ref: /schemas/spi/spi-peripheral-props.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,ade9000
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  spi-max-frequency:
> > +    maximum: 20000000
> > +
> > +  interrupts:
> > +    maxItems: 3
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: irq0
> > +      - const: irq1
> > +      - const: dready
> 
> I always forget how these work.  Does this allow me to say irq1 and dready
> are wired but not irq0? 
> 
> Similar to question on interrupts being required below, if it is plausible
> the driver could be modified to work with a lesser set, the binding should allow
> it.
> 
> > +
> > +  reset-gpios:
> > +    description:
> > +      Must be the device tree identifier of the RESET pin. As the line is
> > +      active low, it should be marked GPIO_ACTIVE_LOW.
> > +    maxItems: 1
> > +
> > +  vdd-supply: true
> > +
> > +  vref-supply: true
> > +
> > +  clocks:
> > +    description: External clock source when not using crystal
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    items:
> > +      - const: clkin
> > +
> > +  "#clock-cells":
> > +    description:
> > +      ADE9000 can provide clock output via CLKOUT pin with external buffer.
> > +    const: 0
> > +
> > +  clock-output-names:
> > +    items:
> > +      - const: clkout
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reset-gpios
> 
> As with interrupts, can we not use it at all if the reset line is tied
> to not reset?   Or is it a driver limitation (which is fine to have but shouldn't
> affect the binding).
> 
> > +  - interrupts
> > +  - interrupt-names
> My usual question on interrupts.  Is the device completely useless without them or
> is it just the case that we currently require them in the driver because we don't
> poll for completion as an alternative?  Fine to require them in the driver even
> if the binding doesn't require them.

That sounds to me the wrong way around. A driver is free to not require 
a property that the binding requires. They should only be not required 
if not hooking them up on the board design is valid (which would then 
imply a driver should be possible without them (or the h/w designer is 
incompetent)).

Rob