On 13/01/2023 12:07, Keguang Zhang wrote: > Add devicetree binding document for the Loongson-1 clock driver. Subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. Subject: Drop driver, not related to hardware. > > Signed-off-by: Keguang Zhang <keguang.zhang@xxxxxxxxx> > --- > .../bindings/clock/loongson,ls1x-clk.yaml | 81 +++++++++++++++++++ > 1 file changed, 81 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/loongson,ls1x-clk.yaml > > diff --git a/Documentation/devicetree/bindings/clock/loongson,ls1x-clk.yaml b/Documentation/devicetree/bindings/clock/loongson,ls1x-clk.yaml > new file mode 100644 > index 000000000000..4709c6757f1e > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/loongson,ls1x-clk.yaml > @@ -0,0 +1,81 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/loongson,ls1x-clk.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Loongson-1 Clock Controller Wasn't this already sent? https://lore.kernel.org/all/20190130194731.GA25716@bogus/ Then this is a v4? Aren't you duplicating efforts (and reviewers efforts)? > + > +maintainers: > + - Keguang Zhang <keguang.zhang@xxxxxxxxx> > + > +properties: compatible is a first property. > + "#clock-cells": > + const: 0 > + > + compatible: > + enum: > + - loongson,ls1b-clk-pll > + - loongson,ls1b-clk-cpu > + - loongson,ls1b-clk-ahb > + - loongson,ls1c-clk-pll > + - loongson,ls1c-clk-cpu > + - loongson,ls1c-clk-ahb Are you registering single clocks? It looks like. No, make a proper clock controller. > + > + reg: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > +required: > + - "#clock-cells" > + - compatible > + - clocks > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + clocks { No, not really related to the binding. > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + xtal: xtal { Incorrect in this context. Missing unit address. > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <33000000>; > + }; > + > + pll: pll@1fe78030 { Node names should be generic, so "clock-controller" https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "loongson,ls1b-clk-pll"; > + #clock-cells = <0>; > + clocks = <&xtal>; > + reg = <0x1fe78030 0x4>; compatible is first property, then reg, then the rest. > + }; > + > + cpu_clk: cpu_clk@1fe78034 { No underscores in node names. Anyway this should be gone - make a proper clock controller. Best regards, Krzysztof
