From: Cousson, BenoitOK
Sent: Tuesday, August 24, 2010 1:45 AM
Hi Partha,
On 8/23/2010 5:44 PM, Basak, Partha wrote:From: Basak, Partha<p-basak2@xxxxxx>function
For every optional clock present per hwmod per omap-device, thisadds an entry in the clocks list of the form<dev-id=dev_name, con-id=role>,if an entry is already present in the list of the form<dev-id=NULL, con-id=role>.omap_device_build_ss(),
The function is called from within the framework insideafter omap_device_register.role
This allows drivers to get a pointer to its optional clocks based on itsby calling clk_get(<dev*>,<role>).++++++++++++++++++++++++++++++-
Link to discussions related to this patch:
http://www.spinics.net/lists/linux-omap/msg34809.html
Signed-off-by: Charulatha V<charu@xxxxxx>
Signed-off-by: Basak, Partha<p-basak2@xxxxxx>
Signed-off-by: Benoit Cousson<b-cousson@xxxxxx>
Signed-off-by: Rajendra Nayak<rnayak@xxxxxx>
Cc: Paul Walmsley<paul@xxxxxxxxx>
Cc: Kevin Hilman<khilman@xxxxxxxxxxxxxxxxxxx>
---
arch/arm/plat-omap/omap_device.c | 31
1 files changed, 30 insertions(+), 1 deletions(-)02:48:30.789079550 +0530
Index: linux-omap-pm/arch/arm/plat-omap/omap_device.c
===================================================================
--- linux-omap-pm.orig/arch/arm/plat-omap/omap_device.c 2010-08-18
+++ linux-omap-pm/arch/arm/plat-omap/omap_device.c 2010-08-2407:19:43.637080138 +0530@@ -82,6 +82,7 @@
#include<linux/slab.h>
#include<linux/err.h>
#include<linux/io.h>
+#include<linux/clk.h>
#include<plat/omap_device.h>
#include<plat/omap_hwmod.h>
@@ -243,7 +244,6 @@ static inline struct omap_device *_find_
return container_of(pdev, struct omap_device, pdev);
}
-
That fix is not related to the patch subject.
/* Public functions for use by core code */
/**
@@ -271,6 +271,47 @@ int omap_device_count_resources(struct o
}
This is not a public function, so you should move it before the /*
Public functions for use by core code */
Not clear on your comment, wanted to take care of the scenario where a device is having multiple hwmods/**function
+ * omap_device_add_opt_clk_alias - Add alias for optional clocks in the
+ * clocks list.
+ *
+ * @od: struct omap_device *od
+ *
+ * For every optional clock present per hwmod per omap-device, this
+ * adds an entry in the clocks list of the form<dev-id=dev_name, con-id=role>+ * if an entry is already present in it with the form<dev-id=NULL,con-id=role>+ *its role
+ * The function is called from inside omap_device_build_ss(),
+ * after omap_device_register.
+ *
+ * This allows drivers to get a pointer to its optional clocks based on
+ * by calling clk_get(<dev*>,<role>).
+ */
+
+static void omap_device_add_opt_clk_alias(struct omap_device *od)
+{
+ int i, j;
+ struct omap_hwmod_opt_clk *oc;
+ struct omap_hwmod *oh;
+
+ for (i = 0, oh = *od->hwmods; i< od->hwmods_cnt; i++, oh++)
You can avoid that iteration and the extra level of indentation by
re-using the iteration done before the call to that function.
+ /* Add Clock alias for all optional clocks*/
+ for (j = oh->opt_clks_cnt, oc = oh->opt_clks;
+ j> 0; j--, oc++) {
+ if ((oc->_clk)&&
+ (IS_ERR(clk_get(&od->pdev.dev, oc->role)))) {
+ int ret;
You can avoid the indentation by returning in case of error.Agreed
You can avoid as well 2 pairs of parens.
+
+ ret = clk_add_alias(oc->role,
+ dev_name(&od->pdev.dev),
+ (char *)oc->clk, NULL);
The indentation is not align with the inner parameters... which is not
easy in your case due to the too many indentation level you have in this
function.
+ if (ret)
+ dev_err(&od->pdev.dev, "omap_device:\
This is not correct way of splitting long string, use "omap_device:"
instead.
Even if dev_err is usable because you have the dev pointer, it is in
fact an error from the omap_device code code, so using pr_err is
probably more appropriate (TBC).
+ clk_add_alias for %s failed\n",
+ oc->role);
+ }
+ }
+}
+/**
* omap_device_fill_resources - fill in array of struct resource
* @od: struct omap_device *
* @res: pointer to an array of struct resource to be filled in
@@ -424,6 +465,12 @@ struct omap_device *omap_device_build_ss
for (i = 0; i< oh_cnt; i++)
hwmods[i]->od = od;
You can call a per hwmod API here in order to avoid the iteration inside
the function.
I see your point, but most of the functions in this layer are having omap_device * as the parameter, so I maintained consistency