Hello Stephen, > -----Original Message----- > From: Stephen Boyd <sboyd@xxxxxxxxxx> > Sent: Tuesday, September 15, 2020 5:37 AM > To: Sagar Kadam <sagar.kadam@xxxxxxxxxxxx>; > devicetree@xxxxxxxxxxxxxxx; linux-clk@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; linux-pwm@xxxxxxxxxxxxxxx; linux- > riscv@xxxxxxxxxxxxxxxxxxx > Cc: mturquette@xxxxxxxxxxxx; robh+dt@xxxxxxxxxx; Paul Walmsley ( Sifive) > <paul.walmsley@xxxxxxxxxx>; palmer@xxxxxxxxxxx; tglx@xxxxxxxxxxxxx; > jason@xxxxxxxxxxxxxx; maz@xxxxxxxxxx; thierry.reding@xxxxxxxxx; > u.kleine-koenig@xxxxxxxxxxxxxx; lee.jones@xxxxxxxxxx; > aou@xxxxxxxxxxxxxxxxx; Yash Shah <yash.shah@xxxxxxxxxxxx>; Sagar > Kadam <sagar.kadam@xxxxxxxxxxxx> > Subject: Re: [PATCH v1 1/3] dt-bindings: fu540: prci: convert PRCI bindings > to json-schema > > [External Email] Do not click links or attachments unless you recognize the > sender and know the content is safe > > Quoting Sagar Kadam (2020-09-10 03:44:02) > > diff --git > > a/Documentation/devicetree/bindings/clock/sifive/fu540-prci.yaml > > b/Documentation/devicetree/bindings/clock/sifive/fu540-prci.yaml > > new file mode 100644 > > index 0000000..49386cd > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/clock/sifive/fu540-prci.yaml > > @@ -0,0 +1,75 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) # Copyright > > +(C) 2020 SiFive, Inc. > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/clock/sifive/fu540-prci.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: SiFive FU540 Power Reset Clock Interrupt Controller (PRCI) > > + > > +maintainers: > > + - Sagar Kadam <sagar.kadam@xxxxxxxxxx> > > + - Paul Walmsley <paul.walmsley@xxxxxxxxxx> > > + > > +description: > > + On the FU540 family of SoCs, most system-wide clock and reset > > +integration > > + is via the PRCI IP block. > > + The clock consumer should specify the desired clock via the clock > > +ID > > + macros defined in include/dt-bindings/clock/sifive-fu540-prci.h. > > + These macros begin with PRCI_CLK_. > > + > > + The hfclk and rtcclk nodes are required, and represent physical > > + crystals or resonators located on the PCB. These nodes should be > > + present underneath /, rather than /soc. > > + > > +properties: > > + compatible: > > + enum: > > + - sifive,fu540-c000-prci > > + description: > > + Should have "sifive,<soc>-prci", only one value is supported > > Drop description and have > > compatible: > const: sifive,fu540-c000-prci > Thank you for suggestion here, I will remove this. > > + > > + reg: > > + maxItems: 1 > > + description: Describe the PRCI's register target physical address > > + region > > Drop description. > Okay. > > + > > + clocks: > > + description: > > + Should point to the hfclk device tree node and the rtcclk device tree > node. > > s/device tree node//g Okay, will remove these. > > > + The RTC clock here is not a time-of-day clock, but is instead a high- > stability > > + clock source for system timers and cycle counters. > > Better to have: > > clocks: > items: > - const: high frequency clock > - const: RTC clock > > Can you add clock-names too? Making it optional is OK. Okay, I will include these optional properties as -const: "hfclk" -const: "rtcclk" > > > + "#clock-cells": > > + const: 1 > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - "#clock-cells" > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + //hfclk and rtcclk present under /, in PCB-specific DT data > > + hfclk: hfclk { > > + #clock-cells = <0>; > > + compatible = "fixed-clock"; > > + clock-frequency = <33333333>; > > + clock-output-names = "hfclk"; > > + }; > > Add a newline here? > Okay. > > + rtcclk: rtcclk { > > + #clock-cells = <0>; > > + compatible = "fixed-clock"; > > + clock-frequency = <1000000>; > > + clock-output-names = "rtcclk"; > > + }; > > These may not be necessary either, just have the clock-controller node > reference phandles? > Okay. > > + > > + //under /soc, in SoC-specific DT data > > Don't think this comment is necessary. > Okay. Thanks & BR, Sagar > > + prci: clock-controller@10000000 { > > + compatible = "sifive,fu540-c000-prci"; > > + reg = <0x10000000 0x1000>; > > + clocks = <&hfclk>, <&rtcclk>; > > + #clock-cells = <1>; > > + };