All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.