Re: [PATCHv4 03/33] CLK: OMAP4: Add DPLL clock support

2013-08-01 Thread Rajendra Nayak
Tero,

On Tuesday 23 July 2013 12:49 PM, Tero Kristo wrote:
 + dd-control_reg = of_iomap(node, 0);
 + dd-idlest_reg = of_iomap(node, 1);
 + dd-autoidle_reg = of_iomap(node, 2);
 + dd-mult_div1_reg = of_iomap(node, 3);
 +
[]...
 + reg = of_iomap(node, 0);

Doing an of_iomap() for every single clock register seems like an overkill
and might have performance penalties at boot.

regards,
Rajendra



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv4 03/33] CLK: OMAP4: Add DPLL clock support

2013-08-01 Thread Nishanth Menon

On 07/31/2013 04:46 AM, Tero Kristo wrote:

On 07/30/2013 07:23 PM, Nishanth Menon wrote:

This patch probably was submitted in the wrong sequence - fails build
and few other issues below.


Yeah, I'll double check the build sequence for these.



On 07/23/2013 02:19 AM, Tero Kristo wrote:

The OMAP clock driver now supports DPLL clock type. This patch also
adds support for DT DPLL nodes.


Then why is $subject specific to OMAP4? is that because of
of_omap4_dpll_setup?


The driver only supports omap4 dpll type at this point, omap3 dplls
require some modifications on top of this, and are provided later in the
series.


ok.







Signed-off-by: Tero Kristo t-kri...@ti.com
---
  drivers/clk/omap/Makefile |2 +-
  drivers/clk/omap/clk.c|1 +
  drivers/clk/omap/dpll.c   |  295
+


Device Tree Binding documentation?


Didn't bother writing those yet as I haven't received too much feedback
whether this approach is acceptable or not.


Documentation helps simplify the understanding of expected usage - this 
is useful to review approach as well :)



diff --git a/drivers/clk/omap/clk.c b/drivers/clk/omap/clk.c
index 4bf1929..1dafdaa 100644
--- a/drivers/clk/omap/clk.c
+++ b/drivers/clk/omap/clk.c
@@ -28,6 +28,7 @@ static const struct of_device_id clk_match[] = {
  .data = of_fixed_factor_clk_setup, },
  {.compatible = divider-clock, .data = of_divider_clk_setup, },
  {.compatible = gate-clock, .data = of_gate_clk_setup, },
+{.compatible = ti,omap4-dpll-clock, .data =
of_omap4_dpll_setup, },
  {},
  };

you would not need this if you did just of_clk_init(NULL); ?


Why not? And I think we still need to do this.


CLK_OF_DECLARE will take care of having all required clks registered 
of_clk_init(NULL); will pick up from that list.


that will remove all extra exports, and make clk.c redundant.
[...]






+#include linux/clk/omap.h


why?


Need dpll_data definition for example.

OK - without the ordering of patches, it was not obvious. structures aside,

We should move the functions to this file instead and empty out 
mach-omap2 gradually, omap_dpll.h should be exported and used by 
mach-omap2, rather than the other way around.



+
+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)


why should this be public?


Currently does not need to be, but someone could in theory build up
cclockXxxx_data.c file and use these calls from there. Kind of legacy
support, see some of the basic clk types. I guess I can add static to
this, and remove some of the params along.



thanks. we should keep unneeded stuff in static unless a specific 
provable need really arises.




I am assuming you do not do parameter check as you expect clk_register
to do the same?


True, so I'll change the above function to static.




+
+/* allocate the divider */
+clk_hw = kzalloc(sizeof(struct clk_hw_omap), GFP_KERNEL);

checkpatch complained:
CHECK: Prefer kzalloc(sizeof(*clk_hw)...) over kzalloc(sizeof(struct
clk_hw_omap)...)


Hmm, I didn't get this with checkpatch. Some special option/version you
use? I still see both types of sizeof used in the kernel.

use checkpatch with --strict option





or given we have dev, devm_kzalloc?


Actually we don't have dev, check how this is called from below.


ok.




+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);

what if init fails? and it is in mach-omap2 at this point in time?


Yea, this just calls the autoidle init part under mach-omap2.


we should make this independent of mach-omap2. calls should be made to 
here if needed from mach-omap2 instead of the other way around. OR the 
required code should be moved over here.







+
+return clk;
+}



snip



