Re: [PATCH 1/8] dt-bindings: clock: Document Loongson 2K0300 clock controller

[Yao Zi <ziyao@disroot.org>]

On Mon, May 26, 2025 at 06:11:22AM +0200, Krzysztof Kozlowski wrote:
> On 23/05/2025 15:28, Yao Zi wrote:
> > On Fri, May 23, 2025 at 08:30:57PM +0800, Binbin Zhou wrote:
> >> On Fri, May 23, 2025 at 6:46 PM Yao Zi <ziyao@disroot.org> wrote:
> >>>
> >>> Document the clock controller shipped in Loongson 2K0300 SoC, which
> >>> generates various clock signals for SoC peripherals.
> >>>
> >>> Signed-off-by: Yao Zi <ziyao@disroot.org>
> >>> ---
> >>>  .../bindings/clock/loongson,ls2k0300-clk.yaml | 52 ++++++++++++++++++
> >>>  .../dt-bindings/clock/loongson,ls2k0300-clk.h | 54 +++++++++++++++++++
> >>>  2 files changed, 106 insertions(+)
> >>>  create mode 100644 Documentation/devicetree/bindings/clock/loongson,ls2k0300-clk.yaml
> >>
> >> I don't think a new binding file for 2K0300 is needed. Adding
> >> compatible entries to loongson,ls2k-clk.yaml would be more appropriate
> >> as they are almost all similar.
> > 
> > Originally I've tried to integrate the 2K0300 stuff with
> > loongson,ls2k-clk.yaml, but found it's hard to describe some properties.
> > 
> > For example, currently in loongson,ls2k-clk.yaml, the clocks property is
> > described as
> > 
> >   clocks:
> >     items:
> >       - description: 100m ref
> > 
> > what should the description look like with 2K0300 introduced, whose
> > reference clock runs at 120MHz instead of 100MHz? It'll be hard to
> > describe things correctly without losing existing information. "120MHz
> > reference clock for Loongson 2K0300, or 100MHz reference clock for other
> > SoCs" sounds even a worse idea.
> 
> Drop the frequency anyway, it is kind of pointless here.
> 
> > 
> > Another example is about the description of clock IDs. loongson,ls2k-clk.yaml
> > describes available clock IDs as
> > 
> >   '#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.
> > 
> > what should the description look like if we add 2K0300 support? With a
> > different header being introduced, the description will be messy.
> 
> No, just list the headers.
> 
> > 
> > I think keeping SoCs peripherals that are different in hardware design
> > in the same binding is really a bad idea. Yes, these clock controllers
> > are similar enough to reuse the clock hardware driver, but they have
> > different clock tree structures and register definitions, making them
> > essentially different things. Trying to keep everything in the same
> > place only makes the binding messy.
> 
> How is binding messy if you only add one compatible?
> 
> 
> Best regards,
> Krzysztof

Okay, thanks for the suggestions. Will merge 2K0300-related stuff into
loongson,ls2k-clk.yaml in the next version.

Thanks,
Yao Zi