Re: [PATCH v5 01/11] 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] The `clocks` property specifies an `items` array with a rigid positional sequence that contradicts the flexible order allowed by `clock-names` and demonstrated in the example.
--

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

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

The FSI on r8a7740 requires the SPU bus/bridge clock to be enabled before
accessing its registers. This update adds multiple clocks to properly
describe the hardware clock tree.

> diff --git a/Documentation/devicetree/bindings/sound/renesas,fsi.yaml b/Documentation/devicetree/bindings/sound/renesas,fsi.yaml
> index df91991699a72..b966b55ff7728 100644
> --- 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
> +

[Severity: Low]
Does this rigid items array cause a regression with dt-schema validation? 

Defining the items as a list of descriptions enforces strict positional
indexing, meaning the schema expects index 3 to specifically be 'CPG DIV6
functional clocks for FSI port B'. This contradicts the flexible enum defined
below in clock-names.

[ ... ]
> @@ -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]
Is this enum-based list in conflict with the strict positional sequence
in the clocks array?

The clock-names property permits these optional clocks to appear
in any arbitrary order, whereas the clocks descriptions require a fixed
sequence.

>  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]
Does this example directly violate the strict positional sequence defined
above in the clocks array? 

The example provides the clocks in an order (fck, spu, icka, xcka, diva...)
that doesn't match the fixed list, which expects ickb in the 4th position.
This leads to semantically incorrect documentation.

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