[PATCH] dt-bindings: can: renesas,rcar-canfd: Fix number of channels for R-Car V3U

[PATCH] dt-bindings: can: renesas,rcar-canfd: Fix number of channels for R-Car V3U

[Geert Uytterhoeven]

According to the bindings, only two channels are supported.
However, R-Car V3U supports eight, leading to "make dtbs" failures:

        arch/arm64/boot/dts/renesas/r8a779a0-falcon.dtb: can@e6660000: Unevaluated properties are not allowed ('channel2', 'channel3', 'channel4', 'channel5', 'channel6', 'channel7' were unexpected)

Update the number of channels to 8 on R-Car V3U.
While at it, prevent adding more properties to the channel nodes, as
they must contain no other properties than a status property.

Fixes: d6254d52d70de530 ("dt-bindings: can: renesas,rcar-canfd: Document r8a779a0 support")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Is there a way to express this using positive logic (i.e. default to 2
channels, extend to more where needed)? R-Car V3H_2 (which is not yet
supported) has 3 channels.
Or perhaps the check should be dropped completely?
---
 .../bindings/net/can/renesas,rcar-canfd.yaml  | 132 ++++++++++--------
 1 file changed, 72 insertions(+), 60 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
index 6f71fc96bc4e3156..6a4fb26cfd7b8979 100644
--- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
+++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
@@ -9,9 +9,6 @@ title: Renesas R-Car CAN FD Controller
 maintainers:
   - Fabrizio Castro <fabrizio.castro.jz@renesas.com>
 
-allOf:
-  - $ref: can-controller.yaml#
-
 properties:
   compatible:
     oneOf:
@@ -77,13 +74,15 @@ properties:
     description: Maximum frequency of the CANFD clock.
 
 patternProperties:
-  "^channel[01]$":
+  "^channel[0-7]$":
     type: object
     description:
-      The controller supports two channels and each is represented as a child
-      node.  Each child node supports the "status" property only, which
+      The controller supports multiple channels and each is represented as a
+      child node.  Each child node supports the "status" property only, which
       is used to enable/disable the respective channel.
 
+    unevaluatedProperties: false
+
 required:
   - compatible
   - reg
@@ -98,60 +97,73 @@ required:
   - channel0
   - channel1
 
-if:
-  properties:
-    compatible:
-      contains:
-        enum:
-          - renesas,rzg2l-canfd
-then:
-  properties:
-    interrupts:
-      items:
-        - description: CAN global error interrupt
-        - description: CAN receive FIFO interrupt
-        - description: CAN0 error interrupt
-        - description: CAN0 transmit interrupt
-        - description: CAN0 transmit/receive FIFO receive completion interrupt
-        - description: CAN1 error interrupt
-        - description: CAN1 transmit interrupt
-        - description: CAN1 transmit/receive FIFO receive completion interrupt
-
-    interrupt-names:
-      items:
-        - const: g_err
-        - const: g_recc
-        - const: ch0_err
-        - const: ch0_rec
-        - const: ch0_trx
-        - const: ch1_err
-        - const: ch1_rec
-        - const: ch1_trx
-
-    resets:
-      maxItems: 2
-
-    reset-names:
-      items:
-        - const: rstp_n
-        - const: rstc_n
-
-  required:
-    - reset-names
-else:
-  properties:
-    interrupts:
-      items:
-        - description: Channel interrupt
-        - description: Global interrupt
-
-    interrupt-names:
-      items:
-        - const: ch_int
-        - const: g_int
-
-    resets:
-      maxItems: 1
+allOf:
+  - $ref: can-controller.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - renesas,rzg2l-canfd
+    then:
+      properties:
+        interrupts:
+          items:
+            - description: CAN global error interrupt
+            - description: CAN receive FIFO interrupt
+            - description: CAN0 error interrupt
+            - description: CAN0 transmit interrupt
+            - description: CAN0 transmit/receive FIFO receive completion interrupt
+            - description: CAN1 error interrupt
+            - description: CAN1 transmit interrupt
+            - description: CAN1 transmit/receive FIFO receive completion interrupt
+
+        interrupt-names:
+          items:
+            - const: g_err
+            - const: g_recc
+            - const: ch0_err
+            - const: ch0_rec
+            - const: ch0_trx
+            - const: ch1_err
+            - const: ch1_rec
+            - const: ch1_trx
+
+        resets:
+          maxItems: 2
+
+        reset-names:
+          items:
+            - const: rstp_n
+            - const: rstc_n
+
+      required:
+        - reset-names
+    else:
+      properties:
+        interrupts:
+          items:
+            - description: Channel interrupt
+            - description: Global interrupt
+
+        interrupt-names:
+          items:
+            - const: ch_int
+            - const: g_int
+
+        resets:
+          maxItems: 1
+
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              const: renesas,r8a779a0-canfd
+    then:
+      patternProperties:
+        "^channel[2-7]$": false
 
 unevaluatedProperties: false
 
