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: 3064 bytes --]

On Wed, May 27, 2026 at 02:44:51PM +0200, Geert Uytterhoeven wrote:
> Hi Conor,
> 
> On Mon, 25 May 2026 at 18:57, Conor Dooley <conor@kernel.org> wrote:
> > 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.
> 
> Calling them optional may be a misnomer: these pins are present.
> Typically we provide zero rate fixed clocks for such clocks in the
> SoC .dtsi, to be filled in or replaced by the board .dts.

IOW, either both clocks will be provided or none will be?

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