| Message ID | 1355370586-6600-1-git-send-email-pgaikwad@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Dec 12, 2012 at 7:49 PM, Prashant Gaikwad <pgaikwad@nvidia.com> wrote: > Adds debug file "clock_tree" in /sys/kernel/debug/clk dir. > It helps to view all the clock registered in tree format. > Prashant, Thanks for submitting this. We've been talking about having a single file for representing the tree for some time. Regarding the output format had you considered using a well known format which can be parsed using well known parsing libs? This avoids needing a custom parser just for this one file. JSON springs to mind as something lightweight and well-understood. > For example: > clock enable_cnt prepare_cnt rate > --------------------------------------------------------------------- > i2s0_sync 0 0 24000000 > spdif_in_sync 0 0 24000000 > spdif_mux 0 0 24000000 > spdif 0 0 24000000 > spdif_doubler 0 0 48000000 > spdif_div 0 0 48000000 > spdif_2x 0 0 48000000 > clk_32k 2 2 32768 > blink_override 1 1 32768 > blink 1 1 32768 > clk_m 2 2 12000000 > clk_out_3_mux 0 0 12000000 > clk_out_3 0 0 12000000 > pll_ref 3 3 12000000 > pll_e_mux 0 0 12000000 > pll_e 0 0 100000000 > cml0 0 0 100000000 > cml1 0 0 100000000 > pciex 0 0 100000000 > pll_d2 0 0 1000000 > pll_d2_out0 0 0 500000 > pll_d 0 0 1000000 > pll_d_out0 0 0 500000 > dsib_mux 0 0 500000 > dsib 0 0 500000 > dsia 0 0 500000 > > Signed-off-by: Prashant Gaikwad <pgaikwad@nvidia.com> > --- > drivers/clk/clk.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 59 insertions(+), 0 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 56e4495e..7daf201 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -34,6 +34,59 @@ static struct dentry *rootdir; > static struct dentry *orphandir; > static int inited = 0; > > +static void clk_tree_show_one(struct seq_file *s, struct clk *c, int level) > +{ > + if (!c) > + return; > + > + seq_printf(s, "%*s%-*s %-11d %-12d %-10lu", > + level * 3 + 1, "", > + 30 - level * 3, c->name, > + c->enable_count, c->prepare_count, c->rate); > + seq_printf(s, "\n"); > +} > + > +static void clk_tree_show_subtree(struct seq_file *s, struct clk *c, int level) > +{ > + struct clk *child; > + struct hlist_node *tmp; > + > + if (!c) > + return; > + > + clk_tree_show_one(s, c, level); > + > + hlist_for_each_entry(child, tmp, &c->children, child_node) > + clk_tree_show_subtree(s, child, level + 1); > +} > + > +static int clk_tree_show(struct seq_file *s, void *data) > +{ > + struct clk *c; > + struct hlist_node *tmp; > + > + seq_printf(s, " clock enable_cnt prepare_cnt rate\n"); > + seq_printf(s, "---------------------------------------------------------------------\n"); > + If we want this to be machine readable then we can probably drop the heading and line of dashes altogether. > + hlist_for_each_entry(c, tmp, &clk_root_list, child_node) > + clk_tree_show_subtree(s, c, 0); > + > + return 0; > +} > + > + > +static int clk_tree_open(struct inode *inode, struct file *file) > +{ > + return single_open(file, clk_tree_show, inode->i_private); > +} > + > +static const struct file_operations clk_tree_fops = { > + .open = clk_tree_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = single_release, > +}; > + > /* caller must hold prepare_lock */ > static int clk_debug_create_one(struct clk *clk, struct dentry *pdentry) > { > @@ -167,12 +220,18 @@ static int __init clk_debug_init(void) > { > struct clk *clk; > struct hlist_node *tmp; > + struct dentry *d; > > rootdir = debugfs_create_dir("clk", NULL); > > if (!rootdir) > return -ENOMEM; > > + d = debugfs_create_file("clock_tree", S_IRUGO, rootdir, NULL, > + &clk_tree_fops); I prefer "clk_tree" versus "clock_tree". If we only spell clock one way then users won't have to spend as much time trying to remember specific spellings of the word. This also brings up the question of whether or not we should keep the directory-based debugfs representation around. I personally don't care and I'm happy to deprecate it and remove at a future date if others do not object. However if you have some tooling based around the existing clk debugfs stuff then please speak up and I'll keep both around. Thanks, Mike > + if (!d) > + return -ENOMEM; > + > orphandir = debugfs_create_dir("orphans", rootdir); > > if (!orphandir) > -- > 1.7.4.1 >
On 12/13/2012 09:27 AM, Mike Turquette wrote: > On Wed, Dec 12, 2012 at 7:49 PM, Prashant Gaikwad <pgaikwad@nvidia.com> wrote: >> Adds debug file "clock_tree" in /sys/kernel/debug/clk dir. >> It helps to view all the clock registered in tree format. >> > > Prashant, > > Thanks for submitting this. We've been talking about having a single > file for representing the tree for some time. > > Regarding the output format had you considered using a well known > format which can be parsed using well known parsing libs? This avoids > needing a custom parser just for this one file. JSON springs to mind > as something lightweight and well-understood. One advantage of the format below is that it's very easily human-readable, and it's not too hard to parse (although I guess you'd have to parse the indent level to get parent/child relation, which would suck a bit). Is there room to provide both? Otherwise, I guess the kernel could include a script to convert from JSON/whatever into the format below. >> For example: >> clock enable_cnt prepare_cnt rate >> --------------------------------------------------------------------- >> i2s0_sync 0 0 24000000 >> spdif_in_sync 0 0 24000000 >> spdif_mux 0 0 24000000 >> spdif 0 0 24000000 >> spdif_doubler 0 0 48000000 >> spdif_div 0 0 48000000 >> spdif_2x 0 0 48000000
On Thu, Dec 13, 2012 at 07:01:31PM +0100, Stephen Warren wrote: > On 12/13/2012 09:27 AM, Mike Turquette wrote: > > On Wed, Dec 12, 2012 at 7:49 PM, Prashant Gaikwad <pgaikwad@nvidia.com> wrote: > >> Adds debug file "clock_tree" in /sys/kernel/debug/clk dir. > >> It helps to view all the clock registered in tree format. > >> > > > > Prashant, > > > > Thanks for submitting this. We've been talking about having a single > > file for representing the tree for some time. > > > > Regarding the output format had you considered using a well known > > format which can be parsed using well known parsing libs? This avoids > > needing a custom parser just for this one file. JSON springs to mind > > as something lightweight and well-understood. > > One advantage of the format below is that it's very easily > human-readable, and it's not too hard to parse (although I guess you'd > have to parse the indent level to get parent/child relation, which would > suck a bit). Is there room to provide both? Otherwise, I guess the > kernel could include a script to convert from JSON/whatever into the > format below. We already have the clk directory for an easy to parse version. So I think this one should be focused on being human readable. Cheers, Peter.
On Fri, Dec 14, 2012 at 2:43 AM, Peter De Schrijver <pdeschrijver@nvidia.com> wrote: > On Thu, Dec 13, 2012 at 07:01:31PM +0100, Stephen Warren wrote: >> On 12/13/2012 09:27 AM, Mike Turquette wrote: >> > On Wed, Dec 12, 2012 at 7:49 PM, Prashant Gaikwad <pgaikwad@nvidia.com> wrote: >> >> Adds debug file "clock_tree" in /sys/kernel/debug/clk dir. >> >> It helps to view all the clock registered in tree format. >> >> >> > >> > Prashant, >> > >> > Thanks for submitting this. We've been talking about having a single >> > file for representing the tree for some time. >> > >> > Regarding the output format had you considered using a well known >> > format which can be parsed using well known parsing libs? This avoids >> > needing a custom parser just for this one file. JSON springs to mind >> > as something lightweight and well-understood. >> >> One advantage of the format below is that it's very easily >> human-readable, and it's not too hard to parse (although I guess you'd >> have to parse the indent level to get parent/child relation, which would >> suck a bit). Is there room to provide both? Otherwise, I guess the >> kernel could include a script to convert from JSON/whatever into the >> format below. > > We already have the clk directory for an easy to parse version. So I think > this one should be focused on being human readable. > I'm really not sure the clk directory structure is such a good idea. Hopping around directories and dumping a few files feels like a strange way to get at the data. Another key point is atomicity. The prepare_lock mutex is only held for each individual read of a file under the current clock directory implementation. So if you wish to read several different clock rates there is a caveat that the rates could change between reads. The "summary" file that dumps all of the data improves on this since it could hold the prepare_lock mutex across the operation. This last point makes me realize that the current code above either needs to hold that lock or at least use hlist_for_each_entry_safe since clocks could be removed from the lists during the operation. As such it is a bit unsafe. I'm OK letting the two implementations live along side each other but I am also sure that the clock summary dump should be reasonably machine readable. Regards, Mike > Cheers, > > Peter.
On Thursday 13 December 2012 11:31 PM, Stephen Warren wrote: > On 12/13/2012 09:27 AM, Mike Turquette wrote: >> On Wed, Dec 12, 2012 at 7:49 PM, Prashant Gaikwad <pgaikwad@nvidia.com> wrote: >>> Adds debug file "clock_tree" in /sys/kernel/debug/clk dir. >>> It helps to view all the clock registered in tree format. >>> >> Prashant, >> >> Thanks for submitting this. We've been talking about having a single >> file for representing the tree for some time. >> >> Regarding the output format had you considered using a well known >> format which can be parsed using well known parsing libs? This avoids >> needing a custom parser just for this one file. JSON springs to mind >> as something lightweight and well-understood. > One advantage of the format below is that it's very easily > human-readable, and it's not too hard to parse (although I guess you'd > have to parse the indent level to get parent/child relation, which would > suck a bit). Is there room to provide both? Otherwise, I guess the > kernel could include a script to convert from JSON/whatever into the > format below. > >>> For example: >>> clock enable_cnt prepare_cnt rate >>> --------------------------------------------------------------------- >>> i2s0_sync 0 0 24000000 >>> spdif_in_sync 0 0 24000000 >>> spdif_mux 0 0 24000000 >>> spdif 0 0 24000000 >>> spdif_doubler 0 0 48000000 >>> spdif_div 0 0 48000000 >>> spdif_2x 0 0 48000000 > Even I think that output must be easily human-readable. How about adding sysfs to switch between human-readable and machine-readable format? I will try come up with a implementation.
On Wed, Dec 19, 2012 at 11:53 PM, Prashant Gaikwad <pgaikwad@nvidia.com> wrote: > On Thursday 13 December 2012 11:31 PM, Stephen Warren wrote: >> >> On 12/13/2012 09:27 AM, Mike Turquette wrote: >>> >>> On Wed, Dec 12, 2012 at 7:49 PM, Prashant Gaikwad <pgaikwad@nvidia.com> >>> wrote: >>>> >>>> Adds debug file "clock_tree" in /sys/kernel/debug/clk dir. >>>> It helps to view all the clock registered in tree format. >>>> >>> Prashant, >>> >>> Thanks for submitting this. We've been talking about having a single >>> file for representing the tree for some time. >>> >>> Regarding the output format had you considered using a well known >>> format which can be parsed using well known parsing libs? This avoids >>> needing a custom parser just for this one file. JSON springs to mind >>> as something lightweight and well-understood. >> >> One advantage of the format below is that it's very easily >> human-readable, and it's not too hard to parse (although I guess you'd >> have to parse the indent level to get parent/child relation, which would >> suck a bit). Is there room to provide both? Otherwise, I guess the >> kernel could include a script to convert from JSON/whatever into the >> format below. >> >>>> For example: >>>> clock enable_cnt prepare_cnt rate >>>> --------------------------------------------------------------------- >>>> i2s0_sync 0 0 24000000 >>>> spdif_in_sync 0 0 24000000 >>>> spdif_mux 0 0 24000000 >>>> spdif 0 0 24000000 >>>> spdif_doubler 0 0 48000000 >>>> spdif_div 0 0 48000000 >>>> spdif_2x 0 0 48000000 >> >> > > Even I think that output must be easily human-readable. How about adding > sysfs to switch between human-readable and machine-readable format? > I will try come up with a implementation. > Do you mean a sysfs file which controls the output format? How about just two different files? One can be clk-dump (machine readable) and the other is clk-summary (human readable). Regards, Mike
On Saturday 22 December 2012 04:26 AM, Mike Turquette wrote: > On Wed, Dec 19, 2012 at 11:53 PM, Prashant Gaikwad <pgaikwad@nvidia.com> wrote: >> On Thursday 13 December 2012 11:31 PM, Stephen Warren wrote: >>> On 12/13/2012 09:27 AM, Mike Turquette wrote: >>>> On Wed, Dec 12, 2012 at 7:49 PM, Prashant Gaikwad <pgaikwad@nvidia.com> >>>> wrote: >>>>> Adds debug file "clock_tree" in /sys/kernel/debug/clk dir. >>>>> It helps to view all the clock registered in tree format. >>>>> >>>> Prashant, >>>> >>>> Thanks for submitting this. We've been talking about having a single >>>> file for representing the tree for some time. >>>> >>>> Regarding the output format had you considered using a well known >>>> format which can be parsed using well known parsing libs? This avoids >>>> needing a custom parser just for this one file. JSON springs to mind >>>> as something lightweight and well-understood. >>> One advantage of the format below is that it's very easily >>> human-readable, and it's not too hard to parse (although I guess you'd >>> have to parse the indent level to get parent/child relation, which would >>> suck a bit). Is there room to provide both? Otherwise, I guess the >>> kernel could include a script to convert from JSON/whatever into the >>> format below. >>> >>>>> For example: >>>>> clock enable_cnt prepare_cnt rate >>>>> --------------------------------------------------------------------- >>>>> i2s0_sync 0 0 24000000 >>>>> spdif_in_sync 0 0 24000000 >>>>> spdif_mux 0 0 24000000 >>>>> spdif 0 0 24000000 >>>>> spdif_doubler 0 0 48000000 >>>>> spdif_div 0 0 48000000 >>>>> spdif_2x 0 0 48000000 >>> >> Even I think that output must be easily human-readable. How about adding >> sysfs to switch between human-readable and machine-readable format? >> I will try come up with a implementation. >> > Do you mean a sysfs file which controls the output format? How about > just two different files? One can be clk-dump (machine readable) and > the other is clk-summary (human readable). It is also fine. Is this patch ok for human-readable format? or any suggestions? I will change the file name to clk-summary. > > Regards, > Mike
On Sun, Dec 23, 2012 at 4:26 AM, Prashant Gaikwad <pgaikwad@nvidia.com> wrote: > On Saturday 22 December 2012 04:26 AM, Mike Turquette wrote: >> >> On Wed, Dec 19, 2012 at 11:53 PM, Prashant Gaikwad <pgaikwad@nvidia.com> >> wrote: >>> >>> On Thursday 13 December 2012 11:31 PM, Stephen Warren wrote: >>>> >>>> On 12/13/2012 09:27 AM, Mike Turquette wrote: >>>>> >>>>> On Wed, Dec 12, 2012 at 7:49 PM, Prashant Gaikwad <pgaikwad@nvidia.com> >>>>> wrote: >>>>>> >>>>>> Adds debug file "clock_tree" in /sys/kernel/debug/clk dir. >>>>>> It helps to view all the clock registered in tree format. >>>>>> >>>>> Prashant, >>>>> >>>>> Thanks for submitting this. We've been talking about having a single >>>>> file for representing the tree for some time. >>>>> >>>>> Regarding the output format had you considered using a well known >>>>> format which can be parsed using well known parsing libs? This avoids >>>>> needing a custom parser just for this one file. JSON springs to mind >>>>> as something lightweight and well-understood. >>>> >>>> One advantage of the format below is that it's very easily >>>> human-readable, and it's not too hard to parse (although I guess you'd >>>> have to parse the indent level to get parent/child relation, which would >>>> suck a bit). Is there room to provide both? Otherwise, I guess the >>>> kernel could include a script to convert from JSON/whatever into the >>>> format below. >>>> >>>>>> For example: >>>>>> clock enable_cnt prepare_cnt rate >>>>>> --------------------------------------------------------------------- >>>>>> i2s0_sync 0 0 24000000 >>>>>> spdif_in_sync 0 0 24000000 >>>>>> spdif_mux 0 0 24000000 >>>>>> spdif 0 0 24000000 >>>>>> spdif_doubler 0 0 48000000 >>>>>> spdif_div 0 0 48000000 >>>>>> spdif_2x 0 0 48000000 >>>> >>>> >>> Even I think that output must be easily human-readable. How about adding >>> sysfs to switch between human-readable and machine-readable format? >>> I will try come up with a implementation. >>> >> Do you mean a sysfs file which controls the output format? How about >> just two different files? One can be clk-dump (machine readable) and >> the other is clk-summary (human readable). > > > It is also fine. Is this patch ok for human-readable format? or any > suggestions? > I will change the file name to clk-summary. > Prashant, Yes the format seems to be agreeable for human-readable format based on the feedback on the list. Go ahead and keep the column titles and the dashed lines that I commented on earlier... my comments are less relevant if a separate machine-readable clk-dump file exists. Are you going to take a crack at JSON-formatted output for clk-dump? Thanks, Mike >> >> Regards, >> Mike > >
On Monday 24 December 2012 11:07 PM, Mike Turquette wrote: > On Sun, Dec 23, 2012 at 4:26 AM, Prashant Gaikwad <pgaikwad@nvidia.com> wrote: >> On Saturday 22 December 2012 04:26 AM, Mike Turquette wrote: >>> On Wed, Dec 19, 2012 at 11:53 PM, Prashant Gaikwad <pgaikwad@nvidia.com> >>> wrote: >>>> On Thursday 13 December 2012 11:31 PM, Stephen Warren wrote: >>>>> On 12/13/2012 09:27 AM, Mike Turquette wrote: >>>>>> On Wed, Dec 12, 2012 at 7:49 PM, Prashant Gaikwad <pgaikwad@nvidia.com> >>>>>> wrote: >>>>>>> Adds debug file "clock_tree" in /sys/kernel/debug/clk dir. >>>>>>> It helps to view all the clock registered in tree format. >>>>>>> >>>>>> Prashant, >>>>>> >>>>>> Thanks for submitting this. We've been talking about having a single >>>>>> file for representing the tree for some time. >>>>>> >>>>>> Regarding the output format had you considered using a well known >>>>>> format which can be parsed using well known parsing libs? This avoids >>>>>> needing a custom parser just for this one file. JSON springs to mind >>>>>> as something lightweight and well-understood. >>>>> One advantage of the format below is that it's very easily >>>>> human-readable, and it's not too hard to parse (although I guess you'd >>>>> have to parse the indent level to get parent/child relation, which would >>>>> suck a bit). Is there room to provide both? Otherwise, I guess the >>>>> kernel could include a script to convert from JSON/whatever into the >>>>> format below. >>>>> >>>>>>> For example: >>>>>>> clock enable_cnt prepare_cnt rate >>>>>>> --------------------------------------------------------------------- >>>>>>> i2s0_sync 0 0 24000000 >>>>>>> spdif_in_sync 0 0 24000000 >>>>>>> spdif_mux 0 0 24000000 >>>>>>> spdif 0 0 24000000 >>>>>>> spdif_doubler 0 0 48000000 >>>>>>> spdif_div 0 0 48000000 >>>>>>> spdif_2x 0 0 48000000 >>>>> >>>> Even I think that output must be easily human-readable. How about adding >>>> sysfs to switch between human-readable and machine-readable format? >>>> I will try come up with a implementation. >>>> >>> Do you mean a sysfs file which controls the output format? How about >>> just two different files? One can be clk-dump (machine readable) and >>> the other is clk-summary (human readable). >> >> It is also fine. Is this patch ok for human-readable format? or any >> suggestions? >> I will change the file name to clk-summary. >> > Prashant, > > Yes the format seems to be agreeable for human-readable format based > on the feedback on the list. Go ahead and keep the column titles and > the dashed lines that I commented on earlier... my comments are less > relevant if a separate machine-readable clk-dump file exists. Thanks Mike!! > Are you going to take a crack at JSON-formatted output for clk-dump? I will work on it. > Thanks, > Mike > >>> Regards, >>> Mike >>
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 56e4495e..7daf201 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -34,6 +34,59 @@ static struct dentry *rootdir; static struct dentry *orphandir; static int inited = 0; +static void clk_tree_show_one(struct seq_file *s, struct clk *c, int level) +{ + if (!c) + return; + + seq_printf(s, "%*s%-*s %-11d %-12d %-10lu", + level * 3 + 1, "", + 30 - level * 3, c->name, + c->enable_count, c->prepare_count, c->rate); + seq_printf(s, "\n"); +} + +static void clk_tree_show_subtree(struct seq_file *s, struct clk *c, int level) +{ + struct clk *child; + struct hlist_node *tmp; + + if (!c) + return; + + clk_tree_show_one(s, c, level); + + hlist_for_each_entry(child, tmp, &c->children, child_node) + clk_tree_show_subtree(s, child, level + 1); +} + +static int clk_tree_show(struct seq_file *s, void *data) +{ + struct clk *c; + struct hlist_node *tmp; + + seq_printf(s, " clock enable_cnt prepare_cnt rate\n"); + seq_printf(s, "---------------------------------------------------------------------\n"); + + hlist_for_each_entry(c, tmp, &clk_root_list, child_node) + clk_tree_show_subtree(s, c, 0); + + return 0; +} + + +static int clk_tree_open(struct inode *inode, struct file *file) +{ + return single_open(file, clk_tree_show, inode->i_private); +} + +static const struct file_operations clk_tree_fops = { + .open = clk_tree_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, +}; + /* caller must hold prepare_lock */ static int clk_debug_create_one(struct clk *clk, struct dentry *pdentry) { @@ -167,12 +220,18 @@ static int __init clk_debug_init(void) { struct clk *clk; struct hlist_node *tmp; + struct dentry *d; rootdir = debugfs_create_dir("clk", NULL); if (!rootdir) return -ENOMEM; + d = debugfs_create_file("clock_tree", S_IRUGO, rootdir, NULL, + &clk_tree_fops); + if (!d) + return -ENOMEM; + orphandir = debugfs_create_dir("orphans", rootdir); if (!orphandir)
Adds debug file "clock_tree" in /sys/kernel/debug/clk dir. It helps to view all the clock registered in tree format. For example: clock enable_cnt prepare_cnt rate --------------------------------------------------------------------- i2s0_sync 0 0 24000000 spdif_in_sync 0 0 24000000 spdif_mux 0 0 24000000 spdif 0 0 24000000 spdif_doubler 0 0 48000000 spdif_div 0 0 48000000 spdif_2x 0 0 48000000 clk_32k 2 2 32768 blink_override 1 1 32768 blink 1 1 32768 clk_m 2 2 12000000 clk_out_3_mux 0 0 12000000 clk_out_3 0 0 12000000 pll_ref 3 3 12000000 pll_e_mux 0 0 12000000 pll_e 0 0 100000000 cml0 0 0 100000000 cml1 0 0 100000000 pciex 0 0 100000000 pll_d2 0 0 1000000 pll_d2_out0 0 0 500000 pll_d 0 0 1000000 pll_d_out0 0 0 500000 dsib_mux 0 0 500000 dsib 0 0 500000 dsia 0 0 500000 Signed-off-by: Prashant Gaikwad <pgaikwad@nvidia.com> --- drivers/clk/clk.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 59 insertions(+), 0 deletions(-)