Devicetree
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Jonathan Cameron <jic23@kernel.org>
Cc: Antoniu Miclaus <antoniu.miclaus@analog.com>,
	conor+dt@kernel.org, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v4 3/5] dt-bindings: iio: adc: add ade9000
Date: Wed, 20 Aug 2025 16:03:40 -0500	[thread overview]
Message-ID: <20250820210340.GB986565-robh@kernel.org> (raw)
In-Reply-To: <20250816175258.42286693@jic23-huawei>

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@analog.com> wrote:
> 
> > Add devicetree bindings support for ade9000.
> > 
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.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@analog.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

  reply	other threads:[~2025-08-20 21:03 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-15  9:56 [PATCH v4 0/5] iio: adc: add support for ADE9000 Energy Monitoring IC Antoniu Miclaus
2025-08-15  9:56 ` [PATCH v4 1/5] iio: add IIO_ALTCURRENT channel type Antoniu Miclaus
2025-08-15  9:56 ` [PATCH v4 2/5] iio: add power and energy measurement modifiers Antoniu Miclaus
2025-08-16 16:40   ` Jonathan Cameron
2025-08-15  9:56 ` [PATCH v4 3/5] dt-bindings: iio: adc: add ade9000 Antoniu Miclaus
2025-08-16 16:52   ` Jonathan Cameron
2025-08-20 21:03     ` Rob Herring [this message]
2025-08-25  9:57       ` Jonathan Cameron
2025-08-20 20:58   ` Rob Herring
2025-08-15  9:56 ` [PATCH v4 4/5] iio: adc: add ade9000 support Antoniu Miclaus
2025-08-16 19:08   ` Jonathan Cameron
2025-08-15  9:56 ` [PATCH v4 5/5] Documentation: ABI: iio: add sinc4+lp Antoniu Miclaus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250820210340.GB986565-robh@kernel.org \
    --to=robh@kernel.org \
    --cc=antoniu.miclaus@analog.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=jic23@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox