| Message ID | 021037bf7e422fcc23700dd62d1174c8e46ac85d.1669969283.git.geert+renesas@glider.be |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | dt-bindings: can: renesas,rcar-canfd: Fix number of channels for R-Car V3U | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 2 warnings, 154 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
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
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
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
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
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
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
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
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
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(-)