Hi Geert,
Thanks for the feedback
> Subject: Re: [PATCH] iio: adc: rzg2l_adc: Add support for RZ/G2UL ADC
>
> Hi Biju,
>
> On Mon, May 2, 2022 at 8:18 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > > Subject: RE: [PATCH] iio: adc: rzg2l_adc: Add support for RZ/G2UL
> > > ADC
> > > > Subject: [PATCH] iio: adc: rzg2l_adc: Add support for RZ/G2UL ADC
> > > >
> > > > ADC found on RZ/G2UL SoC is almost identical to RZ/G2L SoC, but
> > > > RZ/G2UL has 2 analog input channels compared to 8 channels on
> RZ/G2L.
> > > > Therefore, added a new compatible to handle this difference.
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > > ---
> > > > drivers/iio/adc/rzg2l_adc.c | 18 +++++++++++++-----
> > > > 1 file changed, 13 insertions(+), 5 deletions(-)
> > > >
> > > I wonder if this changes is really required. RZ/G2UL can still use
> > > the "renesas,rzg2l-adc". As the driver populates the channels
> > > depending the number of elements in the array passed in the DTS and
> > > not always 8 channels. For example on Renesas SMARC EVK only four
> > > channels are populated.
> >
> > For me that restriction is coming from board design, as SoC is capable
> > of handling 8 channels, But board design allows only 4.
> >
> > But on RZ/G2UL SoC, it is capable of handling only 2 channels. Other
> channels are invalid for RZ/G2UL SoC.
> >
> > That is the difference.
> >
> > > With this we don't have to differentiate RZ/G2UL SoC if just add two
> > > channel entries in the SoC DTSI and the driver will just create two
> > > channels.
> >
> > > @Geert - your thoughts on this.
>
> It depends on the meaning of the channel subnodes: do they indicate
> (a) the number of channels present on the SoC, or (b) the number of
> channels used on the board? The DT bindings are not clear about that.
>
> arch/arm64/boot/dts/renesas/r9a07g044.dtsi lists all channels and doesn't
> keep any disabled, which suggests (a).
> arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi does remove unused
> channels, which suggests (b).
>
> Is there any (perhaps performance?) reason we can't just use the number of
> channels present in DT? "make dtbs_check" can still validate this against
> the SoC-specific compatible value.
No. Krzysztof Kozlowski suggested to validate the number of channels present on the SoC
as there is a difference in hardware capability between the SoCs.
>
> Do we need to know at runtime both the number of channels physically
> present and the number of channels used? If yes, we either need to use
> the SoC-specific compatible value, or add a num-channels property.
Yes, currently driver does the validation with RZG2L_ADC_MAX_CHANNELS(8) which is
wrong for RZ/G2UL as it has only 2 Channels.
That is the reason, new SoC-specific compatible introduced to take care of this difference.
Currently I have done the below changes, which restricts the usage of channel > 1 in DT for RZ/G2UL.
+ reg: true
required:
- reg
additionalProperties: false
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: renesas,r9a07g043-adc
+ then:
+ patternProperties:
+ "^channel@[0-7]$":
+ type: object
+ properties:
+ reg:
+ description: |
+ The channel number. It can have up to 2 channels numbered from 0 to 1.
+ items:
+ - minimum: 0
+ maximum: 1
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: renesas,rzg2l-adc
+ then:
+ patternProperties:
+ "^channel@[0-7]$":
+ type: object
+ properties:
+ reg:
+ description: |
+ The channel number. It can have up to 8 channels numbered from 0 to 7.
+ items:
+ - minimum: 0
+ maximum: 7
Cheers,
Biju
