[powerdebug] clock: show only active clocks

Message ID 1367699278-406-1-git-send-email-sanjay.rawat@linaro.org
State New
Headers show

Commit Message

Sanjay Singh Rawat May 4, 2013, 8:27 p.m.
- use the clk_summary file of CCF for showing clocks
- add option to show active or all clocks

Signed-off-by: Sanjay Singh Rawat <sanjay.rawat@linaro.org>
---
 clocks.c  |   62 +++++++++++++++++++++++++++++++++++++++++++++++++------------
 display.c |    3 +++
 display.h |    1 +
 3 files changed, 54 insertions(+), 12 deletions(-)

Comments

Amit Kucheria May 6, 2013, 9:16 a.m. | #1
Sebastian,

Could you help Sanjay with verification of this feature?

Regards,
Amit


On Sun, May 5, 2013 at 1:57 AM, Sanjay Singh Rawat
<sanjay.rawat@linaro.org> wrote:
> - use the clk_summary file of CCF for showing clocks
> - add option to show active or all clocks
>
> Signed-off-by: Sanjay Singh Rawat <sanjay.rawat@linaro.org>
> ---
>  clocks.c  |   62 +++++++++++++++++++++++++++++++++++++++++++++++++------------
>  display.c |    3 +++
>  display.h |    1 +
>  3 files changed, 54 insertions(+), 12 deletions(-)
>
> diff --git a/clocks.c b/clocks.c
> index 95acf57..a0510b4 100644
> --- a/clocks.c
> +++ b/clocks.c
> @@ -287,9 +287,8 @@ static int clock_print_header(void)
>         int ret;
>
>         if(clock_fw == CCF) {
> -               if (asprintf(&buf, "%-35s %-10s %-12s %-10s %-11s %-15s %-14s %-14s",
> -                    "Name", "Flags", "Rate", "Usecount", "Children", "Prepare_Count",
> -                    "Enable_Count", "Notifier_Count") < 0)
> +               if (asprintf(&buf, "%-30s %-10s %-12s %-10s (clock toggle key-'c')",
> +                    "Name", "Enable_Count", "Prepare_Count", "Rate") < 0)
>                 return -1;
>         }
>         else {
> @@ -297,23 +296,57 @@ static int clock_print_header(void)
>                      "Name", "Flags", "Rate", "Usecount", "Children") < 0)
>                 return -1;
>         }
> -
>         ret = display_column_name(buf);
> -
>         free(buf);
>
>         return ret;
>  }
>
> +/*
> + * Display clocks by refering the clk_summary file of CCF
> + */
> +static int display_clk_summary()
> +{
> +        FILE *fp;
> +        char line[256];
> +        int afterheader;
> +        char clock[30];
> +        int enable_cnt,prepare_cnt,rate;
> +
> +        afterheader = 0;
> +        fp = fopen("/sys/kernel/debug/clk/clk_summary","r");
> +        if (fp == NULL) {
> +                printf("error: failed to open clock tree file\n");
> +                return -1;
> +        }
> +
> +        while (NULL != fgets(line,256,fp)) {
> +                if (afterheader > 1) {
> +                        sscanf(line,"%s %d %d %d",clock,&enable_cnt,&prepare_cnt,&rate);
> +                        if (active_clks) {
> +                               if (enable_cnt)
> +                                       display_print_line(CLOCK,afterheader,
> +                                               line,0,clock_tree);
> +                       }
> +                       else
> +                               display_print_line(CLOCK,afterheader,line,1,clock_tree);
> +                }
> +                afterheader++;
> +        }
> +       return 0;
> +}
> +
>  static int clock_print_info(struct tree *tree)
>  {
>         int ret, line = 0;
>
>         display_reset_cursor(CLOCK);
> -
>         clock_print_header();
>
> -       ret = tree_for_each(tree, clock_print_info_cb, &line);
> +       if (clock_fw == CCF)
> +               ret = display_clk_summary();
> +       else
> +               ret = tree_for_each(tree, clock_print_info_cb, &line);
>
>         display_refresh_pad(CLOCK);
>
> @@ -426,8 +459,10 @@ int clock_init(void)
>
>         sprintf(clk_dir_path[CCF], "%s/clk", clk_dir_path[CCF]);
>         sprintf(clk_dir_path[OCF], "%s/clock", clk_dir_path[OCF]);
> +
>         if (!access(clk_dir_path[CCF], F_OK)) {
>                 clock_fw = CCF;
> +               active_clks = true;
>                 strcpy(clk_dir_path[MAX],clk_dir_path[CCF]);
>         }
>         else if(!access(clk_dir_path[OCF], F_OK)) {
> @@ -437,12 +472,15 @@ int clock_init(void)
>         else
>                 return -1;
>
> -       clock_tree = tree_load(clk_dir_path[MAX], NULL, false);
> -       if (!clock_tree)
> -               return -1;
> +       /* Not preparing tree for CCF, will use the clk_summary file */
> +       if(clock_fw != CCF) {
> +               clock_tree = tree_load(clk_dir_path[MAX], NULL, false);
> +               if (!clock_tree)
> +                       return -1;
>
> -       if (fill_clock_tree())
> -               return -1;
> +               if (fill_clock_tree())
> +                       return -1;
> +       }
>
>         return display_register(CLOCK, &clock_ops);
>  }
> diff --git a/display.c b/display.c
> index e9f4bf6..98544e6 100644
> --- a/display.c
> +++ b/display.c
> @@ -416,6 +416,9 @@ static int display_keystroke(int fd, void *data)
>         case 'r':
>         case 'R':
>                 return display_refresh(current_win, true);
> +       case 'c':
> +               active_clks = active_clks ? false : true;
> +               return display_refresh(current_win, true);
>         default:
>                 return 0;
>         }
> diff --git a/display.h b/display.h
> index 6362a48..24c9d59 100644
> --- a/display.h
> +++ b/display.h
> @@ -33,4 +33,5 @@ extern int display_init(int wdefault);
>  extern int display_register(int win, struct display_ops *ops);
>  extern int display_column_name(const char *line);
>
> +bool active_clks;
>  #define NAME_MAX 255
> --
> 1.7.9.5
>
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
Mike Turquette May 6, 2013, 4:06 p.m. | #2
Quoting Sanjay Singh Rawat (2013-05-04 13:27:58)
> - use the clk_summary file of CCF for showing clocks
> - add option to show active or all clocks
> 
> Signed-off-by: Sanjay Singh Rawat <sanjay.rawat@linaro.org>

