Re: [PATCH v3 1/3] dt-bindings: clock: spacemit: Add clock controllers of Spacemit K1 SoC

[Haylen Chu <heylenay@4d2.org>]

On Wed, Nov 27, 2024 at 09:03:11AM +0100, Krzysztof Kozlowski wrote:
> On Tue, Nov 26, 2024 at 03:48:33PM +0100, Krzysztof Kozlowski wrote:
> > On 26/11/2024 15:31, Haylen Chu wrote:
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - spacemit,k1-ccu-apbs
> > > +      - spacemit,k1-ccu-mpmu
> > > +      - spacemit,k1-ccu-apbc
> > > +      - spacemit,k1-ccu-apmu
> > > +
> > > +  clocks:
> > > +    maxItems: 4
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: osc_32k
> > 
> > osc
> > 
> > > +      - const: vctcxo_1m
> > > +      - const: vctcxo_3m
> > > +      - const: vctcxo_24m
> > > +
> > > +  spacemit,mpmu:
> > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > +    description:
> > > +      Phandle to the syscon managing "Main PMU (MPMU)" registers. It is used to
> > > +      check PLL lock status.
> > 
> > Why your example does not have it? Example code is supposed to be complete.
> 
> I think I understand why - this is for only one variant?

Yes, currently all implemented PLLs are located at APBS region, thus
this property is only meaningful in spacemit,k1-ccu-apbs.

> But then this
> should be disallowed in your binding for others. Currently your binding
> says that it is required for one and allowed for others.

Thanks for the correction. I'm considering moving the definition of
spacemit,mpmu to the if block as well. Is it the correct way to disallow
its usage in other variants?

> 
> Best regards,
> Krzysztof

Thanks,
Haylen Chu