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

[Yao Zi]

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@xxxxxxxxxxx>
> > ---
> >  .../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?

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.

> 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