+
+if (of_property_read_bool(node, ti,dpll-j-type)) {
+dd-sddiv_mask = 0xff00;
+mult_mask = 0xfff  8;
+div1_mask = 0xff;
+max_multiplier = 4095;
+max_divider = 256;
+}
+
+if (of_property_read_bool(node, ti,dpll-regm4xen)) {

I think we need bindings to understand this better.


Or documentation you mean?


yes. Documentation/devicetree/bindings/






+dd-m4xen_mask = 0x800;
+dd-lpmode_mask = 1  10;
+}
+
+dd-mult_mask = mult_mask;
+dd-div1_mask = 

Re: [PATCHv4 03/33] CLK: OMAP4: Add DPLL clock support

2013-08-01 Thread Tero Kristo

On 08/01/2013 05:00 PM, Nishanth Menon wrote:

On 07/31/2013 04:46 AM, Tero Kristo wrote:

On 07/30/2013 07:23 PM, Nishanth Menon wrote:

This patch probably was submitted in the wrong sequence - fails build
and few other issues below.


Yeah, I'll double check the build sequence for these.



On 07/23/2013 02:19 AM, Tero Kristo wrote:

The OMAP clock driver now supports DPLL clock type. This patch also
adds support for DT DPLL nodes.


Then why is $subject specific to OMAP4? is that because of
of_omap4_dpll_setup?


The driver only supports omap4 dpll type at this point, omap3 dplls
require some modifications on top of this, and are provided later in the
series.


ok.







Signed-off-by: Tero Kristo t-kri...@ti.com
---
  drivers/clk/omap/Makefile |2 +-
  drivers/clk/omap/clk.c|1 +
  drivers/clk/omap/dpll.c   |  295
+


Device Tree Binding documentation?


Didn't bother writing those yet as I haven't received too much feedback
whether this approach is acceptable or not.


Documentation helps simplify the understanding of expected usage - this
is useful to review approach as well :)


True, I'll try adding docs for next rev.




diff --git a/drivers/clk/omap/clk.c b/drivers/clk/omap/clk.c
index 4bf1929..1dafdaa 100644
--- a/drivers/clk/omap/clk.c
+++ b/drivers/clk/omap/clk.c
@@ -28,6 +28,7 @@ static const struct of_device_id clk_match[] = {
  .data = of_fixed_factor_clk_setup, },
  {.compatible = divider-clock, .data = of_divider_clk_setup, },
  {.compatible = gate-clock, .data = of_gate_clk_setup, },
+{.compatible = ti,omap4-dpll-clock, .data =
of_omap4_dpll_setup, },
  {},
  };

you would not need this if you did just of_clk_init(NULL); ?


Why not? And I think we still need to do this.


CLK_OF_DECLARE will take care of having all required clks registered
of_clk_init(NULL); will pick up from that list.

that will remove all extra exports, and make clk.c redundant.
[...]


Yep, as agreed on previous patch.








+#include linux/clk/omap.h


why?


Need dpll_data definition for example.

OK - without the ordering of patches, it was not obvious. structures aside,

We should move the functions to this file instead and empty out
mach-omap2 gradually, omap_dpll.h should be exported and used by
mach-omap2, rather than the other way around.


Yeah, the clock stuff should evolve and move here. I just need to start 
from somewhere.





+
+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)


why should this be public?


Currently does not need to be, but someone could in theory build up
cclockXxxx_data.c file and use these calls from there. Kind of legacy
support, see some of the basic clk types. I guess I can add static to
this, and remove some of the params along.



thanks. we should keep unneeded stuff in static unless a specific
provable need really arises.



I am assuming you do not do parameter check as you expect clk_register
to do the same?


True, so I'll change the above function to static.




+
+/* allocate the divider */
+clk_hw = kzalloc(sizeof(struct clk_hw_omap), GFP_KERNEL);

checkpatch complained:
CHECK: Prefer kzalloc(sizeof(*clk_hw)...) over kzalloc(sizeof(struct
clk_hw_omap)...)


Hmm, I didn't get this with checkpatch. Some special option/version you
use? I still see both types of sizeof used in the kernel.

use checkpatch with --strict option


Okay.







or given we have dev, devm_kzalloc?


Actually we don't have dev, check how this is called from below.


ok.




+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);

what if init fails? and it is in mach-omap2 at this point in time?


Yea, this just calls the autoidle init part under mach-omap2.


we should make this independent of mach-omap2. calls should be made to
here if needed from mach-omap2 instead of the other way around. OR the
required code should be moved over here.


