Re: [PATCH 1/2] ARM: OMAP: CLKFW: Initial debugfs support for omap clock framework
Hi Paul, On Thu, 2008-04-17 at 14:47 -0600, ext Paul Walmsley wrote: Hello Igor, On Thu, 17 Apr 2008, Igor Stoppa wrote: On Thu, 2008-04-17 at 13:44 -0600, ext Paul Walmsley wrote: True, but if we can do a debugfs implementation first, then that seems like a good way to start, no? Userspace PM implementations are probably some months in the future, and we can mandate that debugfs be mounted for those. I can hardly see the benefit of a userspace implementation, considering the extra context switch required and the fact the in many cases clocks get enabled in response to irqs. I agree that we shouldn't try to move the clock framework to userspace. But, determining what OPP to switch to next, based on system load or other requirements published by drivers or userspace processes; or determining what power state to put powerdomains to -- it would be nice to experiment with a userspace governor for those tasks. It would certainly make development and testing easier. yes, it can be used to play with it in its infancy state, when one is still not looking for performance/stability stress testing. Afterward it simply doesn't fit the bill of leveraging the HW response time. Notice also that with QoS information coming from different sources, mostly low level ones, this will generate lots of cache trashing. In terms of the clock tree, it would be good to allow userspace-driven OPP changes, analogous to CPUFreq's userspace governor. [ In general, I agree that userspace should not be changing driver clocks directly, just like userspace should not be mucking around in /dev/mem directly :-) ] That also sounds akward at best: cpufreq (or similar) is much better suited for this sort of activity; userspace governor would be the userspace controller you refer to, but it is far from being optimal. Userspace should limit itself to changing policies. CPUFreq is good, but it does not manage non-CPU-frequency knobs very well, and there are plenty of those on OMAP3. But CPUFreq can be expanded. Call it systemfreq or whatever can be more appealing from an advertising point of view. For omap HW anything different from ondemand doesn't make much sense. Maybe performance, would be another viable option, in certain cases. Is there any reason why we should not allow the option of userspace OPP selection/powerdomain control for OMAP? If people don't want to use it, they don't have to :-) No, of course freedom is good. But it shouldn't be freedom of knowingly adopt suboptimal solutions just for the sake of diversity. An approach that doesn't involve requiring userspace components can be used also for more embedded systems where there might be no traditional userspace. A similar case would be in replacing CPUIdle with user space based decision, which is likely far from being optimal. -- Cheers, Igor --- Igor Stoppa Next Generation Software Nokia Devices RD - Helsinki -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: OMAP: CLKFW: Initial debugfs support for omap clock framework
Hello Hiroshi,
On Thu, 17 Apr 2008, Hiroshi DOYU wrote:
debugfs can provide the infrastructure to trace the dependencies of
clock tree hierarchy quite visibly. This patch enables to keep track
of clock tree hierarchy and expose their attributes under each clock
directry as below:
As a replacement for print_parents, which was read-only, it seems good to
me.
But it would be nice to be able to call into clock functions like
round_rate, set_rate, and set_parent via filesystem writes for debugging
purposes, and I don't think that debugfs supports this.
I have a patch here that I use for internal testing that builds the clock
tree in sysfs, and allows for round_rate/set_rate calls, along with
enable/disable testing. But sysfs may not be the right place. It doesn't
seem like there's any logical place to put it that would not involve major
upstream arm-wrestling. Maybe /sys/devices/platform?
There are also procfs and configfs, I suppose.
Obviously, I could continue to carry my debugging patch in my local tree
here. But you mention powerdomain links. I suspect that we will want to
have a standard place for all of these filesystem entities, particularly
since one could conceive of a completely userspace power management policy
that would control powerdomains and clockdomains via the filesystem. So
it probably makes sense to discuss this now.
A quick comment on the patch itself, inlined below.
+static int clk_debugfs_register_one(struct clk *c)
+{
+ int err;
+ struct dentry *d, *child;
+ struct clk *pa = c-parent;
+ char s[255];
+ char *p = s;
+
+ p += sprintf(p, %s, c-name);
+ if (c-id != 0)
+ sprintf(p, %d, c-id);
perhaps separate the ID and the clock name with a colon, if the ID exists?
- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: OMAP: CLKFW: Initial debugfs support for omap clock framework
On Thursday 17 April 2008, Paul Walmsley wrote: But it would be nice to be able to call into clock functions like round_rate, set_rate, and set_parent via filesystem writes for debugging purposes, and I don't think that debugfs supports this. It does, if you set up the files properly ... except maybe the set_parent stuff. - Dave -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: OMAP: CLKFW: Initial debugfs support for omap clock framework
Hello David, On Thu, 17 Apr 2008, David Brownell wrote: On Thursday 17 April 2008, Paul Walmsley wrote: But it would be nice to be able to call into clock functions like round_rate, set_rate, and set_parent via filesystem writes for debugging purposes, and I don't think that debugfs supports this. It does, if you set up the files properly ... except maybe the set_parent stuff. Could you be more specific? The only write support that I see in the debugfs API is to store bytes into memory locations. We'd need to call functions also upon writes. - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: OMAP: CLKFW: Initial debugfs support for omap clock framework
Hi, From: ext Paul Walmsley [EMAIL PROTECTED] Subject: Re: [PATCH 1/2] ARM: OMAP: CLKFW: Initial debugfs support for omap clock framework Date: Thu, 17 Apr 2008 12:45:02 -0600 (MDT) Hello David, On Thu, 17 Apr 2008, David Brownell wrote: On Thursday 17 April 2008, Paul Walmsley wrote: But it would be nice to be able to call into clock functions like round_rate, set_rate, and set_parent via filesystem writes for debugging purposes, and I don't think that debugfs supports this. It does, if you set up the files properly ... except maybe the set_parent stuff. Could you be more specific? The only write support that I see in the debugfs API is to store bytes into memory locations. We'd need to call functions also upon writes. debugfs_create_file(...fops) has take fops as its argument. I guess that this can deal with the above. For set_parent, would it be possible to do that by using debugfs_rename()? And if there will be a little possibility that sysfs attribute can be used by userland in the future, keeping sysfs instead of debugfs doesn't seem not so illegal, does it? Hiroshi DOYU -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: OMAP: CLKFW: Initial debugfs support for omap clock framework
Hello David, On Thu, 17 Apr 2008, David Brownell wrote: struct dentry *debugfs_create_file(const char *name, mode_t mode, struct dentry *parent, void *data, const struct file_operations *fops); ... provide a file_operations vector supporting writes. Or building more complex values with seq_file, etc. thanks, missed that. That would indeed work. - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: OMAP: CLKFW: Initial debugfs support for omap clock framework
Hello Hiroshi, David, On Thu, 17 Apr 2008, David Brownell wrote: On Thursday 17 April 2008, Hiroshi DOYU wrote: And if there will be a little possibility that sysfs attribute can be used by userland in the future, keeping sysfs instead of debugfs doesn't seem not so illegal, does it? True, but if we can do a debugfs implementation first, then that seems like a good way to start, no? Userspace PM implementations are probably some months in the future, and we can mandate that debugfs be mounted for those. I happen to think that the clock tree is sensitive enough that it should not be managed from userspace in production systems. (Except possibly through driver-specific APIs which ensure the right rules are followed.) Too easy to break things otherwise. In terms of the clock tree, it would be good to allow userspace-driven OPP changes, analogous to CPUFreq's userspace governor. [ In general, I agree that userspace should not be changing driver clocks directly, just like userspace should not be mucking around in /dev/mem directly :-) ] Clock tree changes (like OPP changes) which are not initiated by drivers do create some additional complexity. Drivers will need to be notified before and after these changes. It turns out that DSP Bridge already needs post-frequency-change notification for the DSP clock - there is a patch here which will soon be posted to add those. Pre-change notifiers are also needed to quiesce DMA, etc.; those are somewhat more difficult, but also in the works. - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: OMAP: CLKFW: Initial debugfs support for omap clock framework
Hi all, On Thu, 2008-04-17 at 13:44 -0600, ext Paul Walmsley wrote: Hello Hiroshi, David, On Thu, 17 Apr 2008, David Brownell wrote: On Thursday 17 April 2008, Hiroshi DOYU wrote: And if there will be a little possibility that sysfs attribute can be used by userland in the future, keeping sysfs instead of debugfs doesn't seem not so illegal, does it? True, but if we can do a debugfs implementation first, then that seems like a good way to start, no? Userspace PM implementations are probably some months in the future, and we can mandate that debugfs be mounted for those. I can hardly see the benefit of a userspace implementation, considering the extra context switch required and the fact the in many cases clocks get enabled in response to irqs. I happen to think that the clock tree is sensitive enough that it should not be managed from userspace in production systems. (Except possibly through driver-specific APIs which ensure the right rules are followed.) Too easy to break things otherwise. In terms of the clock tree, it would be good to allow userspace-driven OPP changes, analogous to CPUFreq's userspace governor. [ In general, I agree that userspace should not be changing driver clocks directly, just like userspace should not be mucking around in /dev/mem directly :-) ] That also sounds akward at best: cpufreq (or similar) is much better suited for this sort of activity; userspace governor would be the userspace controller you refer to, but it is far from being optimal. Userspace should limit itself to changing policies. -- Cheers, Igor --- Igor Stoppa Next Generation Software Nokia Devices RD - Helsinki -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: OMAP: CLKFW: Initial debugfs support for omap clock framework
From: ext Paul Walmsley [EMAIL PROTECTED]
Subject: Re: [PATCH 1/2] ARM: OMAP: CLKFW: Initial debugfs support for omap
clock framework
Date: Thu, 17 Apr 2008 13:44:04 -0600 (MDT)
Hello Hiroshi, David,
On Thu, 17 Apr 2008, David Brownell wrote:
On Thursday 17 April 2008, Hiroshi DOYU wrote:
And if there will be a little possibility that sysfs attribute can be
used by userland in the future, keeping sysfs instead of debugfs
doesn't seem not so illegal, does it?
True, but if we can do a debugfs implementation first, then that seems
like a good way to start, no? Userspace PM implementations are probably
some months in the future, and we can mandate that debugfs be mounted for
those.
Agreed. Update ones attached. The diffs against the previous are:
diff -u b/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
--- b/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -524,7 +524,7 @@
p += sprintf(p, %s, c-name);
if (c-id != 0)
- sprintf(p, %d, c-id);
+ sprintf(p, :%d, c-id);
d = debugfs_create_dir(s, pa ? pa-dent : clk_debugfs_root);
if (IS_ERR(d))
return PTR_ERR(d);
diff -u b/include/asm-arm/arch-omap/clock.h b/include/asm-arm/arch-omap/clock.h
--- b/include/asm-arm/arch-omap/clock.h
+++ b/include/asm-arm/arch-omap/clock.h
@@ -86,7 +86,7 @@
__u8rate_offset;
__u8src_offset;
#endif
-#ifdef CONFIG_DEBUG_FS
+#if defined(CONFIG_PM_DEBUG) defined(CONFIG_DEBUG_FS)
struct dentry *dent; /* For visible tree hierarchy */
#endif
};
Hiroshi DOYU
From fa06e7a21c08299eaa61e6c367ec8e737a8ff13b Mon Sep 17 00:00:00 2001
From: Hiroshi DOYU [EMAIL PROTECTED]
Date: Thu, 17 Apr 2008 16:51:41 +0300
Subject: [PATCH 2/2] ARM: OMAP: CLKFW: Remove procfs entry for debugging clock
framework
This feature can be covered by debugfs for omap clock framework.
Signed-off-by: Hiroshi DOYU [EMAIL PROTECTED]
---
arch/arm/plat-omap/clock.c | 96
1 files changed, 0 insertions(+), 96 deletions(-)
diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index 2ae87bf..714dbbf 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -34,41 +34,6 @@ static DEFINE_SPINLOCK(clockfw_lock);
static struct clk_functions *arch_clock;
-#ifdef CONFIG_PM_DEBUG
-
-static void print_parents(struct clk *clk)
-{
- struct clk *p;
- int printed = 0;
-
- list_for_each_entry(p, clocks, node) {
- if (p-parent == clk p-usecount) {
- if (!clk-usecount !printed) {
- printk(MISMATCH: %s\n, clk-name);
- printed = 1;
- }
- printk(\t%-15s\n, p-name);
- }
- }
-}
-
-void clk_print_usecounts(void)
-{
- unsigned long flags;
- struct clk *p;
-
- spin_lock_irqsave(clockfw_lock, flags);
- list_for_each_entry(p, clocks, node) {
- if (p-usecount)
- printk(%-15s: %d\n, p-name, p-usecount);
- print_parents(p);
-
- }
- spin_unlock_irqrestore(clockfw_lock, flags);
-}
-
-#endif
-
/*-
* Standard clock functions defined in include/linux/clk.h
*-*/
@@ -447,67 +412,6 @@ int __init clk_init(struct clk_functions * custom_clocks)
return 0;
}
-#if defined(CONFIG_PM_DEBUG) defined(CONFIG_PROC_FS)
-#include linux/proc_fs.h
-#include linux/seq_file.h
-
-static void *omap_ck_start(struct seq_file *m, loff_t *pos)
-{
- return *pos 1 ? (void *)1 : NULL;
-}
-
-static void *omap_ck_next(struct seq_file *m, void *v, loff_t *pos)
-{
- ++*pos;
- return NULL;
-}
-
-static void omap_ck_stop(struct seq_file *m, void *v)
-{
-}
-
-int omap_ck_show(struct seq_file *m, void *v)
-{
- struct clk *cp;
-
- list_for_each_entry(cp, clocks, node)
- seq_printf(m,%s %ld %d\n, cp-name, cp-rate, cp-usecount);
-
- return 0;
-}
-
-static struct seq_operations omap_ck_op = {
- .start =omap_ck_start,
- .next = omap_ck_next,
- .stop = omap_ck_stop,
- .show = omap_ck_show
-};
-
-static int omap_ck_open(struct inode *inode, struct file *file)
-{
- return seq_open(file, omap_ck_op);
-}
-
-static struct file_operations proc_omap_ck_operations = {
- .open = omap_ck_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release= seq_release,
-};
-
-int __init omap_ck_init(void)
-{
- struct proc_dir_entry *entry;
-
- entry = create_proc_entry(omap_clocks, 0, NULL);
- if (entry)
- entry-proc_fops = proc_omap_ck_operations
Re: [PATCH 1/2] ARM: OMAP: CLKFW: Initial debugfs support for omap clock framework
Hello Igor, On Thu, 17 Apr 2008, Igor Stoppa wrote: On Thu, 2008-04-17 at 13:44 -0600, ext Paul Walmsley wrote: True, but if we can do a debugfs implementation first, then that seems like a good way to start, no? Userspace PM implementations are probably some months in the future, and we can mandate that debugfs be mounted for those. I can hardly see the benefit of a userspace implementation, considering the extra context switch required and the fact the in many cases clocks get enabled in response to irqs. I agree that we shouldn't try to move the clock framework to userspace. But, determining what OPP to switch to next, based on system load or other requirements published by drivers or userspace processes; or determining what power state to put powerdomains to -- it would be nice to experiment with a userspace governor for those tasks. It would certainly make development and testing easier. In terms of the clock tree, it would be good to allow userspace-driven OPP changes, analogous to CPUFreq's userspace governor. [ In general, I agree that userspace should not be changing driver clocks directly, just like userspace should not be mucking around in /dev/mem directly :-) ] That also sounds akward at best: cpufreq (or similar) is much better suited for this sort of activity; userspace governor would be the userspace controller you refer to, but it is far from being optimal. Userspace should limit itself to changing policies. CPUFreq is good, but it does not manage non-CPU-frequency knobs very well, and there are plenty of those on OMAP3. Is there any reason why we should not allow the option of userspace OPP selection/powerdomain control for OMAP? If people don't want to use it, they don't have to :-) - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: OMAP: CLKFW: Initial debugfs support for omap clock framework
On Thursday 17 April 2008, Paul Walmsley wrote: Userspace should limit itself to changing policies. CPUFreq is good, but it does not manage non-CPU-frequency knobs very well, and there are plenty of those on OMAP3. Similar issues are widely acknowledged. Is there any reason why we should not allow the option of userspace OPP selection/powerdomain control for OMAP? If people don't want to use it, they don't have to :-) Sure, allow userspace controls. Even use debugs to prototype them. Just don't build fragile interfaces ... don't expect userspace code to do the equivalent of open heart surgery. - Dave -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
