diff mbox

[PATCHv5,02/31] CLK: TI: Add DPLL clock support

Message ID 1375460751-23676-3-git-send-email-t-kristo@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tero Kristo Aug. 2, 2013, 4:25 p.m. UTC
The OMAP clock driver now supports DPLL clock type. This patch also
adds support for DT DPLL nodes.

Signed-off-by: Tero Kristo <t-kristo@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

Comments

Mark Rutland Aug. 13, 2013, 10:50 a.m. UTC | #1
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-kristo@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 0000000..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);
> +       dd->clk_bypass = of_clk_get_from_provider(&clkspec);
> +       if (IS_ERR(dd->clk_bypass)) {
> +               pr_err("%s: ti,clk-bypass for %s not found\n", __func__,
> +                      clk_name);
> +               goto cleanup;
> +       }

You can get these from the standard clocks property. Don't do this.

Look at of_clk_get_by_name().

Thanks,
Mark.
Tero Kristo Aug. 19, 2013, 1:34 p.m. UTC | #2
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-kristo@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 0000000..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>;
>> +               ti,clk-ref = <&sys_clkin_ck>;
>> +               ti,clk-bypass = <&sys_clkin_ck>;
>
> Huh? Why aren't these both in the clocks property?

Will fix based on comments above.

>
> [...]
>
>> +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.

Yea will change this.

>
> [...]
>
>> +       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);
>> +       dd->clk_bypass = of_clk_get_from_provider(&clkspec);
>> +       if (IS_ERR(dd->clk_bypass)) {
>> +               pr_err("%s: ti,clk-bypass for %s not found\n", __func__,
>> +                      clk_name);
>> +               goto cleanup;
>> +       }
>
> You can get these from the standard clocks property. Don't do this.
>
> Look at of_clk_get_by_name().

Or of_clk_get(node, index), which would be better?

-Tero

>
> Thanks,
> Mark.
>
Mark Rutland Aug. 19, 2013, 2:18 p.m. UTC | #3
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-kristo@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 0000000..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 the domains themselves will be represented in dt.

> >
> > [...]
> >
> >> +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.
> 
> Yea will change this.

Cheers.

> 
> >
> > [...]
> >
> >> +       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);
> >> +       dd->clk_bypass = of_clk_get_from_provider(&clkspec);
> >> +       if (IS_ERR(dd->clk_bypass)) {
> >> +               pr_err("%s: ti,clk-bypass for %s not found\n", __func__,
> >> +                      clk_name);
> >> +               goto cleanup;
> >> +       }
> >
> > You can get these from the standard clocks property. Don't do this.
> >
> > Look at of_clk_get_by_name().
> 
> Or of_clk_get(node, index), which would be better?

If you know you always have both clocks, then yes.

Cheers,
Mark.
Tero Kristo Aug. 19, 2013, 3:09 p.m. UTC | #4
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-kristo@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 0000000..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 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.

>
>>>
>>> [...]
>>>
>>>> +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.
>>
>> Yea will change this.
>
> Cheers.
>
>>
>>>
>>> [...]
>>>
>>>> +       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);
>>>> +       dd->clk_bypass = of_clk_get_from_provider(&clkspec);
>>>> +       if (IS_ERR(dd->clk_bypass)) {
>>>> +               pr_err("%s: ti,clk-bypass for %s not found\n", __func__,
>>>> +                      clk_name);
>>>> +               goto cleanup;
>>>> +       }
>>>
>>> You can get these from the standard clocks property. Don't do this.
>>>
>>> Look at of_clk_get_by_name().
>>
>> Or of_clk_get(node, index), which would be better?
>
> If you know you always have both clocks, then yes.

Ok, will do it this way then.

-Tero

>
> Cheers,
> Mark.
>
Mark Rutland Aug. 19, 2013, 4:24 p.m. UTC | #5
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-kristo@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 0000000..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 a domain explicit and in line with the rest of the
common clock bindings. But perhaps I've simplified things a bit too
far.

I'm not sure how easy it would be to use that information for power
management. I don't know what the kernel logic for clock domain power
management needs to know about consumers of the clocks and how it would
need to poke those consumers.

Cheers,
Mark.
Tero Kristo Aug. 19, 2013, 5:06 p.m. UTC | #6
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-kristo@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 0000000..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 a domain explicit and in line with the rest of the
> common clock bindings. But perhaps I've simplified things a bit too
> far.

You got it basically right, but maybe oversimplified it a bit. The 
root/parent clocks can still cross clockdomain boundaries, so mapping 
everything under a simple clockdomain gate would not work. Basically 
each clock has a mapping on the SoC for both its parent clock signal and 
the clockdomain association, kind of having two parents at the same 
time. In OMAP case, most of the clockdomains support hardware autoidle 
type functionality, which puts the domain to idle once all the clocks on 
it are disabled.

-Tero

> I'm not sure how easy it would be to use that information for power
> management. I don't know what the kernel logic for clock domain power
> management needs to know about consumers of the clocks and how it would
> need to poke those consumers.

>
> Cheers,
> Mark.
>
Mike Turquette Aug. 19, 2013, 10 p.m. UTC | #7
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-kristo@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 0000000..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 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 a domain explicit and in line with the rest of the
> > common clock bindings. But perhaps I've simplified things a bit too
> > far.
> 
> You got it basically right, but maybe oversimplified it a bit. The 
> root/parent clocks can still cross clockdomain boundaries, so mapping 
> everything under a simple clockdomain gate would not work. Basically 
> each clock has a mapping on the SoC for both its parent clock signal and 
> the clockdomain association, kind of having two parents at the same 
> time. In OMAP case, most of the clockdomains support hardware autoidle 
> type functionality, which puts the domain to idle once all the clocks on 
> it are disabled.

I always thought that OMAP clock domains were poorly named. Seems to me
that they had more to do with the IP/module programming than clocks per
se.  Again, I'm not sure that putting clkdm data into this binding is
correct.

Is it because you want a call to clk_enable to program the clock domain
in the .enable callback? I always thought that this was better left to a
pm_runtime_get callback...

Regards,
Mike

