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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CA+V-a8tE=tHLWAOnLvrSXeATtevuyP-B=BzowKzvZzqebQNCrg@mail.gmail.com>
Date: Thu, 7 Mar 2024 10:04:59 +0000
From: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: Geert Uytterhoeven <geert+renesas@...der.be>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Jiri Slaby <jirislaby@...nel.org>, 
	Rob Herring <robh+dt@...nel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>, 
	Magnus Damm <magnus.damm@...il.com>, linux-kernel@...r.kernel.org, 
	linux-serial@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-renesas-soc@...r.kernel.org, 
	Fabrizio Castro <fabrizio.castro.jz@...esas.com>, 
	Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH 2/2] dt-bindings: serial: renesas,scif: Validate
 'interrupts' and 'interrupt-names'

Hi Krzysztof,

Thank you for the review.

On Thu, Mar 7, 2024 at 9:53 AM Krzysztof Kozlowski
<krzysztof.kozlowski@...aro.org> wrote:
>
> On 07/03/2024 00:10, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> >
> > 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@...renesas.com>
> > ---
> >  .../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