clk_dump gives a JSON-formatted description of the clock tree, so it is
a bit more machine readable than clk_summary.  Also if you wanted to
actually represent a tree structure then clk_dump would be a better
starting point than using clk_summary, which does not include
parent-child relationship information.

Regards,
Mike

> ---
>  clocks.c  |   62 +++++++++++++++++++++++++++++++++++++++++++++++++------------
>  display.c |    3 +++
>  display.h |    1 +
>  3 files changed, 54 insertions(+), 12 deletions(-)
> 
> diff --git a/clocks.c b/clocks.c
> index 95acf57..a0510b4 100644
> --- a/clocks.c
> +++ b/clocks.c
> @@ -287,9 +287,8 @@ static int clock_print_header(void)
>         int ret;
>  
>         if(clock_fw == CCF) {
> -               if (asprintf(&buf, "%-35s %-10s %-12s %-10s %-11s %-15s %-14s %-14s",
> -                    "Name", "Flags", "Rate", "Usecount", "Children", "Prepare_Count",
> -                    "Enable_Count", "Notifier_Count") < 0)
> +               if (asprintf(&buf, "%-30s %-10s %-12s %-10s (clock toggle key-'c')",
> +                    "Name", "Enable_Count", "Prepare_Count", "Rate") < 0)
>                 return -1;
>         }
>         else {
> @@ -297,23 +296,57 @@ static int clock_print_header(void)
>                      "Name", "Flags", "Rate", "Usecount", "Children") < 0)
>                 return -1;
>         }
> -
>         ret = display_column_name(buf);
> -
>         free(buf);
>  
>         return ret;
>  }
>  
> +/*
> + * Display clocks by refering the clk_summary file of CCF
> + */
> +static int display_clk_summary()
> +{
> +        FILE *fp;
> +        char line[256];
> +        int afterheader;
> +        char clock[30];
> +        int enable_cnt,prepare_cnt,rate;
> +
> +        afterheader = 0;
> +        fp = fopen("/sys/kernel/debug/clk/clk_summary","r");
> +        if (fp == NULL) {
> +                printf("error: failed to open clock tree file\n");
> +                return -1;
> +        }
> +
> +        while (NULL != fgets(line,256,fp)) {
> +                if (afterheader > 1) {
> +                        sscanf(line,"%s %d %d %d",clock,&enable_cnt,&prepare_cnt,&rate);
> +                        if (active_clks) {
> +                               if (enable_cnt)
> +                                       display_print_line(CLOCK,afterheader,
> +                                               line,0,clock_tree);
> +                       }
> +                       else
> +                               display_print_line(CLOCK,afterheader,line,1,clock_tree);
> +                }
> +                afterheader++;
> +        }
> +       return 0;
> +}
> +
>  static int clock_print_info(struct tree *tree)
>  {
>         int ret, line = 0;
>  
>         display_reset_cursor(CLOCK);
> -
>         clock_print_header();
>  
> -       ret = tree_for_each(tree, clock_print_info_cb, &line);
> +       if (clock_fw == CCF)
> +               ret = display_clk_summary();
> +       else
> +               ret = tree_for_each(tree, clock_print_info_cb, &line);
>  
>         display_refresh_pad(CLOCK);
>  
> @@ -426,8 +459,10 @@ int clock_init(void)
>  
>         sprintf(clk_dir_path[CCF], "%s/clk", clk_dir_path[CCF]);
>         sprintf(clk_dir_path[OCF], "%s/clock", clk_dir_path[OCF]);
> +
>         if (!access(clk_dir_path[CCF], F_OK)) {
>                 clock_fw = CCF;
> +               active_clks = true;
>                 strcpy(clk_dir_path[MAX],clk_dir_path[CCF]);
>         }
>         else if(!access(clk_dir_path[OCF], F_OK)) {
> @@ -437,12 +472,15 @@ int clock_init(void)
>         else
>                 return -1;
>  
> -       clock_tree = tree_load(clk_dir_path[MAX], NULL, false);
> -       if (!clock_tree)
> -               return -1;
> +       /* Not preparing tree for CCF, will use the clk_summary file */
> +       if(clock_fw != CCF) {
> +               clock_tree = tree_load(clk_dir_path[MAX], NULL, false);
> +               if (!clock_tree)
> +                       return -1;
>  
> -       if (fill_clock_tree())
> -               return -1;
> +               if (fill_clock_tree())
> +                       return -1;
> +       }
>  
>         return display_register(CLOCK, &clock_ops);
>  }
> diff --git a/display.c b/display.c
> index e9f4bf6..98544e6 100644
> --- a/display.c
> +++ b/display.c
> @@ -416,6 +416,9 @@ static int display_keystroke(int fd, void *data)
>         case 'r':
>         case 'R':
>                 return display_refresh(current_win, true);
> +       case 'c':
> +               active_clks = active_clks ? false : true;
> +               return display_refresh(current_win, true);
>         default:
>                 return 0;
>         }
> diff --git a/display.h b/display.h
> index 6362a48..24c9d59 100644
> --- a/display.h
> +++ b/display.h
> @@ -33,4 +33,5 @@ extern int display_init(int wdefault);
>  extern int display_register(int win, struct display_ops *ops);
>  extern int display_column_name(const char *line);
>  
> +bool active_clks;
>  #define NAME_MAX 255
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
Sanjay Singh Rawat May 7, 2013, 5:19 p.m. | #3
On Monday 06 May 2013 09:36 PM, Mike Turquette wrote:
> Quoting Sanjay Singh Rawat (2013-05-04 13:27:58)
>> - use the clk_summary file of CCF for showing clocks
>> - add option to show active or all clocks
>>
>> Signed-off-by: Sanjay Singh Rawat <sanjay.rawat@linaro.org>
>
> clk_dump gives a JSON-formatted description of the clock tree, so it is
> a bit more machine readable than clk_summary.  Also if you wanted to
> actually represent a tree structure then clk_dump would be a better
> starting point than using clk_summary, which does not include
> parent-child relationship information.
>
> Regards,
> Mike
thanks Mike
- The way the clocks are displayed with this patch, the alignment takes 
care of the parent/child relationship.
- The format of the content doesn't matter here IIUC, as anyway we have 
to parse the content and show it in the clock window.
>
>> ---
>>   clocks.c  |   62 +++++++++++++++++++++++++++++++++++++++++++++++++------------
>>   display.c |    3 +++
>>   display.h |    1 +
>>   3 files changed, 54 insertions(+), 12 deletions(-)
>>
>> diff --git a/clocks.c b/clocks.c
>> index 95acf57..a0510b4 100644
>> --- a/clocks.c
>> +++ b/clocks.c
>> @@ -287,9 +287,8 @@ static int clock_print_header(void)
>>          int ret;
>>
>>          if(clock_fw == CCF) {
>> -               if (asprintf(&buf, "%-35s %-10s %-12s %-10s %-11s %-15s %-14s %-14s",
>> -                    "Name", "Flags", "Rate", "Usecount", "Children", "Prepare_Count",
>> -                    "Enable_Count", "Notifier_Count") < 0)
>> +               if (asprintf(&buf, "%-30s %-10s %-12s %-10s (clock toggle key-'c')",
>> +                    "Name", "Enable_Count", "Prepare_Count", "Rate") < 0)
>>                  return -1;
>>          }
>>          else {
>> @@ -297,23 +296,57 @@ static int clock_print_header(void)
>>                       "Name", "Flags", "Rate", "Usecount", "Children") < 0)
>>                  return -1;
>>          }
>> -
>>          ret = display_column_name(buf);
>> -
>>          free(buf);
>>
>>          return ret;
>>   }
>>
>> +/*
>> + * Display clocks by refering the clk_summary file of CCF
>> + */
>> +static int display_clk_summary()
>> +{
>> +        FILE *fp;
>> +        char line[256];
>> +        int afterheader;
>> +        char clock[30];
>> +        int enable_cnt,prepare_cnt,rate;
>> +
>> +        afterheader = 0;
>> +        fp = fopen("/sys/kernel/debug/clk/clk_summary","r");
>> +        if (fp == NULL) {
>> +                printf("error: failed to open clock tree file\n");
>> +                return -1;
>> +        }
>> +
>> +        while (NULL != fgets(line,256,fp)) {
>> +                if (afterheader > 1) {
>> +                        sscanf(line,"%s %d %d %d",clock,&enable_cnt,&prepare_cnt,&rate);
>> +                        if (active_clks) {
>> +                               if (enable_cnt)
>> +                                       display_print_line(CLOCK,afterheader,
>> +                                               line,0,clock_tree);
>> +                       }
>> +                       else
>> +                               display_print_line(CLOCK,afterheader,line,1,clock_tree);
>> +                }
>> +                afterheader++;
>> +        }
>> +       return 0;
>> +}
>> +
>>   static int clock_print_info(struct tree *tree)
>>   {
>>          int ret, line = 0;
>>
>>          display_reset_cursor(CLOCK);
>> -
>>          clock_print_header();
>>
>> -       ret = tree_for_each(tree, clock_print_info_cb, &line);
>> +       if (clock_fw == CCF)
>> +               ret = display_clk_summary();
>> +       else
>> +               ret = tree_for_each(tree, clock_print_info_cb, &line);
>>
>>          display_refresh_pad(CLOCK);
>>
>> @@ -426,8 +459,10 @@ int clock_init(void)
>>
>>          sprintf(clk_dir_path[CCF], "%s/clk", clk_dir_path[CCF]);
>>          sprintf(clk_dir_path[OCF], "%s/clock", clk_dir_path[OCF]);
>> +
>>          if (!access(clk_dir_path[CCF], F_OK)) {
>>                  clock_fw = CCF;
>> +               active_clks = true;
>>                  strcpy(clk_dir_path[MAX],clk_dir_path[CCF]);
>>          }
>>          else if(!access(clk_dir_path[OCF], F_OK)) {
>> @@ -437,12 +472,15 @@ int clock_init(void)
>>          else
>>                  return -1;
>>
>> -       clock_tree = tree_load(clk_dir_path[MAX], NULL, false);
>> -       if (!clock_tree)
>> -               return -1;
>> +       /* Not preparing tree for CCF, will use the clk_summary file */
>> +       if(clock_fw != CCF) {
>> +               clock_tree = tree_load(clk_dir_path[MAX], NULL, false);
>> +               if (!clock_tree)
>> +                       return -1;
>>
>> -       if (fill_clock_tree())
>> -               return -1;
>> +               if (fill_clock_tree())
>> +                       return -1;
>> +       }
>>
>>          return display_register(CLOCK, &clock_ops);
>>   }
>> diff --git a/display.c b/display.c
>> index e9f4bf6..98544e6 100644
>> --- a/display.c
>> +++ b/display.c
>> @@ -416,6 +416,9 @@ static int display_keystroke(int fd, void *data)
>>          case 'r':
>>          case 'R':
>>                  return display_refresh(current_win, true);
>> +       case 'c':
>> +               active_clks = active_clks ? false : true;
>> +               return display_refresh(current_win, true);
>>          default:
>>                  return 0;
>>          }
>> diff --git a/display.h b/display.h
>> index 6362a48..24c9d59 100644
>> --- a/display.h
>> +++ b/display.h
>> @@ -33,4 +33,5 @@ extern int display_init(int wdefault);
>>   extern int display_register(int win, struct display_ops *ops);
>>   extern int display_column_name(const char *line);
>>
>> +bool active_clks;
>>   #define NAME_MAX 255
>> --
>> 1.7.9.5
>>
>>
>> _______________________________________________
>> linaro-dev mailing list
>> linaro-dev@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/linaro-dev
Sebastian Capella May 7, 2013, 10:05 p.m. | #4
The original clock display in powerdebug would expand and collapse subtrees
in the clock heirarchy using the enter key.  If we continue along the lines
you're following, you'll want to override the actions in clock_select to do
nothing as they're currently trying to expand the subtree which is causing
the segfault we see now.

