| Message ID | 20240306231007.13622-3-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names' | expand |
Hi Prabhakar, On Thu, Mar 7, 2024 at 12:11 AM Prabhakar <prabhakar.csengg@gmail.com> wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.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@bp.renesas.com> Thanks for your patch! > --- 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: > - 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 As the above three groups are increasing supersets, you can just use a single "items" listing all 6 interrupts, and describe the first one as "Error interrupt or single combined interrupt". After that, the SoC-specific logic at the end just needs to specify the appropriate minItems/maxItems. > - > - 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 Likewise, with "interrupt-names: false" below for the ones that don't need it. > - > 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 Please simplify using family-specific names: - renesas,rcar-gen1-scif - renesas,rcar-gen2-scif - renesas,rcar-gen3-scif - renesas,rcar-gen4-scif > + then: > + properties: > + interrupts: > + items: > + - description: A combined interrupt Gr{oetje,eeting}s, Geert
On 07/03/2024 00:10, Prabhakar wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.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@bp.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. > - 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? Best regards, Krzysztof
Hi Geert, Thank you for the review. On Thu, Mar 7, 2024 at 8:58 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Thu, Mar 7, 2024 at 12:11 AM Prabhakar <prabhakar.csengg@gmail.com> wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.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@bp.renesas.com> > > Thanks for your patch! > > > --- 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: > > - 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 > > As the above three groups are increasing supersets, you can just use > a single "items" listing all 6 interrupts, and describe the first one > as "Error interrupt or single combined interrupt". After that, the > SoC-specific logic at the end just needs to specify the appropriate > minItems/maxItems. > Agreed, I will do that. > > - > > - 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 > > Likewise, with "interrupt-names: false" below for the ones that don't > need it. > Agreed. > > - > > 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 > > Please simplify using family-specific names: > - renesas,rcar-gen1-scif > - renesas,rcar-gen2-scif > - renesas,rcar-gen3-scif > - renesas,rcar-gen4-scif > Yep, that makes sense. Cheers, Prabhakar
Hi Krzysztof, Thank you for the review. On Thu, Mar 7, 2024 at 9:53 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 07/03/2024 00:10, Prabhakar wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.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@bp.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
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: - 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 - 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 + then: + properties: + interrupts: + items: + - description: A combined interrupt + + - if: + properties: + compatible: + contains: + enum: + - renesas,scif-r7s72100 + then: + properties: + interrupts: + items: + - description: Error interrupt + - description: Receive buffer full interrupt + - description: Transmit buffer empty interrupt + - description: Break interrupt + interrupt-names: + items: + - const: eri + - const: rxi + - const: txi + - const: bri + + - if: + properties: + compatible: + contains: + enum: + - renesas,scif-r7s9210 + - renesas,scif-r9a07g044 + then: + properties: + interrupts: + 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: + items: + - const: eri + - const: rxi + - const: txi + - const: bri + - const: dri + - const: tei + unevaluatedProperties: false examples: