On Mon, Jan 29, 2018 at 3:14 PM, David Lechner <david@xxxxxxxxxxxxxx> wrote: > On 01/29/2018 01:53 PM, Rob Herring wrote: >> >> On Sat, Jan 20, 2018 at 11:13:40AM -0600, David Lechner wrote: >>> >>> This adds a new binding for the PLL IP blocks in the mach-davinci >>> family of processors. Currently, only da850 has device tree support >>> but these bindings can also work for other SoCs in this family just >>> by adding new compatible strings. >>> >>> Note: Although these PLL controllers are very similar to the TI Keystone >>> SoCs, we are not re-using those bindings. The Keystone bindings use a >>> legacy one-node-per-clock binding. Furthermore, the mach-davinici SoCs >>> have a slightly different PLL register layout and a number of quirks >>> that can't be handled by the existing bindings, so the keystone bindings >>> could not be used as-is anyway. >>> >>> Signed-off-by: David Lechner <david@xxxxxxxxxxxxxx> >>> --- >>> >>> v6 changes: >>> - Added clock-names property >>> - Added ti,clkmode-square-wave property >>> - Added pllout child node >>> - Added obsclk child node >>> - Expanded examples >>> >>> .../devicetree/bindings/clock/ti/davinci/pll.txt | 96 >>> ++++++++++++++++++++++ >>> 1 file changed, 96 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/clock/ti/davinci/pll.txt >>> >>> diff --git a/Documentation/devicetree/bindings/clock/ti/davinci/pll.txt >>> b/Documentation/devicetree/bindings/clock/ti/davinci/pll.txt >>> new file mode 100644 >>> index 0000000..36998e1 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/clock/ti/davinci/pll.txt >>> @@ -0,0 +1,96 @@ >>> +Binding for TI DaVinci PLL Controllers >>> + >>> +The PLL provides clocks to most of the components on the SoC. In >>> addition >>> +to the PLL itself, this controller also contains bypasses, gates, >>> dividers, >>> +an multiplexers for various clock signals. >>> + >>> +Required properties: >>> +- compatible: shall be one of: >>> + - "ti,da850-pll0" for PLL0 on DA850/OMAP-L138/AM18XX >>> + - "ti,da850-pll1" for PLL1 on DA850/OMAP-L138/AM18XX >>> +- reg: physical base address and size of the controller's register area. >>> +- clocks: phandles corresponding to the clock names >>> +- clock-names: names of the clock sources - depends on compatible string >>> + - for "ti,da850-pll0", shall be "clksrc", "extclksrc" >>> + - for "ti,da850-pll1", shall be "clksrc" >>> + >>> +Optional properties: >>> +- ti,clkmode-square-wave: Indicates that the the board is supplying a >>> square >>> + wave input on the OSCIN pin instead of using a crystal >>> oscillator. >>> + This property is only valid when compatible = "ti,da850-pll0". >>> + >>> + >>> +Optional child nodes: >>> + >>> +pllout >>> + Describes the main PLL clock output (before POSTDIV). The node >>> name must >>> + be "pllout". >>> + >>> + Required properties: >>> + - #clock-cells: shall be 0 >>> + >>> +sysclk >>> + Describes the PLLDIVn divider clocks that provide the SYSCLKn >>> clock >>> + domains. The node name must be "sysclk". Consumers of this node >>> should >>> + use "n" in "SYSCLKn" as the index parameter for the clock cell. >>> + >>> + Required properties: >>> + - #clock-cells: shall be 1 >>> + >>> +auxclk >>> + Describes the AUXCLK output of the PLL. The node name must be >>> "auxclk". >>> + This child node is only valid when compatible = "ti,da850-pll0". >>> + >>> + Required properties: >>> + - #clock-cells: shall be 0 >>> + >>> +obsclk >>> + Describes the OBSCLK output of the PLL. The node name must be >>> "obsclk". >>> + >>> + Required properties: >>> + - #clock-cells: shall be 0 >> >> >> So why have all these child nodes vs. just defining a single number >> space of clock ids? >> >> Rob >> > > I think that it makes the bindings more self-documenting. Not all PLLs have > all of possible types of output clocks, so the presence or absence of a > child node indicates if a PLL actually has that output or not. Doesn't the compatible string do that? > It is also complicated by the fact that one of the child nodes (sysclk) > is already an array of clocks. > > To do what you are suggesting might look something like this... > > --- > > Required properties: > - compatible: shall be one of: > - "ti,da850-pll0" for PLL0 on DA850/OMAP-L138/AM18XX > - "ti,da850-pll1" for PLL1 on DA850/OMAP-L138/AM18XX > - reg: physical base address and size of the controller's register area. > - clocks: phandles corresponding to the clock names > - clock-names: names of the clock sources - depends on compatible string > - for "ti,da850-pll0", shall be "clksrc", "extclksrc" > - for "ti,da850-pll1", shall be "clksrc" > - #clock-cells: shall be set to <2>. > > Consumers: > > The clock cell values for consumers work as follows... > > The first index is one of the constants defined in ti-davinci-pll.h > > The second index is 0 unless the first index is TI_DAVINCI_SYSCLK. In the > case > of TI_DAVINCI_SYSCLK the second index the SYSCLK domain ID (n in SYSCLKn). > > For compatible = "ti,da850-pll0": > - <&pll0 TI_DAVINCI_PLLOUT 0> is the PLLOUT clock > - <&pll0 TI_DAVINCI_SYSCLK n> is one of the SYSCLKn clock domains > where n is 1 to 7 > - <&pll0 TI_DAVINCI_AUXCLK 0> is the AUXCLK clock domain > - <&pll0 TI_DAVINCI_OBSCLK 0> is the OBSCLK clock domain > - all other index combinations are invalid > > For compatible = "ti,da850-pll1": > - <&pll0 TI_DAVINCI_SYSCLK n> is one of the SYSCLKn clock domains > where n is 1 to 3 > - <&pll0 TI_DAVINCI_OBSCLK 0> is the OBSCLK clock domain > - all other index combinations are invalid You don't really need 2 cells here. I guess if you want to keep the child nodes, that is fine. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