You may instead want to change it so that the new active clock display
works as you've done, but the unfiltered display works like the original
version with expanded and collapsed subtrees.  This will require a bit more
rework, but as it stands, the addition of the active clock view
substantially changes the behavior of the full clock view.  Was this
intentional?

Thanks,

Sebastian



On 7 May 2013 10:19, Sanjay Singh Rawat <sanjay.rawat@linaro.org> wrote:

> On Monday 06 May 2013 09:36 PM, Mike Turquette wrote:
>
>> Quoting Sanjay Singh Rawat (2013-05-04 13:27:58)
>>
>>> - use the clk_summary file of CCF for showing clocks
>>> - add option to show active or all clocks
>>>
>>> Signed-off-by: Sanjay Singh Rawat <sanjay.rawat@linaro.org>
>>>
>>
>> clk_dump gives a JSON-formatted description of the clock tree, so it is
>> a bit more machine readable than clk_summary.  Also if you wanted to
>> actually represent a tree structure then clk_dump would be a better
>> starting point than using clk_summary, which does not include
>> parent-child relationship information.
>>
>> Regards,
>> Mike
>>
> thanks Mike
> - The way the clocks are displayed with this patch, the alignment takes
> care of the parent/child relationship.
> - The format of the content doesn't matter here IIUC, as anyway we have to
> parse the content and show it in the clock window.
>
>
>>  ---
>>>   clocks.c  |   62 ++++++++++++++++++++++++++++++**
>>> +++++++++++++++++++-----------**-
>>>   display.c |    3 +++
>>>   display.h |    1 +
>>>   3 files changed, 54 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/clocks.c b/clocks.c
>>> index 95acf57..a0510b4 100644
>>> --- a/clocks.c
>>> +++ b/clocks.c
>>> @@ -287,9 +287,8 @@ static int clock_print_header(void)
>>>          int ret;
>>>
>>>          if(clock_fw == CCF) {
>>> -               if (asprintf(&buf, "%-35s %-10s %-12s %-10s %-11s %-15s
>>> %-14s %-14s",
>>> -                    "Name", "Flags", "Rate", "Usecount", "Children",
>>> "Prepare_Count",
>>> -                    "Enable_Count", "Notifier_Count") < 0)
>>> +               if (asprintf(&buf, "%-30s %-10s %-12s %-10s (clock
>>> toggle key-'c')",
>>> +                    "Name", "Enable_Count", "Prepare_Count", "Rate") <
>>> 0)
>>>                  return -1;
>>>          }
>>>          else {
>>> @@ -297,23 +296,57 @@ static int clock_print_header(void)
>>>                       "Name", "Flags", "Rate", "Usecount", "Children") <
>>> 0)
>>>                  return -1;
>>>          }
>>> -
>>>          ret = display_column_name(buf);
>>> -
>>>          free(buf);
>>>
>>>          return ret;
>>>   }
>>>
>>> +/*
>>> + * Display clocks by refering the clk_summary file of CCF
>>> + */
>>> +static int display_clk_summary()
>>> +{
>>> +        FILE *fp;
>>> +        char line[256];
>>> +        int afterheader;
>>> +        char clock[30];
>>> +        int enable_cnt,prepare_cnt,rate;
>>> +
>>> +        afterheader = 0;
>>> +        fp = fopen("/sys/kernel/debug/clk/**clk_summary","r");
>>> +        if (fp == NULL) {
>>> +                printf("error: failed to open clock tree file\n");
>>> +                return -1;
>>> +        }
>>> +
>>> +        while (NULL != fgets(line,256,fp)) {
>>> +                if (afterheader > 1) {
>>> +                        sscanf(line,"%s %d %d %d",clock,&enable_cnt,&**
>>> prepare_cnt,&rate);
>>> +                        if (active_clks) {
>>> +                               if (enable_cnt)
>>> +                                       display_print_line(CLOCK,**
>>> afterheader,
>>> +                                               line,0,clock_tree);
>>> +                       }
>>> +                       else
>>> +                               display_print_line(CLOCK,**
>>> afterheader,line,1,clock_tree)**;
>>> +                }
>>> +                afterheader++;
>>> +        }
>>> +       return 0;
>>> +}
>>> +
>>>   static int clock_print_info(struct tree *tree)
>>>   {
>>>          int ret, line = 0;
>>>
>>>          display_reset_cursor(CLOCK);
>>> -
>>>          clock_print_header();
>>>
>>> -       ret = tree_for_each(tree, clock_print_info_cb, &line);
>>> +       if (clock_fw == CCF)
>>> +               ret = display_clk_summary();
>>> +       else
>>> +               ret = tree_for_each(tree, clock_print_info_cb, &line);
>>>
>>>          display_refresh_pad(CLOCK);
>>>
>>> @@ -426,8 +459,10 @@ int clock_init(void)
>>>
>>>          sprintf(clk_dir_path[CCF], "%s/clk", clk_dir_path[CCF]);
>>>          sprintf(clk_dir_path[OCF], "%s/clock", clk_dir_path[OCF]);
>>> +
>>>          if (!access(clk_dir_path[CCF], F_OK)) {
>>>                  clock_fw = CCF;
>>> +               active_clks = true;
>>>                  strcpy(clk_dir_path[MAX],clk_**dir_path[CCF]);
>>>          }
>>>          else if(!access(clk_dir_path[OCF], F_OK)) {
>>> @@ -437,12 +472,15 @@ int clock_init(void)
>>>          else
>>>                  return -1;
>>>
>>> -       clock_tree = tree_load(clk_dir_path[MAX], NULL, false);
>>> -       if (!clock_tree)
>>> -               return -1;
>>> +       /* Not preparing tree for CCF, will use the clk_summary file */
>>> +       if(clock_fw != CCF) {
>>> +               clock_tree = tree_load(clk_dir_path[MAX], NULL, false);
>>> +               if (!clock_tree)
>>> +                       return -1;
>>>
>>> -       if (fill_clock_tree())
>>> -               return -1;
>>> +               if (fill_clock_tree())
>>> +                       return -1;
>>> +       }
>>>
>>>          return display_register(CLOCK, &clock_ops);
>>>   }
>>> diff --git a/display.c b/display.c
>>> index e9f4bf6..98544e6 100644
>>> --- a/display.c
>>> +++ b/display.c
>>> @@ -416,6 +416,9 @@ static int display_keystroke(int fd, void *data)
>>>          case 'r':
>>>          case 'R':
>>>                  return display_refresh(current_win, true);
>>> +       case 'c':
>>> +               active_clks = active_clks ? false : true;
>>> +               return display_refresh(current_win, true);
>>>          default:
>>>                  return 0;
>>>          }
>>> diff --git a/display.h b/display.h
>>> index 6362a48..24c9d59 100644
>>> --- a/display.h
>>> +++ b/display.h
>>> @@ -33,4 +33,5 @@ extern int display_init(int wdefault);
>>>   extern int display_register(int win, struct display_ops *ops);
>>>   extern int display_column_name(const char *line);
>>>
>>> +bool active_clks;
>>>   #define NAME_MAX 255
>>> --
>>> 1.7.9.5
>>>
>>>
>>> ______________________________**_________________
>>> linaro-dev mailing list
>>> linaro-dev@lists.linaro.org
>>> http://lists.linaro.org/**mailman/listinfo/linaro-dev<http://lists.linaro.org/mailman/listinfo/linaro-dev>
>>>
>>
>
> ______________________________**_________________
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/**mailman/listinfo/linaro-dev<http://lists.linaro.org/mailman/listinfo/linaro-dev>
>
Sebastian Capella May 7, 2013, 10:42 p.m. | #5
Hi Sanjay,

Also, clock tree dump (using powerdebug -dc) does not show the clock info
anymore with this change.

Thanks,

Sebastian


On 7 May 2013 15:05, Sebastian Capella <sebastian.capella@linaro.org> wrote:

> The original clock display in powerdebug would expand and collapse
> subtrees in the clock heirarchy using the enter key.  If we continue along
> the lines you're following, you'll want to override the actions in
> clock_select to do nothing as they're currently trying to expand the
> subtree which is causing the segfault we see now.
>
> You may instead want to change it so that the new active clock display
> works as you've done, but the unfiltered display works like the original
> version with expanded and collapsed subtrees.  This will require a bit more
> rework, but as it stands, the addition of the active clock view
> substantially changes the behavior of the full clock view.  Was this
> intentional?
>
> Thanks,
>
> Sebastian
>
>
>
> On 7 May 2013 10:19, Sanjay Singh Rawat <sanjay.rawat@linaro.org> wrote:
>
>> On Monday 06 May 2013 09:36 PM, Mike Turquette wrote:
>>
>>> Quoting Sanjay Singh Rawat (2013-05-04 13:27:58)
>>>
>>>> - use the clk_summary file of CCF for showing clocks
>>>> - add option to show active or all clocks
>>>>
>>>> Signed-off-by: Sanjay Singh Rawat <sanjay.rawat@linaro.org>
>>>>
>>>
>>> clk_dump gives a JSON-formatted description of the clock tree, so it is
>>> a bit more machine readable than clk_summary.  Also if you wanted to
>>> actually represent a tree structure then clk_dump would be a better
>>> starting point than using clk_summary, which does not include
>>> parent-child relationship information.
>>>
>>> Regards,
>>> Mike
>>>
>> thanks Mike
>> - The way the clocks are displayed with this patch, the alignment takes
>> care of the parent/child relationship.
>> - The format of the content doesn't matter here IIUC, as anyway we have
>> to parse the content and show it in the clock window.
>>
>>
>>>  ---
>>>>   clocks.c  |   62 ++++++++++++++++++++++++++++++**
>>>> +++++++++++++++++++-----------**-
>>>>   display.c |    3 +++
>>>>   display.h |    1 +
>>>>   3 files changed, 54 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/clocks.c b/clocks.c
>>>> index 95acf57..a0510b4 100644
>>>> --- a/clocks.c
>>>> +++ b/clocks.c
>>>> @@ -287,9 +287,8 @@ static int clock_print_header(void)
>>>>          int ret;
>>>>
>>>>          if(clock_fw == CCF) {
>>>> -               if (asprintf(&buf, "%-35s %-10s %-12s %-10s %-11s %-15s
>>>> %-14s %-14s",
>>>> -                    "Name", "Flags", "Rate", "Usecount", "Children",
>>>> "Prepare_Count",
>>>> -                    "Enable_Count", "Notifier_Count") < 0)
>>>> +               if (asprintf(&buf, "%-30s %-10s %-12s %-10s (clock
>>>> toggle key-'c')",
>>>> +                    "Name", "Enable_Count", "Prepare_Count", "Rate") <
>>>> 0)
>>>>                  return -1;
>>>>          }
>>>>          else {
>>>> @@ -297,23 +296,57 @@ static int clock_print_header(void)
>>>>                       "Name", "Flags", "Rate", "Usecount", "Children")
>>>> < 0)
>>>>                  return -1;
>>>>          }
>>>> -
>>>>          ret = display_column_name(buf);
>>>> -
>>>>          free(buf);
>>>>
>>>>          return ret;
>>>>   }
>>>>
>>>> +/*
>>>> + * Display clocks by refering the clk_summary file of CCF
>>>> + */
>>>> +static int display_clk_summary()
>>>> +{
>>>> +        FILE *fp;
>>>> +        char line[256];
>>>> +        int afterheader;
>>>> +        char clock[30];
>>>> +        int enable_cnt,prepare_cnt,rate;
>>>> +
>>>> +        afterheader = 0;
>>>> +        fp = fopen("/sys/kernel/debug/clk/**clk_summary","r");
>>>> +        if (fp == NULL) {
>>>> +                printf("error: failed to open clock tree file\n");
>>>> +                return -1;
>>>> +        }
>>>> +
>>>> +        while (NULL != fgets(line,256,fp)) {
>>>> +                if (afterheader > 1) {
>>>> +                        sscanf(line,"%s %d %d %d",clock,&enable_cnt,&*
>>>> *prepare_cnt,&rate);
>>>> +                        if (active_clks) {
>>>> +                               if (enable_cnt)
>>>> +                                       display_print_line(CLOCK,**
>>>> afterheader,
>>>> +                                               line,0,clock_tree);
>>>> +                       }
>>>> +                       else
>>>> +                               display_print_line(CLOCK,**
>>>> afterheader,line,1,clock_tree)**;
>>>> +                }
>>>> +                afterheader++;
>>>> +        }
>>>> +       return 0;
>>>> +}
>>>> +
>>>>   static int clock_print_info(struct tree *tree)
>>>>   {
>>>>          int ret, line = 0;
>>>>
>>>>          display_reset_cursor(CLOCK);
>>>> -
>>>>          clock_print_header();
>>>>
>>>> -       ret = tree_for_each(tree, clock_print_info_cb, &line);
>>>> +       if (clock_fw == CCF)
>>>> +               ret = display_clk_summary();
>>>> +       else
>>>> +               ret = tree_for_each(tree, clock_print_info_cb, &line);
>>>>
>>>>          display_refresh_pad(CLOCK);
>>>>
>>>> @@ -426,8 +459,10 @@ int clock_init(void)
>>>>
>>>>          sprintf(clk_dir_path[CCF], "%s/clk", clk_dir_path[CCF]);
>>>>          sprintf(clk_dir_path[OCF], "%s/clock", clk_dir_path[OCF]);
>>>> +
>>>>          if (!access(clk_dir_path[CCF], F_OK)) {
>>>>                  clock_fw = CCF;
>>>> +               active_clks = true;
>>>>                  strcpy(clk_dir_path[MAX],clk_**dir_path[CCF]);
>>>>          }
>>>>          else if(!access(clk_dir_path[OCF], F_OK)) {
>>>> @@ -437,12 +472,15 @@ int clock_init(void)
>>>>          else
>>>>                  return -1;
>>>>
>>>> -       clock_tree = tree_load(clk_dir_path[MAX], NULL, false);
>>>> -       if (!clock_tree)
>>>> -               return -1;
>>>> +       /* Not preparing tree for CCF, will use the clk_summary file */
>>>> +       if(clock_fw != CCF) {
>>>> +               clock_tree = tree_load(clk_dir_path[MAX], NULL, false);
>>>> +               if (!clock_tree)
>>>> +                       return -1;
>>>>
>>>> -       if (fill_clock_tree())
>>>> -               return -1;
>>>> +               if (fill_clock_tree())
>>>> +                       return -1;
>>>> +       }
>>>>
>>>>          return display_register(CLOCK, &clock_ops);
>>>>   }
>>>> diff --git a/display.c b/display.c
>>>> index e9f4bf6..98544e6 100644
>>>> --- a/display.c
>>>> +++ b/display.c
>>>> @@ -416,6 +416,9 @@ static int display_keystroke(int fd, void *data)
>>>>          case 'r':
>>>>          case 'R':
>>>>                  return display_refresh(current_win, true);
>>>> +       case 'c':
>>>> +               active_clks = active_clks ? false : true;
>>>> +               return display_refresh(current_win, true);
>>>>          default:
>>>>                  return 0;
>>>>          }
>>>> diff --git a/display.h b/display.h
>>>> index 6362a48..24c9d59 100644
>>>> --- a/display.h
>>>> +++ b/display.h
>>>> @@ -33,4 +33,5 @@ extern int display_init(int wdefault);
>>>>   extern int display_register(int win, struct display_ops *ops);
>>>>   extern int display_column_name(const char *line);
>>>>
>>>> +bool active_clks;
>>>>   #define NAME_MAX 255
>>>> --
>>>> 1.7.9.5
>>>>
>>>>
>>>> ______________________________**_________________
>>>> linaro-dev mailing list
>>>> linaro-dev@lists.linaro.org
>>>> http://lists.linaro.org/**mailman/listinfo/linaro-dev<http://lists.linaro.org/mailman/listinfo/linaro-dev>
>>>>
>>>
>>
>> ______________________________**_________________
>> linaro-dev mailing list
>> linaro-dev@lists.linaro.org
>> http://lists.linaro.org/**mailman/listinfo/linaro-dev<http://lists.linaro.org/mailman/listinfo/linaro-dev>
>>
>
>
Sanjay Singh Rawat May 8, 2013, 1:17 p.m. | #6
thanks Sebastian

