Adding linaro-dev to cc. Kernel consolidation WG might have comments.
On Tue, Oct 12, 2010 at 9:04 AM, Yong Shen yong.shen@linaro.org wrote:
Hi Amit and Jeremy,
This is not a patch review. But patch may better present my idea. Basically, I want to add some code in common clock code to export clock information, so every platform can benefit. This information is present in a tree-like pattern. Currently, each platform uses their own way to show clock info, which is hard to use a common user space tool to collect information. For this purpose, I need do the rest:
- Add a clock name check in the clkdev_add. We don't accept two clocks with
the same name to clkdev_add, do we? otherwise, it is impossible to create a tree-like structure under file system, cause no same names under a directory. 2. Recursive function creates the clock tree in debugfs, which referred omap's clock implementation. 3. Add interface needed to let mach related drivers to report their information. clk_get_rate is already there. Maybe we need clk_get_flags() and clk_get_usecount() and more.
Agreed, this functionality is necessary for common clk infrastructure to be useful.
We've also incorporated this functionality into a tool called powerdebug that'll show runtime state of the clock tree. This is very useful for driver developers.
/Amit
Below is the patch, please share your thoughts.
diff --git a/arch/arm/common/clkdev.c b/arch/arm/common/clkdev.c index e2b2bb6..2ec09ee 100644 --- a/arch/arm/common/clkdev.c +++ b/arch/arm/common/clkdev.c @@ -19,6 +19,7 @@ #include <linux/mutex.h> #include <linux/clk.h> #include <linux/slab.h> +#include <linux/debugfs.h>
#include <asm/clkdev.h> #include <mach/clkdev.h> @@ -95,6 +96,21 @@ EXPORT_SYMBOL(clk_put);
void clkdev_add(struct clk_lookup *cl) { + struct clk_lookup *p; + struct clk *clk = NULL;
+ list_for_each_entry(p, &clocks, node) { + if (p->dev_id) { + if (!cl->dev_id || strcmp(p->dev_id, cl->dev_id)) + continue; + } + if (p->con_id) { + if (!cl->con_id || strcmp(p->con_id, cl->con_id)) + continue; + }
+ return; + } mutex_lock(&clocks_mutex); list_add_tail(&cl->node, &clocks); mutex_unlock(&clocks_mutex); @@ -177,3 +193,131 @@ void clkdev_drop(struct clk_lookup *cl) kfree(cl); } EXPORT_SYMBOL(clkdev_drop);
+#if defined(CONFIG_PM_DEBUG) && defined(CONFIG_DEBUG_FS) +/*
- * debugfs support to trace clock tree hierarchy and attributes
- */
+static struct clk_lookup *clk_lookup_get_parent(struct clk *clk) +{ + struct clk_lookup *cl;
+ list_for_each_entry(cl, &clocks, node) { + if (cl->clk == clk_get_parent(clk)) + return cl; + }
+ return NULL; +} +static struct dentry *clk_debugfs_root;
+static int clk_debugfs_register_one(struct clk_lookup *cl) +{ + int err; + struct dentry *d, *child, *child_tmp; + struct clk_lookup *pa = clk_lookup_get_parent(cl->clk); + char s[255]; + char *p = s;
+ if (cl->dev_id && cl->con_id) + p += sprintf(p, "%s-%s", cl->dev_id, cl->con_id); + else if (cl->dev_id) + p += sprintf(p, "%s-NULL", cl->dev_id); + else if (cl->con_id) + p += sprintf(p, "NULL-%s", cl->con_id); + else + p += sprintf(p, "%s", "unknown");
+ printk("create dir %s, %s....\n", s, pa? "nonroot" : "root"); + d = debugfs_create_dir(s, pa ? pa->dent : clk_debugfs_root); + if (!d) { + printk("failed to create dir....\n"); + return -ENOMEM; + }
+ cl->dent = d;
+ if (cl->usecount != NULL) { + d = debugfs_create_u8("usecount", S_IRUGO, cl->dent, (u8 *)&cl->usecount); + if (!d) { + err = -ENOMEM; + goto err_out; + } + } + if (cl->rate) { + d = debugfs_create_u32("rate", S_IRUGO, cl->dent, (u32 *)&cl->rate); + if (!d) { + err = -ENOMEM; + goto err_out; + } + } + if (cl->flags) { + d = debugfs_create_x32("flags", S_IRUGO, cl->dent, (u32 *)&cl->flags); + if (!d) { + err = -ENOMEM; + goto err_out; + } + } + return 0;
+err_out: + printk("err out....\n"); + return -ENOMEM; + d = cl->dent; + list_for_each_entry_safe(child, child_tmp, &d->d_subdirs, d_u.d_child) + debugfs_remove(child); + debugfs_remove(cl->dent); + return err; +}
+static int clk_debugfs_register(struct clk_lookup *cl) +{ + int err; + struct clk_lookup *pa = clk_lookup_get_parent(cl->clk);
+ if (pa && !pa->dent) { + err = clk_debugfs_register(pa); + if (err) + return err; + }
+ if (!cl->dent) { + err = clk_debugfs_register_one(cl); + if (err) + return err; + } + return 0; +}
+static int __init clk_debugfs_init(void) +{ + struct clk_lookup *cl; + struct dentry *d; + int err;
+ d = debugfs_create_dir("clock", NULL); + if (!d) + return -ENOMEM; + clk_debugfs_root = d;
+ list_for_each_entry(cl, &clocks, node) { + //cl->usercount = clk_get_usercount(cl->clk); + //cl->flags = clk_get_flags(cl->clk); + cl->rate = clk_get_rate(cl->clk); + printk("dev name %s, con name %s\n",\ + cl->dev_id ? cl->dev_id : NULL,\ + cl->con_id ? cl->con_id : NULL); + err = clk_debugfs_register(cl); + if (err) + goto err_out; + } + return 0; +err_out: + printk("on my god ........\n"); + debugfs_remove_recursive(clk_debugfs_root); + return err; +} +late_initcall(clk_debugfs_init);
+#endif /* defined(CONFIG_PM_DEBUG) && defined(CONFIG_DEBUG_FS) */ diff --git a/arch/arm/include/asm/clkdev.h b/arch/arm/include/asm/clkdev.h index b56c138..51734ef 100644 --- a/arch/arm/include/asm/clkdev.h +++ b/arch/arm/include/asm/clkdev.h @@ -20,6 +20,13 @@ struct clk_lookup { const char *dev_id; const char *con_id; struct clk *clk;
+#if defined(CONFIG_PM_DEBUG) && defined(CONFIG_DEBUG_FS) + short usecount; + unsigned short flags; + unsigned long rate; + struct dentry *dent; +#endif };
struct clk_lookup *clkdev_alloc(struct clk *clk, const char *con_id
This is the information output from powerdebug after I apply the patch to mx51 kernel. / |-- fec.0-NULL <flags=0x0:rate=66500000:usecount=0> |-- NULL-gpt <flags=0x0:rate=8000000:usecount=0> |-- NULL-cko1 <flags=0x0:rate=8000000:usecount=0> |-- mxc_w1.0-NULL <flags=0x0:rate=8000000:usecount=0> |-- pata_fsl-NULL <flags=0x0:rate=66500000:usecount=0> |-- NULL-usb_clk <flags=0x0:rate=60000000:usecount=0> |-- NULL-usb_utmi_clk <flags=0x0:rate=60000000:usecount=0> |-- NULL-usb_ahb_clk <flags=0x0:rate=66500000:usecount=0> |-- NULL-iim_clk <flags=0x0:rate=66500000:usecount=0> |-- mxc_spi.2-NULL <flags=0x0:rate=66500000:usecount=0> |-- mxc_spi.1-NULL <flags=0x0:rate=6000000:usecount=0> |-- mxc_spi.0-NULL <flags=0x0:rate=6000000:usecount=0> |-- mxc_pwm.1-NULL <flags=0x0:rate=8000000:usecount=0> |-- mxc_pwm.0-NULL <flags=0x0:rate=8000000:usecount=0> |-- imx-i2c.1-NULL <flags=0x0:rate=8000000:usecount=0> |-- imx-i2c.0-NULL <flags=0x0:rate=8000000:usecount=0> |-- mxcintuart.2-NULL <flags=0x0:rate=66666666:usecount=0> |-- mxcintuart.1-NULL <flags=0x0:rate=66666666:usecount=0> |-- mxcintuart.0-NULL <flags=0x0:rate=66666666:usecount=0> |-- mxc_sdma-sdma_ipg_clk <flags=0x0:rate=66500000:usecount=0> |-- NULL-gpc_dvfs_clk <flags=0x0:rate=66500000:usecount=0> |-- NULL-ckil <flags=0x0:rate=32768:usecount=0> | |-- mxc_rtc.0-NULL <flags=0x0:rate=32768:usecount=0> | |-- NULL-lpsr_clk <flags=0x0:rate=32768:usecount=0> |-- NULL-ckih2 <flags=0x0:rate=24576000:usecount=0> |-- NULL-ckih <flags=0x0:rate=22579200:usecount=0> |-- NULL-osc <flags=0x0:rate=24000000:usecount=0> | |-- NULL-spdif_xtal_clk <flags=0x0:rate=24000000:usecount=0> | | |-- mxc_alsa_spdif.0-NULL <flags=0x0:rate=24000000:usecount=0> | |-- NULL-usb_phy1_clk <flags=0x0:rate=24000000:usecount=0> | |-- NULL-lp_apm <flags=0x0:rate=24000000:usecount=0> | | |-- NULL-ssi_lp_apm_clk <flags=0x0:rate=24000000:usecount=0> | | | |-- mxc_ssi.1-NULL <flags=0x0:rate=12000000:usecount=0> | | | | |-- NULL-ssi_ext2_clk <flags=0x0:rate=12000000:usecount=0> | | | |-- mxc_ssi.0-NULL <flags=0x0:rate=12000000:usecount=0> | | | | |-- NULL-ssi_ext1_clk <flags=0x0:rate=12000000:usecount=0> | |-- NULL-pll3 <flags=0x0:rate=216000000:usecount=0> | | |-- mxc_sim.0-NULL <flags=0x0:rate=54000000:usecount=0> | | |-- mxc_i2c_hs.3-NULL <flags=0x0:rate=54000000:usecount=0> | | |-- tve.0-NULL <flags=0x0:rate=216000000:usecount=0> | | |-- NULL-csi_mclk2 <flags=0x0:rate=54000000:usecount=0> | | |-- NULL-csi_mclk1 <flags=0x0:rate=54000000:usecount=0> | | |-- NULL-ipu_di1_clk <flags=0x0:rate=27000000:usecount=0> | | |-- NULL-ipu_di0_clk <flags=0x0:rate=27000000:usecount=0> | |-- NULL-pll2 <flags=0x0:rate=665000000:usecount=0> | | |-- mxsdhci.1-NULL <flags=0x0:rate=166250000:usecount=0> | | |-- mxsdhci.0-NULL <flags=0x0:rate=166250000:usecount=0> | | | |-- mxsdhci.3-NULL <flags=0x0:rate=166250000:usecount=0> | | | |-- mxsdhci.2-NULL <flags=0x0:rate=166250000:usecount=0>/q | | |-- NULL-usboh3_clk <flags=0x0:rate=66500000:usecount=0> | | |-- NULL-main_bus_clk <flags=0x0:rate=665000000:usecount=0> | | | |-- NULL-ahb_clk <flags=0x0:rate=133000000:usecount=0> | | | | |-- mxc_scc.0-NULL <flags=0x0:rate=133000000:usecount=0> | | | | |-- NULL-sahara_clk <flags=0x0:rate=133000000:usecount=0> | | | | |-- NULL-emi_intr_clk.1 <flags=0x0:rate=133000000:usecount=0> | | | | |-- NULL-emi_intr_clk.0 <flags=0x0:rate=133000000:usecount=0> | | | | |-- mxc_sdma-sdma_ahb_clk <flags=0x0:rate=133000000:usecount=0> | | | | |-- NULL-emi_slow_clk <flags=0x0:rate=133000000:usecount=0> | | | | | |-- NULL-emi_enfc_clk <flags=0x0:rate=33250000:usecount=0> | | | | | |-- NULL-nfc_clk <flags=0x0:rate=33250000:usecount=0> | | | | |-- NULL-ahb_max_clk <flags=0x0:rate=133000000:usecount=0> | | | |-- NULL-axi_b_clk <flags=0x0:rate=133000000:usecount=0> | | | | |-- mxc_vpu.0-NULL <flags=0x0:rate=133000000:usecount=0> | | | | |-- NULL-ipu_clk <flags=0x0:rate=133000000:usecount=0> | | | | | |-- NULL-mipi_hsp_clk <flags=0x0:rate=133000000:usecount=0> | | | | |-- NULL-vpu_core_clk <flags=0x0:rate=133000000:usecount=0> | | | | |-- NULL-vpu_clk <flags=0x0:rate=133000000:usecount=0> | | | |-- NULL-axi_a_clk <flags=0x0:rate=166250000:usecount=0> | | | | |-- NULL-gpu2d_clk <flags=0x0:rate=166250000:usecount=0> | | | | |-- NULL-garb_clk <flags=0x0:rate=166250000:usecount=0> | | | | |-- NULL-gpu3d_clk <flags=0x0:rate=166250000:usecount=0> | |-- NULL-pll1_main_clk <flags=0x0:rate=800000000:usecount=0> | | |-- NULL-pll1_sw_clk <flags=0x0:rate=800000000:usecount=0> | | | |-- NULL-ddr_hf_clk <flags=0x0:rate=0:usecount=0> | | | | |-- NULL-ddr_clk <flags=0x0:rate=200000000:usecount=0> | | | | | |-- NULL-emi_fast_clk <flags=0x0:rate=200000000:usecount=0> | | | |-- NULL-periph_apm_clk <flags=0x0:rate=800000000:usecount=0> | | | |-- NULL-cpu_clk <flags=0x0:rate=800000000:usecount=0>
Yong
On Tuesday 12 October 2010, Amit Kucheria wrote:
Adding linaro-dev to cc. Kernel consolidation WG might have comments.
On Tue, Oct 12, 2010 at 9:04 AM, Yong Shen yong.shen@linaro.org wrote:
Hi Amit and Jeremy,
This is not a patch review. But patch may better present my idea. Basically, I want to add some code in common clock code to export clock information, so every platform can benefit. This information is present in a tree-like pattern. Currently, each platform uses their own way to show clock info, which is hard to use a common user space tool to collect information. For this purpose, I need do the rest:
- Add a clock name check in the clkdev_add. We don't accept two clocks with
the same name to clkdev_add, do we? otherwise, it is impossible to create a tree-like structure under file system, cause no same names under a directory. 2. Recursive function creates the clock tree in debugfs, which referred omap's clock implementation. 3. Add interface needed to let mach related drivers to report their information. clk_get_rate is already there. Maybe we need clk_get_flags() and clk_get_usecount() and more.
Agreed, this functionality is necessary for common clk infrastructure to be useful.
I like the idea, too.
One question I immediately had was whether it should be integrated into sysfs or remain standalone in debugfs.
In general, no core functionality should require debugfs, so if we find it important enough to write user level tools on top of this, it should probably become a stable interface either in sysfs or its own "clkfs" file system if necessary.
The main disadvantage of sysfs is probably memory consumption, which will have to be evaluated carefully: on systems with little RAM but many clocks this might amount to a few percent of the system memory just to manage the inodes.
The main advantage of sysfs is that we can interface it with the device tree, e.g. have a "clk" symlink in each device pointing to the entry in the clock tree, and possibly vice versa.
Arnd
On Tue, 12 Oct 2010, Amit Kucheria wrote:
Adding linaro-dev to cc. Kernel consolidation WG might have comments.
On Tue, Oct 12, 2010 at 9:04 AM, Yong Shen yong.shen@linaro.org wrote:
Hi Amit and Jeremy,
This is not a patch review. But patch may better present my idea. Basically, I want to add some code in common clock code to export clock information, so every platform can benefit. This information is present in a tree-like pattern. Currently, each platform uses their own way to show clock info, which is hard to use a common user space tool to collect information. For this purpose, I need do the rest:
- Add a clock name check in the clkdev_add. We don't accept two clocks with
the same name to clkdev_add, do we? otherwise, it is impossible to create a tree-like structure under file system, cause no same names under a directory. 2. Recursive function creates the clock tree in debugfs, which referred omap's clock implementation. 3. Add interface needed to let mach related drivers to report their information. clk_get_rate is already there. Maybe we need clk_get_flags() and clk_get_usecount() and more.
Agreed, this functionality is necessary for common clk infrastructure to be useful.
We've also incorporated this functionality into a tool called powerdebug that'll show runtime state of the clock tree. This is very useful for driver developers.
Agreed.
Nicolas
On 10 Oct 12, Arnd Bergmann wrote:
On Tuesday 12 October 2010, Amit Kucheria wrote:
Adding linaro-dev to cc. Kernel consolidation WG might have comments.
On Tue, Oct 12, 2010 at 9:04 AM, Yong Shen yong.shen@linaro.org wrote:
Hi Amit and Jeremy,
This is not a patch review. But patch may better present my idea. Basically, I want to add some code in common clock code to export clock information, so every platform can benefit. This information is present in a tree-like pattern. Currently, each platform uses their own way to show clock info, which is hard to use a common user space tool to collect information. For this purpose, I need do the rest:
- Add a clock name check in the clkdev_add. We don't accept two clocks with
the same name to clkdev_add, do we? otherwise, it is impossible to create a tree-like structure under file system, cause no same names under a directory. 2. Recursive function creates the clock tree in debugfs, which referred omap's clock implementation. 3. Add interface needed to let mach related drivers to report their information. clk_get_rate is already there. Maybe we need clk_get_flags() and clk_get_usecount() and more.
Agreed, this functionality is necessary for common clk infrastructure to be useful.
I like the idea, too.
One question I immediately had was whether it should be integrated into sysfs or remain standalone in debugfs.
In general, no core functionality should require debugfs, so if we find it important enough to write user level tools on top of this, it should probably become a stable interface either in sysfs or its own "clkfs" file system if necessary.
That is something I've been wondering about too. At the moment, tools like powerdebug have to periodically re-read the entire clock tree to show updates. AFAIK, sysfs and debugfs don't support inotify/poll/select mechanisms to notify changes.
It would be nice to have notification, but I don't know how hard that would be for a virtual filesystem.
The main disadvantage of sysfs is probably memory consumption, which will have to be evaluated carefully: on systems with little RAM but many clocks this might amount to a few percent of the system memory just to manage the inodes.
The main advantage of sysfs is that we can interface it with the device tree, e.g. have a "clk" symlink in each device pointing to the entry in the clock tree, and possibly vice versa.
Agreed.
/Amit
On Wednesday 13 October 2010 08:49:42 Amit Kucheria wrote:
That is something I've been wondering about too. At the moment, tools like powerdebug have to periodically re-read the entire clock tree to show updates. AFAIK, sysfs and debugfs don't support inotify/poll/select mechanisms to notify changes.
It would be nice to have notification, but I don't know how hard that would be for a virtual filesystem.
Sysfs normally uses udev events to notify the user of changes. Not sure what the mechanics behind this are, probably a netlink socket.
Supporting fanotify or inotify in your own file system should be very easy, the somewhat harder part is to write the file system in the first place.
Arnd
Thanks for your guys' brainstorm. I will take all the thoughts into account while implementing this.
On Wed, Oct 13, 2010 at 2:49 PM, Amit Kucheria amit.kucheria@linaro.orgwrote:
On 10 Oct 12, Arnd Bergmann wrote:
On Tuesday 12 October 2010, Amit Kucheria wrote:
Adding linaro-dev to cc. Kernel consolidation WG might have comments.
On Tue, Oct 12, 2010 at 9:04 AM, Yong Shen yong.shen@linaro.org
wrote:
Hi Amit and Jeremy,
This is not a patch review. But patch may better present my idea.
Basically,
I want to add some code in common clock code to export clock
information, so
every platform can benefit. This information is present in a
tree-like
pattern. Currently, each platform uses their own way to show clock info, which
is
hard to use a common user space tool to collect information. For this purpose, I need do the rest:
- Add a clock name check in the clkdev_add. We don't accept two
clocks with
the same name to clkdev_add, do we? otherwise, it is impossible to
create a
tree-like structure under file system, cause no same names under a directory. 2. Recursive function creates the clock tree in debugfs, which
referred
omap's clock implementation. 3. Add interface needed to let mach related drivers to report their information. clk_get_rate is already there. Maybe we need
clk_get_flags()
and clk_get_usecount() and more.
Agreed, this functionality is necessary for common clk infrastructure to be useful.
I like the idea, too.
One question I immediately had was whether it should be integrated into sysfs or remain standalone in debugfs.
In general, no core functionality should require debugfs, so if we find it important enough to write user level tools on top of this, it should probably become a stable interface either in sysfs or its own "clkfs" file system if necessary.
That is something I've been wondering about too. At the moment, tools like powerdebug have to periodically re-read the entire clock tree to show updates. AFAIK, sysfs and debugfs don't support inotify/poll/select mechanisms to notify changes.
It would be nice to have notification, but I don't know how hard that would be for a virtual filesystem.
The main disadvantage of sysfs is probably memory consumption, which will have to be evaluated carefully: on systems with little RAM but many
clocks
this might amount to a few percent of the system memory just to manage the inodes.
The main advantage of sysfs is that we can interface it with the device
tree,
e.g. have a "clk" symlink in each device pointing to the entry in the clock tree, and possibly vice versa.
Agreed.
/Amit
Hi,
Exporting the clock tree state is help us to monitor clocks state and to find the guilty clocks. But could it be also possible to have a write access to the clock tree ? During power consumption optimization step, we need to identify clock/regulator which should be disable but we also want to know what will be the gain. Instead of waiting for the new code with the right clock management, we generally want to directly disable it and look at the 1st result. It will be also useful when we are looking for the best clock tree configuration without developing/modifying a lot of source code.
For sure, such kind of feature is quite dangerous and must be enable carefully but it would help for optimizing power consumption.
Vincent
On 13 October 2010 12:40, Yong Shen yong.shen@linaro.org wrote:
Thanks for your guys' brainstorm. I will take all the thoughts into account while implementing this.
On Wed, Oct 13, 2010 at 2:49 PM, Amit Kucheria amit.kucheria@linaro.org wrote:
On 10 Oct 12, Arnd Bergmann wrote:
On Tuesday 12 October 2010, Amit Kucheria wrote:
Adding linaro-dev to cc. Kernel consolidation WG might have comments.
On Tue, Oct 12, 2010 at 9:04 AM, Yong Shen yong.shen@linaro.org wrote:
Hi Amit and Jeremy,
This is not a patch review. But patch may better present my idea. Basically, I want to add some code in common clock code to export clock information, so every platform can benefit. This information is present in a tree-like pattern. Currently, each platform uses their own way to show clock info, which is hard to use a common user space tool to collect information. For this purpose, I need do the rest:
- Add a clock name check in the clkdev_add. We don't accept two
clocks with the same name to clkdev_add, do we? otherwise, it is impossible to create a tree-like structure under file system, cause no same names under a directory. 2. Recursive function creates the clock tree in debugfs, which referred omap's clock implementation. 3. Add interface needed to let mach related drivers to report their information. clk_get_rate is already there. Maybe we need clk_get_flags() and clk_get_usecount() and more.
Agreed, this functionality is necessary for common clk infrastructure to be useful.
I like the idea, too.
One question I immediately had was whether it should be integrated into sysfs or remain standalone in debugfs.
In general, no core functionality should require debugfs, so if we find it important enough to write user level tools on top of this, it should probably become a stable interface either in sysfs or its own "clkfs" file system if necessary.
That is something I've been wondering about too. At the moment, tools like powerdebug have to periodically re-read the entire clock tree to show updates. AFAIK, sysfs and debugfs don't support inotify/poll/select mechanisms to notify changes.
It would be nice to have notification, but I don't know how hard that would be for a virtual filesystem.
The main disadvantage of sysfs is probably memory consumption, which will have to be evaluated carefully: on systems with little RAM but many clocks this might amount to a few percent of the system memory just to manage the inodes.
The main advantage of sysfs is that we can interface it with the device tree, e.g. have a "clk" symlink in each device pointing to the entry in the clock tree, and possibly vice versa.
Agreed.
/Amit
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
On Wed, Oct 13, 2010 at 4:35 PM, Vincent Guittot vincent.guittot@linaro.org wrote:
Hi,
Exporting the clock tree state is help us to monitor clocks state and to find the guilty clocks. But could it be also possible to have a write access to the clock tree ? During power consumption optimization
That is an interesting idea.
step, we need to identify clock/regulator which should be disable but we also want to know what will be the gain. Instead of waiting for the new code with the right clock management, we generally want to directly disable it and look at the 1st result. It will be also useful when we are looking for the best clock tree configuration without developing/modifying a lot of source code.
Do you anticipate any other interface besides making the 'rate' sysfs file RW and controlling the clock by writing various rates to the file?
So, 0 - clock off any other positive value - round down to the nearest valid rate
and writing to the file would lead to a call to clk.set_rate()
For sure, such kind of feature is quite dangerous and must be enable carefully but it would help for optimizing power consumption.
I'm not sure that all clock frameworks are even ready for this feature today, especially if they don't handle recalculation of child clock rates as part of set_rate().
/Amit
On 13 October 2010 16:14, Amit Kucheria amit.kucheria@linaro.org wrote:
On Wed, Oct 13, 2010 at 4:35 PM, Vincent Guittot vincent.guittot@linaro.org wrote:
Hi,
Exporting the clock tree state is help us to monitor clocks state and to find the guilty clocks. But could it be also possible to have a write access to the clock tree ? During power consumption optimization
That is an interesting idea.
step, we need to identify clock/regulator which should be disable but we also want to know what will be the gain. Instead of waiting for the new code with the right clock management, we generally want to directly disable it and look at the 1st result. It will be also useful when we are looking for the best clock tree configuration without developing/modifying a lot of source code.
Do you anticipate any other interface besides making the 'rate' sysfs file RW and controlling the clock by writing various rates to the file?
So, 0 - clock off any other positive value - round down to the nearest valid rate
and writing to the file would lead to a call to clk.set_rate()
my idea is to export the clock interface through the debugfs for the optimization phase. Then each machine implements and exports the supported function.
For sure, such kind of feature is quite dangerous and must be enable carefully but it would help for optimizing power consumption.
I'm not sure that all clock frameworks are even ready for this feature today, especially if they don't handle recalculation of child clock rates as part of set_rate().
/Amit
Amit Kucheria amit.kucheria@linaro.org writes:
Adding linaro-dev to cc. Kernel consolidation WG might have comments.
On Tue, Oct 12, 2010 at 9:04 AM, Yong Shen yong.shen@linaro.org wrote:
Hi Amit and Jeremy,
This is not a patch review. But patch may better present my idea. Basically, I want to add some code in common clock code to export clock information, so every platform can benefit. This information is present in a tree-like pattern. Currently, each platform uses their own way to show clock info, which is hard to use a common user space tool to collect information. For this purpose, I need do the rest:
- Add a clock name check in the clkdev_add. We don't accept two clocks with
the same name to clkdev_add, do we? otherwise, it is impossible to create a tree-like structure under file system, cause no same names under a directory. 2. Recursive function creates the clock tree in debugfs, which referred omap's clock implementation. 3. Add interface needed to let mach related drivers to report their information. clk_get_rate is already there. Maybe we need clk_get_flags() and clk_get_usecount() and more.
Agreed, this functionality is necessary for common clk infrastructure to be useful.
We've also incorporated this functionality into a tool called powerdebug that'll show runtime state of the clock tree. This is very useful for driver developers.
IMO, I think this is going somewhat down the wrong path, at least in terms of power management controls.
I know that most drivers assume that a clock is the primary PM "knob" for a given device, but there is *lots* more to device PM than clocks.
For that reason, on OMAP, we are now moving all drivers to use the runtime PM framework, so for power management drivers do not have to know anything about their clocks, and as a result most drivers don't have to know about *any* clocks. They just use runtime PM 'get' and 'put' calls when they want the device to be active.
That being said, I'm not against $SUBJECT patch, but I question it's usefulness in terms of PM. What's more valuable for PM is the statitics and knobs exported on a per-device level by the runtime PM core (and AFAICT, already included into tools like powertop.)
Kevin
On Sat, Oct 16, 2010 at 5:25 AM, Kevin Hilman khilman@deeprootsystems.com wrote:
[..]
That being said, I'm not against $SUBJECT patch, but I question it's usefulness in terms of PM. What's more valuable for PM is the statitics and knobs exported on a per-device level by the runtime PM core (and AFAICT, already included into tools like powertop.)
IIRC, Vincent's suggestion is more valid for the HW guys who want to test out all kinds of possibilities *before* the OS and the support for the drivers becomes mature enough to concentrate on statistics and so on. Even I remember our HW guys requesting for bersek controls over regulators and clocks as a part of platform validation, when the OS was not mature enough in terms of a complete system-support.
Cheers!
Believe me, it is very useful in both BSP development and SOC power management. In the company I am working for, people are keen to have this kind of info.
On Sat, Oct 16, 2010 at 7:55 AM, Kevin Hilman khilman@deeprootsystems.comwrote:
Amit Kucheria amit.kucheria@linaro.org writes:
Adding linaro-dev to cc. Kernel consolidation WG might have comments.
On Tue, Oct 12, 2010 at 9:04 AM, Yong Shen yong.shen@linaro.org wrote:
Hi Amit and Jeremy,
This is not a patch review. But patch may better present my idea.
Basically,
I want to add some code in common clock code to export clock
information, so
every platform can benefit. This information is present in a tree-like pattern. Currently, each platform uses their own way to show clock info, which is hard to use a common user space tool to collect information. For this purpose, I need do the rest:
- Add a clock name check in the clkdev_add. We don't accept two clocks
with
the same name to clkdev_add, do we? otherwise, it is impossible to
create a
tree-like structure under file system, cause no same names under a directory. 2. Recursive function creates the clock tree in debugfs, which referred omap's clock implementation. 3. Add interface needed to let mach related drivers to report their information. clk_get_rate is already there. Maybe we need
clk_get_flags()
and clk_get_usecount() and more.
Agreed, this functionality is necessary for common clk infrastructure to be useful.
We've also incorporated this functionality into a tool called powerdebug that'll show runtime state of the clock tree. This is very useful for driver developers.
IMO, I think this is going somewhat down the wrong path, at least in terms of power management controls.
I know that most drivers assume that a clock is the primary PM "knob" for a given device, but there is *lots* more to device PM than clocks.
For that reason, on OMAP, we are now moving all drivers to use the runtime PM framework, so for power management drivers do not have to know anything about their clocks, and as a result most drivers don't have to know about *any* clocks. They just use runtime PM 'get' and 'put' calls when they want the device to be active.
That being said, I'm not against $SUBJECT patch, but I question it's usefulness in terms of PM. What's more valuable for PM is the statitics and knobs exported on a per-device level by the runtime PM core (and AFAICT, already included into tools like powertop.)
Kevin
Yes, having a interface which can also control clocks is great. I used to write some code myself to debug clocks, so I have the same feeling with what you said.
BTW, 'cheers' is normally used in Australia and New Zealand. Are you from there? Just curious. :)
Yong
On Sat, Oct 16, 2010 at 1:34 PM, Sundar sunder.svit@gmail.com wrote:
On Sat, Oct 16, 2010 at 5:25 AM, Kevin Hilman khilman@deeprootsystems.com wrote:
[..]
That being said, I'm not against $SUBJECT patch, but I question it's usefulness in terms of PM. What's more valuable for PM is the statitics and knobs exported on a per-device level by the runtime PM core (and AFAICT, already included into tools like powertop.)
IIRC, Vincent's suggestion is more valid for the HW guys who want to test out all kinds of possibilities *before* the OS and the support for the drivers becomes mature enough to concentrate on statistics and so on. Even I remember our HW guys requesting for bersek controls over regulators and clocks as a part of platform validation, when the OS was not mature enough in terms of a complete system-support.
Cheers!
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Hi Kevin,
On Sat, Oct 16, 2010 at 2:55 AM, Kevin Hilman khilman@deeprootsystems.com wrote:
Amit Kucheria amit.kucheria@linaro.org writes:
Adding linaro-dev to cc. Kernel consolidation WG might have comments.
On Tue, Oct 12, 2010 at 9:04 AM, Yong Shen yong.shen@linaro.org wrote:
Hi Amit and Jeremy,
This is not a patch review. But patch may better present my idea. Basically, I want to add some code in common clock code to export clock information, so every platform can benefit. This information is present in a tree-like pattern. Currently, each platform uses their own way to show clock info, which is hard to use a common user space tool to collect information. For this purpose, I need do the rest:
- Add a clock name check in the clkdev_add. We don't accept two clocks with
the same name to clkdev_add, do we? otherwise, it is impossible to create a tree-like structure under file system, cause no same names under a directory. 2. Recursive function creates the clock tree in debugfs, which referred omap's clock implementation. 3. Add interface needed to let mach related drivers to report their information. clk_get_rate is already there. Maybe we need clk_get_flags() and clk_get_usecount() and more.
Agreed, this functionality is necessary for common clk infrastructure to be useful.
We've also incorporated this functionality into a tool called powerdebug that'll show runtime state of the clock tree. This is very useful for driver developers.
IMO, I think this is going somewhat down the wrong path, at least in terms of power management controls.
We're not approaching it solely from the angle of PM controls. This work is targeted towards platform consolidation across various SoCs.
I know that most drivers assume that a clock is the primary PM "knob" for a given device, but there is *lots* more to device PM than clocks.
Agreed wholeheartedly. Power domains, per-device runtime states, system states are all critical to satisfy the classic low-power usecases. To that end, we're working to consolidate the information exported by the building blocks (clock framework, regulator framework, cpuidle states, cpufreq states, etc.) and develop tools to provide more insight on the runtime state of the system. At the moment, every SoC vendor is reimplementing the wheel - we want to consolidate those bits first.
For that reason, on OMAP, we are now moving all drivers to use the runtime PM framework, so for power management drivers do not have to know anything about their clocks, and as a result most drivers don't have to know about *any* clocks. They just use runtime PM 'get' and 'put' calls when they want the device to be active.
We're tracking that work with a lot of interest.
That being said, I'm not against $SUBJECT patch, but I question it's usefulness in terms of PM. What's more valuable for PM is the statitics and knobs exported on a per-device level by the runtime PM core (and AFAICT, already included into tools like powertop.)
Other platforms aren't yet using the per-device runtime PM framework. But I am learning a lot from the conversion of OMAP drivers to use these new features. I'm also tracking the work being documented at http://www.omappedia.org/wiki/Power_Management_Domain_Wiki
Regards, Amit
On Sun, Oct 17, 2010 at 9:11 AM, Yong Shen yong.shen@linaro.org wrote:
BTW, 'cheers' is normally used in Australia and New Zealand. Are you from there? Just curious. :)
Oh really??..I am not aware of that all.I am from India :). IMO its cool.
cheers!
and the UK! Cheers, Dave
Sent from yet another ARM powered mobile device
On 17 Oct 2010, at 16:18, Sundar sunder.svit@gmail.com wrote:
On Sun, Oct 17, 2010 at 9:11 AM, Yong Shen yong.shen@linaro.org wrote:
BTW, 'cheers' is normally used in Australia and New Zealand. Are you from there? Just curious. :)
Oh really??..I am not aware of that all.I am from India :). IMO its cool.
cheers!
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Hi,
The goal of the patch is to provide an easy way to find the guilty clocks when we are optimizing the power consumption of a platform. I agree with you that it's not enough for a complete PM and it might be extended to other features (in addition to regulator framework). Such debug interface should used to test and find the optimized configuration that will be used by runtime PM framework as an example. Such optimization step takes a lot of time and a part of this time is generally used to export some debug interfaces and develop some tools.
Vincent
On 17 October 2010 13:55, Amit Kucheria amit.kucheria@linaro.org wrote:
Hi Kevin,
On Sat, Oct 16, 2010 at 2:55 AM, Kevin Hilman khilman@deeprootsystems.com wrote:
Amit Kucheria amit.kucheria@linaro.org writes:
Adding linaro-dev to cc. Kernel consolidation WG might have comments.
On Tue, Oct 12, 2010 at 9:04 AM, Yong Shen yong.shen@linaro.org wrote:
Hi Amit and Jeremy,
This is not a patch review. But patch may better present my idea. Basically, I want to add some code in common clock code to export clock information, so every platform can benefit. This information is present in a tree-like pattern. Currently, each platform uses their own way to show clock info, which is hard to use a common user space tool to collect information. For this purpose, I need do the rest:
- Add a clock name check in the clkdev_add. We don't accept two clocks with
the same name to clkdev_add, do we? otherwise, it is impossible to create a tree-like structure under file system, cause no same names under a directory. 2. Recursive function creates the clock tree in debugfs, which referred omap's clock implementation. 3. Add interface needed to let mach related drivers to report their information. clk_get_rate is already there. Maybe we need clk_get_flags() and clk_get_usecount() and more.
Agreed, this functionality is necessary for common clk infrastructure to be useful.
We've also incorporated this functionality into a tool called powerdebug that'll show runtime state of the clock tree. This is very useful for driver developers.
IMO, I think this is going somewhat down the wrong path, at least in terms of power management controls.
We're not approaching it solely from the angle of PM controls. This work is targeted towards platform consolidation across various SoCs.
I know that most drivers assume that a clock is the primary PM "knob" for a given device, but there is *lots* more to device PM than clocks.
Agreed wholeheartedly. Power domains, per-device runtime states, system states are all critical to satisfy the classic low-power usecases. To that end, we're working to consolidate the information exported by the building blocks (clock framework, regulator framework, cpuidle states, cpufreq states, etc.) and develop tools to provide more insight on the runtime state of the system. At the moment, every SoC vendor is reimplementing the wheel - we want to consolidate those bits first.
For that reason, on OMAP, we are now moving all drivers to use the runtime PM framework, so for power management drivers do not have to know anything about their clocks, and as a result most drivers don't have to know about *any* clocks. They just use runtime PM 'get' and 'put' calls when they want the device to be active.
We're tracking that work with a lot of interest.
That being said, I'm not against $SUBJECT patch, but I question it's usefulness in terms of PM. What's more valuable for PM is the statitics and knobs exported on a per-device level by the runtime PM core (and AFAICT, already included into tools like powertop.)
Other platforms aren't yet using the per-device runtime PM framework. But I am learning a lot from the conversion of OMAP drivers to use these new features. I'm also tracking the work being documented at http://www.omappedia.org/wiki/Power_Management_Domain_Wiki
Regards, Amit
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
-
Amit Kucheria -
Arnd Bergmann -
David Rusling -
Kevin Hilman -
Nicolas Pitre -
Sundar -
Vincent Guittot -
Yong Shen