| Message ID | 1464381494-27096-11-git-send-email-rklein@nvidia.com |
|---|---|
| State | Deferred |
| Headers | show |
On Fri, May 27, 2016 at 04:38:13PM -0400, Rhyland Klein wrote: > Add a '^' character to the beginning of clk entries that are for > CRITICAL clks. > > Signed-off-by: Rhyland Klein <rklein@nvidia.com> > --- > drivers/clk/clk.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 874c7dd8ef66..22dd0ca1e491 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -1948,8 +1948,9 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c, > if (!c) > return; > > - seq_printf(s, "%*s%-*s %11d %12d %11lu %10lu %-3d\n", > - level * 3 + 1, "", > + seq_printf(s, "%s%*s%-*s %11d %12d %11lu %10lu %-3d\n", > + (c->flags & CLK_IS_CRITICAL ? "^" : ""), > + level * 3 + (c->flags & CLK_IS_CRITICAL ? 0 : 1), "", Maybe output " " instead of "" for CLK_IS_CRITICAL, that way you can omit the second conditional. I wonder if it might be easier to read if this flag was at the end of the line. There's also the fact that someone may have written a script that expects the clock name as the first word on the line and may get confused by this change. If you put it at the very end of the line the likelihood of upsetting scripts will be reduced. Thierry
On 6/22/2016 8:24 AM, Thierry Reding wrote: > * PGP Signed by an unknown key > > On Fri, May 27, 2016 at 04:38:13PM -0400, Rhyland Klein wrote: >> Add a '^' character to the beginning of clk entries that are for >> CRITICAL clks. >> >> Signed-off-by: Rhyland Klein <rklein@nvidia.com> >> --- >> drivers/clk/clk.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c >> index 874c7dd8ef66..22dd0ca1e491 100644 >> --- a/drivers/clk/clk.c >> +++ b/drivers/clk/clk.c >> @@ -1948,8 +1948,9 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c, >> if (!c) >> return; >> >> - seq_printf(s, "%*s%-*s %11d %12d %11lu %10lu %-3d\n", >> - level * 3 + 1, "", >> + seq_printf(s, "%s%*s%-*s %11d %12d %11lu %10lu %-3d\n", >> + (c->flags & CLK_IS_CRITICAL ? "^" : ""), >> + level * 3 + (c->flags & CLK_IS_CRITICAL ? 0 : 1), "", > > Maybe output " " instead of "" for CLK_IS_CRITICAL, that way you can > omit the second conditional. > > I wonder if it might be easier to read if this flag was at the end of > the line. There's also the fact that someone may have written a script > that expects the clock name as the first word on the line and may get > confused by this change. If you put it at the very end of the line the > likelihood of upsetting scripts will be reduced. Yah we can put the mark at the end of the line. I wasn't sure if there was a strong motivation to avoid extending the the width of each line, as sometimes people prefer to try to keep it close to 80 char as possible. I think right now, it was close to that, but might be a little over already. I can switch to that though, as it is less likely to break any automatic parsing scripts. -rhyland
On 06/22, Rhyland Klein wrote: > On 6/22/2016 8:24 AM, Thierry Reding wrote: > > > > Maybe output " " instead of "" for CLK_IS_CRITICAL, that way you can > > omit the second conditional. > > > > I wonder if it might be easier to read if this flag was at the end of > > the line. There's also the fact that someone may have written a script > > that expects the clock name as the first word on the line and may get > > confused by this change. If you put it at the very end of the line the > > likelihood of upsetting scripts will be reduced. > > Yah we can put the mark at the end of the line. I wasn't sure if there > was a strong motivation to avoid extending the the width of each line, > as sometimes people prefer to try to keep it close to 80 char as > possible. I think right now, it was close to that, but might be a little > over already. I can switch to that though, as it is less likely to break > any automatic parsing scripts. > Nak. clk_summary is about taking a snapshot of the system state for things that may be changing rapidly, like consumers (which sounds fun to add!), rates, enable/prepare state. Flags are not changing. If you want to add flag info into some summary then a script should be able to augment clk_summary info (really should use the clk_dump in this case though) with whatever flags can be read through debugfs already.
On 6/28/2016 1:40 PM, Stephen Boyd wrote: > On 06/22, Rhyland Klein wrote: >> On 6/22/2016 8:24 AM, Thierry Reding wrote: >>> >>> Maybe output " " instead of "" for CLK_IS_CRITICAL, that way you can >>> omit the second conditional. >>> >>> I wonder if it might be easier to read if this flag was at the end of >>> the line. There's also the fact that someone may have written a script >>> that expects the clock name as the first word on the line and may get >>> confused by this change. If you put it at the very end of the line the >>> likelihood of upsetting scripts will be reduced. >> >> Yah we can put the mark at the end of the line. I wasn't sure if there >> was a strong motivation to avoid extending the the width of each line, >> as sometimes people prefer to try to keep it close to 80 char as >> possible. I think right now, it was close to that, but might be a little >> over already. I can switch to that though, as it is less likely to break >> any automatic parsing scripts. >> > > Nak. clk_summary is about taking a snapshot of the system state > for things that may be changing rapidly, like consumers (which > sounds fun to add!), rates, enable/prepare state. Flags are not > changing. If you want to add flag info into some summary then a > script should be able to augment clk_summary info (really should > use the clk_dump in this case though) with whatever flags can be > read through debugfs already. > That is fine with me. This was more of something I was using locally to verify things and thought it might be useful in some manner upstream. However, a script which read the clk_flags from debugfs for each clk could do the same thing without changing the summary. -rhyland
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 874c7dd8ef66..22dd0ca1e491 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1948,8 +1948,9 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c, if (!c) return; - seq_printf(s, "%*s%-*s %11d %12d %11lu %10lu %-3d\n", - level * 3 + 1, "", + seq_printf(s, "%s%*s%-*s %11d %12d %11lu %10lu %-3d\n", + (c->flags & CLK_IS_CRITICAL ? "^" : ""), + level * 3 + (c->flags & CLK_IS_CRITICAL ? 0 : 1), "", 30 - level * 3, c->name, c->enable_count, c->prepare_count, clk_core_get_rate(c), clk_core_get_accuracy(c), clk_core_get_phase(c)); @@ -1985,6 +1986,8 @@ static int clk_summary_show(struct seq_file *s, void *data) clk_prepare_unlock(); + seq_puts(s, "clocks starting with ^ are marked as CRITICAL\n"); + return 0; }
Add a '^' character to the beginning of clk entries that are for CRITICAL clks. Signed-off-by: Rhyland Klein <rklein@nvidia.com> --- drivers/clk/clk.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)