Message-ID:
 <TYWPR01MB88052B9ACC279F3C00AEAC21851DA@TYWPR01MB8805.jpnprd01.prod.outlook.com>
Date: Tue, 23 Sep 2025 20:14:09 +0000
From: Cosmin-Gabriel Tanislav <cosmin-gabriel.tanislav.xa@...esas.com>
To: Conor Dooley <conor@...nel.org>
CC: Jonathan Cameron <jic23@...nel.org>, David Lechner
	<dlechner@...libre.com>, Nuno Sá <nuno.sa@...log.com>,
	Andy Shevchenko <andy@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof
 Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Geert
 Uytterhoeven <geert+renesas@...der.be>, magnus.damm <magnus.damm@...il.com>,
	Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>,
	Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>,
	"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>
Subject: RE: [PATCH 2/7] dt-bindings: iio: adc: document RZ/T2H and RZ/N2H ADC



> -----Original Message-----
> From: Conor Dooley <conor@...nel.org>
> Sent: Tuesday, September 23, 2025 9:42 PM
> To: Cosmin-Gabriel Tanislav <cosmin-gabriel.tanislav.xa@...esas.com>
> Cc: Jonathan Cameron <jic23@...nel.org>; David Lechner
> <dlechner@...libre.com>; Nuno Sá <nuno.sa@...log.com>; Andy Shevchenko
> <andy@...nel.org>; Rob Herring <robh@...nel.org>; Krzysztof Kozlowski
> <krzk+dt@...nel.org>; Conor Dooley <conor+dt@...nel.org>; Geert
> Uytterhoeven <geert+renesas@...der.be>; magnus.damm
> <magnus.damm@...il.com>; Michael Turquette <mturquette@...libre.com>;
> Stephen Boyd <sboyd@...nel.org>; Prabhakar Mahadev Lad <prabhakar.mahadev-
> lad.rj@...renesas.com>; linux-iio@...r.kernel.org; linux-renesas-
> soc@...r.kernel.org; devicetree@...r.kernel.org; linux-
> kernel@...r.kernel.org; linux-clk@...r.kernel.org
> Subject: Re: [PATCH 2/7] dt-bindings: iio: adc: document RZ/T2H and RZ/N2H
> ADC
>
> On Tue, Sep 23, 2025 at 07:05:16PM +0300, Cosmin Tanislav wrote:
> > Document the A/D 12-Bit successive approximation converters found in the
> > Renesas RZ/T2H (R9A09G077) and RZ/N2H (R9A09G087) SoCs.
> >
> > RZ/T2H has two ADCs with 4 channels and one with 6.
> > RZ/N2H has two ADCs with 4 channels and one with 15.
> >
> > Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@...esas.com>
> > ---
> >  .../iio/adc/renesas,r9a09g077-adc.yaml        | 170 ++++++++++++++++++
> >  MAINTAINERS                                   |   7 +
> >  2 files changed, 177 insertions(+)
> >  create mode 100644
> Documentation/devicetree/bindings/iio/adc/renesas,r9a09g077-adc.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/iio/adc/renesas,r9a09g077-
> adc.yaml b/Documentation/devicetree/bindings/iio/adc/renesas,r9a09g077-
> adc.yaml
> > new file mode 100644
> > index 000000000000..840108cd317e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/renesas,r9a09g077-
> adc.yaml
> > @@ -0,0 +1,170 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/adc/renesas,r9a09g077-adc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas RZ/T2H / RZ/N2H ADC12
> > +
> > +maintainers:
> > +  - Cosmin Tanislav <cosmin-gabriel.tanislav.xa@...esas.com>
> > +
> > +description: |
> > +  A/D Converter block is a successive approximation analog-to-digital
> converter
> > +  with a 12-bit accuracy. Up to 15 analog input channels can be
> selected.
> > +  Conversions can be performed in single or continuous mode. Result of
> the ADC
> > +  is stored in a 16-bit data register corresponding to each channel.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - renesas,r9a09g077-adc # RZ/T2H
> > +      - renesas,r9a09g087-adc # RZ/N2H
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    items:
> > +      - description: A/D scan end interrupt
> > +      - description: A/D scan end interrupt for Group B
> > +      - description: A/D scan end interrupt for Group C
> > +      - description: Window A compare match
> > +      - description: Window B compare match
> > +      - description: Compare match
> > +      - description: Compare mismatch
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: adi
> > +      - const: gbadi
> > +      - const: gcadi
> > +      - const: cmpai
> > +      - const: cmpbi
> > +      - const: wcmpm
> > +      - const: wcmpum
> > +
> > +  clocks:
> > +    items:
> > +      - description: converter clock
> > +      - description: peripheral clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: adclk
> > +      - const: pclk
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  renesas,max-channels:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: |
> > +      Maximum number of channels supported by the ADC.
> > +      RZ/T2H has two ADCs with 4 channels and one with 6 channels.
> > +      RZ/N2H has two ADCs with 4 channels and one with 15 channels.
>
> What is the point of this? Why do you need to know how many channels
> there can be in the driver, isn't it enough to just figure out how many
> child nodes you have?
>

The idea here was that the SoC dtsi file would define the number of
channels supported by each instance of the ADC peripheral, while the
board dtsi (which includes the SoC dtsi) would define the number of
channels actually wired up on the.