Same comment as above, I did not want to move the allow / deny idle 
portion of every possible clock under drivers/clk/omap yet. This is on 
the evolution path for this driver.









+
+return clk;
+}



snip



+
+if (of_property_read_bool(node, ti,dpll-j-type)) {
+dd-sddiv_mask = 0xff00;
+mult_mask = 0xfff  8;
+

Re: [PATCHv4 03/33] CLK: OMAP4: Add DPLL clock support

2013-08-01 Thread Nishanth Menon

On 08/01/2013 03:29 AM, Rajendra Nayak wrote:

Tero,

On Tuesday 23 July 2013 12:49 PM, Tero Kristo wrote:

+   dd-control_reg = of_iomap(node, 0);
+   dd-idlest_reg = of_iomap(node, 1);
+   dd-autoidle_reg = of_iomap(node, 2);
+   dd-mult_div1_reg = of_iomap(node, 3);
+

[]...

+   reg = of_iomap(node, 0);


Doing an of_iomap() for every single clock register seems like an overkill
and might have performance penalties at boot.


the other option might be to use offset and a single allocation - but I 
think Tero should comment if this is possible or if registers on some 
SoCs are strewn all over the place


--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv4 03/33] CLK: OMAP4: Add DPLL clock support

2013-08-01 Thread Nishanth Menon
On Thu, Aug 1, 2013 at 10:08 AM, Tero Kristo t-kri...@ti.com wrote:
 We should move the functions to this file instead and empty out
 mach-omap2 gradually, omap_dpll.h should be exported and used by
 mach-omap2, rather than the other way around.


 Yeah, the clock stuff should evolve and move here. I just need to start from
 somewhere.

yep, this step is good and as part of this step, moving the used
functions from mach-omap2 to dpll.c is indeed the right step for us to
do proper cleanup.

it is a question of what we bring into drivers/clk and ensuring what
we bring in is clean as well.

Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv4 03/33] CLK: OMAP4: Add DPLL clock support

2013-08-01 Thread Tero Kristo

On 08/01/2013 06:10 PM, Nishanth Menon wrote:

On 08/01/2013 03:29 AM, Rajendra Nayak wrote:

Tero,

On Tuesday 23 July 2013 12:49 PM, Tero Kristo wrote:

+dd-control_reg = of_iomap(node, 0);
+dd-idlest_reg = of_iomap(node, 1);
+dd-autoidle_reg = of_iomap(node, 2);
+dd-mult_div1_reg = of_iomap(node, 3);
+

[]...

+reg = of_iomap(node, 0);


Doing an of_iomap() for every single clock register seems like an
overkill
and might have performance penalties at boot.


the other option might be to use offset and a single allocation - but I
think Tero should comment if this is possible or if registers on some
SoCs are strewn all over the place


Well, currently the basic clock nodes also do their individual 
of_iomaps, so doing a tweak only for the OMAP DPLLs is not going to 
change the figure much.


A generic solution is needed but I think this was commented elsewhere by 
Mike to remain as future optimization (can't find the reference to this 
with a quick search though.)


-Tero

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv4 03/33] CLK: OMAP4: Add DPLL clock support

2013-07-31 Thread Tero Kristo

On 07/30/2013 07:23 PM, Nishanth Menon wrote:

This patch probably was submitted in the wrong sequence - fails build
and few other issues below.


Yeah, I'll double check the build sequence for these.



On 07/23/2013 02:19 AM, Tero Kristo wrote:

The OMAP clock driver now supports DPLL clock type. This patch also
adds support for DT DPLL nodes.


Then why is $subject specific to OMAP4? is that because of
of_omap4_dpll_setup?


The driver only supports omap4 dpll type at this point, omap3 dplls 
require some modifications on top of this, and are provided later in the 
series.






Signed-off-by: Tero Kristo t-kri...@ti.com
---
  drivers/clk/omap/Makefile |2 +-
  drivers/clk/omap/clk.c|1 +
  drivers/clk/omap/dpll.c   |  295
+


Device Tree Binding documentation?


Didn't bother writing those yet as I haven't received too much feedback 
whether this approach is acceptable or not.





  3 files changed, 297 insertions(+), 1 deletion(-)
  create mode 100644 drivers/clk/omap/dpll.c

