RE: [PATCH] iio: adc: rzg2l_adc: Add support for RZ/G2UL ADC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux