Re: [PATCH v4 1/8] dt-bindings: clock: renesas: Add audio clock inputs for RZ/V2H family

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2427 bytes --]

On Mon, May 25, 2026 at 11:19:19AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Strict array ordering prevents the independent use of newly added optional clocks.
> --
> 
> commit 3723d06ba80fe72a3e7492af52e5e54aa0f36b03
> Author: John Madieu <john.madieu.xa@bp.renesas.com>
> 
> dt-bindings: clock: renesas: Add audio clock inputs for RZ/V2H family
> 
> This commit updates the clock bindings for the RZ/V2H family SoCs to include
> the optional external audio clock inputs AUDIO_CLKB and AUDIO_CLKC. The
> AUDIO_CLKA input is fed internally and thus does not require a binding entry.
> 
> > diff --git a/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml b/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml
> > --- a/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml
> > +++ b/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml
> > @@ -26,16 +26,22 @@ properties:
> >      maxItems: 1
> >  
> >    clocks:
> > +    minItems: 3
> >      items:
> >        - description: AUDIO_EXTAL clock input
> >        - description: RTXIN clock input
> >        - description: QEXTAL clock input
> > +      - description: AUDIO_CLKB clock input
> > +      - description: AUDIO_CLKC clock input
> >  
> >    clock-names:
> > +    minItems: 3
> >      items:
> >        - const: audio_extal
> >        - const: rtxin
> >        - const: qextal
> > +      - const: audio_clkb
> > +      - const: audio_clkc
> 
> [Severity: Medium]
> Will this strict array ordering prevent the independent use of the newly
> added optional clocks?
> 
> Because JSON schema validates arrays strictly by position based on the items
> list, if a hardware board connects AUDIO_CLKC but does not connect AUDIO_CLKB,
> the device tree node would provide an array of 4 clocks where the 4th name
> is audio_clkc.
> 
> When dt_binding_check evaluates this, won't it check the 4th element against
> the schema's expectation of audio_clkb and fail, incorrectly rejecting a valid
> hardware configuration?
> 
> Should this use an anyOf block enumerating the valid permutations of optional
> clocks to avoid forcing users to write incorrect device trees to pass
> validation?

Seems like a valid question to me. Can be solved by making the second
last clock an enum.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]