diff --git a/drivers/clk/omap/Makefile b/drivers/clk/omap/Makefile
index 8195931..4cad480 100644
--- a/drivers/clk/omap/Makefile
+++ b/drivers/clk/omap/Makefile
@@ -1 +1 @@
-obj-y+= clk.o
+obj-y+= clk.o dpll.o
diff --git a/drivers/clk/omap/clk.c b/drivers/clk/omap/clk.c
index 4bf1929..1dafdaa 100644
--- a/drivers/clk/omap/clk.c
+++ b/drivers/clk/omap/clk.c
@@ -28,6 +28,7 @@ static const struct of_device_id clk_match[] = {
  .data = of_fixed_factor_clk_setup, },
  {.compatible = divider-clock, .data = of_divider_clk_setup, },
  {.compatible = gate-clock, .data = of_gate_clk_setup, },
+{.compatible = ti,omap4-dpll-clock, .data =
of_omap4_dpll_setup, },
  {},
  };

you would not need this if you did just of_clk_init(NULL); ?


Why not? And I think we still need to do this.



Further, at this patch, build fails with:
drivers/clk/omap/clk.c:31:55: error: undefined identifier
'of_omap4_dpll_setup'
drivers/clk/omap/clk.c:31:48: error: ‘of_omap4_dpll_setup’ undeclared
here (not in a function)

which makes sense since we did not export the function.


Yea seems like wrong ordering of patches, sorry about that. .



diff --git a/drivers/clk/omap/dpll.c b/drivers/clk/omap/dpll.c
new file mode 100644
index 000..66e82be
--- /dev/null
+++ b/drivers/clk/omap/dpll.c
@@ -0,0 +1,295 @@
+/*
+ * OMAP DPLL clock support
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * Tero Kristo t-kri...@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/module.h
+#include linux/slab.h
+#include linux/io.h
+#include linux/err.h
+#include linux/string.h
+#include linux/log2.h
+#include linux/of.h
+#include linux/of_address.h

after a quick check, are all these required?


Seems like some might not be needed, I'll double check this.




+#include linux/clk/omap.h


why?


Need dpll_data definition for example.




+
+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 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_x2_ck_ops = {
+.recalc_rate= omap3_clkoutx2_recalc,
+};


none of these are defined at this stage of the patch, generates a huge
build error for dpll.c
http://pastebin.com/GJucv1A5


Yea, wrong ordering, linux/clk/omap.h is not up to date. I'll fix this 
and rest of the similar issues.



+
+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)


why should this be public?


Currently does not need to be, but someone could in theory build up 
cclockXxxx_data.c file and use these calls 

Re: [PATCHv4 03/33] CLK: OMAP4: Add DPLL clock support

2013-07-30 Thread Nishanth Menon
This patch probably was submitted in the wrong sequence - fails build 
and few other issues below.


On 07/23/2013 02:19 AM, Tero Kristo wrote:

The OMAP clock driver now supports DPLL clock type. This patch also
adds support for DT DPLL nodes.


Then why is $subject specific to OMAP4? is that because of 
of_omap4_dpll_setup?




Signed-off-by: Tero Kristo t-kri...@ti.com
---
  drivers/clk/omap/Makefile |2 +-
  drivers/clk/omap/clk.c|1 +
  drivers/clk/omap/dpll.c   |  295 +


Device Tree Binding documentation?


  3 files changed, 297 insertions(+), 1 deletion(-)
  create mode 100644 drivers/clk/omap/dpll.c

diff --git a/drivers/clk/omap/Makefile b/drivers/clk/omap/Makefile
index 8195931..4cad480 100644
--- a/drivers/clk/omap/Makefile
+++ b/drivers/clk/omap/Makefile
@@ -1 +1 @@
-obj-y  += clk.o
+obj-y  += clk.o dpll.o
diff --git a/drivers/clk/omap/clk.c b/drivers/clk/omap/clk.c
index 4bf1929..1dafdaa 100644
--- a/drivers/clk/omap/clk.c
+++ b/drivers/clk/omap/clk.c
@@ -28,6 +28,7 @@ static const struct of_device_id clk_match[] = {
.data = of_fixed_factor_clk_setup, },
{.compatible = divider-clock, .data = of_divider_clk_setup, },
{.compatible = gate-clock, .data = of_gate_clk_setup, },
+   {.compatible = ti,omap4-dpll-clock, .data = of_omap4_dpll_setup, },
{},
  };

you would not need this if you did just of_clk_init(NULL); ?

Further, at this patch, build fails with:
drivers/clk/omap/clk.c:31:55: error: undefined identifier 
'of_omap4_dpll_setup'
drivers/clk/omap/clk.c:31:48: error: ‘of_omap4_dpll_setup’ undeclared 
here (not in a function)


which makes sense since we did not export the function. 


diff --git a/drivers/clk/omap/dpll.c b/drivers/clk/omap/dpll.c
new file mode 100644
index 000..66e82be
--- /dev/null
+++ b/drivers/clk/omap/dpll.c
@@ -0,0 +1,295 @@
+/*
+ * OMAP DPLL clock support
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * Tero Kristo t-kri...@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/module.h
+#include linux/slab.h
+#include linux/io.h
+#include linux/err.h
+#include linux/string.h
+#include linux/log2.h
+#include linux/of.h
+#include linux/of_address.h

after a quick check, are all these required?


+#include linux/clk/omap.h


why?


+
+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 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_x2_ck_ops = {
+   .recalc_rate= omap3_clkoutx2_recalc,
+};


none of these are defined at this stage of the patch, generates a huge 
build error for dpll.c

http://pastebin.com/GJucv1A5

+
+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)


why should this be public?


+{
+   struct clk *clk;
+   struct clk_init_data init;


init = { 0 };  just to future proof?


+   struct clk_hw_omap *clk_hw;


does not exist yet in generic header?

I am assuming you do not do parameter check as you expect clk_register 
to do the same?



+
+   /* allocate the divider */
+   clk_hw = kzalloc(sizeof(struct clk_hw_omap), GFP_KERNEL);