-- 
2.25.1

Re: [PATCH] dt-bindings: can: renesas,rcar-canfd: Fix number of channels for R-Car V3U

[Krzysztof Kozlowski]

On 02/12/2022 09:22, Geert Uytterhoeven wrote:
> According to the bindings, only two channels are supported.
> However, R-Car V3U supports eight, leading to "make dtbs" failures:
> 
>         arch/arm64/boot/dts/renesas/r8a779a0-falcon.dtb: can@e6660000: Unevaluated properties are not allowed ('channel2', 'channel3', 'channel4', 'channel5', 'channel6', 'channel7' were unexpected)
> 
> Update the number of channels to 8 on R-Car V3U.
> While at it, prevent adding more properties to the channel nodes, as
> they must contain no other properties than a status property.
> 
> Fixes: d6254d52d70de530 ("dt-bindings: can: renesas,rcar-canfd: Document r8a779a0 support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Is there a way to express this using positive logic (i.e. default to 2
> channels, extend to more where needed)? R-Car V3H_2 (which is not yet
> supported) has 3 channels.
> Or perhaps the check should be dropped completely?
> ---
>  .../bindings/net/can/renesas,rcar-canfd.yaml  | 132 ++++++++++--------
>  1 file changed, 72 insertions(+), 60 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> index 6f71fc96bc4e3156..6a4fb26cfd7b8979 100644
> --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> @@ -9,9 +9,6 @@ title: Renesas R-Car CAN FD Controller
>  maintainers:
>    - Fabrizio Castro <fabrizio.castro.jz@renesas.com>
>  
> -allOf:
> -  - $ref: can-controller.yaml#
> -
>  properties:
>    compatible:
>      oneOf:
> @@ -77,13 +74,15 @@ properties:
>      description: Maximum frequency of the CANFD clock.
>  
>  patternProperties:
> -  "^channel[01]$":
> +  "^channel[0-7]$":
>      type: object
>      description:
> -      The controller supports two channels and each is represented as a child
> -      node.  Each child node supports the "status" property only, which
> +      The controller supports multiple channels and each is represented as a
> +      child node.  Each child node supports the "status" property only, which
>        is used to enable/disable the respective channel.
>  
> +    unevaluatedProperties: false

There are no other properties within a channel, so this should be rather
additionalProperties: false.


Best regards,
Krzysztof

Re: [PATCH] dt-bindings: can: renesas,rcar-canfd: Fix number of channels for R-Car V3U

[Geert Uytterhoeven]

Hi Krzysztof,

On Fri, Dec 2, 2022 at 10:01 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 02/12/2022 09:22, Geert Uytterhoeven wrote:
> > According to the bindings, only two channels are supported.
> > However, R-Car V3U supports eight, leading to "make dtbs" failures:
> >
> >         arch/arm64/boot/dts/renesas/r8a779a0-falcon.dtb: can@e6660000: Unevaluated properties are not allowed ('channel2', 'channel3', 'channel4', 'channel5', 'channel6', 'channel7' were unexpected)
> >
> > Update the number of channels to 8 on R-Car V3U.
> > While at it, prevent adding more properties to the channel nodes, as
> > they must contain no other properties than a status property.
> >
> > Fixes: d6254d52d70de530 ("dt-bindings: can: renesas,rcar-canfd: Document r8a779a0 support")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

