Re: [PATCHv5 02/31] CLK: TI: Add DPLL clock support
On 08/20/2013 01:00 AM, Mike Turquette wrote: Quoting Tero Kristo (2013-08-19 10:06:39) On 08/19/2013 07:24 PM, Mark Rutland wrote: On Mon, Aug 19, 2013 at 04:09:37PM +0100, Tero Kristo wrote: On 08/19/2013 05:18 PM, Mark Rutland wrote: On Mon, Aug 19, 2013 at 02:34:45PM +0100, Tero Kristo wrote: On 08/13/2013 01:50 PM, Mark Rutland wrote: Hi, On Fri, Aug 02, 2013 at 05:25:21PM +0100, Tero Kristo wrote: The OMAP clock driver now supports DPLL clock type. This patch also adds support for DT DPLL nodes. Signed-off-by: Tero Kristo t-kri...@ti.com --- .../devicetree/bindings/clock/ti/dpll.txt | 70 +++ arch/arm/mach-omap2/clock.h| 144 +- arch/arm/mach-omap2/clock3xxx.h|2 - drivers/clk/Makefile |1 + drivers/clk/ti/Makefile|3 + drivers/clk/ti/dpll.c | 488 include/linux/clk/ti.h | 164 +++ 7 files changed, 727 insertions(+), 145 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/ti/dpll.txt create mode 100644 drivers/clk/ti/Makefile create mode 100644 drivers/clk/ti/dpll.c create mode 100644 include/linux/clk/ti.h diff --git a/Documentation/devicetree/bindings/clock/ti/dpll.txt b/Documentation/devicetree/bindings/clock/ti/dpll.txt new file mode 100644 index 000..dcd6e63 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/ti/dpll.txt @@ -0,0 +1,70 @@ +Binding for Texas Instruments DPLL clock. + +This binding uses the common clock binding[1]. It assumes a +register-mapped DPLL with usually two selectable input clocks +(reference clock and bypass clock), with digital phase locked +loop logic for multiplying the input clock to a desired output +clock. This clock also typically supports different operation +modes (locked, low power stop etc.) This binding has several +sub-types, which effectively result in slightly different setup +for the actual DPLL clock. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be one of: + ti,omap4-dpll-x2-clock, + ti,omap3-dpll-clock, + ti,omap3-dpll-core-clock, + ti,omap3-dpll-per-clock, + ti,omap3-dpll-per-j-type-clock, + ti,omap4-dpll-clock, + ti,omap4-dpll-core-clock, + ti,omap4-dpll-m4xen-clock, + ti,omap4-dpll-j-type-clock, + ti,omap4-dpll-no-gate-clock, + ti,omap4-dpll-no-gate-j-type-clock, + +- #clock-cells : from common clock binding; shall be set to 0. +- clocks : link phandles of parent clocks (clk-ref and clk-bypass) It might be a good idea to use clock-names to clarify which clocks are present (I notice your examples contain only one clock input). All DPLLs have both bypass and reference clocks, but these can be the same. Is it better to just list both always (and hence drop clock-names), or provide clock-names always? If they're separate inputs, it's worth listing both (even if they're fed by the same provider). If it's possible one input might not be wired up, use clock-names. Ref and bypass clocks are used currently by all DPLLs (also the APLL) so I guess I just enforce both to be specified. Ok. It's always possible to add clock-names later if a platform doesn't wire an input. We lose the possibility of future compatibility, but backwards compatibility is easy enough to maintain. +- ti,clkdm-name : clockdomain name for the DPLL Could you elaborate on what this is for? What constitutes a valid name? I'm not sure a string is the best way to define the linkage of several elements to a domain... Well, I am not sure if we can do this any better at this point, we don't have DT nodes for clockdomain at the moment (I am not sure if we will have those either as there are only a handful of those per SoC) but I'll add some more documentation for this. I'll have to see the documentation, but I'd be very wary of putting that in as-is. Does having the clock domain string link this up to domains in platform data? I'm not sure how well we'll be able to maintain support for that in future if it requires other platform code to use now, and we're not sure how the domains themselves will be represented in dt. Hmm so, should I add a stub representation for the clockdomains to the DT then for now or how should this be handled? The stubs could then be mapped to the actual clock domains based on their node names. I'm not sure that this binding should know about the clock domain at all. Maybe the clock domain binding should list the clocks that are within the domain? Ok, I experimented with this stuff a bit to have a proper reply, and it looks like I can get this done with a clockdomain mapping, which has its own binding. Something like
Re: [PATCHv5 02/31] CLK: TI: Add DPLL clock support
On 08/13/2013 01:50 PM, Mark Rutland wrote:
Hi,
On Fri, Aug 02, 2013 at 05:25:21PM +0100, Tero Kristo wrote:
The OMAP clock driver now supports DPLL clock type. This patch also
adds support for DT DPLL nodes.
Signed-off-by: Tero Kristo t-kri...@ti.com
---
.../devicetree/bindings/clock/ti/dpll.txt | 70 +++
arch/arm/mach-omap2/clock.h| 144 +-
arch/arm/mach-omap2/clock3xxx.h|2 -
drivers/clk/Makefile |1 +
drivers/clk/ti/Makefile|3 +
drivers/clk/ti/dpll.c | 488
include/linux/clk/ti.h | 164 +++
7 files changed, 727 insertions(+), 145 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/ti/dpll.txt
create mode 100644 drivers/clk/ti/Makefile
create mode 100644 drivers/clk/ti/dpll.c
create mode 100644 include/linux/clk/ti.h
diff --git a/Documentation/devicetree/bindings/clock/ti/dpll.txt
b/Documentation/devicetree/bindings/clock/ti/dpll.txt
new file mode 100644
index 000..dcd6e63
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/ti/dpll.txt
@@ -0,0 +1,70 @@
+Binding for Texas Instruments DPLL clock.
+
+This binding uses the common clock binding[1]. It assumes a
+register-mapped DPLL with usually two selectable input clocks
+(reference clock and bypass clock), with digital phase locked
+loop logic for multiplying the input clock to a desired output
+clock. This clock also typically supports different operation
+modes (locked, low power stop etc.) This binding has several
+sub-types, which effectively result in slightly different setup
+for the actual DPLL clock.
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : shall be one of:
+ ti,omap4-dpll-x2-clock,
+ ti,omap3-dpll-clock,
+ ti,omap3-dpll-core-clock,
+ ti,omap3-dpll-per-clock,
+ ti,omap3-dpll-per-j-type-clock,
+ ti,omap4-dpll-clock,
+ ti,omap4-dpll-core-clock,
+ ti,omap4-dpll-m4xen-clock,
+ ti,omap4-dpll-j-type-clock,
+ ti,omap4-dpll-no-gate-clock,
+ ti,omap4-dpll-no-gate-j-type-clock,
+
+- #clock-cells : from common clock binding; shall be set to 0.
+- clocks : link phandles of parent clocks (clk-ref and clk-bypass)
It might be a good idea to use clock-names to clarify which clocks are
present (I notice your examples contain only one clock input).
All DPLLs have both bypass and reference clocks, but these can be the
same. Is it better to just list both always (and hence drop
clock-names), or provide clock-names always?
+- reg : array of register base addresses for controlling the DPLL (some
+ of these can also be left as NULL):
+ reg[0] = control register
+ reg[1] = idle status register
+ reg[2] = autoidle register
+ reg[3] = multiplier / divider set register
Are all of these always present? Using reg-names seems sensible here.
Not always, I'll change the code. I am quite new to DT and didn't
actually know of the existence of reg-names prop.
+- ti,clk-ref : link phandle for the reference clock
+- ti,clk-bypass : link phandle for the bypass clock
You already have these in clocks, why do you need them again here?
Yea good point. Will fix based on the above.
+
+Optional properties:
+- ti,modes : available modes for the DPLL
Huh? What type is that property? What does it mean?
Will add some documentation to this.
+- ti,recal-en-bit : recalibration enable bit
+- ti,recal-st-bit : recalibration status bit
+- ti,auto-recal-bit : auto recalibration enable bit
These seem rather low-level, but I see other clock bindings are doing
similar things. When are these needed, and why? What type are they?
Bit shift values for the auto recalibration feature. HOWEVER, it seems
these are actually legacy, kernel does not have support for these
anymore if it ever had I think I can just drop these for now as they
are unused by the support code even.
+- ti,clkdm-name : clockdomain name for the DPLL
Could you elaborate on what this is for? What constitutes a valid name?
I'm not sure a string is the best way to define the linkage of several
elements to a domain...
Well, I am not sure if we can do this any better at this point, we don't
have DT nodes for clockdomain at the moment (I am not sure if we will
have those either as there are only a handful of those per SoC) but I'll
add some more documentation for this.
+
+Examples:
+ dpll_core_ck: dpll_core_ck@44e00490 {
+ #clock-cells = 0;
+ compatible = ti,omap4-dpll-core-clock;
+ clocks = sys_clkin_ck;
+ reg = 0x44e00490 0x4, 0x44e0045c 0x4, 0x0 0x4,
+ 0x44e00468 0x4;
+
Re: [PATCHv5 02/31] CLK: TI: Add DPLL clock support
On Mon, Aug 19, 2013 at 02:34:45PM +0100, Tero Kristo wrote: On 08/13/2013 01:50 PM, Mark Rutland wrote: Hi, On Fri, Aug 02, 2013 at 05:25:21PM +0100, Tero Kristo wrote: The OMAP clock driver now supports DPLL clock type. This patch also adds support for DT DPLL nodes. Signed-off-by: Tero Kristo t-kri...@ti.com --- .../devicetree/bindings/clock/ti/dpll.txt | 70 +++ arch/arm/mach-omap2/clock.h| 144 +- arch/arm/mach-omap2/clock3xxx.h|2 - drivers/clk/Makefile |1 + drivers/clk/ti/Makefile|3 + drivers/clk/ti/dpll.c | 488 include/linux/clk/ti.h | 164 +++ 7 files changed, 727 insertions(+), 145 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/ti/dpll.txt create mode 100644 drivers/clk/ti/Makefile create mode 100644 drivers/clk/ti/dpll.c create mode 100644 include/linux/clk/ti.h diff --git a/Documentation/devicetree/bindings/clock/ti/dpll.txt b/Documentation/devicetree/bindings/clock/ti/dpll.txt new file mode 100644 index 000..dcd6e63 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/ti/dpll.txt @@ -0,0 +1,70 @@ +Binding for Texas Instruments DPLL clock. + +This binding uses the common clock binding[1]. It assumes a +register-mapped DPLL with usually two selectable input clocks +(reference clock and bypass clock), with digital phase locked +loop logic for multiplying the input clock to a desired output +clock. This clock also typically supports different operation +modes (locked, low power stop etc.) This binding has several +sub-types, which effectively result in slightly different setup +for the actual DPLL clock. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be one of: + ti,omap4-dpll-x2-clock, + ti,omap3-dpll-clock, + ti,omap3-dpll-core-clock, + ti,omap3-dpll-per-clock, + ti,omap3-dpll-per-j-type-clock, + ti,omap4-dpll-clock, + ti,omap4-dpll-core-clock, + ti,omap4-dpll-m4xen-clock, + ti,omap4-dpll-j-type-clock, + ti,omap4-dpll-no-gate-clock, + ti,omap4-dpll-no-gate-j-type-clock, + +- #clock-cells : from common clock binding; shall be set to 0. +- clocks : link phandles of parent clocks (clk-ref and clk-bypass) It might be a good idea to use clock-names to clarify which clocks are present (I notice your examples contain only one clock input). All DPLLs have both bypass and reference clocks, but these can be the same. Is it better to just list both always (and hence drop clock-names), or provide clock-names always? If they're separate inputs, it's worth listing both (even if they're fed by the same provider). If it's possible one input might not be wired up, use clock-names. +- reg : array of register base addresses for controlling the DPLL (some + of these can also be left as NULL): + reg[0] = control register + reg[1] = idle status register + reg[2] = autoidle register + reg[3] = multiplier / divider set register Are all of these always present? Using reg-names seems sensible here. Not always, I'll change the code. I am quite new to DT and didn't actually know of the existence of reg-names prop. Ok. :) +- ti,recal-en-bit : recalibration enable bit +- ti,recal-st-bit : recalibration status bit +- ti,auto-recal-bit : auto recalibration enable bit These seem rather low-level, but I see other clock bindings are doing similar things. When are these needed, and why? What type are they? Bit shift values for the auto recalibration feature. HOWEVER, it seems these are actually legacy, kernel does not have support for these anymore if it ever had I think I can just drop these for now as they are unused by the support code even. Ok. +- ti,clkdm-name : clockdomain name for the DPLL Could you elaborate on what this is for? What constitutes a valid name? I'm not sure a string is the best way to define the linkage of several elements to a domain... Well, I am not sure if we can do this any better at this point, we don't have DT nodes for clockdomain at the moment (I am not sure if we will have those either as there are only a handful of those per SoC) but I'll add some more documentation for this. I'll have to see the documentation, but I'd be very wary of putting that in as-is. Does having the clock domain string link this up to domains in platform data? I'm not sure how well we'll be able to maintain support for that in future if it requires other platform code to use now, and we're not sure how
Re: [PATCHv5 02/31] CLK: TI: Add DPLL clock support
On 08/19/2013 05:18 PM, Mark Rutland wrote: On Mon, Aug 19, 2013 at 02:34:45PM +0100, Tero Kristo wrote: On 08/13/2013 01:50 PM, Mark Rutland wrote: Hi, On Fri, Aug 02, 2013 at 05:25:21PM +0100, Tero Kristo wrote: The OMAP clock driver now supports DPLL clock type. This patch also adds support for DT DPLL nodes. Signed-off-by: Tero Kristo t-kri...@ti.com --- .../devicetree/bindings/clock/ti/dpll.txt | 70 +++ arch/arm/mach-omap2/clock.h| 144 +- arch/arm/mach-omap2/clock3xxx.h|2 - drivers/clk/Makefile |1 + drivers/clk/ti/Makefile|3 + drivers/clk/ti/dpll.c | 488 include/linux/clk/ti.h | 164 +++ 7 files changed, 727 insertions(+), 145 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/ti/dpll.txt create mode 100644 drivers/clk/ti/Makefile create mode 100644 drivers/clk/ti/dpll.c create mode 100644 include/linux/clk/ti.h diff --git a/Documentation/devicetree/bindings/clock/ti/dpll.txt b/Documentation/devicetree/bindings/clock/ti/dpll.txt new file mode 100644 index 000..dcd6e63 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/ti/dpll.txt @@ -0,0 +1,70 @@ +Binding for Texas Instruments DPLL clock. + +This binding uses the common clock binding[1]. It assumes a +register-mapped DPLL with usually two selectable input clocks +(reference clock and bypass clock), with digital phase locked +loop logic for multiplying the input clock to a desired output +clock. This clock also typically supports different operation +modes (locked, low power stop etc.) This binding has several +sub-types, which effectively result in slightly different setup +for the actual DPLL clock. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be one of: + ti,omap4-dpll-x2-clock, + ti,omap3-dpll-clock, + ti,omap3-dpll-core-clock, + ti,omap3-dpll-per-clock, + ti,omap3-dpll-per-j-type-clock, + ti,omap4-dpll-clock, + ti,omap4-dpll-core-clock, + ti,omap4-dpll-m4xen-clock, + ti,omap4-dpll-j-type-clock, + ti,omap4-dpll-no-gate-clock, + ti,omap4-dpll-no-gate-j-type-clock, + +- #clock-cells : from common clock binding; shall be set to 0. +- clocks : link phandles of parent clocks (clk-ref and clk-bypass) It might be a good idea to use clock-names to clarify which clocks are present (I notice your examples contain only one clock input). All DPLLs have both bypass and reference clocks, but these can be the same. Is it better to just list both always (and hence drop clock-names), or provide clock-names always? If they're separate inputs, it's worth listing both (even if they're fed by the same provider). If it's possible one input might not be wired up, use clock-names. Ref and bypass clocks are used currently by all DPLLs (also the APLL) so I guess I just enforce both to be specified. +- reg : array of register base addresses for controlling the DPLL (some + of these can also be left as NULL): + reg[0] = control register + reg[1] = idle status register + reg[2] = autoidle register + reg[3] = multiplier / divider set register Are all of these always present? Using reg-names seems sensible here. Not always, I'll change the code. I am quite new to DT and didn't actually know of the existence of reg-names prop. Ok. :) +- ti,recal-en-bit : recalibration enable bit +- ti,recal-st-bit : recalibration status bit +- ti,auto-recal-bit : auto recalibration enable bit These seem rather low-level, but I see other clock bindings are doing similar things. When are these needed, and why? What type are they? Bit shift values for the auto recalibration feature. HOWEVER, it seems these are actually legacy, kernel does not have support for these anymore if it ever had I think I can just drop these for now as they are unused by the support code even. Ok. +- ti,clkdm-name : clockdomain name for the DPLL Could you elaborate on what this is for? What constitutes a valid name? I'm not sure a string is the best way to define the linkage of several elements to a domain... Well, I am not sure if we can do this any better at this point, we don't have DT nodes for clockdomain at the moment (I am not sure if we will have those either as there are only a handful of those per SoC) but I'll add some more documentation for this. I'll have to see the documentation, but I'd be very wary of putting that in as-is. Does having the clock domain string link this up to domains in platform data? I'm not sure how well we'll be able to maintain support for that in future if it requires other platform code to use now, and
Re: [PATCHv5 02/31] CLK: TI: Add DPLL clock support
On Mon, Aug 19, 2013 at 04:09:37PM +0100, Tero Kristo wrote: On 08/19/2013 05:18 PM, Mark Rutland wrote: On Mon, Aug 19, 2013 at 02:34:45PM +0100, Tero Kristo wrote: On 08/13/2013 01:50 PM, Mark Rutland wrote: Hi, On Fri, Aug 02, 2013 at 05:25:21PM +0100, Tero Kristo wrote: The OMAP clock driver now supports DPLL clock type. This patch also adds support for DT DPLL nodes. Signed-off-by: Tero Kristo t-kri...@ti.com --- .../devicetree/bindings/clock/ti/dpll.txt | 70 +++ arch/arm/mach-omap2/clock.h| 144 +- arch/arm/mach-omap2/clock3xxx.h|2 - drivers/clk/Makefile |1 + drivers/clk/ti/Makefile|3 + drivers/clk/ti/dpll.c | 488 include/linux/clk/ti.h | 164 +++ 7 files changed, 727 insertions(+), 145 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/ti/dpll.txt create mode 100644 drivers/clk/ti/Makefile create mode 100644 drivers/clk/ti/dpll.c create mode 100644 include/linux/clk/ti.h diff --git a/Documentation/devicetree/bindings/clock/ti/dpll.txt b/Documentation/devicetree/bindings/clock/ti/dpll.txt new file mode 100644 index 000..dcd6e63 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/ti/dpll.txt @@ -0,0 +1,70 @@ +Binding for Texas Instruments DPLL clock. + +This binding uses the common clock binding[1]. It assumes a +register-mapped DPLL with usually two selectable input clocks +(reference clock and bypass clock), with digital phase locked +loop logic for multiplying the input clock to a desired output +clock. This clock also typically supports different operation +modes (locked, low power stop etc.) This binding has several +sub-types, which effectively result in slightly different setup +for the actual DPLL clock. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be one of: + ti,omap4-dpll-x2-clock, + ti,omap3-dpll-clock, + ti,omap3-dpll-core-clock, + ti,omap3-dpll-per-clock, + ti,omap3-dpll-per-j-type-clock, + ti,omap4-dpll-clock, + ti,omap4-dpll-core-clock, + ti,omap4-dpll-m4xen-clock, + ti,omap4-dpll-j-type-clock, + ti,omap4-dpll-no-gate-clock, + ti,omap4-dpll-no-gate-j-type-clock, + +- #clock-cells : from common clock binding; shall be set to 0. +- clocks : link phandles of parent clocks (clk-ref and clk-bypass) It might be a good idea to use clock-names to clarify which clocks are present (I notice your examples contain only one clock input). All DPLLs have both bypass and reference clocks, but these can be the same. Is it better to just list both always (and hence drop clock-names), or provide clock-names always? If they're separate inputs, it's worth listing both (even if they're fed by the same provider). If it's possible one input might not be wired up, use clock-names. Ref and bypass clocks are used currently by all DPLLs (also the APLL) so I guess I just enforce both to be specified. Ok. It's always possible to add clock-names later if a platform doesn't wire an input. We lose the possibility of future compatibility, but backwards compatibility is easy enough to maintain. +- ti,clkdm-name : clockdomain name for the DPLL Could you elaborate on what this is for? What constitutes a valid name? I'm not sure a string is the best way to define the linkage of several elements to a domain... Well, I am not sure if we can do this any better at this point, we don't have DT nodes for clockdomain at the moment (I am not sure if we will have those either as there are only a handful of those per SoC) but I'll add some more documentation for this. I'll have to see the documentation, but I'd be very wary of putting that in as-is. Does having the clock domain string link this up to domains in platform data? I'm not sure how well we'll be able to maintain support for that in future if it requires other platform code to use now, and we're not sure how the domains themselves will be represented in dt. Hmm so, should I add a stub representation for the clockdomains to the DT then for now or how should this be handled? The stubs could then be mapped to the actual clock domains based on their node names. I unfortunately don't have a good answer here, because I'm not that familiar with how we handle clockdomains for power management purposes. As I understand it, each clock domain is essentially a clock gate controlling multiple clock signals, so it's possible to describe that with the common clock bindings, with a domain's clocks
Re: [PATCHv5 02/31] CLK: TI: Add DPLL clock support
On 08/19/2013 07:24 PM, Mark Rutland wrote: On Mon, Aug 19, 2013 at 04:09:37PM +0100, Tero Kristo wrote: On 08/19/2013 05:18 PM, Mark Rutland wrote: On Mon, Aug 19, 2013 at 02:34:45PM +0100, Tero Kristo wrote: On 08/13/2013 01:50 PM, Mark Rutland wrote: Hi, On Fri, Aug 02, 2013 at 05:25:21PM +0100, Tero Kristo wrote: The OMAP clock driver now supports DPLL clock type. This patch also adds support for DT DPLL nodes. Signed-off-by: Tero Kristo t-kri...@ti.com --- .../devicetree/bindings/clock/ti/dpll.txt | 70 +++ arch/arm/mach-omap2/clock.h| 144 +- arch/arm/mach-omap2/clock3xxx.h|2 - drivers/clk/Makefile |1 + drivers/clk/ti/Makefile|3 + drivers/clk/ti/dpll.c | 488 include/linux/clk/ti.h | 164 +++ 7 files changed, 727 insertions(+), 145 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/ti/dpll.txt create mode 100644 drivers/clk/ti/Makefile create mode 100644 drivers/clk/ti/dpll.c create mode 100644 include/linux/clk/ti.h diff --git a/Documentation/devicetree/bindings/clock/ti/dpll.txt b/Documentation/devicetree/bindings/clock/ti/dpll.txt new file mode 100644 index 000..dcd6e63 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/ti/dpll.txt @@ -0,0 +1,70 @@ +Binding for Texas Instruments DPLL clock. + +This binding uses the common clock binding[1]. It assumes a +register-mapped DPLL with usually two selectable input clocks +(reference clock and bypass clock), with digital phase locked +loop logic for multiplying the input clock to a desired output +clock. This clock also typically supports different operation +modes (locked, low power stop etc.) This binding has several +sub-types, which effectively result in slightly different setup +for the actual DPLL clock. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be one of: + ti,omap4-dpll-x2-clock, + ti,omap3-dpll-clock, + ti,omap3-dpll-core-clock, + ti,omap3-dpll-per-clock, + ti,omap3-dpll-per-j-type-clock, + ti,omap4-dpll-clock, + ti,omap4-dpll-core-clock, + ti,omap4-dpll-m4xen-clock, + ti,omap4-dpll-j-type-clock, + ti,omap4-dpll-no-gate-clock, + ti,omap4-dpll-no-gate-j-type-clock, + +- #clock-cells : from common clock binding; shall be set to 0. +- clocks : link phandles of parent clocks (clk-ref and clk-bypass) It might be a good idea to use clock-names to clarify which clocks are present (I notice your examples contain only one clock input). All DPLLs have both bypass and reference clocks, but these can be the same. Is it better to just list both always (and hence drop clock-names), or provide clock-names always? If they're separate inputs, it's worth listing both (even if they're fed by the same provider). If it's possible one input might not be wired up, use clock-names. Ref and bypass clocks are used currently by all DPLLs (also the APLL) so I guess I just enforce both to be specified. Ok. It's always possible to add clock-names later if a platform doesn't wire an input. We lose the possibility of future compatibility, but backwards compatibility is easy enough to maintain. +- ti,clkdm-name : clockdomain name for the DPLL Could you elaborate on what this is for? What constitutes a valid name? I'm not sure a string is the best way to define the linkage of several elements to a domain... Well, I am not sure if we can do this any better at this point, we don't have DT nodes for clockdomain at the moment (I am not sure if we will have those either as there are only a handful of those per SoC) but I'll add some more documentation for this. I'll have to see the documentation, but I'd be very wary of putting that in as-is. Does having the clock domain string link this up to domains in platform data? I'm not sure how well we'll be able to maintain support for that in future if it requires other platform code to use now, and we're not sure how the domains themselves will be represented in dt. Hmm so, should I add a stub representation for the clockdomains to the DT then for now or how should this be handled? The stubs could then be mapped to the actual clock domains based on their node names. I unfortunately don't have a good answer here, because I'm not that familiar with how we handle clockdomains for power management purposes. As I understand it, each clock domain is essentially a clock gate controlling multiple clock signals, so it's possible to describe that with the common clock bindings, with a domain's clocks all coming from a domain-gate-clock node (or something like that). That would make the wiring of clocks to
Re: [PATCHv5 02/31] CLK: TI: Add DPLL clock support
Quoting Tero Kristo (2013-08-19 10:06:39) On 08/19/2013 07:24 PM, Mark Rutland wrote: On Mon, Aug 19, 2013 at 04:09:37PM +0100, Tero Kristo wrote: On 08/19/2013 05:18 PM, Mark Rutland wrote: On Mon, Aug 19, 2013 at 02:34:45PM +0100, Tero Kristo wrote: On 08/13/2013 01:50 PM, Mark Rutland wrote: Hi, On Fri, Aug 02, 2013 at 05:25:21PM +0100, Tero Kristo wrote: The OMAP clock driver now supports DPLL clock type. This patch also adds support for DT DPLL nodes. Signed-off-by: Tero Kristo t-kri...@ti.com --- .../devicetree/bindings/clock/ti/dpll.txt | 70 +++ arch/arm/mach-omap2/clock.h| 144 +- arch/arm/mach-omap2/clock3xxx.h|2 - drivers/clk/Makefile |1 + drivers/clk/ti/Makefile|3 + drivers/clk/ti/dpll.c | 488 include/linux/clk/ti.h | 164 +++ 7 files changed, 727 insertions(+), 145 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/ti/dpll.txt create mode 100644 drivers/clk/ti/Makefile create mode 100644 drivers/clk/ti/dpll.c create mode 100644 include/linux/clk/ti.h diff --git a/Documentation/devicetree/bindings/clock/ti/dpll.txt b/Documentation/devicetree/bindings/clock/ti/dpll.txt new file mode 100644 index 000..dcd6e63 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/ti/dpll.txt @@ -0,0 +1,70 @@ +Binding for Texas Instruments DPLL clock. + +This binding uses the common clock binding[1]. It assumes a +register-mapped DPLL with usually two selectable input clocks +(reference clock and bypass clock), with digital phase locked +loop logic for multiplying the input clock to a desired output +clock. This clock also typically supports different operation +modes (locked, low power stop etc.) This binding has several +sub-types, which effectively result in slightly different setup +for the actual DPLL clock. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be one of: + ti,omap4-dpll-x2-clock, + ti,omap3-dpll-clock, + ti,omap3-dpll-core-clock, + ti,omap3-dpll-per-clock, + ti,omap3-dpll-per-j-type-clock, + ti,omap4-dpll-clock, + ti,omap4-dpll-core-clock, + ti,omap4-dpll-m4xen-clock, + ti,omap4-dpll-j-type-clock, + ti,omap4-dpll-no-gate-clock, + ti,omap4-dpll-no-gate-j-type-clock, + +- #clock-cells : from common clock binding; shall be set to 0. +- clocks : link phandles of parent clocks (clk-ref and clk-bypass) It might be a good idea to use clock-names to clarify which clocks are present (I notice your examples contain only one clock input). All DPLLs have both bypass and reference clocks, but these can be the same. Is it better to just list both always (and hence drop clock-names), or provide clock-names always? If they're separate inputs, it's worth listing both (even if they're fed by the same provider). If it's possible one input might not be wired up, use clock-names. Ref and bypass clocks are used currently by all DPLLs (also the APLL) so I guess I just enforce both to be specified. Ok. It's always possible to add clock-names later if a platform doesn't wire an input. We lose the possibility of future compatibility, but backwards compatibility is easy enough to maintain. +- ti,clkdm-name : clockdomain name for the DPLL Could you elaborate on what this is for? What constitutes a valid name? I'm not sure a string is the best way to define the linkage of several elements to a domain... Well, I am not sure if we can do this any better at this point, we don't have DT nodes for clockdomain at the moment (I am not sure if we will have those either as there are only a handful of those per SoC) but I'll add some more documentation for this. I'll have to see the documentation, but I'd be very wary of putting that in as-is. Does having the clock domain string link this up to domains in platform data? I'm not sure how well we'll be able to maintain support for that in future if it requires other platform code to use now, and we're not sure how the domains themselves will be represented in dt. Hmm so, should I add a stub representation for the clockdomains to the DT then for now or how should this be handled? The stubs could then be mapped to the actual clock domains based on their node names. I'm not sure that this binding should know about the clock domain at all. Maybe the clock domain binding should list the clocks that are within the domain? I unfortunately don't have a good answer here, because
Re: [PATCHv5 02/31] CLK: TI: Add DPLL clock support
Hi,
On Fri, Aug 02, 2013 at 05:25:21PM +0100, Tero Kristo wrote:
The OMAP clock driver now supports DPLL clock type. This patch also
adds support for DT DPLL nodes.
Signed-off-by: Tero Kristo t-kri...@ti.com
---
.../devicetree/bindings/clock/ti/dpll.txt | 70 +++
arch/arm/mach-omap2/clock.h| 144 +-
arch/arm/mach-omap2/clock3xxx.h|2 -
drivers/clk/Makefile |1 +
drivers/clk/ti/Makefile|3 +
drivers/clk/ti/dpll.c | 488
include/linux/clk/ti.h | 164 +++
7 files changed, 727 insertions(+), 145 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/ti/dpll.txt
create mode 100644 drivers/clk/ti/Makefile
create mode 100644 drivers/clk/ti/dpll.c
create mode 100644 include/linux/clk/ti.h
diff --git a/Documentation/devicetree/bindings/clock/ti/dpll.txt
b/Documentation/devicetree/bindings/clock/ti/dpll.txt
new file mode 100644
index 000..dcd6e63
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/ti/dpll.txt
@@ -0,0 +1,70 @@
+Binding for Texas Instruments DPLL clock.
+
+This binding uses the common clock binding[1]. It assumes a
+register-mapped DPLL with usually two selectable input clocks
+(reference clock and bypass clock), with digital phase locked
+loop logic for multiplying the input clock to a desired output
+clock. This clock also typically supports different operation
+modes (locked, low power stop etc.) This binding has several
+sub-types, which effectively result in slightly different setup
+for the actual DPLL clock.
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : shall be one of:
+ ti,omap4-dpll-x2-clock,
+ ti,omap3-dpll-clock,
+ ti,omap3-dpll-core-clock,
+ ti,omap3-dpll-per-clock,
+ ti,omap3-dpll-per-j-type-clock,
+ ti,omap4-dpll-clock,
+ ti,omap4-dpll-core-clock,
+ ti,omap4-dpll-m4xen-clock,
+ ti,omap4-dpll-j-type-clock,
+ ti,omap4-dpll-no-gate-clock,
+ ti,omap4-dpll-no-gate-j-type-clock,
+
+- #clock-cells : from common clock binding; shall be set to 0.
+- clocks : link phandles of parent clocks (clk-ref and clk-bypass)
It might be a good idea to use clock-names to clarify which clocks are
present (I notice your examples contain only one clock input).
+- reg : array of register base addresses for controlling the DPLL (some
+ of these can also be left as NULL):
+ reg[0] = control register
+ reg[1] = idle status register
+ reg[2] = autoidle register
+ reg[3] = multiplier / divider set register
Are all of these always present? Using reg-names seems sensible here.
+- ti,clk-ref : link phandle for the reference clock
+- ti,clk-bypass : link phandle for the bypass clock
You already have these in clocks, why do you need them again here?
+
+Optional properties:
+- ti,modes : available modes for the DPLL
Huh? What type is that property? What does it mean?
+- ti,recal-en-bit : recalibration enable bit
+- ti,recal-st-bit : recalibration status bit
+- ti,auto-recal-bit : auto recalibration enable bit
These seem rather low-level, but I see other clock bindings are doing
similar things. When are these needed, and why? What type are they?
+- ti,clkdm-name : clockdomain name for the DPLL
Could you elaborate on what this is for? What constitutes a valid name?
I'm not sure a string is the best way to define the linkage of several
elements to a domain...
+
+Examples:
+ dpll_core_ck: dpll_core_ck@44e00490 {
+ #clock-cells = 0;
+ compatible = ti,omap4-dpll-core-clock;
+ clocks = sys_clkin_ck;
+ reg = 0x44e00490 0x4, 0x44e0045c 0x4, 0x0 0x4,
+ 0x44e00468 0x4;
+ ti,clk-ref = sys_clkin_ck;
+ ti,clk-bypass = sys_clkin_ck;
Huh? Why aren't these both in the clocks property?
[...]
+static inline void __iomem *get_dt_register(struct device_node *node,
+ int index)
+{
+ u32 val;
+
+ of_property_read_u32_index(node, reg, 2 * index, val);
+ if (val)
+ return of_iomap(node, index);
+ else
+ return NULL;
NAK. Use reg-names to handle registers being optional.
[...]
+ clkspec.np = of_parse_phandle(node, ti,clk-ref, 0);
+ dd-clk_ref = of_clk_get_from_provider(clkspec);
+ if (IS_ERR(dd-clk_ref)) {
+ pr_err(%s: ti,clk-ref for %s not found\n, __func__,
+ clk_name);
+ goto cleanup;
+ }
+
+ clkspec.np = of_parse_phandle(node, ti,clk-bypass, 0);
+
[PATCHv5 02/31] CLK: TI: Add DPLL clock support
The OMAP clock driver now supports DPLL clock type. This patch also
adds support for DT DPLL nodes.
Signed-off-by: Tero Kristo t-kri...@ti.com
---
.../devicetree/bindings/clock/ti/dpll.txt | 70 +++
arch/arm/mach-omap2/clock.h| 144 +-
arch/arm/mach-omap2/clock3xxx.h|2 -
drivers/clk/Makefile |1 +
drivers/clk/ti/Makefile|3 +
drivers/clk/ti/dpll.c | 488
include/linux/clk/ti.h | 164 +++
7 files changed, 727 insertions(+), 145 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/ti/dpll.txt
create mode 100644 drivers/clk/ti/Makefile
create mode 100644 drivers/clk/ti/dpll.c
create mode 100644 include/linux/clk/ti.h
diff --git a/Documentation/devicetree/bindings/clock/ti/dpll.txt
b/Documentation/devicetree/bindings/clock/ti/dpll.txt
new file mode 100644
index 000..dcd6e63
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/ti/dpll.txt
@@ -0,0 +1,70 @@
+Binding for Texas Instruments DPLL clock.
+
+This binding uses the common clock binding[1]. It assumes a
+register-mapped DPLL with usually two selectable input clocks
+(reference clock and bypass clock), with digital phase locked
+loop logic for multiplying the input clock to a desired output
+clock. This clock also typically supports different operation
+modes (locked, low power stop etc.) This binding has several
+sub-types, which effectively result in slightly different setup
+for the actual DPLL clock.
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : shall be one of:
+ ti,omap4-dpll-x2-clock,
+ ti,omap3-dpll-clock,
+ ti,omap3-dpll-core-clock,
+ ti,omap3-dpll-per-clock,
+ ti,omap3-dpll-per-j-type-clock,
+ ti,omap4-dpll-clock,
+ ti,omap4-dpll-core-clock,
+ ti,omap4-dpll-m4xen-clock,
+ ti,omap4-dpll-j-type-clock,
+ ti,omap4-dpll-no-gate-clock,
+ ti,omap4-dpll-no-gate-j-type-clock,
+
+- #clock-cells : from common clock binding; shall be set to 0.
+- clocks : link phandles of parent clocks (clk-ref and clk-bypass)
+- reg : array of register base addresses for controlling the DPLL (some
+ of these can also be left as NULL):
+ reg[0] = control register
+ reg[1] = idle status register
+ reg[2] = autoidle register
+ reg[3] = multiplier / divider set register
+- ti,clk-ref : link phandle for the reference clock
+- ti,clk-bypass : link phandle for the bypass clock
+
+Optional properties:
+- ti,modes : available modes for the DPLL
+- ti,recal-en-bit : recalibration enable bit
+- ti,recal-st-bit : recalibration status bit
+- ti,auto-recal-bit : auto recalibration enable bit
+- ti,clkdm-name : clockdomain name for the DPLL
+
+Examples:
+ dpll_core_ck: dpll_core_ck@44e00490 {
+ #clock-cells = 0;
+ compatible = ti,omap4-dpll-core-clock;
+ clocks = sys_clkin_ck;
+ reg = 0x44e00490 0x4, 0x44e0045c 0x4, 0x0 0x4,
+ 0x44e00468 0x4;
+ ti,clk-ref = sys_clkin_ck;
+ ti,clk-bypass = sys_clkin_ck;
+ };
+
+ dpll2_ck: dpll2_ck@48004004 {
+ #clock-cells = 0;
+ compatible = ti,omap3-dpll-clock;
+ clocks = sys_ck;
+ ti,modes = 0xa2;
+ ti,clk-bypass = dpll2_fck;
+ reg = 0x48004004 0x4, 0x48004024 0x4, 0x48004034 0x4,
+ 0x48004040 0x4;
+ ti,clkdm-name = dpll2_clkdm;
+ ti,clk-ref = sys_ck;
+ ti,recal-en-bit = 0x8;
+ ti,auto-recal-bit = 0x3;
+ ti,recal-st-bit = 0x8;
+ };
diff --git a/arch/arm/mach-omap2/clock.h b/arch/arm/mach-omap2/clock.h
index 7aa32cd..079536a 100644
--- a/arch/arm/mach-omap2/clock.h
+++ b/arch/arm/mach-omap2/clock.h
@@ -21,6 +21,7 @@
#include linux/clkdev.h
#include linux/clk-provider.h
+#include linux/clk/ti.h
struct omap_clk {
u16 cpu;
@@ -178,83 +179,6 @@ struct clksel {
const struct clksel_rate *rates;
};
-/**
- * struct dpll_data - DPLL registers and integration data
- * @mult_div1_reg: register containing the DPLL M and N bitfields
- * @mult_mask: mask of the DPLL M bitfield in @mult_div1_reg
- * @div1_mask: mask of the DPLL N bitfield in @mult_div1_reg
- * @clk_bypass: struct clk pointer to the clock's bypass clock input
- * @clk_ref: struct clk pointer to the clock's reference clock input
- * @control_reg: register containing the DPLL mode bitfield
- * @enable_mask: mask of the DPLL mode bitfield in @control_reg
- * @last_rounded_rate: cache of the last rate result of omap2_dpll_round_rate()
- * @last_rounded_m: cache
