Hi Jeremy,
Thanks for comments. Please see my further discussion.
- Create clock information based on common clock device, more specific,
based on struct clk_lookup. Since platform drivers are supposed to
register
their clock using 'void clkdev_add(struct clk_lookup *cl)', clk_lookup should
contain
enough information for clock display, like clock name and platform clk definition 'struck clk'. An extra field need to be added into clk_lookup
is
a 'struct dentry *' to store a pointer of the dentry node which will be exported in debug fs, like below: struct clk_lookup { struct list_head node; const char *dev_id; const char *con_id; struct clk *clk;
+#if defined(CONFIG_PM_DEBUG) && defined(CONFIG_DEBUG_FS)
struct dentry *dent;+#endif };
I think that struct clk_lookup is the wrong place to put this information; you want to have debug information about the clock, so it should live in struct clk. struct clk_lookup represents a relationship between a clock and (potentially) a device using that clock, and this is why you're having the naming issues.
True. I misunderstood the purpose of clk_lookup.
Unfortunately, that means modifying the 21-or-so different definitions of struct clk. I'm guessing this is why you decided on using clk_lookup instead :)
This is something I try to avoid. Originally, I thought I could write a unified interface for all the platform without caring about different clock definitions.
I'd see this better done as adding a 'const char name[CLK_NAME_LEN]' to struct clk, and populating this in the relevant platform code. Using the common clk API, you'd just have to change the one 'struct clk'. We already have a name parameter to INIT_CLK, so we could trivially set the name:
This would give us:
#define CLK_NAME_LEN 16
struct clk { const struct clk_ops *ops; unsigned int enable_count; struct mutex mutex; const char name[CLK_NAME_LEN]; };
#define INIT_CLK(name, o) { \ .ops = &o, \ .enable_count = 0, \ .lock.mutex = __MUTEX_INITIALIZER(name.lock.mutex), \ .name = #name, \ }
This means that you have already forced each SOC to have a unified 'struct
clk', or at least force some fields inside the struct tp be unified, right? My understanding is that leaving 'struct clk' definition to SOC related code is to give freedom for 'struct clk' definition to SOC vendors and they can define their own 'struct clk' as they want. If 'struct clk' has this much in common, why cannot we define it or define some common fields in common code, say, <linux/clk.h> or <asm/clkdev.h>?
I think that allocating a dentry per clk_lookup is quite heavyweight; we
could just expose one debugfs file, with the whole set of clocks available through this file, one clock per line. The seq_file interface makes this fairly straightforward to do.
This is another way to expose information, but we plan to have a more vivid
tree-like clock information, which had been adopted by omap already. The whole clock information is presented in a directory with many sub directories and sub nodes and powerdebug will walk through the directories to create tree-like output.
- Currently, 'struct clk' is defined in platform related code, common
clk interface did not impose any assumption on how each platform define their own 'struct clk', therefore, clock debug interface can not make any assumption on how much information the platform defined clock contains as well. So far, only clk_get_rate is there as a common defined interface which I can use to export clock rate information, and I plan to add some optional interface like clk_get_count, clk_get_flag, so more useful information can be displayed.
You probably don't need to add the clk_get_count parameter, as this isn't something we would want to expose to drivers through the clk API; instead, just access clk->enable_count directly. With the common clk API, this will be valid on all platforms, as struct clk always has the enable_count. I'm not sure what information you want to expose with clk_get_flag, but the same might apply there too.
About what can be exposed, we can decide later, they don't have to be flag or count, but the goal is to provide useful information as we can.
If you like, after I've finished some updates to the common clock code, I can put together a patch to deomstrate what I mean. It shouldn't be too long :)
Can't wait to see that. Can I know a rough schedule of that? Since this is a high priority task for me on the blueprints, I am keen to know more. Maybe if you have draft code done, I can have test as soon as possible.
Regards,
Jeremy