Re: [PATCH net-next 01/12] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC

[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] In this patch's tree state, the binding documents
  `renesas,r9a08g045-canfd` as a standalone compatible with no fallback,…

Pre-existing issues:
- [Low] The concern says the RZ/G3S compatible `renesas,r9a08g045-canfd`
  gets the unchanged top-level `clocks`/`clock-names` schema, and that…

--

dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC

This adds the renesas,r9a08g045-canfd compatible for the RZ/G3S CAN FD
controller and adds it to the existing allOf conditional blocks. It also
moves the assigned-clocks/assigned-clock-rates requirement into a new
conditional that exempts RZ/G3S, and disallows renesas,no-can-fd and
renesas,fd-only for RZ/G3S.

Link: https://patch.msgid.link/20260902141544.565763-2-claudiu.beznea+renesas@tuxon.dev

> diff --git a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> index b9d9dd7a79676..bbaaef8f4282e 100644
> --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> @@ -13,6 +13,7 @@ properties:
>    compatible:
>      oneOf:
>        - enum:
> +          - renesas,r9a08g045-canfd        # RZ/G3S
>            - renesas,r9a09g047-canfd        # RZ/G3E
>            - renesas,r9a09g077-canfd        # RZ/T2H

[Severity: Low]
At this point in the series, does anything bind to this compatible?

The patch adds it as a standalone string with no fallback. However,
rcar_canfd_of_table[] in drivers/net/can/rcar/rcar_canfd.c has no
renesas,r9a08g045-canfd entry yet. An RZ/G3S node that passes validation
would therefore not probe.

A later patch in the series, "can: rcar_canfd: Add support for Renesas
RZ/G3S", appears to fix this by adding:

    { .compatible = "renesas,r9a08g045-canfd", .data = &r9a08g045_hw_info },

[ ... ]

> @@ -350,6 +352,20 @@ allOf:
>        properties:
>          reset-names: false
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,r9a08g045-canfd
> +    then:
> +      properties:
> +        renesas,no-can-fd: false
> +        renesas,fd-only: false
> +    else:
> +      required:
> +        - assigned-clocks
> +        - assigned-clock-rates

[Severity: Low]
This is a pre-existing issue, but should this block also narrow
clock-names for RZ/G3S? The top-level clock-names still accepts both
forms:

  clock-names:
    oneOf:
      - items:
          - const: fck
          - const: canfd
          - const: can_clk
      - items:
          - const: fck
          - const: ram_clk
          - const: can_clk

No allOf entry picks one of these forms per compatible.

The RZ/G3S CPG only supplies the peripheral and RAM clocks. Even so, an
RZ/G3S node that uses the canfd form would still pass dt_binding_check.
The driver would then get a NULL clk_ram from devm_clk_get_optional(),
and the RAM clock would never be enabled.

The renesas,rzg2l-canfd and renesas,r9a09g047-canfd compatibles already
have the same gap.

Requiring the third can_clk entry looks fine, though. The internal
divider setup can be described with a 0 Hz can_clk fixed-clock, as in
r9a07g044.dtsi. The fck fallback that "can: rcar_canfd: Allow the CAN FD
clock to be sourced from fck" adds to rcar_canfd_probe() then handles
that case.

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