Move the corresponding function to the right header file and rename the function with <subsystem>_function_name.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- powerdebug.c | 3 ++- powerdebug.h | 1 - regulator.c | 2 +- regulator.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index 2f0992b..fa26e77 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -15,6 +15,7 @@
#include <getopt.h> #include <stdbool.h> +#include "regulator.h" #include "powerdebug.h"
int highlighted_row; @@ -352,7 +353,7 @@ int main(int argc, char **argv) return 1; }
- if (init_regulator_ds()) + if (regulator_init()) return 1;
if (mainloop(options)) diff --git a/powerdebug.h b/powerdebug.h index cef5570..42add86 100644 --- a/powerdebug.h +++ b/powerdebug.h @@ -32,7 +32,6 @@ extern struct regulator_info *regulators_info;
extern int numregulators;
-extern int init_regulator_ds(void); extern void print_regulator_info(int verbose); extern void read_regulator_info(void); extern void print_regulator_info(int verbose); diff --git a/regulator.c b/regulator.c index 18c9777..395571b 100644 --- a/regulator.c +++ b/regulator.c @@ -15,7 +15,7 @@
#include "regulator.h"
-int init_regulator_ds(void) +int regulator_init(void) { DIR *regdir; struct dirent *item; diff --git a/regulator.h b/regulator.h index 3ecf6b8..b6780ab 100644 --- a/regulator.h +++ b/regulator.h @@ -40,4 +40,4 @@ struct regulator_info { int num_users; } *regulators_info;
-extern int numregulators; +extern int regulator_init(void);
Move the corresponding functions to the right header file and rename the function with <subsystem>_function_name.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- powerdebug.c | 4 ++-- powerdebug.h | 4 ---- regulator.c | 8 ++++---- regulator.h | 2 ++ 4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index fa26e77..0469eb8 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -261,13 +261,13 @@ int mainloop(struct powerdebug_options *options) }
if (options->regulators || options->selectedwindow == REGULATOR) { - read_regulator_info(); + regulator_read_info(); if (!options->dump) { create_selectedwindow(options->selectedwindow); show_regulator_info(options->verbose); } else - print_regulator_info(options->verbose); + regulator_print_info(options->verbose); }
if (options->clocks || options->selectedwindow == CLOCK) { diff --git a/powerdebug.h b/powerdebug.h index 42add86..d04cfce 100644 --- a/powerdebug.h +++ b/powerdebug.h @@ -32,10 +32,6 @@ extern struct regulator_info *regulators_info;
extern int numregulators;
-extern void print_regulator_info(int verbose); -extern void read_regulator_info(void); -extern void print_regulator_info(int verbose); - extern void read_and_dump_clock_info(int verbose); extern void read_and_dump_clock_info_one(char *clk, bool dump); extern void read_clock_info(char *clkpath); diff --git a/regulator.c b/regulator.c index 395571b..255a56d 100644 --- a/regulator.c +++ b/regulator.c @@ -42,14 +42,14 @@ int regulator_init(void) return(0); }
-void print_string_val(char *name, char *val) +static void print_string_val(char *name, char *val) { printf("\t%s=%s", name, val); if (!strchr(val, '\n')) printf("\n"); }
-void print_regulator_info(int verbose) +void regulator_print_info(int verbose) { int i;
@@ -109,7 +109,7 @@ void print_regulator_info(int verbose) printf("\n\n"); }
-void read_info_from_dirent(struct dirent *ritem, char *str, int idx) +static void read_info_from_dirent(struct dirent *ritem, char *str, int idx) { if (!strcmp(ritem->d_name, "name")) strcpy(regulators_info[idx].name, str); @@ -143,7 +143,7 @@ void read_info_from_dirent(struct dirent *ritem, char *str, int idx) regulators_info[idx].num_users = atoi(str); }
-int read_regulator_info(void) +int regulator_read_info(void) { FILE *file = NULL; DIR *regdir, *dir; diff --git a/regulator.h b/regulator.h index b6780ab..bda46fd 100644 --- a/regulator.h +++ b/regulator.h @@ -41,3 +41,5 @@ struct regulator_info { } *regulators_info;
extern int regulator_init(void); +extern int regulator_read_info(void); +extern void regulator_print_info(int verbose);
All the files found in the /sys/class/regulator directory should be regulators. Don't assume their name begin with 'regulator'.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- regulator.c | 18 ++++++++++++++---- 1 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/regulator.c b/regulator.c index 255a56d..342a1a7 100644 --- a/regulator.c +++ b/regulator.c @@ -15,20 +15,30 @@
#include "regulator.h"
+#define SYSFS_REGULATOR "/sys/class/regulator" + int regulator_init(void) { DIR *regdir; struct dirent *item;
- regdir = opendir("/sys/class/regulator"); - if (!regdir) - return(1); + regdir = opendir(SYSFS_REGULATOR); + if (!regdir) { + fprintf(stderr, "failed to open '%s': %m\n", SYSFS_REGULATOR); + return -1; + } + while ((item = readdir(regdir))) { - if (strncmp(item->d_name, "regulator", 9)) + + if (!strcmp(item->d_name, ".")) + continue; + + if (!strcmp(item->d_name, "..")) continue;
numregulators++; } + closedir(regdir);
regulators_info = (struct regulator_info *)malloc(numregulators*
The objective is to make the regulators_info variable local to the main function. This first patch pass the regulators_info variable to the function instead of having this one relying on a global variable definition.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- powerdebug.c | 2 +- regulator.c | 52 ++++++++++++++++++++++++++-------------------------- regulator.h | 2 +- 3 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index 0469eb8..81875eb 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -267,7 +267,7 @@ int mainloop(struct powerdebug_options *options) show_regulator_info(options->verbose); } else - regulator_print_info(options->verbose); + regulator_print_info(regulators_info, options->verbose); }
if (options->clocks || options->selectedwindow == CLOCK) { diff --git a/regulator.c b/regulator.c index 342a1a7..91c3903 100644 --- a/regulator.c +++ b/regulator.c @@ -59,7 +59,7 @@ static void print_string_val(char *name, char *val) printf("\n"); }
-void regulator_print_info(int verbose) +void regulator_print_info(struct regulator_info *reg_info, int verbose) { int i;
@@ -68,46 +68,46 @@ void regulator_print_info(int verbose)
for (i = 0; i < numregulators; i++) { printf("Regulator %d:\n", i + 1); - print_string_val("name", regulators_info[i].name); - if (strcmp(regulators_info[i].status, "")) - print_string_val("status", regulators_info[i].status); - if (strcmp(regulators_info[i].state, "")) - print_string_val("state", regulators_info[i].state); + print_string_val("name", reg_info[i].name); + if (strcmp(reg_info[i].status, "")) + print_string_val("status", reg_info[i].status); + if (strcmp(reg_info[i].state, "")) + print_string_val("state", reg_info[i].state);
if (!verbose) continue;
- if (strcmp(regulators_info[i].type, "")) - print_string_val("type", regulators_info[i].type); - if (strcmp(regulators_info[i].opmode, "")) - print_string_val("opmode", regulators_info[i].opmode); + if (strcmp(reg_info[i].type, "")) + print_string_val("type", reg_info[i].type); + if (strcmp(reg_info[i].opmode, "")) + print_string_val("opmode", reg_info[i].opmode);
- if (regulators_info[i].microvolts) + if (reg_info[i].microvolts) printf("\tmicrovolts=%d\n", - regulators_info[i].microvolts); - if (regulators_info[i].min_microvolts) + reg_info[i].microvolts); + if (reg_info[i].min_microvolts) printf("\tmin_microvolts=%d\n", - regulators_info[i].min_microvolts); - if (regulators_info[i].max_microvolts) + reg_info[i].min_microvolts); + if (reg_info[i].max_microvolts) printf("\tmax_microvolts=%d\n", - regulators_info[i].max_microvolts); + reg_info[i].max_microvolts);
- if (regulators_info[i].microamps) + if (reg_info[i].microamps) printf("\tmicroamps=%d\n", - regulators_info[i].microamps); - if (regulators_info[i].min_microamps) + reg_info[i].microamps); + if (reg_info[i].min_microamps) printf("\tmin_microamps=%d\n", - regulators_info[i].min_microamps); - if (regulators_info[i].max_microamps) + reg_info[i].min_microamps); + if (reg_info[i].max_microamps) printf("\tmax_microamps=%d\n", - regulators_info[i].max_microamps); - if (regulators_info[i].requested_microamps) + reg_info[i].max_microamps); + if (reg_info[i].requested_microamps) printf("\trequested_microamps=%d\n", - regulators_info[i].requested_microamps); + reg_info[i].requested_microamps);
- if (regulators_info[i].num_users) + if (reg_info[i].num_users) printf("\tnum_users=%d\n", - regulators_info[i].num_users); + reg_info[i].num_users); printf("\n"); }
diff --git a/regulator.h b/regulator.h index bda46fd..d46114a 100644 --- a/regulator.h +++ b/regulator.h @@ -42,4 +42,4 @@ struct regulator_info {
extern int regulator_init(void); extern int regulator_read_info(void); -extern void regulator_print_info(int verbose); +extern void regulator_print_info(struct regulator_info *reg_info, int verbose);
The regulator_init function does no longer use the global defined function.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- powerdebug.c | 5 ++++- regulator.c | 18 ++++++------------ regulator.h | 2 +- 3 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index 81875eb..5f23e0c 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -353,8 +353,11 @@ int main(int argc, char **argv) return 1; }
- if (regulator_init()) + regulators_info = regulator_init(&numregulators); + if (!regulators_info) { + printf("not enough memory to allocate regulators info\n"); return 1; + }
if (mainloop(options)) return 1; diff --git a/regulator.c b/regulator.c index 91c3903..ddbeea3 100644 --- a/regulator.c +++ b/regulator.c @@ -17,15 +17,17 @@
#define SYSFS_REGULATOR "/sys/class/regulator"
-int regulator_init(void) +struct regulator_info *regulator_init(int *nr_regulators) { DIR *regdir; struct dirent *item;
+ *nr_regulators = 0; + regdir = opendir(SYSFS_REGULATOR); if (!regdir) { fprintf(stderr, "failed to open '%s': %m\n", SYSFS_REGULATOR); - return -1; + return NULL; }
while ((item = readdir(regdir))) { @@ -36,20 +38,12 @@ int regulator_init(void) if (!strcmp(item->d_name, "..")) continue;
- numregulators++; + (*nr_regulators)++; }
closedir(regdir);
- regulators_info = (struct regulator_info *)malloc(numregulators* - sizeof(struct regulator_info)); - if (!regulators_info) { - fprintf(stderr, "init_regulator_ds: Not enough memory to " - "read information for %d regulators!\n", numregulators); - return(1); - } - - return(0); + return malloc(*nr_regulators * sizeof(struct regulator_info)); }
static void print_string_val(char *name, char *val) diff --git a/regulator.h b/regulator.h index d46114a..a753299 100644 --- a/regulator.h +++ b/regulator.h @@ -40,6 +40,6 @@ struct regulator_info { int num_users; } *regulators_info;
-extern int regulator_init(void); +extern struct regulator_info *regulator_init(int *nr_regulators); extern int regulator_read_info(void); extern void regulator_print_info(struct regulator_info *reg_info, int verbose);
The read_info_from_dirent does no longer rely on the regulators_info global variable.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- regulator.c | 33 ++++++++++++++++++--------------- 1 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/regulator.c b/regulator.c index ddbeea3..fc40f27 100644 --- a/regulator.c +++ b/regulator.c @@ -113,38 +113,39 @@ void regulator_print_info(struct regulator_info *reg_info, int verbose) printf("\n\n"); }
-static void read_info_from_dirent(struct dirent *ritem, char *str, int idx) +static void read_info_from_dirent(struct regulator_info *reg_info, + struct dirent *ritem, char *str, int idx) { if (!strcmp(ritem->d_name, "name")) - strcpy(regulators_info[idx].name, str); + strcpy(reg_info[idx].name, str); if (!strcmp(ritem->d_name, "state")) - strcpy(regulators_info[idx].state, str); + strcpy(reg_info[idx].state, str); if (!strcmp(ritem->d_name, "status")) - strcpy(regulators_info[idx].status, str); + strcpy(reg_info[idx].status, str);
if (!strcmp(ritem->d_name, "type")) - strcpy(regulators_info[idx].type, str); + strcpy(reg_info[idx].type, str); if (!strcmp(ritem->d_name, "opmode")) - strcpy(regulators_info[idx].opmode, str); + strcpy(reg_info[idx].opmode, str);
if (!strcmp(ritem->d_name, "microvolts")) - regulators_info[idx].microvolts = atoi(str); + reg_info[idx].microvolts = atoi(str); if (!strcmp(ritem->d_name, "min_microvolts")) - regulators_info[idx].min_microvolts = atoi(str); + reg_info[idx].min_microvolts = atoi(str); if (!strcmp(ritem->d_name, "max_microvolts")) - regulators_info[idx].max_microvolts = atoi(str); + reg_info[idx].max_microvolts = atoi(str);
if (!strcmp(ritem->d_name, "microamps")) - regulators_info[idx].microamps = atoi(str); + reg_info[idx].microamps = atoi(str); if (!strcmp(ritem->d_name, "min_microamps")) - regulators_info[idx].min_microamps = atoi(str); + reg_info[idx].min_microamps = atoi(str); if (!strcmp(ritem->d_name, "max_microamps")) - regulators_info[idx].max_microamps = atoi(str); + reg_info[idx].max_microamps = atoi(str); if (!strcmp(ritem->d_name, "requested_microamps")) - regulators_info[idx].requested_microamps = atoi(str); + reg_info[idx].requested_microamps = atoi(str);
if (!strcmp(ritem->d_name, "num_users")) - regulators_info[idx].num_users = atoi(str); + reg_info[idx].num_users = atoi(str); }
int regulator_read_info(void) @@ -192,7 +193,9 @@ int regulator_read_info(void) fclose(file); if (!fptr) continue; - read_info_from_dirent(ritem, fptr, count - 1); + + read_info_from_dirent(regulators_info, ritem, + fptr, count - 1); } exit: closedir(dir);
The regulator_read_info function does no longer rely on the regulators_info global variable.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- powerdebug.c | 2 +- regulator.c | 13 ++++++------- regulator.h | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index 5f23e0c..a8e98c9 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -261,7 +261,7 @@ int mainloop(struct powerdebug_options *options) }
if (options->regulators || options->selectedwindow == REGULATOR) { - regulator_read_info(); + regulator_read_info(regulators_info); if (!options->dump) { create_selectedwindow(options->selectedwindow); show_regulator_info(options->verbose); diff --git a/regulator.c b/regulator.c index fc40f27..0371961 100644 --- a/regulator.c +++ b/regulator.c @@ -107,7 +107,7 @@ void regulator_print_info(struct regulator_info *reg_info, int verbose)
if (!numregulators && verbose) { printf("Could not find regulator information!"); - printf(" Looks like /sys/class/regulator is empty.\n\n"); + printf(" Looks like %s is empty.\n\n", SYSFS_REGULATOR); }
printf("\n\n"); @@ -148,7 +148,7 @@ static void read_info_from_dirent(struct regulator_info *reg_info, reg_info[idx].num_users = atoi(str); }
-int regulator_read_info(void) +int regulator_read_info(struct regulator_info *reg_info) { FILE *file = NULL; DIR *regdir, *dir; @@ -156,7 +156,7 @@ int regulator_read_info(void) char line[1024], filename[1024], *fptr; struct dirent *item, *ritem;
- regdir = opendir("/sys/class/regulator"); + regdir = opendir(SYSFS_REGULATOR); if (!regdir) return(1); while ((item = readdir(regdir))) { @@ -166,8 +166,7 @@ int regulator_read_info(void) if (strncmp(item->d_name, "regulator", 9)) continue;
- len = sprintf(filename, "/sys/class/regulator/%s", - item->d_name); + len = sprintf(filename, "%s/%s", SYSFS_REGULATOR, item->d_name);
dir = opendir(filename); if (!dir) @@ -179,7 +178,7 @@ int regulator_read_info(void) goto exit; }
- strcpy(regulators_info[count-1].name, item->d_name); + strcpy(reg_info[count-1].name, item->d_name); while ((ritem = readdir(dir))) { if (strlen(ritem->d_name) < 3) continue; @@ -194,7 +193,7 @@ int regulator_read_info(void) if (!fptr) continue;
- read_info_from_dirent(regulators_info, ritem, + read_info_from_dirent(reg_info, ritem, fptr, count - 1); } exit: diff --git a/regulator.h b/regulator.h index a753299..115780e 100644 --- a/regulator.h +++ b/regulator.h @@ -41,5 +41,5 @@ struct regulator_info { } *regulators_info;
extern struct regulator_info *regulator_init(int *nr_regulators); -extern int regulator_read_info(void); +extern int regulator_read_info(struct regulator_info *reg_info); extern void regulator_print_info(struct regulator_info *reg_info, int verbose);
The show_regulator_info function does no longer rely on the global regulators_info variable.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- display.c | 20 ++++++++++---------- powerdebug.c | 2 +- powerdebug.h | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/display.c b/display.c index 8b03444..b439814 100644 --- a/display.c +++ b/display.c @@ -176,7 +176,7 @@ void show_header(int selectedwindow) }
-void show_regulator_info(int verbose) +void show_regulator_info(struct regulator_info *reg_info, int verbose) { int i, count = 1;
@@ -200,34 +200,34 @@ void show_regulator_info(int verbose) if ((i + 2) > (maxy-2)) break;
- if (regulators_info[i].num_users > 0) + if (reg_info[i].num_users > 0) wattron(regulator_win, WA_BOLD); else wattroff(regulator_win, WA_BOLD);
print(regulator_win, col, count, "%s", - regulators_info[i].name); + reg_info[i].name); col += 12; print(regulator_win, col, count, "%s", - regulators_info[i].status); + reg_info[i].status); col += 12; print(regulator_win, col, count, "%s", - regulators_info[i].state); + reg_info[i].state); col += 12; print(regulator_win, col, count, "%s", - regulators_info[i].type); + reg_info[i].type); col += 12; print(regulator_win, col, count, "%d", - regulators_info[i].num_users); + reg_info[i].num_users); col += 12; print(regulator_win, col, count, "%d", - regulators_info[i].microvolts); + reg_info[i].microvolts); col += 12; print(regulator_win, col, count, "%d", - regulators_info[i].min_microvolts); + reg_info[i].min_microvolts); col += 12; print(regulator_win, col, count, "%d", - regulators_info[i].max_microvolts); + reg_info[i].max_microvolts);
count++; } diff --git a/powerdebug.c b/powerdebug.c index a8e98c9..15194da 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -264,7 +264,7 @@ int mainloop(struct powerdebug_options *options) regulator_read_info(regulators_info); if (!options->dump) { create_selectedwindow(options->selectedwindow); - show_regulator_info(options->verbose); + show_regulator_info(regulators_info, options->verbose); } else regulator_print_info(regulators_info, options->verbose); diff --git a/powerdebug.h b/powerdebug.h index d04cfce..ac2b042 100644 --- a/powerdebug.h +++ b/powerdebug.h @@ -58,4 +58,4 @@ extern void killall_windows(int all); extern void show_header(int selectedwindow); extern void create_windows(int selectedwindow); extern void create_selectedwindow(int selectedwindow); -extern void show_regulator_info(int verbose); +extern void show_regulator_info(struct regulator_info *reg_info, int verbose);
As no function rely on the global variable regulators_info, let's make it static and reduce the scope to powerdebug.c
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- powerdebug.c | 2 ++ powerdebug.h | 4 ++-- regulator.h | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index 15194da..7d91845 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -20,6 +20,8 @@
int highlighted_row;
+static struct regulator_info *regulators_info; + void usage(void) { printf("Usage: powerdebug [OPTIONS]\n"); diff --git a/powerdebug.h b/powerdebug.h index ac2b042..e92049a 100644 --- a/powerdebug.h +++ b/powerdebug.h @@ -28,8 +28,6 @@ enum {CLOCK, REGULATOR, SENSOR}; enum {CLOCK_SELECTED = 1, REFRESH_WINDOW};
-extern struct regulator_info *regulators_info; - extern int numregulators;
extern void read_and_dump_clock_info(int verbose); @@ -58,4 +56,6 @@ extern void killall_windows(int all); extern void show_header(int selectedwindow); extern void create_windows(int selectedwindow); extern void create_selectedwindow(int selectedwindow); + +struct regulator_info; extern void show_regulator_info(struct regulator_info *reg_info, int verbose); diff --git a/regulator.h b/regulator.h index 115780e..814691e 100644 --- a/regulator.h +++ b/regulator.h @@ -38,7 +38,7 @@ struct regulator_info { int max_microamps; int requested_microamps; int num_users; -} *regulators_info; +};
extern struct regulator_info *regulator_init(int *nr_regulators); extern int regulator_read_info(struct regulator_info *reg_info);
The regulator_print_info does no longer rely on the numregulators global variable.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- powerdebug.c | 3 ++- regulator.c | 6 +++--- regulator.h | 3 ++- 3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index 7d91845..d310d5c 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -269,7 +269,8 @@ int mainloop(struct powerdebug_options *options) show_regulator_info(regulators_info, options->verbose); } else - regulator_print_info(regulators_info, options->verbose); + regulator_print_info(regulators_info, numregulators, + options->verbose); }
if (options->clocks || options->selectedwindow == CLOCK) { diff --git a/regulator.c b/regulator.c index 0371961..93a03cf 100644 --- a/regulator.c +++ b/regulator.c @@ -53,14 +53,14 @@ static void print_string_val(char *name, char *val) printf("\n"); }
-void regulator_print_info(struct regulator_info *reg_info, int verbose) +void regulator_print_info(struct regulator_info *reg_info, int nr_reg, int verbose) { int i;
printf("\nRegulator Information:\n"); printf("*********************\n\n");
- for (i = 0; i < numregulators; i++) { + for (i = 0; i < nr_reg; i++) { printf("Regulator %d:\n", i + 1); print_string_val("name", reg_info[i].name); if (strcmp(reg_info[i].status, "")) @@ -105,7 +105,7 @@ void regulator_print_info(struct regulator_info *reg_info, int verbose) printf("\n"); }
- if (!numregulators && verbose) { + if (!nr_reg && verbose) { printf("Could not find regulator information!"); printf(" Looks like %s is empty.\n\n", SYSFS_REGULATOR); } diff --git a/regulator.h b/regulator.h index 814691e..0c843b3 100644 --- a/regulator.h +++ b/regulator.h @@ -42,4 +42,5 @@ struct regulator_info {
extern struct regulator_info *regulator_init(int *nr_regulators); extern int regulator_read_info(struct regulator_info *reg_info); -extern void regulator_print_info(struct regulator_info *reg_info, int verbose); +extern void regulator_print_info(struct regulator_info *reg_info, + int nr_reg, int verbose);
The regulator_read_info does no longer depend no the global variable numregulators.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- powerdebug.c | 2 +- regulator.c | 4 ++-- regulator.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index d310d5c..d705bb9 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -263,7 +263,7 @@ int mainloop(struct powerdebug_options *options) }
if (options->regulators || options->selectedwindow == REGULATOR) { - regulator_read_info(regulators_info); + regulator_read_info(regulators_info, numregulators); if (!options->dump) { create_selectedwindow(options->selectedwindow); show_regulator_info(regulators_info, options->verbose); diff --git a/regulator.c b/regulator.c index 93a03cf..60529ed 100644 --- a/regulator.c +++ b/regulator.c @@ -148,7 +148,7 @@ static void read_info_from_dirent(struct regulator_info *reg_info, reg_info[idx].num_users = atoi(str); }
-int regulator_read_info(struct regulator_info *reg_info) +int regulator_read_info(struct regulator_info *reg_info, int nr_reg) { FILE *file = NULL; DIR *regdir, *dir; @@ -173,7 +173,7 @@ int regulator_read_info(struct regulator_info *reg_info) continue; count++;
- if (count > numregulators) { + if (count > nr_reg) { ret = 1; goto exit; } diff --git a/regulator.h b/regulator.h index 0c843b3..8781f36 100644 --- a/regulator.h +++ b/regulator.h @@ -41,6 +41,6 @@ struct regulator_info { };
extern struct regulator_info *regulator_init(int *nr_regulators); -extern int regulator_read_info(struct regulator_info *reg_info); +extern int regulator_read_info(struct regulator_info *reg_info, int nr_reg); extern void regulator_print_info(struct regulator_info *reg_info, int nr_reg, int verbose);
The show_regulator_info function does no longer depend no the global variable numregulators.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- display.c | 4 ++-- powerdebug.c | 3 ++- powerdebug.h | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/display.c b/display.c index b439814..fb252cb 100644 --- a/display.c +++ b/display.c @@ -176,7 +176,7 @@ void show_header(int selectedwindow) }
-void show_regulator_info(struct regulator_info *reg_info, int verbose) +void show_regulator_info(struct regulator_info *reg_info, int nr_reg, int verbose) { int i, count = 1;
@@ -194,7 +194,7 @@ void show_regulator_info(struct regulator_info *reg_info, int verbose) print(regulator_win, 84, 0, "Max u-volts"); wattroff(regulator_win, A_BOLD);
- for (i = 0; i < numregulators; i++) { + for (i = 0; i < nr_reg; i++) { int col = 0;
if ((i + 2) > (maxy-2)) diff --git a/powerdebug.c b/powerdebug.c index d705bb9..6e9e896 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -266,7 +266,8 @@ int mainloop(struct powerdebug_options *options) regulator_read_info(regulators_info, numregulators); if (!options->dump) { create_selectedwindow(options->selectedwindow); - show_regulator_info(regulators_info, options->verbose); + show_regulator_info(regulators_info, numregulators, + options->verbose); } else regulator_print_info(regulators_info, numregulators, diff --git a/powerdebug.h b/powerdebug.h index e92049a..3c4f1e8 100644 --- a/powerdebug.h +++ b/powerdebug.h @@ -58,4 +58,5 @@ extern void create_windows(int selectedwindow); extern void create_selectedwindow(int selectedwindow);
struct regulator_info; -extern void show_regulator_info(struct regulator_info *reg_info, int verbose); +extern void show_regulator_info(struct regulator_info *reg_info, + int nr_reg, int verbose);
Reduce the scope of the numregulators to powerdebug.c
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- powerdebug.c | 1 + powerdebug.h | 2 -- regulator.h | 2 -- 3 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index 6e9e896..6dd4b8c 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -21,6 +21,7 @@ int highlighted_row;
static struct regulator_info *regulators_info; +static int numregulators;
void usage(void) { diff --git a/powerdebug.h b/powerdebug.h index 3c4f1e8..7ec7d96 100644 --- a/powerdebug.h +++ b/powerdebug.h @@ -28,8 +28,6 @@ enum {CLOCK, REGULATOR, SENSOR}; enum {CLOCK_SELECTED = 1, REFRESH_WINDOW};
-extern int numregulators; - extern void read_and_dump_clock_info(int verbose); extern void read_and_dump_clock_info_one(char *clk, bool dump); extern void read_clock_info(char *clkpath); diff --git a/regulator.h b/regulator.h index 8781f36..1633cfd 100644 --- a/regulator.h +++ b/regulator.h @@ -22,8 +22,6 @@
#define VALUE_MAX 16
-int numregulators; - struct regulator_info { char name[NAME_MAX]; char state[VALUE_MAX];
Kill the global variables and make them local to the main function.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- powerdebug.c | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index 6dd4b8c..0b26678 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -20,9 +20,6 @@
int highlighted_row;
-static struct regulator_info *regulators_info; -static int numregulators; - void usage(void) { printf("Usage: powerdebug [OPTIONS]\n"); @@ -239,7 +236,8 @@ int keystroke_callback(bool *enter_hit, bool *findparent_ncurses, return 0; }
-int mainloop(struct powerdebug_options *options) +int mainloop(struct powerdebug_options *options, + struct regulator_info *reg_info, int nr_reg) { bool findparent_ncurses = false; bool refreshwin = false; @@ -264,14 +262,14 @@ int mainloop(struct powerdebug_options *options) }
if (options->regulators || options->selectedwindow == REGULATOR) { - regulator_read_info(regulators_info, numregulators); + regulator_read_info(reg_info, nr_reg); if (!options->dump) { create_selectedwindow(options->selectedwindow); - show_regulator_info(regulators_info, numregulators, + show_regulator_info(reg_info, nr_reg, options->verbose); } else - regulator_print_info(regulators_info, numregulators, + regulator_print_info(reg_info, nr_reg, options->verbose); }
@@ -346,6 +344,8 @@ int mainloop(struct powerdebug_options *options) int main(int argc, char **argv) { struct powerdebug_options *options; + struct regulator_info *regulators_info; + int numregulators;
options = malloc(sizeof(*options)); if (!options) { @@ -364,7 +364,7 @@ int main(int argc, char **argv) return 1; }
- if (mainloop(options)) + if (mainloop(options, regulators_info, numregulators)) return 1;
return 0;
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- powerdebug.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index 0b26678..7eb9c5e 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -18,7 +18,7 @@ #include "regulator.h" #include "powerdebug.h"
-int highlighted_row; +static int highlighted_row;
void usage(void) {
Let's create a specific function to dump instead of managing with ncurses
Signed-off-by: Daniel Lezcano daniel.lezcano@free.frIndex: powerdebug/powerdebug.c =================================================================== --- powerdebug.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index 7eb9c5e..bb024fc 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -341,6 +341,12 @@ int mainloop(struct powerdebug_options *options, return 0; }
+static int powerdebug_dump(struct powerdebug_options *options, + struct regulator_info *reg_info, int nr_reg) +{ + return 0; +} + int main(int argc, char **argv) { struct powerdebug_options *options; @@ -364,6 +370,13 @@ int main(int argc, char **argv) return 1; }
+ /* we just dump the informations */ + if (options->dump) { + if (powerdebug_dump(options, regulators_info, numregulators)) + return 1; + return 0; + } + if (mainloop(options, regulators_info, numregulators)) return 1;
When we reach the mainloop, that means we are not in dump-mode, so we can remove the test with the dump mode. In the other side, let's use the dump function when we are in dump-mode.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- powerdebug.c | 57 ++++++++++++++++++++++++++++----------------------------- 1 files changed, 28 insertions(+), 29 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index bb024fc..ca7db7a 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -254,23 +254,16 @@ int mainloop(struct powerdebug_options *options, struct timeval tval; fd_set readfds;
- if (!options->dump) { - if (firsttime[0]) - init_curses(); - create_windows(options->selectedwindow); - show_header(options->selectedwindow); - } + if (firsttime[0]) + init_curses(); + create_windows(options->selectedwindow); + show_header(options->selectedwindow);
if (options->regulators || options->selectedwindow == REGULATOR) { regulator_read_info(reg_info, nr_reg); - if (!options->dump) { - create_selectedwindow(options->selectedwindow); - show_regulator_info(reg_info, nr_reg, - options->verbose); - } - else - regulator_print_info(reg_info, nr_reg, - options->verbose); + create_selectedwindow(options->selectedwindow); + show_regulator_info(reg_info, nr_reg, + options->verbose); }
if (options->clocks || options->selectedwindow == CLOCK) { @@ -282,7 +275,7 @@ int mainloop(struct powerdebug_options *options, firsttime[CLOCK] = 0; strcpy(clkname_str, ""); } - if (!ret && !options->dump) { + if (!ret) { int hrow;
create_selectedwindow(options->selectedwindow); @@ -304,25 +297,13 @@ int mainloop(struct powerdebug_options *options, enter_hit, options->dump); } - if (!ret && options->dump) { - if (options->findparent) - read_and_dump_clock_info_one(options->clkarg, options->dump); - else - read_and_dump_clock_info(options->verbose); - } }
if (options->sensors || options->selectedwindow == SENSOR) { - if (!options->dump) { - create_selectedwindow(options->selectedwindow); - print_sensor_header(); - } else - read_and_print_sensor_info(options->verbose); + create_selectedwindow(options->selectedwindow); + print_sensor_header(); }
- if (options->dump) - break; - FD_ZERO(&readfds); FD_SET(0, &readfds); tval.tv_sec = options->ticktime; @@ -344,6 +325,24 @@ int mainloop(struct powerdebug_options *options, static int powerdebug_dump(struct powerdebug_options *options, struct regulator_info *reg_info, int nr_reg) { + if (options->regulators) { + regulator_read_info(reg_info, nr_reg); + regulator_print_info(reg_info, nr_reg, options->verbose); + } + + if (options->clocks) { + init_clock_details(options->dump, options->selectedwindow); + + if (options->findparent) + read_and_dump_clock_info_one(options->clkarg, + options->dump); + else + read_and_dump_clock_info(options->verbose); + } + + if (options->sensors) + read_and_print_sensor_info(options->verbose); + return 0; }
The modified function are used on the display mode context, we no longer need to pass the dump option.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- clocks.c | 4 ++-- powerdebug.c | 3 +-- powerdebug.h | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/clocks.c b/clocks.c index 4b5b0b7..c42f589 100644 --- a/clocks.c +++ b/clocks.c @@ -146,7 +146,7 @@ static void dump_all_parents(char *clkarg, bool dump) } }
-void find_parents_for_clock(char *clkname, int complete, bool dump) +void find_parents_for_clock(char *clkname, int complete) { char name[256];
@@ -161,7 +161,7 @@ void find_parents_for_clock(char *clkname, int complete, bool dump) } sprintf(name, "Parents for "%s" Clock : \n", clkname); print_one_clock(0, name, 1, 1); - dump_all_parents(clkname, dump); + dump_all_parents(clkname, false); }
int read_and_print_clock_info(int verbose, int hrow, int selected) diff --git a/powerdebug.c b/powerdebug.c index ca7db7a..3832da6 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -294,8 +294,7 @@ int mainloop(struct powerdebug_options *options, enter_hit = false; } else find_parents_for_clock(clkname_str, - enter_hit, - options->dump); + enter_hit); } }
diff --git a/powerdebug.h b/powerdebug.h index 7ec7d96..d2bd4ec 100644 --- a/powerdebug.h +++ b/powerdebug.h @@ -35,7 +35,7 @@ extern struct clock_info *read_clock_info_recur(char *clkpath, int level, struct clock_info *parent); extern void dump_clock_info(struct clock_info *clk, int level, int bmp); extern void insert_children(struct clock_info **parent, struct clock_info *clk); -extern void find_parents_for_clock(char *clkname, int complete, bool dump); +extern void find_parents_for_clock(char *clkname, int complete); extern int read_and_print_clock_info(int verbose, int hrow, int selected); extern void print_clock_info(int verbose, int hrow, int selected); extern void print_string_val(char *name, char *val);
If we allocate a clock name, that means we can just check with the clock name pointer, no need of the findparent boolean.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- powerdebug.c | 39 +++++++++++++++++++++++++-------------- 1 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index 3832da6..7ef0a22 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -74,14 +74,13 @@ static struct option long_options[] = {
struct powerdebug_options { bool verbose; - bool findparent; bool regulators; bool sensors; bool clocks; bool dump; unsigned int ticktime; int selectedwindow; - char *clkarg; + char *clkname; };
int getoptions(int argc, char *argv[], struct powerdebug_options *options) @@ -114,9 +113,8 @@ int getoptions(int argc, char *argv[], struct powerdebug_options *options) options->selectedwindow = CLOCK; break; case 'p': - options->findparent = true; - options->clkarg = strdup(optarg); - if (!options->clkarg) { + options->clkname = strdup(optarg); + if (!options->clkname) { fprintf(stderr, "failed to allocate memory"); return -1; } @@ -332,8 +330,8 @@ static int powerdebug_dump(struct powerdebug_options *options, if (options->clocks) { init_clock_details(options->dump, options->selectedwindow);
- if (options->findparent) - read_and_dump_clock_info_one(options->clkarg, + if (options->clkname) + read_and_dump_clock_info_one(options->clkname, options->dump); else read_and_dump_clock_info(options->verbose); @@ -345,20 +343,28 @@ static int powerdebug_dump(struct powerdebug_options *options, return 0; }
+static struct powerdebug_options *powerdebug_init(void) +{ + struct powerdebug_options *options; + + options = malloc(sizeof(*options)); + if (!options) + return NULL; + + memset(options, 0, sizeof(*options)); + + return options; +} + int main(int argc, char **argv) { struct powerdebug_options *options; struct regulator_info *regulators_info; int numregulators;
- options = malloc(sizeof(*options)); + options = powerdebug_init(); if (!options) { - fprintf(stderr, "failed to allocated memory\n"); - return -1; - } - - if (getoptions(argc, argv, options)) { - usage(); + fprintf(stderr, "not enough memory to allocate options\n"); return 1; }
@@ -368,6 +374,11 @@ int main(int argc, char **argv) return 1; }
+ if (getoptions(argc, argv, options)) { + usage(); + return 1; + } + /* we just dump the informations */ if (options->dump) { if (powerdebug_dump(options, regulators_info, numregulators))
Change the debugfs_locate_mpoint function to be nicer and more efficient. Another effect is that fixes a segmentation fault when calling powerdebug --dump
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- clocks.c | 68 +++++++++++++++++++++++++-------------------------------- clocks.h | 7 ------ powerdebug.h | 1 - 3 files changed, 30 insertions(+), 46 deletions(-)
diff --git a/clocks.c b/clocks.c index c42f589..ff79235 100644 --- a/clocks.c +++ b/clocks.c @@ -13,6 +13,9 @@ * - initial API and implementation *******************************************************************************/
+#include <stdio.h> +#include <mntent.h> + #include "powerdebug.h" #include "clocks.h"
@@ -20,14 +23,36 @@ static char clk_dir_path[PATH_MAX]; static char clk_name[NAME_MAX]; static int bold[MAX_LINES];
+static int locate_debugfs(char *clk_path) +{ + const char *mtab = "/proc/mounts"; + struct mntent *mntent; + int ret = -1; + FILE *file = NULL; + + file = setmntent(mtab, "r"); + if (!file) + return -1; + + while ((mntent = getmntent(file))) { + + if (strcmp(mntent->mnt_type, "debugfs")) + continue; + + strcpy(clk_path, mntent->mnt_dir); + ret = 0; + break; + } + + fclose(file); + return ret; +} + int init_clock_details(bool dump, int selectedwindow) { - char *path = debugfs_locate_mpoint(); struct stat buf;
- if (path) - strcpy(clk_dir_path, path); - else { + if (locate_debugfs(clk_dir_path)) { if (!dump) { create_selectedwindow(selectedwindow); sprintf(clock_lines[0], "Unable to locate debugfs " @@ -43,6 +68,7 @@ int init_clock_details(bool dump, int selectedwindow) exit(1); } } + sprintf(clk_dir_path, "%s/clock", clk_dir_path); //strcpy(clk_dir_path, "/debug/clock"); // Hardcoded for testing.. if (stat(clk_dir_path, &buf)) { @@ -539,37 +565,3 @@ void dump_clock_info(struct clock_info *clk, int level, int bmp) } } } - -char *debugfs_locate_mpoint(void) -{ - int ret; - FILE *filep; - char **path; - char fsname[64]; - struct statfs sfs; - - path = likely_mpoints; - while (*path) { - ret = statfs(*path, &sfs); - if (ret >= 0 && sfs.f_type == (long)DEBUGFS_MAGIC) - return *path; - path++; - } - - filep = fopen("/proc/mounts", "r"); - if (filep == NULL) { - fprintf(stderr, "powerdebug: Error opening /proc/mounts."); - exit(1); - } - - while (fscanf(filep, "%*s %s %s %*s %*d %*d\n", - debugfs_mntpoint, fsname) == 2) - if (!strcmp(fsname, "debugfs")) - break; - fclose(filep); - - if (strcmp(fsname, "debugfs")) - return NULL; - - return debugfs_mntpoint; -} diff --git a/clocks.h b/clocks.h index 4b176d0..dd9216a 100644 --- a/clocks.h +++ b/clocks.h @@ -36,17 +36,10 @@ struct clock_info { struct clock_info **children; } *clocks_info;
-char debugfs_mntpoint[1024]; char clock_lines[MAX_LINES][128]; int clock_line_no; int old_clock_line_no;
-char *likely_mpoints[] = { - "/sys/kernel/debug", - "/debug", - NULL -}; - void add_clock_details_recur(struct clock_info *clk, int hrow, int selected); void destroy_clocks_info(void); void destroy_clocks_info_recur(struct clock_info *clock); diff --git a/powerdebug.h b/powerdebug.h index d2bd4ec..f424c1f 100644 --- a/powerdebug.h +++ b/powerdebug.h @@ -42,7 +42,6 @@ extern void print_string_val(char *name, char *val); extern int init_clock_details(bool dump, int selectedwindow); extern void print_clock_header(void); extern void print_one_clock(int line, char *str, int bold, int highlight); -extern char *debugfs_locate_mpoint(void);
extern void get_sensor_info(char *path, char *name, char *sensor, int verbose); extern int read_and_print_sensor_info(int verbose);
Use a simple function to read the content of the file. Fix the format of the flags which were always zero by reading in heaxdecimal mode.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- clocks.c | 37 ++++++++++++++++++++++--------------- 1 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/clocks.c b/clocks.c index ff79235..2c9597d 100644 --- a/clocks.c +++ b/clocks.c @@ -89,21 +89,28 @@ int init_clock_details(bool dump, int selectedwindow) return(0); }
-int get_int_from(char *file) +static int file_read_from_format(char *file, int *value, const char *format) { - FILE *filep; - char result[NAME_MAX]; + FILE *f; int ret;
- filep = fopen(file, "r"); + f = fopen(file, "r"); + if (!f) + return -1; + ret = fscanf(f, format, value); + fclose(f);
- if (!filep) - return -1; //TBD : What should we return on failure, here ? + return !ret ? -1 : 0; +}
- ret = fscanf(filep, "%s", result); - fclose(filep); +static inline int file_read_int(char *file, int *value) +{ + return file_read_from_format(file, value, "%d"); +}
- return atoi(result); +static inline int file_read_hex(char *file, int *value) +{ + return file_read_from_format(file, value, "%x"); }
static void dump_parent(struct clock_info *clk, int line, bool dump) @@ -132,13 +139,13 @@ static void dump_parent(struct clock_info *clk, int line, bool dump) if (clk == clocks_info) { line++; strcpy(spaces, ""); - sprintf(str, "%s%s (flags:%d,usecount:%d,rate:%5.2f %s)\n", + sprintf(str, "%s%s (flags:0x%x,usecount:%d,rate:%5.2f %s)\n", spaces, clk->name, clk->flags, clk->usecount, drate, unit); } else { if (!(clk->parent == clocks_info)) strcat(spaces, " "); - sprintf(str, "%s`- %s (flags:%d,usecount:%d,rate:%5.2f %s)\n", + sprintf(str, "%s`- %s (flags:0x%x,usecount:%d,rate:%5.2f %s)\n", spaces, clk->name, clk->flags, clk->usecount, drate, unit); } @@ -439,11 +446,11 @@ struct clock_info *read_clock_info_recur(char *clkpath, int level,
if (S_ISREG(buf.st_mode)) { if (!strcmp(item->d_name, "flags")) - parent->flags = get_int_from(filename); + file_read_hex(filename, &parent->flags); if (!strcmp(item->d_name, "rate")) - parent->rate = get_int_from(filename); + file_read_int(filename, &parent->rate); if (!strcmp(item->d_name, "usecount")) - parent->usecount = get_int_from(filename); + file_read_int(filename, &parent->usecount); continue; }
@@ -546,7 +553,7 @@ void dump_clock_info(struct clock_info *clk, int level, int bmp) unit = "MHz"; drate /= 1000000; } - printf("%s (flags:%d,usecount:%d,rate:%5.2f %s)\n", + printf("%s (flags:0x%x,usecount:%d,rate:%5.2f %s)\n", clk->name, clk->flags, clk->usecount, drate, unit); } if (clk->children) {
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- powerdebug.c | 18 +++++++++++++----- 1 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index 7ef0a22..d337cc8 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -141,13 +141,21 @@ int getoptions(int argc, char *argv[], struct powerdebug_options *options) } }
- if (options->dump && !(options->regulators || - options->clocks || options->sensors)) { - /* By Default lets show everything we have */ - options->regulators = options->clocks = options->sensors = true; + if (options->dump) { + + /* No system specified to be dump, let's default to all */ + if (!options->regulators && + !options->clocks && + !options->sensors) { + options->regulators = options->clocks = + options->sensors = true; + + return 0; + } + }
- if (!options->dump && options->selectedwindow == -1) + if (options->selectedwindow == -1) options->selectedwindow = CLOCK;
return 0;
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- clocks.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/clocks.c b/clocks.c index 2c9597d..5c5c24f 100644 --- a/clocks.c +++ b/clocks.c @@ -20,7 +20,6 @@ #include "clocks.h"
static char clk_dir_path[PATH_MAX]; -static char clk_name[NAME_MAX]; static int bold[MAX_LINES];
static int locate_debugfs(char *clk_path) @@ -85,8 +84,8 @@ int init_clock_details(bool dump, int selectedwindow) exit(1); } } - strcpy(clk_name, ""); - return(0); + + return 0; }
static int file_read_from_format(char *file, int *value, const char *format)
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- clocks.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/clocks.c b/clocks.c index 5c5c24f..9577267 100644 --- a/clocks.c +++ b/clocks.c @@ -64,7 +64,7 @@ int init_clock_details(bool dump, int selectedwindow) fprintf(stderr, "powerdebug: Unable to locate debugfs " "mount point. Mount debugfs and try " "again..\n"); - exit(1); + return -1; } }
@@ -81,7 +81,7 @@ int init_clock_details(bool dump, int selectedwindow) } else { fprintf(stderr, "powerdebug: Unable to find clock tree" " information at %s.\n", clk_dir_path); - exit(1); + return -1; } }
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- display.h | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/display.h b/display.h index f00a4f8..a73ae43 100644 --- a/display.h +++ b/display.h @@ -13,10 +13,6 @@ * - initial API and implementation *******************************************************************************/
-#define VALUE_MAX 16 - -WINDOW windows[TOTAL_FEATURE_WINS]; - #define PT_COLOR_DEFAULT 1 #define PT_COLOR_HEADER_BAR 2 #define PT_COLOR_ERROR 3
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- display.c | 2 +- display.h | 2 ++ powerdebug.c | 3 ++- powerdebug.h | 1 - 4 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/display.c b/display.c index fb252cb..8ea796a 100644 --- a/display.c +++ b/display.c @@ -65,7 +65,7 @@ void killall_windows(int all) } }
-void init_curses(void) +void display_init(void) { initscr(); start_color(); diff --git a/display.h b/display.h index a73ae43..d3c875a 100644 --- a/display.h +++ b/display.h @@ -21,3 +21,5 @@ #define PT_COLOR_GREEN 6 #define PT_COLOR_BRIGHT 7 #define PT_COLOR_BLUE 8 + +extern void display_init(void); diff --git a/powerdebug.c b/powerdebug.c index d337cc8..8244df2 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -16,6 +16,7 @@ #include <getopt.h> #include <stdbool.h> #include "regulator.h" +#include "display.h" #include "powerdebug.h"
static int highlighted_row; @@ -261,7 +262,7 @@ int mainloop(struct powerdebug_options *options, fd_set readfds;
if (firsttime[0]) - init_curses(); + display_init(); create_windows(options->selectedwindow); show_header(options->selectedwindow);
diff --git a/powerdebug.h b/powerdebug.h index f424c1f..29b1177 100644 --- a/powerdebug.h +++ b/powerdebug.h @@ -47,7 +47,6 @@ extern void get_sensor_info(char *path, char *name, char *sensor, int verbose); extern int read_and_print_sensor_info(int verbose); extern void print_sensor_header(void);
-extern void init_curses(void); extern void fini_curses(void); extern void killall_windows(int all); extern void show_header(int selectedwindow);
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- display.c | 12 ++++++------ powerdebug.h | 1 - 2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/display.c b/display.c index 8ea796a..cf1d5d7 100644 --- a/display.c +++ b/display.c @@ -36,10 +36,6 @@ static char *win_names[TOTAL_FEATURE_WINS] = { "Sensors" };
-void fini_curses(void) { - endwin(); -} - /* "all" : Kill header and footer windows too ? */ void killall_windows(int all) { @@ -65,6 +61,11 @@ void killall_windows(int all) } }
+static void display_fini(void) +{ + endwin(); +} + void display_init(void) { initscr(); @@ -85,10 +86,9 @@ void display_init(void) init_pair(PT_COLOR_BLUE, COLOR_WHITE, COLOR_BLUE); init_pair(PT_COLOR_RED, COLOR_WHITE, COLOR_RED);
- atexit(fini_curses); + atexit(display_fini); }
- void create_windows(int selectedwindow) {
diff --git a/powerdebug.h b/powerdebug.h index 29b1177..d8a40a9 100644 --- a/powerdebug.h +++ b/powerdebug.h @@ -47,7 +47,6 @@ extern void get_sensor_info(char *path, char *name, char *sensor, int verbose); extern int read_and_print_sensor_info(int verbose); extern void print_sensor_header(void);
-extern void fini_curses(void); extern void killall_windows(int all); extern void show_header(int selectedwindow); extern void create_windows(int selectedwindow);
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- display.c | 28 ++++++++++++++++------------ display.h | 2 +- powerdebug.c | 10 ++++++++-- 3 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/display.c b/display.c index cf1d5d7..aee5503 100644 --- a/display.c +++ b/display.c @@ -66,27 +66,31 @@ static void display_fini(void) endwin(); }
-void display_init(void) +int display_init(void) { - initscr(); + if (!initscr()) + return -1; + start_color(); + use_default_colors(); + keypad(stdscr, TRUE); noecho(); cbreak(); curs_set(0); nonl(); - use_default_colors(); - - init_pair(PT_COLOR_DEFAULT, COLOR_WHITE, COLOR_BLACK); - init_pair(PT_COLOR_ERROR, COLOR_BLACK, COLOR_RED); - init_pair(PT_COLOR_HEADER_BAR, COLOR_WHITE, COLOR_BLACK); - init_pair(PT_COLOR_YELLOW, COLOR_WHITE, COLOR_YELLOW); - init_pair(PT_COLOR_GREEN, COLOR_WHITE, COLOR_GREEN); - init_pair(PT_COLOR_BRIGHT, COLOR_WHITE, COLOR_BLACK); - init_pair(PT_COLOR_BLUE, COLOR_WHITE, COLOR_BLUE); - init_pair(PT_COLOR_RED, COLOR_WHITE, COLOR_RED);
- atexit(display_fini); + if (init_pair(PT_COLOR_DEFAULT, COLOR_WHITE, COLOR_BLACK) || + init_pair(PT_COLOR_ERROR, COLOR_BLACK, COLOR_RED) || + init_pair(PT_COLOR_HEADER_BAR, COLOR_WHITE, COLOR_BLACK) || + init_pair(PT_COLOR_YELLOW, COLOR_WHITE, COLOR_YELLOW) || + init_pair(PT_COLOR_GREEN, COLOR_WHITE, COLOR_GREEN) || + init_pair(PT_COLOR_BRIGHT, COLOR_WHITE, COLOR_BLACK) || + init_pair(PT_COLOR_BLUE, COLOR_WHITE, COLOR_BLUE) || + init_pair(PT_COLOR_RED, COLOR_WHITE, COLOR_RED)) + return -1; + + return atexit(display_fini); }
void create_windows(int selectedwindow) diff --git a/display.h b/display.h index d3c875a..044af6c 100644 --- a/display.h +++ b/display.h @@ -22,4 +22,4 @@ #define PT_COLOR_BRIGHT 7 #define PT_COLOR_BLUE 8
-extern void display_init(void); +extern int display_init(void); diff --git a/powerdebug.c b/powerdebug.c index 8244df2..215b0fe 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -261,8 +261,9 @@ int mainloop(struct powerdebug_options *options, struct timeval tval; fd_set readfds;
- if (firsttime[0]) - display_init(); + if (firsttime[0] && display_init()) + return -1; + create_windows(options->selectedwindow); show_header(options->selectedwindow);
@@ -395,6 +396,11 @@ int main(int argc, char **argv) return 0; }
+ if (display_init()) { + printf("failed to initialize display\n"); + return 1; + } + if (mainloop(options, regulators_info, numregulators)) return 1;
and reduce the scope by moving it to the c file.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- display.c | 10 ++++++++++ display.h | 9 --------- 2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/display.c b/display.c index aee5503..2d63908 100644 --- a/display.c +++ b/display.c @@ -20,6 +20,16 @@ #define print(w, x, y, fmt, args...) do { mvwprintw(w, y, x, fmt, ##args); } while (0) #define NUM_FOOTER_ITEMS 5
+enum { PT_COLOR_DEFAULT = 1, + PT_COLOR_HEADER_BAR, + PT_COLOR_ERROR, + PT_COLOR_RED, + PT_COLOR_YELLOW, + PT_COLOR_GREEN, + PT_COLOR_BRIGHT, + PT_COLOR_BLUE, +}; + static WINDOW *header_win; static WINDOW *regulator_win; static WINDOW *clock_win; diff --git a/display.h b/display.h index 044af6c..047d674 100644 --- a/display.h +++ b/display.h @@ -13,13 +13,4 @@ * - initial API and implementation *******************************************************************************/
-#define PT_COLOR_DEFAULT 1 -#define PT_COLOR_HEADER_BAR 2 -#define PT_COLOR_ERROR 3 -#define PT_COLOR_RED 4 -#define PT_COLOR_YELLOW 5 -#define PT_COLOR_GREEN 6 -#define PT_COLOR_BRIGHT 7 -#define PT_COLOR_BLUE 8 - extern int display_init(void);
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- powerdebug.c | 42 ++++++++++++------------------------------ 1 files changed, 12 insertions(+), 30 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index 215b0fe..ef2098c 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -142,19 +142,9 @@ int getoptions(int argc, char *argv[], struct powerdebug_options *options) } }
- if (options->dump) { - - /* No system specified to be dump, let's default to all */ - if (!options->regulators && - !options->clocks && - !options->sensors) { - options->regulators = options->clocks = - options->sensors = true; - - return 0; - } - - } + /* No system specified to be dump, let's default to all */ + if (!options->regulators && !options->clocks && !options->sensors) + options->regulators = options->clocks = options->sensors = true;
if (options->selectedwindow == -1) options->selectedwindow = CLOCK; @@ -249,21 +239,15 @@ int mainloop(struct powerdebug_options *options, bool findparent_ncurses = false; bool refreshwin = false; bool enter_hit = false; - int firsttime[TOTAL_FEATURE_WINS]; - int i; char clkname_str[64];
- for (i = 0; i < TOTAL_FEATURE_WINS; i++) - firsttime[i] = 1; + strcpy(clkname_str, "");
while (1) { int key = 0; struct timeval tval; fd_set readfds;
- if (firsttime[0] && display_init()) - return -1; - create_windows(options->selectedwindow); show_header(options->selectedwindow);
@@ -274,16 +258,9 @@ int mainloop(struct powerdebug_options *options, options->verbose); }
- if (options->clocks || options->selectedwindow == CLOCK) { - int ret = 0; - if (firsttime[CLOCK]) { - ret = init_clock_details(options->dump, - options->selectedwindow); - if (!ret) - firsttime[CLOCK] = 0; - strcpy(clkname_str, ""); - } - if (!ret) { + if (options->selectedwindow == CLOCK) { + + if (options->clocks) { int hrow;
create_selectedwindow(options->selectedwindow); @@ -401,6 +378,11 @@ int main(int argc, char **argv) return 1; }
+ if (init_clock_details(options->dump, options->selectedwindow)) { + printf("failed initialize clock details\n"); + options->clocks = false; + } + if (mainloop(options, regulators_info, numregulators)) return 1;
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- clocks.c | 39 ++++----------------------------------- clocks.h | 2 ++ powerdebug.c | 12 ++++++------ powerdebug.h | 1 - 4 files changed, 12 insertions(+), 42 deletions(-)
diff --git a/clocks.c b/clocks.c index 9577267..c83055f 100644 --- a/clocks.c +++ b/clocks.c @@ -47,45 +47,14 @@ static int locate_debugfs(char *clk_path) return ret; }
-int init_clock_details(bool dump, int selectedwindow) +int clock_init(void) { - struct stat buf; - - if (locate_debugfs(clk_dir_path)) { - if (!dump) { - create_selectedwindow(selectedwindow); - sprintf(clock_lines[0], "Unable to locate debugfs " - "mount point. Mount debugfs " - "and try again..\n"); - print_one_clock(0, clock_lines[0], 1, 0); - old_clock_line_no = 1; - return(1); - } else { - fprintf(stderr, "powerdebug: Unable to locate debugfs " - "mount point. Mount debugfs and try " - "again..\n"); - return -1; - } - } + if (locate_debugfs(clk_dir_path)) + return -1;
sprintf(clk_dir_path, "%s/clock", clk_dir_path); - //strcpy(clk_dir_path, "/debug/clock"); // Hardcoded for testing.. - if (stat(clk_dir_path, &buf)) { - if (!dump) { - create_selectedwindow(selectedwindow); - sprintf(clock_lines[0], "Unable to find clock tree" - " information at %s.\n", clk_dir_path); - print_one_clock(0, clock_lines[0], 1, 0); - old_clock_line_no = 1; - return(1); - } else { - fprintf(stderr, "powerdebug: Unable to find clock tree" - " information at %s.\n", clk_dir_path); - return -1; - } - }
- return 0; + return access(clk_dir_path, F_OK); }
static int file_read_from_format(char *file, int *value, const char *format) diff --git a/clocks.h b/clocks.h index dd9216a..cc17e86 100644 --- a/clocks.h +++ b/clocks.h @@ -40,6 +40,8 @@ char clock_lines[MAX_LINES][128]; int clock_line_no; int old_clock_line_no;
+extern int clock_init(void); + void add_clock_details_recur(struct clock_info *clk, int hrow, int selected); void destroy_clocks_info(void); void destroy_clocks_info_recur(struct clock_info *clock); diff --git a/powerdebug.c b/powerdebug.c index ef2098c..c7f9574 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -17,6 +17,7 @@ #include <stdbool.h> #include "regulator.h" #include "display.h" +#include "clocks.h" #include "powerdebug.h"
static int highlighted_row; @@ -315,7 +316,6 @@ static int powerdebug_dump(struct powerdebug_options *options, }
if (options->clocks) { - init_clock_details(options->dump, options->selectedwindow);
if (options->clkname) read_and_dump_clock_info_one(options->clkname, @@ -366,6 +366,11 @@ int main(int argc, char **argv) return 1; }
+ if (clock_init()) { + printf("failed to initialize clock details (check debugfs)\n"); + options->clocks = false; + } + /* we just dump the informations */ if (options->dump) { if (powerdebug_dump(options, regulators_info, numregulators)) @@ -378,11 +383,6 @@ int main(int argc, char **argv) return 1; }
- if (init_clock_details(options->dump, options->selectedwindow)) { - printf("failed initialize clock details\n"); - options->clocks = false; - } - if (mainloop(options, regulators_info, numregulators)) return 1;
diff --git a/powerdebug.h b/powerdebug.h index d8a40a9..a122b7f 100644 --- a/powerdebug.h +++ b/powerdebug.h @@ -39,7 +39,6 @@ extern void find_parents_for_clock(char *clkname, int complete); extern int read_and_print_clock_info(int verbose, int hrow, int selected); extern void print_clock_info(int verbose, int hrow, int selected); extern void print_string_val(char *name, char *val); -extern int init_clock_details(bool dump, int selectedwindow); extern void print_clock_header(void); extern void print_one_clock(int line, char *str, int bold, int highlight);
By moving the functions in the right order in the file, we can get ride of their definitions and we can set them static.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- clocks.c | 195 ++++++++++++++++++++++++++++++++------------------------------ clocks.h | 18 ------ 2 files changed, 101 insertions(+), 112 deletions(-)
diff --git a/clocks.c b/clocks.c index c83055f..b556644 100644 --- a/clocks.c +++ b/clocks.c @@ -15,12 +15,18 @@
#include <stdio.h> #include <mntent.h> +#include <sys/stat.h>
#include "powerdebug.h" #include "clocks.h"
+#define MAX_LINES 120 + static char clk_dir_path[PATH_MAX]; static int bold[MAX_LINES]; +static char clock_lines[MAX_LINES][128]; +static int clock_line_no; +static int old_clock_line_no;
static int locate_debugfs(char *clk_path) { @@ -124,6 +130,29 @@ static void dump_parent(struct clock_info *clk, int line, bool dump) print_one_clock(maxline - line + 2, str, 1, 0); }
+static struct clock_info *find_clock(struct clock_info *clk, char *clkarg) +{ + int i; + struct clock_info *ret = clk; + + if (!strcmp(clk->name, clkarg)) + return ret; + + if (clk->children) { + for (i = 0; i < clk->num_children; i++) { + if (!strcmp(clk->children[i]->name, clkarg)) + return clk->children[i]; + } + for (i = 0; i < clk->num_children; i++) { + ret = find_clock(clk->children[i], clkarg); + if (ret) + return ret; + } + } + + return NULL; +} + static void dump_all_parents(char *clkarg, bool dump) { struct clock_info *clk; @@ -165,6 +194,45 @@ void find_parents_for_clock(char *clkname, int complete) dump_all_parents(clkname, false); }
+static void destroy_clocks_info_recur(struct clock_info *clock) +{ + int i; + + if (clock && clock->num_children) { + for (i = (clock->num_children - 1); i >= 0; i--) { + fflush(stdin); + destroy_clocks_info_recur(clock->children[i]); + if (!i) { + free(clock->children); + clock->children = NULL; + clock->num_children = 0; + } + } + } +} + +static void destroy_clocks_info(void) +{ + int i; + + if (!clocks_info) + return; + + if (clocks_info->num_children) { + for (i = (clocks_info->num_children - 1); i >= 0 ; i--) { + destroy_clocks_info_recur(clocks_info->children[i]); + if (!i) { + free(clocks_info->children); + clocks_info->children = NULL; + } + } + } + clocks_info->num_children = 0; + free(clocks_info); + clocks_info = NULL; +} + + int read_and_print_clock_info(int verbose, int hrow, int selected) { print_one_clock(0, "Reading Clock Tree ...", 1, 1); @@ -190,7 +258,7 @@ int read_and_print_clock_info(int verbose, int hrow, int selected) return hrow; }
-int calc_delta_screen_size(int hrow) +static int calc_delta_screen_size(int hrow) { if (hrow >= (maxy - 3)) return hrow - (maxy - 4); @@ -198,36 +266,7 @@ int calc_delta_screen_size(int hrow) return 0; }
-void print_clock_info(int verbose, int hrow, int selected) -{ - int i, count = 0, delta; - - (void)verbose; - - print_clock_header(); - - for (i = 0; i < clocks_info->num_children; i++) - add_clock_details_recur(clocks_info->children[i], - hrow, selected); - - delta = calc_delta_screen_size(hrow); - - while (clock_lines[count + delta] && - strcmp(clock_lines[count + delta], "")) { - if (count < delta) { - count++; - continue; - } - print_one_clock(count - delta, clock_lines[count + delta], - bold[count + delta], (hrow == (count + delta))); - count++; - } - - old_clock_line_no = clock_line_no; - clock_line_no = 0; -} - -void prepare_name_str(char *namestr, struct clock_info *clock) +static void prepare_name_str(char *namestr, struct clock_info *clock) { int i;
@@ -238,7 +277,18 @@ void prepare_name_str(char *namestr, struct clock_info *clock) strcat(namestr, clock->name); }
-void add_clock_details_recur(struct clock_info *clock, int hrow, int selected) +static void collapse_all_subclocks(struct clock_info *clock) +{ + int i; + + clock->expanded = 0; + if (clock->num_children) + for (i = 0; i < clock->num_children; i++) + collapse_all_subclocks(clock->children[i]); +} + +static void add_clock_details_recur(struct clock_info *clock, + int hrow, int selected) { int i; char *unit = " Hz"; @@ -280,52 +330,33 @@ void add_clock_details_recur(struct clock_info *clock, int hrow, int selected) strcpy(clock_lines[clock_line_no], ""); }
-void collapse_all_subclocks(struct clock_info *clock) +void print_clock_info(int verbose, int hrow, int selected) { - int i; - - clock->expanded = 0; - if (clock->num_children) - for (i = 0; i < clock->num_children; i++) - collapse_all_subclocks(clock->children[i]); -} + int i, count = 0, delta;
-void destroy_clocks_info(void) -{ - int i; + (void)verbose;
- if (!clocks_info) - return; + print_clock_header();
- if (clocks_info->num_children) { - for (i = (clocks_info->num_children - 1); i >= 0 ; i--) { - destroy_clocks_info_recur(clocks_info->children[i]); - if (!i) { - free(clocks_info->children); - clocks_info->children = NULL; - } - } - } - clocks_info->num_children = 0; - free(clocks_info); - clocks_info = NULL; -} + for (i = 0; i < clocks_info->num_children; i++) + add_clock_details_recur(clocks_info->children[i], + hrow, selected);
-void destroy_clocks_info_recur(struct clock_info *clock) -{ - int i; + delta = calc_delta_screen_size(hrow);
- if (clock && clock->num_children) { - for (i = (clock->num_children - 1); i >= 0; i--) { - fflush(stdin); - destroy_clocks_info_recur(clock->children[i]); - if (!i) { - free(clock->children); - clock->children = NULL; - clock->num_children = 0; - } + while (clock_lines[count + delta] && + strcmp(clock_lines[count + delta], "")) { + if (count < delta) { + count++; + continue; } + print_one_clock(count - delta, clock_lines[count + delta], + bold[count + delta], (hrow == (count + delta))); + count++; } + + old_clock_line_no = clock_line_no; + clock_line_no = 0; }
void read_and_dump_clock_info_one(char *clk, bool dump) @@ -462,30 +493,6 @@ void insert_children(struct clock_info **parent, struct clock_info *clk) (*parent)->num_children++; }
-struct clock_info *find_clock(struct clock_info *clk, char *clkarg) -{ - int i; - struct clock_info *ret = clk; - - if (!strcmp(clk->name, clkarg)) - return ret; - - if (clk->children) { - for (i = 0; i < clk->num_children; i++) { - if (!strcmp(clk->children[i]->name, clkarg)) - return clk->children[i]; - } - for (i = 0; i < clk->num_children; i++) { - ret = find_clock(clk->children[i], clkarg); - if (ret) - return ret; - } - } - - return NULL; -} - - void dump_clock_info(struct clock_info *clk, int level, int bmp) { int i, j; diff --git a/clocks.h b/clocks.h index cc17e86..9ad9804 100644 --- a/clocks.h +++ b/clocks.h @@ -13,16 +13,8 @@ * - initial API and implementation *******************************************************************************/
-#include <sys/stat.h> -#include <sys/vfs.h> -#include <errno.h> -#include <sys/stat.h> -#include <linux/magic.h> - extern int maxy;
-#define MAX_LINES 120 - struct clock_info { char name[NAME_MAX]; int flags; @@ -36,14 +28,4 @@ struct clock_info { struct clock_info **children; } *clocks_info;
-char clock_lines[MAX_LINES][128]; -int clock_line_no; -int old_clock_line_no; - extern int clock_init(void); - -void add_clock_details_recur(struct clock_info *clk, int hrow, int selected); -void destroy_clocks_info(void); -void destroy_clocks_info_recur(struct clock_info *clock); -void collapse_all_subclocks(struct clock_info *clock); -struct clock_info *find_clock(struct clock_info *clk, char *clkarg);
On Sun, Mar 27, 2011 at 5:06 AM, Daniel Lezcano daniel.lezcano@free.frwrote:
By moving the functions in the right order in the file, we can get ride of their definitions and we can set them static.
Technically, those function names in head files should be declarations, not definitions. For this patch, it could be further split, since it is a little bit messed up, for example: for print_clock_info(), you moved it from the original place, but for others, it is really hard to see this. It looks like you removed it and then changed collapse_all_subclocks to a new print_clock_info somehow.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr
clocks.c | 195 ++++++++++++++++++++++++++++++++------------------------------ clocks.h | 18 ------ 2 files changed, 101 insertions(+), 112 deletions(-)
diff --git a/clocks.c b/clocks.c index c83055f..b556644 100644 --- a/clocks.c +++ b/clocks.c @@ -15,12 +15,18 @@
#include <stdio.h> #include <mntent.h> +#include <sys/stat.h>
#include "powerdebug.h" #include "clocks.h"
+#define MAX_LINES 120
static char clk_dir_path[PATH_MAX]; static int bold[MAX_LINES]; +static char clock_lines[MAX_LINES][128]; +static int clock_line_no; +static int old_clock_line_no;
static int locate_debugfs(char *clk_path) { @@ -124,6 +130,29 @@ static void dump_parent(struct clock_info *clk, int line, bool dump) print_one_clock(maxline - line + 2, str, 1, 0); }
+static struct clock_info *find_clock(struct clock_info *clk, char *clkarg) +{
int i;struct clock_info *ret = clk;if (!strcmp(clk->name, clkarg))return ret;if (clk->children) {for (i = 0; i < clk->num_children; i++) {if (!strcmp(clk->children[i]->name, clkarg))return clk->children[i];}for (i = 0; i < clk->num_children; i++) {ret = find_clock(clk->children[i], clkarg);if (ret)return ret;}}return NULL;+}
static void dump_all_parents(char *clkarg, bool dump) { struct clock_info *clk; @@ -165,6 +194,45 @@ void find_parents_for_clock(char *clkname, int complete) dump_all_parents(clkname, false); }
+static void destroy_clocks_info_recur(struct clock_info *clock) +{
int i;if (clock && clock->num_children) {for (i = (clock->num_children - 1); i >= 0; i--) {fflush(stdin);destroy_clocks_info_recur(clock->children[i]);if (!i) {free(clock->children);clock->children = NULL;clock->num_children = 0;}}}+}
+static void destroy_clocks_info(void) +{
int i;if (!clocks_info)return;if (clocks_info->num_children) {for (i = (clocks_info->num_children - 1); i >= 0 ; i--) {destroy_clocks_info_recur(clocks_info->children[i]);
if (!i) {free(clocks_info->children);clocks_info->children = NULL;}}}clocks_info->num_children = 0;free(clocks_info);clocks_info = NULL;+}
int read_and_print_clock_info(int verbose, int hrow, int selected) { print_one_clock(0, "Reading Clock Tree ...", 1, 1); @@ -190,7 +258,7 @@ int read_and_print_clock_info(int verbose, int hrow, int selected) return hrow; }
-int calc_delta_screen_size(int hrow) +static int calc_delta_screen_size(int hrow) { if (hrow >= (maxy - 3)) return hrow - (maxy - 4); @@ -198,36 +266,7 @@ int calc_delta_screen_size(int hrow) return 0; }
-void print_clock_info(int verbose, int hrow, int selected) -{
int i, count = 0, delta;(void)verbose;print_clock_header();for (i = 0; i < clocks_info->num_children; i++)add_clock_details_recur(clocks_info->children[i],hrow, selected);delta = calc_delta_screen_size(hrow);while (clock_lines[count + delta] &&strcmp(clock_lines[count + delta], "")) {if (count < delta) {count++;continue;}print_one_clock(count - delta, clock_lines[count + delta],bold[count + delta], (hrow == (count +delta)));
count++;}old_clock_line_no = clock_line_no;clock_line_no = 0;-}
-void prepare_name_str(char *namestr, struct clock_info *clock) +static void prepare_name_str(char *namestr, struct clock_info *clock) { int i;
@@ -238,7 +277,18 @@ void prepare_name_str(char *namestr, struct clock_info *clock) strcat(namestr, clock->name); }
-void add_clock_details_recur(struct clock_info *clock, int hrow, int selected) +static void collapse_all_subclocks(struct clock_info *clock) +{
int i;clock->expanded = 0;if (clock->num_children)for (i = 0; i < clock->num_children; i++)collapse_all_subclocks(clock->children[i]);+}
+static void add_clock_details_recur(struct clock_info *clock,
int hrow, int selected){ int i; char *unit = " Hz"; @@ -280,52 +330,33 @@ void add_clock_details_recur(struct clock_info *clock, int hrow, int selected) strcpy(clock_lines[clock_line_no], ""); }
-void collapse_all_subclocks(struct clock_info *clock) +void print_clock_info(int verbose, int hrow, int selected) {
int i;clock->expanded = 0;if (clock->num_children)for (i = 0; i < clock->num_children; i++)collapse_all_subclocks(clock->children[i]);-}
int i, count = 0, delta;-void destroy_clocks_info(void) -{
int i;
(void)verbose;
if (!clocks_info)return;
print_clock_header();
if (clocks_info->num_children) {for (i = (clocks_info->num_children - 1); i >= 0 ; i--) {destroy_clocks_info_recur(clocks_info->children[i]);
if (!i) {free(clocks_info->children);clocks_info->children = NULL;}}}clocks_info->num_children = 0;free(clocks_info);clocks_info = NULL;-}
for (i = 0; i < clocks_info->num_children; i++)add_clock_details_recur(clocks_info->children[i],hrow, selected);-void destroy_clocks_info_recur(struct clock_info *clock) -{
int i;
delta = calc_delta_screen_size(hrow);
if (clock && clock->num_children) {for (i = (clock->num_children - 1); i >= 0; i--) {fflush(stdin);destroy_clocks_info_recur(clock->children[i]);if (!i) {free(clock->children);clock->children = NULL;clock->num_children = 0;}
while (clock_lines[count + delta] &&strcmp(clock_lines[count + delta], "")) {if (count < delta) {count++;continue; }print_one_clock(count - delta, clock_lines[count + delta],bold[count + delta], (hrow == (count +delta)));
count++; }old_clock_line_no = clock_line_no;clock_line_no = 0;}
void read_and_dump_clock_info_one(char *clk, bool dump) @@ -462,30 +493,6 @@ void insert_children(struct clock_info **parent, struct clock_info *clk) (*parent)->num_children++; }
-struct clock_info *find_clock(struct clock_info *clk, char *clkarg) -{
int i;struct clock_info *ret = clk;if (!strcmp(clk->name, clkarg))return ret;if (clk->children) {for (i = 0; i < clk->num_children; i++) {if (!strcmp(clk->children[i]->name, clkarg))return clk->children[i];}for (i = 0; i < clk->num_children; i++) {ret = find_clock(clk->children[i], clkarg);if (ret)return ret;}}return NULL;-}
void dump_clock_info(struct clock_info *clk, int level, int bmp) { int i, j; diff --git a/clocks.h b/clocks.h index cc17e86..9ad9804 100644 --- a/clocks.h +++ b/clocks.h @@ -13,16 +13,8 @@
- initial API and implementation*******************************************************************************/
-#include <sys/stat.h> -#include <sys/vfs.h> -#include <errno.h> -#include <sys/stat.h> -#include <linux/magic.h>
extern int maxy;
-#define MAX_LINES 120
struct clock_info { char name[NAME_MAX]; int flags; @@ -36,14 +28,4 @@ struct clock_info { struct clock_info **children; } *clocks_info;
-char clock_lines[MAX_LINES][128]; -int clock_line_no; -int old_clock_line_no;
extern int clock_init(void);
-void add_clock_details_recur(struct clock_info *clk, int hrow, int selected); -void destroy_clocks_info(void); -void destroy_clocks_info_recur(struct clock_info *clock); -void collapse_all_subclocks(struct clock_info *clock);
-struct clock_info *find_clock(struct clock_info *clk, char *clkarg);
1.7.1
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
On 03/28/2011 08:28 AM, Yong Shen wrote:
On Sun, Mar 27, 2011 at 5:06 AM, Daniel Lezcanodaniel.lezcano@free.frwrote:
By moving the functions in the right order in the file, we can get ride of their definitions and we can set them static.
Technically, those function names in head files should be declarations, not definitions.
Right, I meant "declarations", thanks.
For this patch, it could be further split, since it is a little bit messed up, for example: for print_clock_info(), you moved it from the original place, but for others, it is really hard to see this. It looks like you removed it and then changed collapse_all_subclocks to a new print_clock_info somehow.
Yep, sometimes the changes are trivial (here just moving the functions up) but the diff result is hard to understand. I can split this patch into several but is it really worth to do that ?
On Mon, Mar 28, 2011 at 4:56 PM, Daniel Lezcano daniel.lezcano@free.frwrote:
On 03/28/2011 08:28 AM, Yong Shen wrote:
On Sun, Mar 27, 2011 at 5:06 AM, Daniel Lezcano<daniel.lezcano@free.fr
wrote:
By moving the functions in the right order in the file, we can get ride
of their definitions and we can set them static.
Technically, those function names in head files should be declarations,
not definitions.
Right, I meant "declarations", thanks.
For this patch, it could be further split, since it is a little bit messed
up, for example: for print_clock_info(), you moved it from the original place, but for others, it is really hard to see this. It looks like you removed it and then changed collapse_all_subclocks to a new print_clock_info somehow.
Yep, sometimes the changes are trivial (here just moving the functions up) but the diff result is hard to understand. I can split this patch into several but is it really worth to do that ?
Personally, I am OK with this as long as it does not let errors creep in. It is Amit's call for the acceptance of this patch.
Yong
In order to have the code more clear, let's create a function for the display like what we did with the dump function.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- powerdebug.c | 45 ++++++++++++++++++++++++--------------------- 1 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index c7f9574..86f51bc 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -330,6 +330,20 @@ static int powerdebug_dump(struct powerdebug_options *options, return 0; }
+static int powerdebug_display(struct powerdebug_options *options, + struct regulator_info *reg_info, int nr_reg) +{ + if (display_init()) { + printf("failed to initialize display\n"); + return -1; + } + + if (mainloop(options, reg_info, nr_reg)) + return -1; + + return 0; +} + static struct powerdebug_options *powerdebug_init(void) { struct powerdebug_options *options; @@ -347,7 +361,7 @@ int main(int argc, char **argv) { struct powerdebug_options *options; struct regulator_info *regulators_info; - int numregulators; + int numregulators, ret;
options = powerdebug_init(); if (!options) { @@ -355,14 +369,14 @@ int main(int argc, char **argv) return 1; }
- regulators_info = regulator_init(&numregulators); - if (!regulators_info) { - printf("not enough memory to allocate regulators info\n"); + if (getoptions(argc, argv, options)) { + usage(); return 1; }
- if (getoptions(argc, argv, options)) { - usage(); + regulators_info = regulator_init(&numregulators); + if (!regulators_info) { + printf("not enough memory to allocate regulators info\n"); return 1; }
@@ -371,20 +385,9 @@ int main(int argc, char **argv) options->clocks = false; }
- /* we just dump the informations */ - if (options->dump) { - if (powerdebug_dump(options, regulators_info, numregulators)) - return 1; - return 0; - } - - if (display_init()) { - printf("failed to initialize display\n"); - return 1; - } + ret = options->dump ? + powerdebug_dump(options, regulators_info, numregulators) : + powerdebug_display(options, regulators_info, numregulators);
- if (mainloop(options, regulators_info, numregulators)) - return 1; - - return 0; + return ret < 0; }
These variable are only used in the sensor.c file.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- sensor.c | 9 +++++++++ sensor.h | 8 -------- 2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/sensor.c b/sensor.c index f5fcd40..df07593 100644 --- a/sensor.c +++ b/sensor.c @@ -16,6 +16,15 @@ #include "powerdebug.h" #include "sensor.h"
+static char *items_temp[32] = {"min", "max", "input", "label", ""}; +static char *suffix_temp[32] = {"°C", "°C", "°C", "", ""}; +static char *items_in[32] = {"min", "max", "input", "label", ""}; +static char *suffix_in[32] = {"Volts", "Volts", "Volts", "", ""}; +static char *items_fan[32] = {"min", "max", "input", "label", "div", "target", ""}; +static char *suffix_fan[32] = {"RPM", "RPM", "RPM", "", "", "RPM", ""}; +static char *items_pwm[32] = {"freq", "enable", "mode", ""}; +static char *suffix_pwm[32] = {"Hz", "", "", ""}; + char *get_num(char *fname, char *sensor) { char tmpstr[NAME_MAX]; diff --git a/sensor.h b/sensor.h index cca11d1..8537149 100644 --- a/sensor.h +++ b/sensor.h @@ -13,11 +13,3 @@ * - initial API and implementation *******************************************************************************/
-char *items_temp[32] = {"min", "max", "input", "label", ""}; -char *suffix_temp[32] = {"°C", "°C", "°C", "", ""}; -char *items_in[32] = {"min", "max", "input", "label", ""}; -char *suffix_in[32] = {"Volts", "Volts", "Volts", "", ""}; -char *items_fan[32] = {"min", "max", "input", "label", "div", "target", ""}; -char *suffix_fan[32] = {"RPM", "RPM", "RPM", "", "", "RPM", ""}; -char *items_pwm[32] = {"freq", "enable", "mode", ""}; -char *suffix_pwm[32] = {"Hz", "", "", ""};
We just disable the option
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr --- powerdebug.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index 86f51bc..0f86e0c 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -377,7 +377,7 @@ int main(int argc, char **argv) regulators_info = regulator_init(&numregulators); if (!regulators_info) { printf("not enough memory to allocate regulators info\n"); - return 1; + options->regulators = false; }
if (clock_init()) {
Daniel,
I've now reviewed, tested and merged this entire series. Some minor changes were made to a handful of commit messages.
Thanks for taking the time to make the changes self-contained, easily bisectable and making sure they don't break builds (I checked with git test-sequence).
One quick note: I still see the segmentation fault with your patch in dump mode.
Regards, Amit
On 11 Mar 26, Daniel Lezcano wrote:
Move the corresponding function to the right header file and rename the function with <subsystem>_function_name.
Signed-off-by: Daniel Lezcano daniel.lezcano@free.fr
powerdebug.c | 3 ++- powerdebug.h | 1 - regulator.c | 2 +- regulator.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/powerdebug.c b/powerdebug.c index 2f0992b..fa26e77 100644 --- a/powerdebug.c +++ b/powerdebug.c @@ -15,6 +15,7 @@ #include <getopt.h> #include <stdbool.h> +#include "regulator.h" #include "powerdebug.h" int highlighted_row; @@ -352,7 +353,7 @@ int main(int argc, char **argv) return 1; }
- if (init_regulator_ds())
- if (regulator_init()) return 1;
if (mainloop(options)) diff --git a/powerdebug.h b/powerdebug.h index cef5570..42add86 100644 --- a/powerdebug.h +++ b/powerdebug.h @@ -32,7 +32,6 @@ extern struct regulator_info *regulators_info; extern int numregulators; -extern int init_regulator_ds(void); extern void print_regulator_info(int verbose); extern void read_regulator_info(void); extern void print_regulator_info(int verbose); diff --git a/regulator.c b/regulator.c index 18c9777..395571b 100644 --- a/regulator.c +++ b/regulator.c @@ -15,7 +15,7 @@ #include "regulator.h" -int init_regulator_ds(void) +int regulator_init(void) { DIR *regdir; struct dirent *item; diff --git a/regulator.h b/regulator.h index 3ecf6b8..b6780ab 100644 --- a/regulator.h +++ b/regulator.h @@ -40,4 +40,4 @@ struct regulator_info { int num_users; } *regulators_info; -extern int numregulators;
+extern int regulator_init(void);
1.7.1
On 04/04/2011 11:53 AM, Amit Kucheria wrote:
Daniel,
I've now reviewed, tested and merged this entire series. Some minor changes were made to a handful of commit messages.
Thanks for taking the time to make the changes self-contained, easily bisectable and making sure they don't break builds (I checked with git test-sequence).
Thanks for testing and merging.
One quick note: I still see the segmentation fault with your patch in dump mode.
Hmm, weird. I will check that.
Thanks
-- Daniel
On 04/04/2011 11:53 AM, Amit Kucheria wrote:
Daniel,
I've now reviewed, tested and merged this entire series. Some minor changes were made to a handful of commit messages.
Thanks for taking the time to make the changes self-contained, easily bisectable and making sure they don't break builds (I checked with git test-sequence).
One quick note: I still see the segmentation fault with your patch in dump mode.
Hi Amit,
I was not able to reproduce the segfault. That does not mean there isn't a bug but possibly it appears on a different hardware, I am using a igep-v2.
-
Amit Kucheria -
Daniel Lezcano -
Yong Shen