Re: [PATCH v2 1/8] dt-bindings: clock: loongson2: Add Loongson 2K0300 compatible

[Yao Zi <ziyao@disroot.org>]

On Fri, Jun 27, 2025 at 11:52:41AM +0000, Yao Zi wrote:
> On Fri, Jun 27, 2025 at 10:03:53AM +0200, Krzysztof Kozlowski wrote:
> > On Tue, Jun 17, 2025 at 04:24:19PM +0000, Yao Zi wrote:
> > > Document the clock controller shipped in Loongson 2K0300 SoC, which
> > > generates various clock signals for SoC peripherals.
> > > 
> > > Differing from previous generations of SoCs, 2K0300 requires a 120MHz
> > > external clock input, and a separate dt-binding header is used for
> > > cleanness.
> > > 
> > > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > > ---
> > >  .../bindings/clock/loongson,ls2k-clk.yaml     | 26 ++++++---
> > >  MAINTAINERS                                   |  1 +
> > >  .../dt-bindings/clock/loongson,ls2k0300-clk.h | 54 +++++++++++++++++++
> > >  3 files changed, 75 insertions(+), 6 deletions(-)
> > >  create mode 100644 include/dt-bindings/clock/loongson,ls2k0300-clk.h
> > > 
> > > diff --git a/Documentation/devicetree/bindings/clock/loongson,ls2k-clk.yaml b/Documentation/devicetree/bindings/clock/loongson,ls2k-clk.yaml
> > > index 4f79cdb417ab..3e0a894cfb2f 100644
> > > --- a/Documentation/devicetree/bindings/clock/loongson,ls2k-clk.yaml
> > > +++ b/Documentation/devicetree/bindings/clock/loongson,ls2k-clk.yaml
> > > @@ -16,6 +16,7 @@ description: |
> > >  properties:
> > >    compatible:
> > >      enum:
> > > +      - loongson,ls2k0300-clk
> > >        - loongson,ls2k0500-clk
> > >        - loongson,ls2k-clk  # This is for Loongson-2K1000
> > >        - loongson,ls2k2000-clk
> > > @@ -24,19 +25,32 @@ properties:
> > >      maxItems: 1
> > >  
> > >    clocks:
> > > -    items:
> > > -      - description: 100m ref
> > > +    maxItems: 1
> > >  
> > > -  clock-names:
> > > -    items:
> > > -      - const: ref_100m
> > > +  clock-names: true
> > 
> > No. How does this implement my comment?

Hi Krzysztof,

> I'm sorry that I forgot about the suggestion of dropping clock-names for
> the new compatible.
> 
> Is it acceptable to remove the description of clocks property, keep
> clock-names property as-is, and use an allOf block to disallow
> clocks-names for the new 2K0300 compatible? Thanks for your explanation.

Could you please provide some further comments on this? The schema will
look like,

	clocks:
	  maxItems: 1

	clock-names:
	  items:
	    - const: ref_100m

	...

	allOf:
	  - if:
	      properties:
	        compatible:
	          contains:
	            const: loongson,ls2k0300-clk
	    then:
	      properties:
	        clock-names: false
	    else:
	      required:
	        - clock-names

Thanks for your time and suggestion.

> > It makes no sense, why 100m even appeared here. I already objected last
> > time!
> > 
> > >  
> > >    '#clock-cells':
> > >      const: 1
> > >      description:
> > >        The clock consumer should specify the desired clock by having the clock
> > >        ID in its "clocks" phandle cell. See include/dt-bindings/clock/loongson,ls2k-clk.h
> > > -      for the full list of Loongson-2 SoC clock IDs.
> > > +      and include/dt-bindings/clock/loongson,ls2k0300-clk.h for the full list of
> > > +      Loongson-2 SoC clock IDs.
> > > +
> > > +allOf:
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            const: loongson,ls2k0300-clk
> > > +    then:
> > > +      properties:
> > > +        clock-names:
> > > +          const: ref_120m
> > 
> > NAK, stop doing this pattern. You already got comment on this.
> 
> Oops, I missed the comment about dropping the frequency (or the full
> clock-names property) from clock-names when writing v2, and I've decided
> to drop the clock-names property completely for the 2K0300 compatible.
> 
> Sorry again for my mistake.
> 
> > Best regards,
> > Krzysztof
> > 
> > 
> 
> Best regards,
> Yao Zi
> 

Regards,
Yao Zi