> 
> -Tero
> 
> > I'm not sure how easy it would be to use that information for power
> > management. I don't know what the kernel logic for clock domain power
> > management needs to know about consumers of the clocks and how it would
> > need to poke those consumers.
> 
> >
> > Cheers,
> > Mark.
> >
Tero Kristo Aug. 21, 2013, 4:16 p.m. UTC | #8
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-kristo@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 0000000..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 this:

         clockdomains {
                 usbhost_clkdm: usbhost_clkdm {
                         compatible = "ti,clockdomain";
                         clocks = <&usbhost_48m_fck>, <&usbhost_ick>;
                 };
	};

Mike, what is your approach on this? Are you okay having the 
implementation for this under drivers/clk? I recall you mentioned 
earlier that you don't want clockdomain stuff under drivers/clk, hence I 
am asking.

Here is the initial implementation for the binding:

void __init of_omap_clockdomain_setup(struct device_node *node)
{
         struct clk *clk;
         struct clk_hw *clk_hw;
         const char *clkdm_name = node->name;
         int i;
         int num_clks;

         num_clks = of_count_phandle_with_args(node, "clocks", 
"#clock-cells");

         for (i = 0; i < num_clks; i++) {
                 clk = of_clk_get(node, i);
                 if (__clk_get_flags(clk) & CLK_IS_BASIC) {
                         pr_warn("%s: can't setup clkdm for basic clk %s\n",
                                 __func__, __clk_get_name(clk));
                         continue;
                 }
                 clk_hw = __clk_get_hw(clk);
                 to_clk_hw_omap(clk_hw)->clkdm_name = clkdm_name;
                 omap2_init_clk_clkdm(clk_hw);
         }
}
CLK_OF_DECLARE(omap_clockdomain, "ti,clockdomain", 
of_omap_clockdomain_setup);

>
>>>>
>>>
>>> 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 a domain explicit and in line with the rest of the
>>> common clock bindings. But perhaps I've simplified things a bit too
>>> far.
>>
>> You got it basically right, but maybe oversimplified it a bit. The
>> root/parent clocks can still cross clockdomain boundaries, so mapping
>> everything under a simple clockdomain gate would not work. Basically
>> each clock has a mapping on the SoC for both its parent clock signal and
>> the clockdomain association, kind of having two parents at the same
>> time. In OMAP case, most of the clockdomains support hardware autoidle
>> type functionality, which puts the domain to idle once all the clocks on
>> it are disabled.
>
> I always thought that OMAP clock domains were poorly named. Seems to me
> that they had more to do with the IP/module programming than clocks per
> se.  Again, I'm not sure that putting clkdm data into this binding is
> correct.
>
> Is it because you want a call to clk_enable to program the clock domain
> in the .enable callback? I always thought that this was better left to a
> pm_runtime_get callback...

My guess is that some clocks require the clockdomain itself to be forced 
on before they are enabled, some of the DPLLs do this for example. I am 
just trying not to cause any regressions with this set, thus attempting 
to keep most of the implementation specifics intact.

-Tero

>
> Regards,
> Mike
>
>>
>> -Tero
>>
>>> I'm not sure how easy it would be to use that information for power
>>> management. I don't know what the kernel logic for clock domain power
>>> management needs to know about consumers of the clocks and how it would
>>> need to poke those consumers.
>>
>>>
>>> Cheers,
>>> Mark.
>>>
Mike Turquette Aug. 22, 2013, 8:04 a.m. UTC | #9
Quoting Tero Kristo (2013-08-21 09:16:45)
> 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-kristo@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 0000000..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 this:
> 
>          clockdomains {
>                  usbhost_clkdm: usbhost_clkdm {
>                          compatible = "ti,clockdomain";
>                          clocks = <&usbhost_48m_fck>, <&usbhost_ick>;
>                  };
>         };

Cool, this is much closer to what I had in mind. I guess the clock
domain DT binding can be marked as "Unstable - ABI compatibility may be
broken in the future" since the binding isn't really fleshed out yet.

> 
> Mike, what is your approach on this? Are you okay having the 
> implementation for this under drivers/clk? I recall you mentioned 
> earlier that you don't want clockdomain stuff under drivers/clk, hence I 
> am asking.

I do not want them to live there permanently. There has been some talk
about drivers/syscon/ or whatever for a while now, but this clkdm stuff
is also not a good candidate for that since none of the clkdm bits here
are really fleshed out with an eye towards a reusable core framework.
Maybe someday though...

I guess I could take it into drivers/clk/ti/ on a temporary basis, with
a commitment to move it out when the right driver infrastructure for
clock domains comes along.

Regards,
Mike

