Re: [PATCH v4 01/10] ASoC: dt-bindings: renesas,fsi: add support multiple clocks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Contradiction between the strict positional order defined in the `clocks` index descriptions and the flexible ordering permitted by `clock-names`.
--

commit f8c8566ea84f655cc7929bfa8c21e1ba2f97abe4
Author: bui duc phuc <phucduc.bui@gmail.com>

ASoC: dt-bindings: renesas,fsi: add support multiple clocks

This commit updates the Renesas FSI binding to support multiple clocks to
properly describe the hardware clock tree. It includes the SPU bus/bridge
clock, CPG DIV6 functional clocks, FSI dividers, and external clock inputs.

> diff --git a/Documentation/devicetree/bindings/sound/renesas,fsi.yaml b/Documentation/devicetree/bindings/sound/renesas,fsi.yaml
> --- a/Documentation/devicetree/bindings/sound/renesas,fsi.yaml
> +++ b/Documentation/devicetree/bindings/sound/renesas,fsi.yaml
> @@ -38,7 +35,32 @@ properties:
>      maxItems: 1
>  
>    clocks:
> -    maxItems: 1
> +    minItems: 1
> +    items:
> +      - description: Main FSI module clock
> +      - description: |
> +          SPU bus/bridge clock. On R8A7740, this clock must be enabled to allow
> +          register access as the FSI block is connected behind the SPU bus.
> +      - description: CPG DIV6 functional clocks for FSI port A
> +      - description: CPG DIV6 functional clocks for FSI port B
> +      - description: FSI dividers for port A used for audio clock generation
> +      - description: FSI dividers for port B used for audio clock generation
> +      - description: External clock inputs for FSI port A provided by the board
> +      - description: External clock inputs for FSI port B provided by the board

[ ... ]

> @@ -69,6 +91,31 @@ required:
>  
>  unevaluatedProperties: false
>  
> +allOf:
> +  - $ref: dai-common.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,fsi2-r8a7740
> +    then:
> +      required:
> +        - clock-names
> +
> +      properties:
> +        clock-names:
> +          minItems: 2
> +          uniqueItems: true
> +          items:
> +            - const: fck
> +            - const: spu
> +            - enum: [icka, ickb, diva, divb, xcka, xckb]
> +            - enum: [icka, ickb, diva, divb, xcka, xckb]
> +            - enum: [icka, ickb, diva, divb, xcka, xckb]
> +            - enum: [icka, ickb, diva, divb, xcka, xckb]
> +            - enum: [icka, ickb, diva, divb, xcka, xckb]
> +            - enum: [icka, ickb, diva, divb, xcka, xckb]

[Severity: Low]
Does this flexible enum ordering conflict with the strict positional sequence
defined in the clocks property description above?

The clocks property specifies an items array with 8 distinct descriptions
that enforces a strict positional mapping (e.g., the fourth slot is the
functional clock for port B). However, this clock-names constraint permits
these optional clocks to be specified in an arbitrary order.

If a board omits an optional clock but provides subsequent ones, will
dt-schema incorrectly map the provided clock to the wrong description since
DT arrays do not support skipping elements?

>  examples:
>    - |
>      #include <dt-bindings/clock/r8a7740-clock.h>
> @@ -77,7 +124,11 @@ examples:
>              compatible = "renesas,fsi2-r8a7740", "renesas,sh_fsi2";
>              reg = <0xfe1f0000 0x400>;
>              interrupts = <GIC_SPI 9 0x4>;
> -            clocks = <&mstp3_clks R8A7740_CLK_FSI>;
> +            clocks = <&mstp3_clks R8A7740_CLK_FSI>, <&spu_clk>,
> +                    <&fsia_clk>, <&fsiack_clk>, <&fsidiva_clk>,
> +                    <&fsib_clk>, <&fsibck_clk>, <&fsidivb_clk>;
> +            clock-names = "fck", "spu", "icka", "xcka", "diva",
> +                         "ickb", "xckb", "divb";

[Severity: Low]
Can this example be updated to follow the positional sequence defined in the
clocks property?

The clocks definition reserves the fourth position (index 3) for the CPG DIV6
functional clock for FSI port B ("ickb"), but the example places the external
clock input "xcka" at that position instead.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605121955.105661-1-phucduc.bui@gmail.com?part=1