checkpatch complained:
CHECK: Prefer kzalloc(sizeof(*clk_hw)...) over kzalloc(sizeof(struct 
clk_hw_omap)...)


or given we have dev, devm_kzalloc?

+   if (!clk_hw) {
+   pr_err(%s: could not allocate clk_hw_omap\n, __func__);
+   return ERR_PTR(-ENOMEM);
+   }
+
+   

[PATCHv4 03/33] CLK: OMAP4: Add DPLL clock support

2013-07-23 Thread Tero Kristo
The OMAP clock driver now supports DPLL clock type. This patch also
adds support for DT DPLL nodes.

Signed-off-by: Tero Kristo t-kri...@ti.com
---
 drivers/clk/omap/Makefile |2 +-
 drivers/clk/omap/clk.c|1 +
 drivers/clk/omap/dpll.c   |  295 +
 3 files changed, 297 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/omap/dpll.c

diff --git a/drivers/clk/omap/Makefile b/drivers/clk/omap/Makefile
index 8195931..4cad480 100644
--- a/drivers/clk/omap/Makefile
+++ b/drivers/clk/omap/Makefile
@@ -1 +1 @@
-obj-y  += clk.o
+obj-y  += clk.o dpll.o
diff --git a/drivers/clk/omap/clk.c b/drivers/clk/omap/clk.c
index 4bf1929..1dafdaa 100644
--- a/drivers/clk/omap/clk.c
+++ b/drivers/clk/omap/clk.c
@@ -28,6 +28,7 @@ static const struct of_device_id clk_match[] = {
.data = of_fixed_factor_clk_setup, },
{.compatible = divider-clock, .data = of_divider_clk_setup, },
{.compatible = gate-clock, .data = of_gate_clk_setup, },
+   {.compatible = ti,omap4-dpll-clock, .data = of_omap4_dpll_setup, },
{},
 };
 
diff --git a/drivers/clk/omap/dpll.c b/drivers/clk/omap/dpll.c
new file mode 100644
index 000..66e82be
--- /dev/null
+++ b/drivers/clk/omap/dpll.c
@@ -0,0 +1,295 @@
+/*
+ * OMAP DPLL clock support
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * Tero Kristo t-kri...@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/module.h
+#include linux/slab.h
+#include linux/io.h
+#include linux/err.h
+#include linux/string.h
+#include linux/log2.h
+#include linux/of.h
+#include linux/of_address.h
+#include linux/clk/omap.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 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_x2_ck_ops = {
+   .recalc_rate= omap3_clkoutx2_recalc,
+};
+
+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;
+   struct clk_hw_omap *clk_hw;
+
+   /* allocate the divider */
+   clk_hw = kzalloc(sizeof(struct clk_hw_omap), 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;
+}
+
+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;
+   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(struct clk_hw_omap), GFP_KERNEL);
+   if (!clk_hw) {
+   pr_err(%s: could not allocate clk_hw_omap\n, __func__);
+   return