> 
> Here is the initial implementation for the binding:
> 
> void __init of_omap_clockdomain_setup(struct device_node *node)
> {
>          struct clk *clk;
>          struct clk_hw *clk_hw;
>          const char *clkdm_name = node->name;
>          int i;
>          int num_clks;
> 
>          num_clks = of_count_phandle_with_args(node, "clocks", 
> "#clock-cells");
> 
>          for (i = 0; i < num_clks; i++) {
>                  clk = of_clk_get(node, i);
>                  if (__clk_get_flags(clk) & CLK_IS_BASIC) {
>                          pr_warn("%s: can't setup clkdm for basic clk %s\n",
>                                  __func__, __clk_get_name(clk));
>                          continue;
>                  }
>                  clk_hw = __clk_get_hw(clk);
>                  to_clk_hw_omap(clk_hw)->clkdm_name = clkdm_name;
>                  omap2_init_clk_clkdm(clk_hw);
>          }
> }
> CLK_OF_DECLARE(omap_clockdomain, "ti,clockdomain", 
> of_omap_clockdomain_setup);
> 
> >
> >>>>
> >>>
> >>> 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 a domain explicit and in line with the rest of the
> >>> common clock bindings. But perhaps I've simplified things a bit too
> >>> far.
> >>
> >> You got it basically right, but maybe oversimplified it a bit. The
> >> root/parent clocks can still cross clockdomain boundaries, so mapping
> >> everything under a simple clockdomain gate would not work. Basically
> >> each clock has a mapping on the SoC for both its parent clock signal and
> >> the clockdomain association, kind of having two parents at the same
> >> time. In OMAP case, most of the clockdomains support hardware autoidle
> >> type functionality, which puts the domain to idle once all the clocks on
> >> it are disabled.
> >
> > I always thought that OMAP clock domains were poorly named. Seems to me
> > that they had more to do with the IP/module programming than clocks per
> > se.  Again, I'm not sure that putting clkdm data into this binding is
> > correct.
> >
> > Is it because you want a call to clk_enable to program the clock domain
> > in the .enable callback? I always thought that this was better left to a
> > pm_runtime_get callback...
> 
> My guess is that some clocks require the clockdomain itself to be forced 
> on before they are enabled, some of the DPLLs do this for example. I am 
> just trying not to cause any regressions with this set, thus attempting 
> to keep most of the implementation specifics intact.
> 
> -Tero
> 
> >
> > Regards,
> > Mike
> >
> >>
> >> -Tero
> >>
> >>> I'm not sure how easy it would be to use that information for power
> >>> management. I don't know what the kernel logic for clock domain power
> >>> management needs to know about consumers of the clocks and how it would
> >>> need to poke those consumers.
> >>
> >>>
> >>> Cheers,
> >>> Mark.
> >>>
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/clock/ti/dpll.txt b/Documentation/devicetree/bindings/clock/ti/dpll.txt
new file mode 100644
index 0000000..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 of the last M result of omap2_dpll_round_rate()
- * @last_rounded_m4xen: cache of the last M4X result of
- *			omap4_dpll_regm4xen_round_rate()
- * @last_rounded_lpmode: cache of the last lpmode result of
- *			 omap4_dpll_lpmode_recalc()
- * @max_multiplier: maximum valid non-bypass multiplier value (actual)
- * @last_rounded_n: cache of the last N result of omap2_dpll_round_rate()
- * @min_divider: minimum valid non-bypass divider value (actual)
- * @max_divider: maximum valid non-bypass divider value (actual)
- * @modes: possible values of @enable_mask
- * @autoidle_reg: register containing the DPLL autoidle mode bitfield
- * @idlest_reg: register containing the DPLL idle status bitfield
- * @autoidle_mask: mask of the DPLL autoidle mode bitfield in @autoidle_reg
- * @freqsel_mask: mask of the DPLL jitter correction bitfield in @control_reg
- * @idlest_mask: mask of the DPLL idle status bitfield in @idlest_reg
- * @lpmode_mask: mask of the DPLL low-power mode bitfield in @control_reg
- * @m4xen_mask: mask of the DPLL M4X multiplier bitfield in @control_reg
- * @auto_recal_bit: bitshift of the driftguard enable bit in @control_reg
- * @recal_en_bit: bitshift of the PRM_IRQENABLE_* bit for recalibration IRQs
- * @recal_st_bit: bitshift of the PRM_IRQSTATUS_* bit for recalibration IRQs
- * @flags: DPLL type/features (see below)
- *
- * Possible values for @flags:
- * DPLL_J_TYPE: "J-type DPLL" (only some 36xx, 4xxx DPLLs)
- *
- * @freqsel_mask is only used on the OMAP34xx family and AM35xx.
- *
- * XXX Some DPLLs have multiple bypass inputs, so it's not technically
- * correct to only have one @clk_bypass pointer.
- *
- * XXX The runtime-variable fields (@last_rounded_rate, @last_rounded_m,
- * @last_rounded_n) should be separated from the runtime-fixed fields
- * and placed into a different structure, so that the runtime-fixed data
- * can be placed into read-only space.
- */
-struct dpll_data {
-	void __iomem		*mult_div1_reg;
-	u32			mult_mask;
-	u32			div1_mask;
-	struct clk		*clk_bypass;
-	struct clk		*clk_ref;
-	void __iomem		*control_reg;
-	u32			enable_mask;
-	unsigned long		last_rounded_rate;
-	u16			last_rounded_m;
-	u8			last_rounded_m4xen;
-	u8			last_rounded_lpmode;
-	u16			max_multiplier;
-	u8			last_rounded_n;
-	u8			min_divider;
-	u16			max_divider;
-	u8			modes;
-	void __iomem		*autoidle_reg;
-	void __iomem		*idlest_reg;
-	u32			autoidle_mask;
-	u32			freqsel_mask;
-	u32			idlest_mask;
-	u32			dco_mask;
-	u32			sddiv_mask;
-	u32			lpmode_mask;
-	u32			m4xen_mask;
-	u8			auto_recal_bit;
-	u8			recal_en_bit;
-	u8			recal_st_bit;
-	u8			flags;
-};
-
 /*
  * struct clk.flags possibilities
  *
@@ -274,45 +198,6 @@  struct dpll_data {
 #define INVERT_ENABLE		(1 << 4)	/* 0 enables, 1 disables */
 #define CLOCK_CLKOUTX2		(1 << 5)
 
