| Message ID | 1516468460-4908-2-git-send-email-david@lechnology.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@lechnology.com> > --- > > 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
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@lechnology.com> >> --- >> >> 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. 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 --- I guess it is not too bad (writing a clock lookup function that knows all of these rules could get interesting though).
On Mon, Jan 29, 2018 at 3:14 PM, David Lechner <david@lechnology.com> 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@lechnology.com> >>> --- >>> >>> 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
On 01/30/2018 08:50 AM, Rob Herring wrote: > On Mon, Jan 29, 2018 at 3:14 PM, David Lechner <david@lechnology.com> 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@lechnology.com> >>>> --- >>>> >>>> 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? Sure. > >> 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. OK, I can see how it could work with one cell. Since this is already implemented and working, I'm inclined to leave it as-is if it is "good enough". But, I am fine going either way if there are other opinions on the matter.
On Wednesday 31 January 2018 12:16 AM, David Lechner wrote: > On 01/30/2018 08:50 AM, Rob Herring wrote: >> On Mon, Jan 29, 2018 at 3:14 PM, David Lechner <david@lechnology.com> >> 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@lechnology.com> >>>>> --- >>>>> >>>>> 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? > > Sure. > >> >>> 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. > > OK, I can see how it could work with one cell. > > Since this is already implemented and working, I'm inclined to leave it > as-is if it is "good enough". But, I am fine going either way if there > are other opinions on the matter. FWIW, I think the current binding is fine too. The clocks are of different kind (fixed clock, fixed divider, flexible divider etc). So, its slightly better to have child nodes, I guess. Thanks, Sekhar
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 + + +Examples: + + pll0: clock-controller@11000 { + compatible = "ti,da850-pll0"; + reg = <0x11000 0x1000>; + clocks = <&ref_clk>, <&pll1_sysclk 3>; + clock-names = "clksrc", "extclksrc"; + ti,clkmode-square-wave; + + pll0_pllout: pllout { + #clock-cells = <0>; + }; + + pll0_sysclk: sysclk { + #clock-cells = <1>; + }; + + pll0_auxclk: auxclk { + #clock-cells = <0>; + }; + + pll0_obsclk: obsclk { + #clock-cells = <0>; + }; + }; + + pll1: clock-controller@21a000 { + compatible = "ti,da850-pll1"; + reg = <0x21a000 0x1000>; + clocks = <&ref_clk>; + clock-names = "clksrc"; + + pll0_sysclk: sysclk { + #clock-cells = <1>; + }; + + pll0_obsclk: obsclk { + #clock-cells = <0>; + }; + }; + +Also see: +- Documentation/devicetree/bindings/clock/clock-bindings.txt
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@lechnology.com> --- 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