> > --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> > +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml

> >      description: Maximum frequency of the CANFD clock.
> >
> >  patternProperties:
> > -  "^channel[01]$":
> > +  "^channel[0-7]$":
> >      type: object
> >      description:
> > -      The controller supports two channels and each is represented as a child
> > -      node.  Each child node supports the "status" property only, which
> > +      The controller supports multiple channels and each is represented as a
> > +      child node.  Each child node supports the "status" property only, which
> >        is used to enable/disable the respective channel.
> >
> > +    unevaluatedProperties: false
>
> There are no other properties within a channel, so this should be rather
> additionalProperties: false.

Are you sure? Then I also have to add:

        properties:
          status: true

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] dt-bindings: can: renesas,rcar-canfd: Fix number of channels for R-Car V3U

[Krzysztof Kozlowski]

On 02/12/2022 10:25, Geert Uytterhoeven wrote:
> Hi Krzysztof,
> 
> On Fri, Dec 2, 2022 at 10:01 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>> On 02/12/2022 09:22, Geert Uytterhoeven wrote:
>>> According to the bindings, only two channels are supported.
>>> However, R-Car V3U supports eight, leading to "make dtbs" failures:
>>>
>>>         arch/arm64/boot/dts/renesas/r8a779a0-falcon.dtb: can@e6660000: Unevaluated properties are not allowed ('channel2', 'channel3', 'channel4', 'channel5', 'channel6', 'channel7' were unexpected)
>>>
>>> Update the number of channels to 8 on R-Car V3U.
>>> While at it, prevent adding more properties to the channel nodes, as
>>> they must contain no other properties than a status property.
>>>
>>> Fixes: d6254d52d70de530 ("dt-bindings: can: renesas,rcar-canfd: Document r8a779a0 support")
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
>>> --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
>>> +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> 
>>>      description: Maximum frequency of the CANFD clock.
>>>
>>>  patternProperties:
>>> -  "^channel[01]$":
>>> +  "^channel[0-7]$":
>>>      type: object
>>>      description:
>>> -      The controller supports two channels and each is represented as a child
>>> -      node.  Each child node supports the "status" property only, which
>>> +      The controller supports multiple channels and each is represented as a
>>> +      child node.  Each child node supports the "status" property only, which
>>>        is used to enable/disable the respective channel.
>>>
>>> +    unevaluatedProperties: false
>>
>> There are no other properties within a channel, so this should be rather
>> additionalProperties: false.
> 
> Are you sure? Then I also have to add:
> 
>         properties:
>           status: true
> 

Do you? I think it would be first schema needing it, so maybe that would
be a problem for dtschema...

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: can: renesas,rcar-canfd: Fix number of channels for R-Car V3U

[Geert Uytterhoeven]

Hi Krzysztof,

On Fri, Dec 2, 2022 at 11:49 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 02/12/2022 10:25, Geert Uytterhoeven wrote:
> > On Fri, Dec 2, 2022 at 10:01 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >> On 02/12/2022 09:22, Geert Uytterhoeven wrote:
> >>> According to the bindings, only two channels are supported.
> >>> However, R-Car V3U supports eight, leading to "make dtbs" failures:
> >>>
> >>>         arch/arm64/boot/dts/renesas/r8a779a0-falcon.dtb: can@e6660000: Unevaluated properties are not allowed ('channel2', 'channel3', 'channel4', 'channel5', 'channel6', 'channel7' were unexpected)
> >>>
> >>> Update the number of channels to 8 on R-Car V3U.
> >>> While at it, prevent adding more properties to the channel nodes, as
> >>> they must contain no other properties than a status property.
> >>>
> >>> Fixes: d6254d52d70de530 ("dt-bindings: can: renesas,rcar-canfd: Document r8a779a0 support")
> >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> >>> --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> >>> +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> >
> >>>      description: Maximum frequency of the CANFD clock.
> >>>
> >>>  patternProperties:
> >>> -  "^channel[01]$":
> >>> +  "^channel[0-7]$":
> >>>      type: object
> >>>      description:
> >>> -      The controller supports two channels and each is represented as a child
> >>> -      node.  Each child node supports the "status" property only, which
> >>> +      The controller supports multiple channels and each is represented as a
> >>> +      child node.  Each child node supports the "status" property only, which
> >>>        is used to enable/disable the respective channel.
> >>>
> >>> +    unevaluatedProperties: false
> >>
> >> There are no other properties within a channel, so this should be rather
> >> additionalProperties: false.
> >
> > Are you sure? Then I also have to add:
> >
> >         properties:
> >           status: true
>
> Do you? I think it would be first schema needing it, so maybe that would
> be a problem for dtschema...