Alternatively, we could have multiple compatibles for each SoC, like
renesas,r9a09g077-adc-4, which would only have 4 channels, while
the main renesas,r9a09g077-adc compatible would be the one with the
most channels, 6.

There might exist instances where knowing how many channels the chip
has might be useful inside the driver, but the bindings themselves
shouldn't really be addressing driver requirements, they should be
describing the hardware properties.

The maximum number of supported channels of each ADC instance is a
property of the hardware, which is fine to have in the bindings.

Also, it is useful to know the maximum number of channels,
otherwise, we would have to iterate over the iio_chan_spec
populated by devm_iio_adc_device_alloc_chaninfo_se() to figure out
what is the maximum used channel. We will surely need this
information when implementing buffered mode, to know up to which
register to read data from, and we already need it when iterating
over the enabled channels for the same reason.

All things considered, I think it is useful to have this property
here, considering the separation between SoC capabilities and board
implementation.

> > +
> > +  '#address-cells':
> > +    const: 1
> > +
> > +  '#size-cells':
> > +    const: 0
> > +
> > +  "#io-channel-cells":
> > +    const: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - power-domains
> > +  - renesas,max-channels
>
> This should be after patternProperties.
>

Ack.

> > +
> > +patternProperties:
> > +  "^channel@[0-9a-e]$":
> > +    $ref: adc.yaml
> > +    type: object
> > +    description: The external channels which are connected to the ADC.
> > +
> > +    properties:
> > +      reg:
> > +        description: The channel number.
> > +        maximum: 14
> > +
> > +    required:
> > +      - reg
> > +
>
> > +    additionalProperties: false
>
> You don't include any properties other than reg from adc.yaml, and using
> additionalProperties: false blocks their use. Is that intentional or
> should this be unevaluatedProperties: false?
>

I don't think we need any other properties besides reg at this point,
and reg is the only property actually handled by the driver, via
devm_iio_adc_device_alloc_chaninfo_se().

> Cheers,
> Conor.
>
> > +
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: renesas,r9a09g077-adc
> > +    then:
> > +      properties:
> > +        renesas,max-channels:
> > +          enum: [4, 6]
> > +
> > +      patternProperties:
> > +        "^channel@[6-9a-e]$": false
> > +        "^channel@[0-5]$":
> > +          properties:
> > +            reg:
> > +              maximum: 5
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - renesas,r9a09g087-adc
> > +    then:
> > +      properties:
> > +        renesas,max-channels:
> > +          enum: [4, 15]
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/renesas,r9a09g077-cpg-mssr.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    adc@...08000 {
> > +      compatible = "renesas,r9a09g077-adc";
> > +      reg = <0x80008000 0x400>;
> > +      interrupts = <GIC_SPI 708 IRQ_TYPE_EDGE_RISING>,
> > +                   <GIC_SPI 709 IRQ_TYPE_EDGE_RISING>,
> > +                   <GIC_SPI 710 IRQ_TYPE_EDGE_RISING>,
> > +                   <GIC_SPI 711 IRQ_TYPE_LEVEL_HIGH>,
> > +                   <GIC_SPI 712 IRQ_TYPE_LEVEL_HIGH>,
> > +                   <GIC_SPI 855 IRQ_TYPE_EDGE_RISING>,
> > +                   <GIC_SPI 856 IRQ_TYPE_EDGE_RISING>;
> > +      interrupt-names = "adi", "gbadi", "gcadi",
> > +                        "cmpai", "cmpbi", "wcmpm", "wcmpum";
> > +      clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKL>,
> > +               <&cpg CPG_MOD 225>;
> > +      clock-names = "adclk", "pclk";
> > +      power-domains = <&cpg>;
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +      #io-channel-cells = <1>;
> > +      renesas,max-channels = <6>;
> > +
> > +      channel@0 {
> > +        reg = <0x0>;
> > +      };
> > +      channel@1 {
> > +        reg = <0x1>;
> > +      };
> > +      channel@2 {
> > +        reg = <0x2>;
> > +      };
> > +      channel@3 {
> > +        reg = <0x3>;
> > +      };
> > +    };
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 9f4b48801879..07e0d37cf468 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -21822,6 +21822,13 @@ S: Supported
> >  F: Documentation/devicetree/bindings/timer/renesas,rz-mtu3.yaml
> >  F: drivers/counter/rz-mtu3-cnt.c
> >
> > +RENESAS RZ/T2H / RZ/N2H A/D DRIVER
> > +M: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@...esas.com>
> > +L: linux-iio@...r.kernel.org
> > +L: linux-renesas-soc@...r.kernel.org
> > +S: Supported
> > +F: Documentation/devicetree/bindings/iio/adc/renesas,r9a09g077-adc.yaml
> > +
> >  RENESAS RTCA-3 RTC DRIVER
> >  M: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> >  L: linux-rtc@...r.kernel.org
> > --
> > 2.51.0
> >
________________________________

Renesas Electronics Europe GmbH
Registered Office: Arcadiastrasse 10
DE-40472 Duesseldorf
Commercial Registry: Duesseldorf, HRB 3708
Managing Director: Carsten Jauch
VAT-No.: DE 14978647
Tax-ID-No: 105/5839/1793

Legal Disclaimer: This e-mail communication (and any attachment/s) is confidential and contains proprietary information, some or all of which may be legally privileged. It is intended solely for the use of the individual or entity to which it is addressed. Access to this email by anyone else is unauthorized. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited and may be unlawful.

Powered by blists - more mailing lists