On Wednesday 08 May 2013 04:12 AM, Sebastian Capella wrote:
> Hi Sanjay,
>
> Also, clock tree dump (using powerdebug -dc) does not show the clock
> info anymore with this change.
>
> Thanks,
Ack, will take care in next patch.
>
> Sebastian
>
>
> On 7 May 2013 15:05, Sebastian Capella <sebastian.capella@linaro.org
> <mailto:sebastian.capella@linaro.org>> wrote:
>
>     The original clock display in powerdebug would expand and collapse
>     subtrees in the clock heirarchy using the enter key.  If we continue
>     along the lines you're following, you'll want to override the
>     actions in clock_select to do nothing as they're currently trying to
>     expand the subtree which is causing the segfault we see now.
ditto
>
>     You may instead want to change it so that the new active clock
>     display works as you've done, but the unfiltered display works like
>     the original version with expanded and collapsed subtrees.  This
>     will require a bit more rework, but as it stands, the addition of
>     the active clock view substantially changes the behavior of the full
>     clock view.  Was this intentional?
The requirement as per the blueprint is to show *ONLY* the active clocks 
in the window. I thought it will be good to keep all clock info also, so 
added that part.
>
>     Thanks,
>
>     Sebastian
>
>
>
>     On 7 May 2013 10:19, Sanjay Singh Rawat <sanjay.rawat@linaro.org
>     <mailto:sanjay.rawat@linaro.org>> wrote:
>
>         On Monday 06 May 2013 09:36 PM, Mike Turquette wrote:
>
>             Quoting Sanjay Singh Rawat (2013-05-04 13:27:58)
>
>                 - use the clk_summary file of CCF for showing clocks
>                 - add option to show active or all clocks
>
>                 Signed-off-by: Sanjay Singh Rawat
>                 <sanjay.rawat@linaro.org <mailto:sanjay.rawat@linaro.org>>
>
>
>             clk_dump gives a JSON-formatted description of the clock
>             tree, so it is
>             a bit more machine readable than clk_summary.  Also if you
>             wanted to
>             actually represent a tree structure then clk_dump would be a
>             better
>             starting point than using clk_summary, which does not include
>             parent-child relationship information.
>
>             Regards,
>             Mike
>
>         thanks Mike
>         - The way the clocks are displayed with this patch, the
>         alignment takes care of the parent/child relationship.
>         - The format of the content doesn't matter here IIUC, as anyway
>         we have to parse the content and show it in the clock window.
>
>
>                 ---
>                    clocks.c  |   62
>                 ++++++++++++++++++++++++++++++__+++++++++++++++++++-----------__-
>                    display.c |    3 +++
>                    display.h |    1 +
>                    3 files changed, 54 insertions(+), 12 deletions(-)
>
>                 diff --git a/clocks.c b/clocks.c
>                 index 95acf57..a0510b4 100644
>                 --- a/clocks.c
>                 +++ b/clocks.c
>                 @@ -287,9 +287,8 @@ static int clock_print_header(void)
>                           int ret;
>
>                           if(clock_fw == CCF) {
>                 -               if (asprintf(&buf, "%-35s %-10s %-12s
>                 %-10s %-11s %-15s %-14s %-14s",
>                 -                    "Name", "Flags", "Rate",
>                 "Usecount", "Children", "Prepare_Count",
>                 -                    "Enable_Count", "Notifier_Count") < 0)
>                 +               if (asprintf(&buf, "%-30s %-10s %-12s
>                 %-10s (clock toggle key-'c')",
>                 +                    "Name", "Enable_Count",
>                 "Prepare_Count", "Rate") < 0)
>                                   return -1;
>                           }
>                           else {
>                 @@ -297,23 +296,57 @@ static int clock_print_header(void)
>                                        "Name", "Flags", "Rate",
>                 "Usecount", "Children") < 0)
>                                   return -1;
>                           }
>                 -
>                           ret = display_column_name(buf);
>                 -
>                           free(buf);
>
>                           return ret;
>                    }
>
>                 +/*
>                 + * Display clocks by refering the clk_summary file of CCF
>                 + */
>                 +static int display_clk_summary()
>                 +{
>                 +        FILE *fp;
>                 +        char line[256];
>                 +        int afterheader;
>                 +        char clock[30];
>                 +        int enable_cnt,prepare_cnt,rate;
>                 +
>                 +        afterheader = 0;
>                 +        fp =
>                 fopen("/sys/kernel/debug/clk/__clk_summary","r");
>                 +        if (fp == NULL) {
>                 +                printf("error: failed to open clock
>                 tree file\n");
>                 +                return -1;
>                 +        }
>                 +
>                 +        while (NULL != fgets(line,256,fp)) {
>                 +                if (afterheader > 1) {
>                 +                        sscanf(line,"%s %d %d
>                 %d",clock,&enable_cnt,&__prepare_cnt,&rate);
>                 +                        if (active_clks) {
>                 +                               if (enable_cnt)
>                 +
>                 display_print_line(CLOCK,__afterheader,
>                 +
>                 line,0,clock_tree);
>                 +                       }
>                 +                       else
>                 +
>                 display_print_line(CLOCK,__afterheader,line,1,clock_tree)__;
>                 +                }
>                 +                afterheader++;
>                 +        }
>                 +       return 0;
>                 +}
>                 +
>                    static int clock_print_info(struct tree *tree)
>                    {
>                           int ret, line = 0;
>
>                           display_reset_cursor(CLOCK);
>                 -
>                           clock_print_header();
>
>                 -       ret = tree_for_each(tree, clock_print_info_cb,
>                 &line);
>                 +       if (clock_fw == CCF)
>                 +               ret = display_clk_summary();
>                 +       else
>                 +               ret = tree_for_each(tree,
>                 clock_print_info_cb, &line);
>
>                           display_refresh_pad(CLOCK);
>
>                 @@ -426,8 +459,10 @@ int clock_init(void)
>
>                           sprintf(clk_dir_path[CCF], "%s/clk",
>                 clk_dir_path[CCF]);
>                           sprintf(clk_dir_path[OCF], "%s/clock",
>                 clk_dir_path[OCF]);
>                 +
>                           if (!access(clk_dir_path[CCF], F_OK)) {
>                                   clock_fw = CCF;
>                 +               active_clks = true;
>
>                   strcpy(clk_dir_path[MAX],clk___dir_path[CCF]);
>                           }
>                           else if(!access(clk_dir_path[OCF], F_OK)) {
>                 @@ -437,12 +472,15 @@ int clock_init(void)
>                           else
>                                   return -1;
>
>                 -       clock_tree = tree_load(clk_dir_path[MAX], NULL,
>                 false);
>                 -       if (!clock_tree)
>                 -               return -1;
>                 +       /* Not preparing tree for CCF, will use the
>                 clk_summary file */
>                 +       if(clock_fw != CCF) {
>                 +               clock_tree =
>                 tree_load(clk_dir_path[MAX], NULL, false);
>                 +               if (!clock_tree)
>                 +                       return -1;
>
>                 -       if (fill_clock_tree())
>                 -               return -1;
>                 +               if (fill_clock_tree())
>                 +                       return -1;
>                 +       }
>
>                           return display_register(CLOCK, &clock_ops);
>                    }
>                 diff --git a/display.c b/display.c
>                 index e9f4bf6..98544e6 100644
>                 --- a/display.c
>                 +++ b/display.c
>                 @@ -416,6 +416,9 @@ static int display_keystroke(int fd,
>                 void *data)
>                           case 'r':
>                           case 'R':
>                                   return display_refresh(current_win, true);
>                 +       case 'c':
>                 +               active_clks = active_clks ? false : true;
>                 +               return display_refresh(current_win, true);
>                           default:
>                                   return 0;
>                           }
>                 diff --git a/display.h b/display.h
>                 index 6362a48..24c9d59 100644
>                 --- a/display.h
>                 +++ b/display.h
>                 @@ -33,4 +33,5 @@ extern int display_init(int wdefault);
>                    extern int display_register(int win, struct
>                 display_ops *ops);
>                    extern int display_column_name(const char *line);
>
>                 +bool active_clks;
>                    #define NAME_MAX 255
>                 --
>                 1.7.9.5
>
>
>                 _________________________________________________
>                 linaro-dev mailing list
>                 linaro-dev@lists.linaro.org
>                 <mailto:linaro-dev@lists.linaro.org>
>                 http://lists.linaro.org/__mailman/listinfo/linaro-dev
>                 <http://lists.linaro.org/mailman/listinfo/linaro-dev>
>
>
>
>         _________________________________________________
>         linaro-dev mailing list
>         linaro-dev@lists.linaro.org <mailto:linaro-dev@lists.linaro.org>
>         http://lists.linaro.org/__mailman/listinfo/linaro-dev
>         <http://lists.linaro.org/mailman/listinfo/linaro-dev>
>
>
>

Patch

diff --git a/clocks.c b/clocks.c
index 95acf57..a0510b4 100644
--- a/clocks.c
+++ b/clocks.c
@@ -287,9 +287,8 @@  static int clock_print_header(void)
 	int ret;
 
 	if(clock_fw == CCF) {
-		if (asprintf(&buf, "%-35s %-10s %-12s %-10s %-11s %-15s %-14s %-14s",
-		     "Name", "Flags", "Rate", "Usecount", "Children", "Prepare_Count",
-		     "Enable_Count", "Notifier_Count") < 0)
+		if (asprintf(&buf, "%-30s %-10s %-12s %-10s (clock toggle key-'c')",
+		     "Name", "Enable_Count", "Prepare_Count", "Rate") < 0)
 		return -1;
 	}
 	else {
@@ -297,23 +296,57 @@  static int clock_print_header(void)
 		     "Name", "Flags", "Rate", "Usecount", "Children") < 0)
 		return -1;
 	}
