Re: [PATCH 2/7] dt-bindings: iio: adc: document RZ/T2H and RZ/N2H ADC

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4451 bytes --]

On Tue, Sep 23, 2025 at 08:14:09PM +0000, Cosmin-Gabriel Tanislav wrote:
> > From: Conor Dooley <conor@kernel.org>
> > 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@renesas.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@renesas.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

Your mail client is screwing up quoting somehow. Please fix it.

> > > +  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.

"There might", so in other words: have written the driver and have not
had any need for this information. ;)

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

That is true, but also there's no interest in having properties that
you can obtain by other means - or don't need at all.

> 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

Right, you can get the information from another source. You only need to
do that exactly once, during probe, so whatever overhead that produces
isn't meaningful. I think this property should be removed.

> 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.
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]