You think I haven't tried? ;-)

    arch/arm64/boot/dts/renesas/r8a774a1-beacon-rzg2m-kit.dtb:
can@e66c0000: channel0: Additional properties are not allowed
('status' was unexpected)

And indeed, that would be the first reference to status in a bindings
file...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] dt-bindings: can: renesas,rcar-canfd: Fix number of channels for R-Car V3U

[Rob Herring]

On Fri, Dec 02, 2022 at 11:58:23AM +0100, Geert Uytterhoeven wrote:
> Hi Krzysztof,
> 
> On Fri, Dec 2, 2022 at 11:49 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> > On 02/12/2022 10:25, Geert Uytterhoeven wrote:
> > > On Fri, Dec 2, 2022 at 10:01 AM Krzysztof Kozlowski
> > > <krzysztof.kozlowski@linaro.org> wrote:
> > >> On 02/12/2022 09:22, Geert Uytterhoeven wrote:
> > >>> According to the bindings, only two channels are supported.
> > >>> However, R-Car V3U supports eight, leading to "make dtbs" failures:
> > >>>
> > >>>         arch/arm64/boot/dts/renesas/r8a779a0-falcon.dtb: can@e6660000: Unevaluated properties are not allowed ('channel2', 'channel3', 'channel4', 'channel5', 'channel6', 'channel7' were unexpected)
> > >>>
> > >>> Update the number of channels to 8 on R-Car V3U.
> > >>> While at it, prevent adding more properties to the channel nodes, as
> > >>> they must contain no other properties than a status property.
> > >>>
> > >>> Fixes: d6254d52d70de530 ("dt-bindings: can: renesas,rcar-canfd: Document r8a779a0 support")
> > >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > >
> > >>> --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> > >>> +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> > >
> > >>>      description: Maximum frequency of the CANFD clock.
> > >>>
> > >>>  patternProperties:
> > >>> -  "^channel[01]$":
> > >>> +  "^channel[0-7]$":
> > >>>      type: object
> > >>>      description:
> > >>> -      The controller supports two channels and each is represented as a child
> > >>> -      node.  Each child node supports the "status" property only, which
> > >>> +      The controller supports multiple channels and each is represented as a
> > >>> +      child node.  Each child node supports the "status" property only, which
> > >>>        is used to enable/disable the respective channel.
> > >>>
> > >>> +    unevaluatedProperties: false
> > >>
> > >> There are no other properties within a channel, so this should be rather
> > >> additionalProperties: false.
> > >
> > > Are you sure? Then I also have to add:
> > >
> > >         properties:
> > >           status: true
> >
> > Do you? I think it would be first schema needing it, so maybe that would
> > be a problem for dtschema...
> 
> You think I haven't tried? ;-)
> 
>     arch/arm64/boot/dts/renesas/r8a774a1-beacon-rzg2m-kit.dtb:
> can@e66c0000: channel0: Additional properties are not allowed
> ('status' was unexpected)

I guess nodes with no properties aren't too common.

Now fixed in dtschema.

Rob

Re: [PATCH] dt-bindings: can: renesas,rcar-canfd: Fix number of channels for R-Car V3U

[Rob Herring]