-
 	ret = display_column_name(buf);
-
 	free(buf);
 
 	return ret;
 }
 
+/*
+ * Display clocks by refering the clk_summary file of CCF
+ */
+static int display_clk_summary()
+{
+        FILE *fp;
+        char line[256];
+        int afterheader;
+        char clock[30];
+        int enable_cnt,prepare_cnt,rate;
+
+        afterheader = 0;
+        fp = fopen("/sys/kernel/debug/clk/clk_summary","r");
+        if (fp == NULL) {
+                printf("error: failed to open clock tree file\n");
+                return -1;
+        }
+
+        while (NULL != fgets(line,256,fp)) {
+                if (afterheader > 1) {
+                        sscanf(line,"%s %d %d %d",clock,&enable_cnt,&prepare_cnt,&rate);
+                        if (active_clks) {
+				if (enable_cnt)
+					display_print_line(CLOCK,afterheader,
+						line,0,clock_tree);
+			}
+			else
+				display_print_line(CLOCK,afterheader,line,1,clock_tree);
+                }
+                afterheader++;
+        }
+	return 0;
+}
+
 static int clock_print_info(struct tree *tree)
 {
 	int ret, line = 0;
 
 	display_reset_cursor(CLOCK);
-
 	clock_print_header();
 
-	ret = tree_for_each(tree, clock_print_info_cb, &line);
+	if (clock_fw == CCF)
+		ret = display_clk_summary();
+	else
+		ret = tree_for_each(tree, clock_print_info_cb, &line);
 
 	display_refresh_pad(CLOCK);
 
@@ -426,8 +459,10 @@  int clock_init(void)
 
 	sprintf(clk_dir_path[CCF], "%s/clk", clk_dir_path[CCF]);
 	sprintf(clk_dir_path[OCF], "%s/clock", clk_dir_path[OCF]);
+
 	if (!access(clk_dir_path[CCF], F_OK)) {
 		clock_fw = CCF;
+		active_clks = true;
 		strcpy(clk_dir_path[MAX],clk_dir_path[CCF]);
 	}
 	else if(!access(clk_dir_path[OCF], F_OK)) {
@@ -437,12 +472,15 @@  int clock_init(void)
 	else
 		return -1;
 
-	clock_tree = tree_load(clk_dir_path[MAX], NULL, false);
-	if (!clock_tree)
-		return -1;
+	/* Not preparing tree for CCF, will use the clk_summary file */
+	if(clock_fw != CCF) {
+		clock_tree = tree_load(clk_dir_path[MAX], NULL, false);
+		if (!clock_tree)
+			return -1;
 
-	if (fill_clock_tree())
-		return -1;
+		if (fill_clock_tree())
+			return -1;
+	}
 
 	return display_register(CLOCK, &clock_ops);
 }
diff --git a/display.c b/display.c
index e9f4bf6..98544e6 100644
--- a/display.c
+++ b/display.c
@@ -416,6 +416,9 @@  static int display_keystroke(int fd, void *data)
 	case 'r':
 	case 'R':
 		return display_refresh(current_win, true);
+	case 'c':
+		active_clks = active_clks ? false : true;
+		return display_refresh(current_win, true);
 	default:
 		return 0;
 	}
diff --git a/display.h b/display.h
index 6362a48..24c9d59 100644
--- a/display.h
+++ b/display.h
@@ -33,4 +33,5 @@  extern int display_init(int wdefault);
 extern int display_register(int win, struct display_ops *ops);
 extern int display_column_name(const char *line);
 
+bool active_clks;
 #define NAME_MAX 255