Re: [PATCH net-next 18/37] dt-bindings: can: renesas,rcar-canfd: Restrict resets in top-level

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message says the patch will "disallow" `reset-names`
  for the two variants that do not use it, i.e. renesas,rcar-gen3-canfd…

--

dt-bindings: can: renesas,rcar-canfd: Restrict resets in top-level

This patch gives the top-level resets property its broadest limits
(minItems 1, maxItems 2) and drops the now redundant per-variant
maxItems: 2. It also adds reset-names: false to the R-Car Gen3/Gen4
conditional block.

Link: https://patch.msgid.link/20261001060825.53335-2-krzysztof.kozlowski@oss.qualcomm.com

> Since top-level part says "reset-names" cannot accept one item, disallow
> it for the two variants which do not use it in practice.

[Severity: Low]
Is this part of the commit message accurate? Before this patch, the
binding already disallowed reset-names for renesas,rcar-gen3-canfd and
renesas,rcar-gen4-canfd, in a later allOf entry:

  - if:
      properties:
        compatible:
          contains:
            enum:
              - renesas,rcar-gen3-canfd
              - renesas,rcar-gen4-canfd
    then:
      required:
        - resets
      properties:
        reset-names: false

That block came from commit af6b427c7ad09 ("dt-bindings: can:
renesas,rcar-canfd: Specify reset-names") and commit 1e3710b1fe915
("dt-bindings: can: renesas,rcar-canfd: Document RZ/T2H and RZ/N2H
SoCs").

> diff --git a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> index bbaaef8f4282..1a7a9ce45e97 100644
> --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml

[ ... ]

> @@ -231,6 +232,8 @@ allOf:
>          resets:
>            maxItems: 1
>
> +        reset-names: false
> +

With this line, two allOf entries now have the same if condition (contains
enum renesas,rcar-gen3-canfd / renesas,rcar-gen4-canfd), and both set
reset-names: false. The schema accepts and rejects the same nodes as
before.

Could the duplicate be dropped, or the two Gen3/Gen4 blocks be merged into
one? Could the commit message also say that the only real change is the
top-level resets bounds? The duplicate is still there at the end of the
series.

>    - if:
>        properties:
>          compatible:

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261015015336.816543-2-mkl%40pengutronix.de