On Fri, Dec 02, 2022 at 09:22:11AM +0100, Geert Uytterhoeven wrote:
> According to the bindings, only two channels are supported.
> However, R-Car V3U supports eight, leading to "make dtbs" failures:
> 
>         arch/arm64/boot/dts/renesas/r8a779a0-falcon.dtb: can@e6660000: Unevaluated properties are not allowed ('channel2', 'channel3', 'channel4', 'channel5', 'channel6', 'channel7' were unexpected)
> 
> Update the number of channels to 8 on R-Car V3U.
> While at it, prevent adding more properties to the channel nodes, as
> they must contain no other properties than a status property.
> 
> Fixes: d6254d52d70de530 ("dt-bindings: can: renesas,rcar-canfd: Document r8a779a0 support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Is there a way to express this using positive logic (i.e. default to 2
> channels, extend to more where needed)? R-Car V3H_2 (which is not yet
> supported) has 3 channels.

I think you'd need an if/elif/elif/else construct which is doable, but 
not pretty.

> Or perhaps the check should be dropped completely?

I'm fine with that.

Rob

Re: [PATCH] dt-bindings: can: renesas,rcar-canfd: Fix number of channels for R-Car V3U

[Geert Uytterhoeven]

Hi Rob,

On Mon, Dec 5, 2022 at 9:39 PM Rob Herring <robh@kernel.org> wrote:
> On Fri, Dec 02, 2022 at 11:58:23AM +0100, Geert Uytterhoeven wrote:
> > On Fri, Dec 2, 2022 at 11:49 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> > > On 02/12/2022 10:25, Geert Uytterhoeven wrote:
> > > > On Fri, Dec 2, 2022 at 10:01 AM Krzysztof Kozlowski
> > > > <krzysztof.kozlowski@linaro.org> wrote:
> > > >> On 02/12/2022 09:22, Geert Uytterhoeven wrote:
> > > >>> According to the bindings, only two channels are supported.
> > > >>> However, R-Car V3U supports eight, leading to "make dtbs" failures:
> > > >>>
> > > >>>         arch/arm64/boot/dts/renesas/r8a779a0-falcon.dtb: can@e6660000: Unevaluated properties are not allowed ('channel2', 'channel3', 'channel4', 'channel5', 'channel6', 'channel7' were unexpected)
> > > >>>
> > > >>> Update the number of channels to 8 on R-Car V3U.
> > > >>> While at it, prevent adding more properties to the channel nodes, as
> > > >>> they must contain no other properties than a status property.
> > > >>>
> > > >>> Fixes: d6254d52d70de530 ("dt-bindings: can: renesas,rcar-canfd: Document r8a779a0 support")
> > > >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > >
> > > >>> --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> > > >>> +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> > > >
> > > >>>      description: Maximum frequency of the CANFD clock.
> > > >>>
> > > >>>  patternProperties:
> > > >>> -  "^channel[01]$":
> > > >>> +  "^channel[0-7]$":
> > > >>>      type: object
> > > >>>      description:
> > > >>> -      The controller supports two channels and each is represented as a child
> > > >>> -      node.  Each child node supports the "status" property only, which
> > > >>> +      The controller supports multiple channels and each is represented as a
> > > >>> +      child node.  Each child node supports the "status" property only, which
> > > >>>        is used to enable/disable the respective channel.
> > > >>>
> > > >>> +    unevaluatedProperties: false
> > > >>
> > > >> There are no other properties within a channel, so this should be rather
> > > >> additionalProperties: false.
> > > >
> > > > Are you sure? Then I also have to add:
> > > >
> > > >         properties:
> > > >           status: true
> > >
> > > Do you? I think it would be first schema needing it, so maybe that would
> > > be a problem for dtschema...
> >
> > You think I haven't tried? ;-)
> >
> >     arch/arm64/boot/dts/renesas/r8a774a1-beacon-rzg2m-kit.dtb:
> > can@e66c0000: channel0: Additional properties are not allowed
> > ('status' was unexpected)
>
> I guess nodes with no properties aren't too common.
>
> Now fixed in dtschema.

Thanks, confirmed.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds