Hi Yong,
To do so, I need to make somethings done, and some of them are related to common clock code, for which I am more than happy to hear your comments.
Definitely, I'm happy to help out.
- 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.
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 :)
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, \ }
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.
- 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.
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 :)
Regards,
Jeremy