On 20/03/2023 17:18, Sergio Paracuellos wrote: > Adds device tree binding documentation for clocks and resets in the > Mediatek MIPS and Ralink SOCs. This covers RT2880, RT3050, RT3052, RT3350, > RT3883, RT5350, MT7620, MT7628 and MT7688 SoCs. Use subject prefixes matching the subsystem (which you can get for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching). Subject: drop second/last, redundant "device tree binding documentation". The "dt-bindings" prefix is already stating that these are bindings. (BTW, that's the longest redundant component I ever saw) > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx> > --- > .../bindings/clock/mtmips-clock.yaml | 68 +++++++++++++++++++ > 1 file changed, 68 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/mtmips-clock.yaml > > diff --git a/Documentation/devicetree/bindings/clock/mtmips-clock.yaml b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml > new file mode 100644 > index 000000000000..c92969ce231d > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/mtmips-clock.yaml Filename matching compatible, so vendor prefix and device name (or family of names). > @@ -0,0 +1,68 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/mtmips-clock.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MTMIPS SoCs Clock One clock? Are you sure these describe exactly one clock? > + > +maintainers: > + - Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx> > + > +description: | > + MediaTek MIPS and Ralink SoCs have an XTAL from where the cpu clock is > + provided as well as derived clocks for the bus and the peripherals. > + > + Each clock is assigned an identifier and client nodes use this identifier > + to specify the clock which they consume. Drop useless or obvious pieces of description. Describe the hardware. > + > + The clocks are provided inside a system controller node. ??? > + > + This node is also a reset provider for all the peripherals. ??? Does it mean it is not only "Clock" but also reset controller? > + > +properties: > + compatible: > + items: > + - enum: > + - ralink,rt2880-sysc > + - ralink,rt3050-sysc > + - ralink,rt3052-sysc > + - ralink,rt3352-sysc > + - ralink,rt3883-sysc > + - ralink,rt5350-sysc > + - ralink,mt7620-sysc > + - ralink,mt7620a-sysc > + - ralink,mt7628-sysc > + - ralink,mt7688-sysc > + - ralink,rt2880-reset That's odd. rt2880 is sysc and reset? One device with two compatibles? Also, order these by name. > + - const: syscon > + > + reg: > + maxItems: 1 > + > + '#clock-cells': > + description: > + The first cell indicates the clock number. > + const: 1 > + > + '#reset-cells': > + description: > + The first cell indicates the reset bit within the register. > + const: 1 Wait, only rt2880-reset is reset controller? This is confusing. > + > +required: > + - compatible > + - reg > + - '#clock-cells' > + - '#reset-cells' > + > +additionalProperties: false > + > +examples: > + - | > + sysc: sysc@0 { Drop label. Node names should be generic, clock-controller or reset-controller or system-controller sometimes. https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "ralink,rt5350-sysc", "syscon"; > + reg = <0x0 0x100>; > + #clock-cells = <1>; > + #reset-cells = <1>; > + }; Best regards, Krzysztof
