Re: [PATCH 2/2] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'

[Lad, Prabhakar]

Hi Krzysztof,

Thank you for the review.

On Thu, Mar 7, 2024 at 9:53 AM Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> On 07/03/2024 00:10, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> >
> > This commit adds support to validate the 'interrupts' and 'interrupt-names'
> > properties for every supported SoC. This ensures proper handling and
> > configuration of interrupt-related properties across supported platforms.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > ---
> >  .../bindings/serial/renesas,scif.yaml         | 117 +++++++++++++-----
> >  1 file changed, 85 insertions(+), 32 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/serial/renesas,scif.yaml b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> > index af72c3420453..1a5f4568ae4f 100644
> > --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> > +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> > @@ -82,38 +82,6 @@ properties:
> >    reg:
> >      maxItems: 1
> >
> > -  interrupts:
>
> Properties should be defined in top-level block with widest constraints
> (min/maxItems). Your if:then: narrows them per each variant.
>
Agreed, I will fix that.

> > -    oneOf:
> > -      - items:
> > -          - description: A combined interrupt
> > -      - items:
> > -          - description: Error interrupt
> > -          - description: Receive buffer full interrupt
> > -          - description: Transmit buffer empty interrupt
> > -          - description: Break interrupt
> > -      - items:
> > -          - description: Error interrupt
> > -          - description: Receive buffer full interrupt
> > -          - description: Transmit buffer empty interrupt
> > -          - description: Break interrupt
> > -          - description: Data Ready interrupt
> > -          - description: Transmit End interrupt
> > -
> > -  interrupt-names:
> > -    oneOf:
> > -      - items:
> > -          - const: eri
> > -          - const: rxi
> > -          - const: txi
> > -          - const: bri
> > -      - items:
> > -          - const: eri
> > -          - const: rxi
> > -          - const: txi
> > -          - const: bri
> > -          - const: dri
> > -          - const: tei
>
> This probably could be:
>
>         minItems: 4
>         items:
>           - const: eri
>           - const: rxi
>           - const: txi
>           - const: bri
>           - const: dri
>           - const: tei
>
> > -
> >    clocks:
> >      minItems: 1
> >      maxItems: 4
> > @@ -173,6 +141,91 @@ allOf:
> >        required:
> >          - resets
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - renesas,scif-r8a7742
> > +              - renesas,scif-r8a7743
> > +              - renesas,scif-r8a7744
> > +              - renesas,scif-r8a7745
> > +              - renesas,scif-r8a77470
> > +              - renesas,scif-r8a774a1
> > +              - renesas,scif-r8a774b1
> > +              - renesas,scif-r8a774c0
> > +              - renesas,scif-r8a774e1
> > +              - renesas,scif-r8a7778
> > +              - renesas,scif-r8a7779
> > +              - renesas,scif-r8a7790
> > +              - renesas,scif-r8a7791
> > +              - renesas,scif-r8a7792
> > +              - renesas,scif-r8a7793
> > +              - renesas,scif-r8a7794
> > +              - renesas,scif-r8a7795
> > +              - renesas,scif-r8a7796
> > +              - renesas,scif-r8a77961
> > +              - renesas,scif-r8a77965
> > +              - renesas,scif-r8a77970
> > +              - renesas,scif-r8a77980
> > +              - renesas,scif-r8a77990
> > +              - renesas,scif-r8a77995
> > +              - renesas,scif-r8a779a0
> > +              - renesas,scif-r8a779f0
> > +              - renesas,scif-r8a779g0
>
> There is no way compatibility strings express it?
>
I'll cut it short to below as suggested by Geert,

   - renesas,rcar-gen1-scif
   - renesas,rcar-gen2-scif
   - renesas,rcar-gen3-scif
   - renesas,rcar-gen4-scif

Cheers,
Prabhakar