-/**
- * struct clk_hw_omap - OMAP struct clk
- * @node: list_head connecting this clock into the full clock list
- * @enable_reg: register to write to enable the clock (see @enable_bit)
- * @enable_bit: bitshift to write to enable/disable the clock (see @enable_reg)
- * @flags: see "struct clk.flags possibilities" above
- * @clksel_reg: for clksel clks, register va containing src/divisor select
- * @clksel_mask: bitmask in @clksel_reg for the src/divisor selector
- * @clksel: for clksel clks, pointer to struct clksel for this clock
- * @dpll_data: for DPLLs, pointer to struct dpll_data for this clock
- * @clkdm_name: clockdomain name that this clock is contained in
- * @clkdm: pointer to struct clockdomain, resolved from @clkdm_name at runtime
- * @rate_offset: bitshift for rate selection bitfield (OMAP1 only)
- * @src_offset: bitshift for source selection bitfield (OMAP1 only)
- *
- * XXX @rate_offset, @src_offset should probably be removed and OMAP1
- * clock code converted to use clksel.
- *
- */
-
-struct clk_hw_omap_ops;
-
-struct clk_hw_omap {
-	struct clk_hw		hw;
-	struct list_head	node;
-	unsigned long		fixed_rate;
-	u8			fixed_div;
-	void __iomem		*enable_reg;
-	u8			enable_bit;
-	u8			flags;
-	void __iomem		*clksel_reg;
-	u32			clksel_mask;
-	const struct clksel	*clksel;
-	struct dpll_data	*dpll_data;
-	const char		*clkdm_name;
-	struct clockdomain	*clkdm;
-	const struct clk_hw_omap_ops	*ops;
-};
-
 struct clk_hw_omap_ops {
 	void			(*find_idlest)(struct clk_hw_omap *oclk,
 					void __iomem **idlest_reg,
@@ -348,36 +233,13 @@  unsigned long omap_fixed_divisor_recalc(struct clk_hw *hw,
 #define OMAP4XXX_EN_DPLL_FRBYPASS		0x6
 #define OMAP4XXX_EN_DPLL_LOCKED			0x7
 
-/* CM_CLKEN_PLL*.EN* bit values - not all are available for every DPLL */
-#define DPLL_LOW_POWER_STOP	0x1
-#define DPLL_LOW_POWER_BYPASS	0x5
-#define DPLL_LOCKED		0x7
-
-/* DPLL Type and DCO Selection Flags */
-#define DPLL_J_TYPE		0x1
-
-long omap2_dpll_round_rate(struct clk_hw *hw, unsigned long target_rate,
-			unsigned long *parent_rate);
-unsigned long omap3_dpll_recalc(struct clk_hw *hw, unsigned long parent_rate);
-int omap3_noncore_dpll_enable(struct clk_hw *hw);
-void omap3_noncore_dpll_disable(struct clk_hw *hw);
-int omap3_noncore_dpll_set_rate(struct clk_hw *hw, unsigned long rate,
-				unsigned long parent_rate);
 u32 omap3_dpll_autoidle_read(struct clk_hw_omap *clk);
 void omap3_dpll_allow_idle(struct clk_hw_omap *clk);
 void omap3_dpll_deny_idle(struct clk_hw_omap *clk);
-unsigned long omap3_clkoutx2_recalc(struct clk_hw *hw,
-				    unsigned long parent_rate);
 int omap4_dpllmx_gatectrl_read(struct clk_hw_omap *clk);
 void omap4_dpllmx_allow_gatectrl(struct clk_hw_omap *clk);
 void omap4_dpllmx_deny_gatectrl(struct clk_hw_omap *clk);
-unsigned long omap4_dpll_regm4xen_recalc(struct clk_hw *hw,
-				unsigned long parent_rate);
-long omap4_dpll_regm4xen_round_rate(struct clk_hw *hw,
-				    unsigned long target_rate,
-				    unsigned long *parent_rate);
 
-void omap2_init_clk_clkdm(struct clk_hw *clk);
 void __init omap2_clk_disable_clkdm_control(void);
 
 /* clkt_clksel.c public functions */
@@ -396,7 +258,6 @@  int omap2_clksel_set_parent(struct clk_hw *hw, u8 field_val);
 extern void omap2_clkt_iclk_allow_idle(struct clk_hw_omap *clk);
 extern void omap2_clkt_iclk_deny_idle(struct clk_hw_omap *clk);
 
-u8 omap2_init_dpll_parent(struct clk_hw *hw);
 unsigned long omap2_get_dpll_rate(struct clk_hw_omap *clk);
 
 int omap2_dflt_clk_enable(struct clk_hw *hw);
@@ -408,7 +269,6 @@  void omap2_clk_dflt_find_companion(struct clk_hw_omap *clk,
 void omap2_clk_dflt_find_idlest(struct clk_hw_omap *clk,
 				void __iomem **idlest_reg,
 				u8 *idlest_bit, u8 *idlest_val);
-void omap2_init_clk_hw_omap_clocks(struct clk *clk);
 int omap2_clk_enable_autoidle_all(void);
 int omap2_clk_disable_autoidle_all(void);
 void omap2_clk_enable_init_clocks(const char **clk_names, u8 num_clocks);
@@ -431,10 +291,8 @@  extern const struct clksel_rate gfx_l3_rates[];
 extern const struct clksel_rate dsp_ick_rates[];
 extern struct clk dummy_ck;
 
-extern const struct clk_hw_omap_ops clkhwops_omap3_dpll;
 extern const struct clk_hw_omap_ops clkhwops_iclk_wait;
 extern const struct clk_hw_omap_ops clkhwops_wait;
-extern const struct clk_hw_omap_ops clkhwops_omap4_dpllmx;
 extern const struct clk_hw_omap_ops clkhwops_iclk;
 extern const struct clk_hw_omap_ops clkhwops_omap3430es2_ssi_wait;
 extern const struct clk_hw_omap_ops clkhwops_omap3430es2_iclk_ssi_wait;
diff --git a/arch/arm/mach-omap2/clock3xxx.h b/arch/arm/mach-omap2/clock3xxx.h
index 8cd4b0a..dab90e2 100644
--- a/arch/arm/mach-omap2/clock3xxx.h
+++ b/arch/arm/mach-omap2/clock3xxx.h
@@ -9,8 +9,6 @@ 
 #define __ARCH_ARM_MACH_OMAP2_CLOCK3XXX_H
 
 int omap3xxx_clk_init(void);
-int omap3_dpll4_set_rate(struct clk_hw *clk, unsigned long rate,
-					unsigned long parent_rate);
 int omap3_core_dpll_m2_set_rate(struct clk_hw *clk, unsigned long rate,
 					unsigned long parent_rate);
 void omap3_clk_lock_dpll5(void);
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 4038c2b..cf55585 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -32,6 +32,7 @@  obj-$(CONFIG_ARCH_VT8500)	+= clk-vt8500.o
 obj-$(CONFIG_ARCH_ZYNQ)		+= zynq/
 obj-$(CONFIG_ARCH_TEGRA)	+= tegra/
 obj-$(CONFIG_PLAT_SAMSUNG)	+= samsung/
+obj-$(CONFIG_ARCH_OMAP)		+= ti/
 
 obj-$(CONFIG_X86)		+= x86/
 
diff --git a/drivers/clk/ti/Makefile b/drivers/clk/ti/Makefile
new file mode 100644
index 0000000..93177987
--- /dev/null
+++ b/drivers/clk/ti/Makefile
@@ -0,0 +1,3 @@ 
+ifneq ($(CONFIG_OF),)
+obj-y					+= dpll.o
+endif
diff --git a/drivers/clk/ti/dpll.c b/drivers/clk/ti/dpll.c
new file mode 100644
index 0000000..089b68d
--- /dev/null
+++ b/drivers/clk/ti/dpll.c
@@ -0,0 +1,488 @@ 
+/*
+ * OMAP DPLL clock support
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * Tero Kristo <t-kristo@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/clk/ti.h>
+
+static const struct clk_ops dpll_m4xen_ck_ops = {
+	.enable		= &omap3_noncore_dpll_enable,
+	.disable	= &omap3_noncore_dpll_disable,
+	.recalc_rate	= &omap4_dpll_regm4xen_recalc,
+	.round_rate	= &omap4_dpll_regm4xen_round_rate,
+	.set_rate	= &omap3_noncore_dpll_set_rate,
+	.get_parent	= &omap2_init_dpll_parent,
+};
+
+static const struct clk_ops dpll_core_ck_ops = {
+	.recalc_rate	= &omap3_dpll_recalc,
+	.get_parent	= &omap2_init_dpll_parent,
+};
+
+static const struct clk_ops omap3_dpll_core_ck_ops = {
+	.init		= &omap2_init_clk_clkdm,
+	.get_parent	= &omap2_init_dpll_parent,
+	.recalc_rate	= &omap3_dpll_recalc,
+	.round_rate	= &omap2_dpll_round_rate,
+};
+
+static const struct clk_ops dpll_ck_ops = {
+	.enable		= &omap3_noncore_dpll_enable,
+	.disable	= &omap3_noncore_dpll_disable,
+	.recalc_rate	= &omap3_dpll_recalc,
+	.round_rate	= &omap2_dpll_round_rate,
+	.set_rate	= &omap3_noncore_dpll_set_rate,
+	.get_parent	= &omap2_init_dpll_parent,
+	.init		= &omap2_init_clk_clkdm,
+};
+
+static const struct clk_ops dpll_no_gate_ck_ops = {
+	.recalc_rate	= &omap3_dpll_recalc,
+	.get_parent	= &omap2_init_dpll_parent,
+	.round_rate	= &omap2_dpll_round_rate,
+	.set_rate	= &omap3_noncore_dpll_set_rate,
+};
+
+static const struct clk_ops omap3_dpll_ck_ops = {
+	.init		= &omap2_init_clk_clkdm,
+	.enable		= &omap3_noncore_dpll_enable,
+	.disable	= &omap3_noncore_dpll_disable,
+	.get_parent	= &omap2_init_dpll_parent,
+	.recalc_rate	= &omap3_dpll_recalc,
+	.set_rate	= &omap3_noncore_dpll_set_rate,
+	.round_rate	= &omap2_dpll_round_rate,
+};
+
+static const struct clk_ops omap3_dpll_per_ck_ops = {
+	.init		= &omap2_init_clk_clkdm,
+	.enable		= &omap3_noncore_dpll_enable,
+	.disable	= &omap3_noncore_dpll_disable,
+	.get_parent	= &omap2_init_dpll_parent,
+	.recalc_rate	= &omap3_dpll_recalc,
+	.set_rate	= &omap3_dpll4_set_rate,
+	.round_rate	= &omap2_dpll_round_rate,
+};
+
+static const struct clk_ops dpll_x2_ck_ops = {
+	.recalc_rate	= &omap3_clkoutx2_recalc,
+};
+
+static struct clk *omap_clk_register_dpll(struct device *dev, const char *name,
+		const char **parent_names, int num_parents, unsigned long flags,
+		struct dpll_data *dpll_data, const char *clkdm_name,
+		const struct clk_ops *ops)
+{
+	struct clk *clk;
+	struct clk_init_data init = { 0 };
+	struct clk_hw_omap *clk_hw;
+
+	/* allocate the divider */
+	clk_hw = kzalloc(sizeof(*clk_hw), GFP_KERNEL);
+	if (!clk_hw) {
+		pr_err("%s: could not allocate clk_hw_omap\n", __func__);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	clk_hw->dpll_data = dpll_data;
+	clk_hw->ops = &clkhwops_omap3_dpll;
+	clk_hw->clkdm_name = clkdm_name;
+	clk_hw->hw.init = &init;
+
+	init.name = name;
+	init.ops = ops;
+	init.flags = flags;
+	init.parent_names = parent_names;
+	init.num_parents = num_parents;
+
+	/* register the clock */
+	clk = clk_register(dev, &clk_hw->hw);
+
+	if (IS_ERR(clk))
+		kfree(clk_hw);
+	else
+		omap2_init_clk_hw_omap_clocks(clk);
+
+	return clk;
+}
+
+static struct clk *omap_clk_register_dpll_x2(struct device *dev,
+		const char *name, const char *parent_name, void __iomem *reg,
+		const struct clk_ops *ops)
+{
+	struct clk *clk;
+	struct clk_init_data init = { 0 };
+	struct clk_hw_omap *clk_hw;
+
+	if (!parent_name) {
+		pr_err("%s: dpll_x2 must have parent\n", __func__);
+		return ERR_PTR(-EINVAL);
+	}
+
+	clk_hw = kzalloc(sizeof(*clk_hw), GFP_KERNEL);
+	if (!clk_hw) {
+		pr_err("%s: could not allocate clk_hw_omap\n", __func__);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	clk_hw->ops = &clkhwops_omap4_dpllmx;
+	clk_hw->clksel_reg = reg;
+	clk_hw->hw.init = &init;
+
+	init.name = name;
+	init.ops = ops;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	/* register the clock */
+	clk = clk_register(dev, &clk_hw->hw);
+
+	if (IS_ERR(clk))
+		kfree(clk_hw);
+	else
+		omap2_init_clk_hw_omap_clocks(clk);
+
+	return clk;
+}
+
+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;
+}
+
+/**
+ * of_omap_dpll_setup() - Setup function for OMAP DPLL clocks
+ *
+ * @node: device node containing the DPLL info
+ * @ops: ops for the DPLL
+ * @ddt: DPLL data template to use
+ */
+static void __init of_omap_dpll_setup(struct device_node *node,
+					const struct clk_ops *ops,
+					const struct dpll_data *ddt)
+{
+	struct clk *clk;
+	const char *clk_name = node->name;
+	int num_parents;
+	const char **parent_names = NULL;
+	const char *clkdm_name = NULL;
+	struct of_phandle_args clkspec;
+	u8 dpll_flags = 0;
+	struct dpll_data *dd;
+	int i;
+	u32 val;
+
+	dd = kzalloc(sizeof(*dd), GFP_KERNEL);
+	if (!dd) {
+		pr_err("%s: could not allocate dpll_data\n", __func__);
+		return;
+	}
+
+	memcpy(dd, ddt, sizeof(*dd));
+
+	of_property_read_string(node, "clock-output-names", &clk_name);
+
+	num_parents = of_clk_get_parent_count(node);
+	if (num_parents < 1) {
+		pr_err("%s: omap dpll %s must have parent(s)\n",
+		       __func__, node->name);
+		goto cleanup;
+	}
+
+	parent_names = kzalloc(sizeof(char *) * num_parents, GFP_KERNEL);
+
+	for (i = 0; i < num_parents; i++)
+		parent_names[i] = of_clk_get_parent_name(node, i);
+
+	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);
+	dd->clk_bypass = of_clk_get_from_provider(&clkspec);
+	if (IS_ERR(dd->clk_bypass)) {
+		pr_err("%s: ti,clk-bypass for %s not found\n", __func__,
+		       clk_name);
+		goto cleanup;
+	}
+
+	of_property_read_string(node, "ti,clkdm-name", &clkdm_name);
+
+	dd->control_reg = get_dt_register(node, 0);
+	dd->idlest_reg = get_dt_register(node, 1);
+	dd->autoidle_reg = get_dt_register(node, 2);
+	dd->mult_div1_reg = get_dt_register(node, 3);
+
+	if (!of_property_read_u32(node, "ti,modes", &val))
+		dd->modes = val;
+
+	if (!of_property_read_u32(node, "ti,recal-en-bit", &val))
+		dd->recal_en_bit = val;
+
+	if (!of_property_read_u32(node, "ti,recal-st-bit", &val))
+		dd->recal_st_bit = val;
+
+	if (!of_property_read_u32(node, "ti,auto-recal-bit", &val))
+		dd->auto_recal_bit = val;
+
+	clk = omap_clk_register_dpll(NULL, clk_name, parent_names,
+				num_parents, dpll_flags, dd,
+				clkdm_name, ops);
+
+	if (!IS_ERR(clk))
+		of_clk_add_provider(node, of_clk_src_simple_get, clk);
+	return;
+
+cleanup:
+	kfree(dd);
+	kfree(parent_names);
+	return;
+}
+
+static void __init of_omap_dpll_x2_setup(struct device_node *node)
+{
+	struct clk *clk;
+	const char *clk_name = node->name;
+	void __iomem *reg;
+	const char *parent_name;
+
+	of_property_read_string(node, "clock-output-names", &clk_name);
+
+	parent_name = of_clk_get_parent_name(node, 0);
+
+	reg = get_dt_register(node, 0);
+
+	clk = omap_clk_register_dpll_x2(NULL, clk_name, parent_name,
+				reg, &dpll_x2_ck_ops);
+
+	if (!IS_ERR(clk))
+		of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(omap_dpll_x2_clock, "ti,omap4-dpll-x2-clock",
+	       of_omap_dpll_x2_setup);
+
+static void __init of_omap3_dpll_setup(struct device_node *node)
+{
+	const struct dpll_data dd = {
+		.idlest_mask = 0x1,
+		.enable_mask = 0x7,
+		.autoidle_mask = 0x7,
+		.mult_mask = 0x7ff << 8,
+		.div1_mask = 0x7f,
+		.max_multiplier = 2047,
+		.max_divider = 128,
+		.min_divider = 1,
+		.freqsel_mask = 0xf0,
+		.modes = (1 << DPLL_LOW_POWER_BYPASS) | (1 << DPLL_LOCKED),
+	};
+
+	of_omap_dpll_setup(node, &omap3_dpll_ck_ops, &dd);
+}
+CLK_OF_DECLARE(omap3_dpll_clock, "ti,omap3-dpll-clock", of_omap3_dpll_setup);
+
+static void __init of_omap3_core_dpll_setup(struct device_node *node)
+{
+	const struct dpll_data dd = {
+		.idlest_mask = 0x1,
+		.enable_mask = 0x7,
+		.autoidle_mask = 0x7,
+		.mult_mask = 0x7ff << 16,
+		.div1_mask = 0x7f << 8,
+		.max_multiplier = 2047,
+		.max_divider = 128,
+		.min_divider = 1,
+		.freqsel_mask = 0xf0,
+	};
+
+	of_omap_dpll_setup(node, &omap3_dpll_core_ck_ops, &dd);
+}
+CLK_OF_DECLARE(omap3_core_dpll_clock, "ti,omap3-dpll-core-clock",
+	       of_omap3_core_dpll_setup);
+
+static void __init of_omap3_per_dpll_setup(struct device_node *node)
+{
+	const struct dpll_data dd = {
+		.idlest_mask = 0x1 << 1,
+		.enable_mask = 0x7 << 16,
+		.autoidle_mask = 0x7 << 3,
+		.mult_mask = 0x7ff << 8,
+		.div1_mask = 0x7f,
+		.max_multiplier = 2047,
+		.max_divider = 128,
+		.min_divider = 1,
+		.freqsel_mask = 0xf00000,
+		.modes = (1 << DPLL_LOW_POWER_STOP) | (1 << DPLL_LOCKED),
+	};
+
+	of_omap_dpll_setup(node, &omap3_dpll_per_ck_ops, &dd);
+}
+CLK_OF_DECLARE(omap3_per_dpll_clock, "ti,omap3-dpll-per-clock",
+	       of_omap3_per_dpll_setup);
+
+static void __init of_omap3_per_jtype_dpll_setup(struct device_node *node)
+{
+	const struct dpll_data dd = {
+		.idlest_mask = 0x1 << 1,
+		.enable_mask = 0x7 << 16,
+		.autoidle_mask = 0x7 << 3,
+		.mult_mask = 0xfff << 8,
+		.div1_mask = 0x7f,
+		.max_multiplier = 4095,
+		.max_divider = 128,
+		.min_divider = 1,
+		.sddiv_mask = 0xff << 24,
+		.dco_mask = 0xe << 20,
+		.flags = DPLL_J_TYPE,
+		.modes = (1 << DPLL_LOW_POWER_STOP) | (1 << DPLL_LOCKED),
+	};
+
+	of_omap_dpll_setup(node, &omap3_dpll_per_ck_ops, &dd);
+}
+CLK_OF_DECLARE(omap3_per_jtype_dpll_clock, "ti,omap3-dpll-per-j-type-clock",
+	       of_omap3_per_jtype_dpll_setup);
+
+static void __init of_omap4_dpll_setup(struct device_node *node)
+{
+	const struct dpll_data dd = {
+		.idlest_mask = 0x1,
+		.enable_mask = 0x7,
+		.autoidle_mask = 0x7,
+		.mult_mask = 0x7ff << 8,
+		.div1_mask = 0x7f,
+		.max_multiplier = 2047,
+		.max_divider = 128,
+		.min_divider = 1,
+		.modes = (1 << DPLL_LOW_POWER_BYPASS) | (1 << DPLL_LOCKED),
+	};
+
+	of_omap_dpll_setup(node, &dpll_ck_ops, &dd);
+}
+CLK_OF_DECLARE(omap4_dpll_clock, "ti,omap4-dpll-clock", of_omap4_dpll_setup);
+
+static void __init of_omap4_core_dpll_setup(struct device_node *node)
+{
+	const struct dpll_data dd = {
+		.idlest_mask = 0x1,
+		.enable_mask = 0x7,
+		.autoidle_mask = 0x7,
+		.mult_mask = 0x7ff << 8,
+		.div1_mask = 0x7f,
+		.max_multiplier = 2047,
+		.max_divider = 128,
+		.min_divider = 1,
+		.modes = (1 << DPLL_LOW_POWER_BYPASS) | (1 << DPLL_LOCKED),
+	};
+
+	of_omap_dpll_setup(node, &dpll_core_ck_ops, &dd);
+}
+CLK_OF_DECLARE(omap4_core_dpll_clock, "ti,omap4-dpll-core-clock",
+	       of_omap4_core_dpll_setup);
+
+static void __init of_omap4_m4xen_dpll_setup(struct device_node *node)
+{
+	const struct dpll_data dd = {
+		.idlest_mask = 0x1,
+		.enable_mask = 0x7,
+		.autoidle_mask = 0x7,
+		.mult_mask = 0x7ff << 8,
+		.div1_mask = 0x7f,
+		.max_multiplier = 2047,
+		.max_divider = 128,
+		.min_divider = 1,
+		.m4xen_mask = 0x800,
+		.lpmode_mask = 1 << 10,
+		.modes = (1 << DPLL_LOW_POWER_BYPASS) | (1 << DPLL_LOCKED),
+	};
+
+	of_omap_dpll_setup(node, &dpll_m4xen_ck_ops, &dd);
+}
+CLK_OF_DECLARE(omap4_m4xen_dpll_clock, "ti,omap4-dpll-m4xen-clock",
+	       of_omap4_m4xen_dpll_setup);
+
+static void __init of_omap4_jtype_dpll_setup(struct device_node *node)
+{
+	const struct dpll_data dd = {
+		.idlest_mask = 0x1,
+		.enable_mask = 0x7,
+		.autoidle_mask = 0x7,
+		.mult_mask = 0xfff << 8,
+		.div1_mask = 0xff,
+		.max_multiplier = 4095,
+		.max_divider = 256,
+		.min_divider = 1,
+		.sddiv_mask = 0xff << 24,
+		.flags = DPLL_J_TYPE,
+		.modes = (1 << DPLL_LOW_POWER_BYPASS) | (1 << DPLL_LOCKED),
+	};
+
+	of_omap_dpll_setup(node, &dpll_m4xen_ck_ops, &dd);
+}
+CLK_OF_DECLARE(omap4_jtype_dpll_clock, "ti,omap4-dpll-j-type-clock",
+	       of_omap4_jtype_dpll_setup);
+
+static void __init of_omap4_no_gate_dpll_setup(struct device_node *node)
+{
+	const struct dpll_data dd = {
+		.idlest_mask = 0x1,
+		.enable_mask = 0x7,
+		.autoidle_mask = 0x7,
+		.mult_mask = 0x7ff << 8,
+		.div1_mask = 0x7f,
+		.max_multiplier = 2047,
+		.max_divider = 128,
+		.min_divider = 1,
+		.modes = (1 << DPLL_LOW_POWER_BYPASS) | (1 << DPLL_LOCKED),
+	};
+
+	of_omap_dpll_setup(node, &dpll_no_gate_ck_ops, &dd);
+}
+CLK_OF_DECLARE(omap4_no_gate_dpll_clock, "ti,omap4-dpll-no-gate-clock",
+	       of_omap4_no_gate_dpll_setup);
+
+static void __init of_omap4_no_gate_jtype_dpll_setup(struct device_node *node)
+{
+	const struct dpll_data dd = {
+		.idlest_mask = 0x1,
+		.enable_mask = 0x7,
+		.autoidle_mask = 0x7,
+		.mult_mask = 0x7ff << 8,
+		.div1_mask = 0x7f,
+		.max_multiplier = 2047,
+		.max_divider = 128,
+		.min_divider = 1,
+		.flags = DPLL_J_TYPE,
+		.modes = (1 << DPLL_LOW_POWER_BYPASS) | (1 << DPLL_LOCKED),
+	};
+
+	of_omap_dpll_setup(node, &dpll_no_gate_ck_ops, &dd);
+}
+CLK_OF_DECLARE(omap4_no_gate_jtype_dpll_clock,
+	       "ti,omap4-dpll-no-gate-j-type-clock",
+	       of_omap4_no_gate_jtype_dpll_setup);
diff --git a/include/linux/clk/ti.h b/include/linux/clk/ti.h
new file mode 100644
index 0000000..385384a
--- /dev/null
+++ b/include/linux/clk/ti.h
@@ -0,0 +1,164 @@ 
+/*
+ * TI clock drivers support
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#ifndef __LINUX_CLK_TI_H__
+#define __LINUX_CLK_TI_H__
+
+/**
+ * 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 of the last M result of omap2_dpll_round_rate()
+ * @last_rounded_m4xen: cache of the last M4X result of
+ *                     omap4_dpll_regm4xen_round_rate()
+ * @last_rounded_lpmode: cache of the last lpmode result of
+ *                      omap4_dpll_lpmode_recalc()
+ * @max_multiplier: maximum valid non-bypass multiplier value (actual)
+ * @last_rounded_n: cache of the last N result of omap2_dpll_round_rate()
+ * @min_divider: minimum valid non-bypass divider value (actual)
+ * @max_divider: maximum valid non-bypass divider value (actual)
+ * @modes: possible values of @enable_mask
+ * @autoidle_reg: register containing the DPLL autoidle mode bitfield
+ * @idlest_reg: register containing the DPLL idle status bitfield
+ * @autoidle_mask: mask of the DPLL autoidle mode bitfield in @autoidle_reg
+ * @freqsel_mask: mask of the DPLL jitter correction bitfield in @control_reg
+ * @idlest_mask: mask of the DPLL idle status bitfield in @idlest_reg
+ * @lpmode_mask: mask of the DPLL low-power mode bitfield in @control_reg
+ * @m4xen_mask: mask of the DPLL M4X multiplier bitfield in @control_reg
+ * @auto_recal_bit: bitshift of the driftguard enable bit in @control_reg
+ * @recal_en_bit: bitshift of the PRM_IRQENABLE_* bit for recalibration IRQs
+ * @recal_st_bit: bitshift of the PRM_IRQSTATUS_* bit for recalibration IRQs
+ * @flags: DPLL type/features (see below)
+ *
+ * Possible values for @flags:
+ * DPLL_J_TYPE: "J-type DPLL" (only some 36xx, 4xxx DPLLs)
+ *
+ * @freqsel_mask is only used on the OMAP34xx family and AM35xx.
+ *
+ * XXX Some DPLLs have multiple bypass inputs, so it's not technically
+ * correct to only have one @clk_bypass pointer.
+ *
+ * XXX The runtime-variable fields (@last_rounded_rate, @last_rounded_m,
+ * @last_rounded_n) should be separated from the runtime-fixed fields
+ * and placed into a different structure, so that the runtime-fixed data
+ * can be placed into read-only space.
+ */
+struct dpll_data {
+	void __iomem		*mult_div1_reg;
+	u32			mult_mask;
+	u32			div1_mask;
+	struct clk		*clk_bypass;
+	struct clk		*clk_ref;
+	void __iomem		*control_reg;
+	u32			enable_mask;
+	unsigned long		last_rounded_rate;
+	u16			last_rounded_m;
+	u8			last_rounded_m4xen;
+	u8			last_rounded_lpmode;
+	u16			max_multiplier;
+	u8			last_rounded_n;
+	u8			min_divider;
+	u16			max_divider;
+	u8			modes;
+	void __iomem		*autoidle_reg;
+	void __iomem		*idlest_reg;
+	u32			autoidle_mask;
+	u32			freqsel_mask;
+	u32			idlest_mask;
+	u32			dco_mask;
+	u32			sddiv_mask;
+	u32			lpmode_mask;
+	u32			m4xen_mask;
+	u8			auto_recal_bit;
+	u8			recal_en_bit;
+	u8			recal_st_bit;
+	u8			flags;
+};
+
+struct clk_hw_omap_ops;
+
+/**
+ * struct clk_hw_omap - OMAP struct clk
+ * @node: list_head connecting this clock into the full clock list
+ * @enable_reg: register to write to enable the clock (see @enable_bit)
+ * @enable_bit: bitshift to write to enable/disable the clock (see @enable_reg)
+ * @flags: see "struct clk.flags possibilities" above
+ * @clksel_reg: for clksel clks, register va containing src/divisor select
+ * @clksel_mask: bitmask in @clksel_reg for the src/divisor selector
+ * @clksel: for clksel clks, pointer to struct clksel for this clock
+ * @dpll_data: for DPLLs, pointer to struct dpll_data for this clock
+ * @clkdm_name: clockdomain name that this clock is contained in
+ * @clkdm: pointer to struct clockdomain, resolved from @clkdm_name at runtime
+ * @rate_offset: bitshift for rate selection bitfield (OMAP1 only)
+ * @src_offset: bitshift for source selection bitfield (OMAP1 only)
+ *
+ * XXX @rate_offset, @src_offset should probably be removed and OMAP1
+ * clock code converted to use clksel.
+ *
+ */
+struct clk_hw_omap {
+	struct clk_hw		hw;
+	struct list_head	node;
+	unsigned long		fixed_rate;
+	u8			fixed_div;
+	void __iomem		*enable_reg;
+	u8			enable_bit;
+	u8			flags;
+	void __iomem		*clksel_reg;
+	u32			clksel_mask;
+	const struct clksel	*clksel;
+	struct dpll_data	*dpll_data;
+	const char		*clkdm_name;
+	struct clockdomain	*clkdm;
+	const struct clk_hw_omap_ops	*ops;
+};
+
+/* CM_CLKEN_PLL*.EN* bit values - not all are available for every DPLL */
+#define DPLL_LOW_POWER_STOP	0x1
+#define DPLL_LOW_POWER_BYPASS	0x5
+#define DPLL_LOCKED		0x7
+
+/* DPLL Type and DCO Selection Flags */
+#define DPLL_J_TYPE		0x1
+
+void omap2_init_clk_hw_omap_clocks(struct clk *clk);
+int omap3_noncore_dpll_enable(struct clk_hw *hw);
+void omap3_noncore_dpll_disable(struct clk_hw *hw);
+int omap3_noncore_dpll_set_rate(struct clk_hw *hw, unsigned long rate,
+				unsigned long parent_rate);
+unsigned long omap4_dpll_regm4xen_recalc(struct clk_hw *hw,
+					 unsigned long parent_rate);
+long omap4_dpll_regm4xen_round_rate(struct clk_hw *hw,
+				    unsigned long target_rate,
+				    unsigned long *parent_rate);
+u8 omap2_init_dpll_parent(struct clk_hw *hw);
+unsigned long omap3_dpll_recalc(struct clk_hw *hw, unsigned long parent_rate);
+long omap2_dpll_round_rate(struct clk_hw *hw, unsigned long target_rate,
+			   unsigned long *parent_rate);
+void omap2_init_clk_clkdm(struct clk_hw *clk);
+unsigned long omap3_clkoutx2_recalc(struct clk_hw *hw,
+				    unsigned long parent_rate);
+int omap3_dpll4_set_rate(struct clk_hw *clk, unsigned long rate,
+			 unsigned long parent_rate);
+
+extern const struct clk_hw_omap_ops clkhwops_omap3_dpll;
+extern const struct clk_hw_omap_ops clkhwops_omap4_dpllmx;
+
+#endif