| Message ID | 1374751928-23378-1-git-send-email-shaojie.sun@linaro.com |
|---|---|
| State | New |
| Headers | show |
Good to have the GPIO configuration steps in the README file. Please include as part of this commit. Few comments below. On Thursday 25 July 2013 05:02 PM, Shaojie Sun wrote: > For power consumption test, we can change gpio direction and value > and check that power consumption is falled or not. > use 'D' key to change gpio direction. > And when gpio direction is "out", use 'V' key to change gpio value. > > Signed-off-by: Shaojie Sun <shaojie.sun@linaro.com> > --- > display.c | 22 ++++++++++++++++++++++ > display.h | 1 + > gpio.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > utils.c | 36 ++++++++++++++++++++++++++++++++++++ > utils.h | 2 ++ > 5 files changed, 109 insertions(+) > > diff --git a/display.c b/display.c > index 41a511d..748cc1b 100644 > --- a/display.c > +++ b/display.c > @@ -164,6 +164,17 @@ static int display_select(void) > return 0; > } > > +static int display_change(int keyvalue) > +{ > + if (!(windata[current_win].nrdata)) > + return 0; > + > + if (windata[current_win].ops && windata[current_win].ops->change) > + return windata[current_win].ops->change(keyvalue); > + > + return 0; > +} > + > static int display_next_panel(void) > { > size_t array_size = sizeof(windata) / sizeof(windata[0]); > @@ -406,6 +417,17 @@ static int display_keystroke(int fd, void *data) > display_select(); > break; > > + case 'v': > + case 'V': > + case 'a': > + case 'A': > + case 'e': > + case 'E': Not sure, are we going to configure edge? > + case 'd': > + case 'D': > + display_change(toupper(keystroke)); > + break; > + > case EOF: > case 'q': > case 'Q': > diff --git a/display.h b/display.h > index 6362a48..b28d26e 100644 > --- a/display.h > +++ b/display.h > @@ -20,6 +20,7 @@ struct display_ops { > int (*select)(void); > int (*find)(const char *); > int (*selectf)(void); > + int (*change)(int keyvalue); > }; > > extern int display_print_line(int window, int line, char *str, > diff --git a/gpio.c b/gpio.c > index 3ecc393..5ab7c2a 100644 > --- a/gpio.c > +++ b/gpio.c > @@ -264,8 +264,56 @@ static int gpio_display(bool refresh) > return gpio_print_info(gpio_tree); > } > > +static int gpio_change(int keyvalue) > +{ > + struct tree *t = display_get_row_data(GPIO); > + struct gpio_info *gpio = t->private; > + int tmp; > + char atmp[MAX_VALUE_BYTE]; > + > + if (!t || !gpio) > + return -1; > + > + switch (keyvalue) { Only if the gpio pin is not used as irq, we will be configuring it right? In that case, if the edge is "none" then only we will proceed ahead. > + case 'D': > + if (strstr(gpio->direction, "in")) > + strcpy(atmp, "out"); > + else if (strstr(gpio->direction, "out")) > + strcpy(atmp, "in"); > + else > + strcpy(atmp, gpio->direction); what is the need of this else case?` > + if (!file_write_value(t->path, "direction", "%s", &atmp)) > + strcpy(gpio->direction, atmp); > + break; > + > + case 'V': > + if (strstr(gpio->direction, "out")) { > + tmp = !gpio->value; > + if (!file_write_value(t->path, "value", "%d", &tmp)) > + gpio->value = tmp; > + } > + break; > + > +/* It is not good choise to change gpio irq status from userspace. */ > +/* > + case 'A': > + gpio->active_low = !gpio->active_low; why you are not writing to file here? > + break; > + > + case 'E': > + break; > +*/ > + > + default: > + return -1; > + } > + > + return 0; > +} > + > static struct display_ops gpio_ops = { > .display = gpio_display, > + .change = gpio_change, > }; > > /* > diff --git a/utils.c b/utils.c > index e47c58e..5423c49 100644 > --- a/utils.c > +++ b/utils.c > @@ -53,3 +53,39 @@ out_free: > free(rpath); > return ret; > } > + > +/* > + * This functions is a helper to write a specific file content and store > + * the content inside a variable pointer passed as parameter, the format > + * parameter gives the variable type to be write to the file. > + * > + * @path : directory path containing the file > + * @name : name of the file to be read > + * @format : the format of the format > + * @value : a pointer to a variable to store the content of the file > + * Returns 0 on success, -1 otherwise > + */ > +int file_write_value(const char *path, const char *name, > + const char *format, void *value) > +{ > + FILE *file; > + char *rpath; > + int ret; > + > + ret = asprintf(&rpath, "%s/%s", path, name); > + if (ret < 0) > + return ret; > + > + file = fopen(rpath, "wr"); > + if (!file) { > + ret = -1; > + goto out_free; > + } > + > + ret = fprintf(file, format, value) < 0 ? -1 : 0; > + > + fclose(file); > +out_free: > + free(rpath); > + return ret; > +} > diff --git a/utils.h b/utils.h > index d4ac65a..73159b9 100644 > --- a/utils.h > +++ b/utils.h > @@ -17,6 +17,8 @@ > > extern int file_read_value(const char *path, const char *name, > const char *format, void *value); > +extern int file_write_value(const char *path, const char *name, > + const char *format, void *value); > > > #endif >
Thank sanjar. On 29 July 2013 18:41, Sanjay Singh Rawat <sanjay.rawat@linaro.org> wrote: > Good to have the GPIO configuration steps in the README file. Please include > as part of this commit. Few comments below. Yes, I will add it. > > On Thursday 25 July 2013 05:02 PM, Shaojie Sun wrote: >> >> For power consumption test, we can change gpio direction and value >> and check that power consumption is falled or not. >> use 'D' key to change gpio direction. >> And when gpio direction is "out", use 'V' key to change gpio value. >> >> Signed-off-by: Shaojie Sun <shaojie.sun@linaro.com> >> --- >> display.c | 22 ++++++++++++++++++++++ >> display.h | 1 + >> gpio.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ >> utils.c | 36 ++++++++++++++++++++++++++++++++++++ >> utils.h | 2 ++ >> 5 files changed, 109 insertions(+) >> >> diff --git a/display.c b/display.c >> index 41a511d..748cc1b 100644 >> --- a/display.c >> +++ b/display.c >> @@ -164,6 +164,17 @@ static int display_select(void) >> return 0; >> } >> >> +static int display_change(int keyvalue) >> +{ >> + if (!(windata[current_win].nrdata)) >> + return 0; >> + >> + if (windata[current_win].ops && windata[current_win].ops->change) >> + return windata[current_win].ops->change(keyvalue); >> + >> + return 0; >> +} >> + >> static int display_next_panel(void) >> { >> size_t array_size = sizeof(windata) / sizeof(windata[0]); >> @@ -406,6 +417,17 @@ static int display_keystroke(int fd, void *data) >> display_select(); >> break; >> >> + case 'v': >> + case 'V': >> + case 'a': >> + case 'A': >> + case 'e': >> + case 'E': > > Not sure, are we going to configure edge? No, I don't want to config edge, Because I didn't find the benefit from this. I will delete this code. > >> + case 'd': >> + case 'D': >> + display_change(toupper(keystroke)); >> + break; >> + >> case EOF: >> case 'q': >> case 'Q': >> diff --git a/display.h b/display.h >> index 6362a48..b28d26e 100644 >> --- a/display.h >> +++ b/display.h >> @@ -20,6 +20,7 @@ struct display_ops { >> int (*select)(void); >> int (*find)(const char *); >> int (*selectf)(void); >> + int (*change)(int keyvalue); >> }; >> >> extern int display_print_line(int window, int line, char *str, >> diff --git a/gpio.c b/gpio.c >> index 3ecc393..5ab7c2a 100644 >> --- a/gpio.c >> +++ b/gpio.c >> @@ -264,8 +264,56 @@ static int gpio_display(bool refresh) >> return gpio_print_info(gpio_tree); >> } >> >> +static int gpio_change(int keyvalue) >> +{ >> + struct tree *t = display_get_row_data(GPIO); >> + struct gpio_info *gpio = t->private; >> + int tmp; >> + char atmp[MAX_VALUE_BYTE]; >> + >> + if (!t || !gpio) >> + return -1; >> + >> + switch (keyvalue) { > > Only if the gpio pin is not used as irq, we will be configuring it right? In > that case, if the edge is "none" then only we will proceed ahead. > >> + case 'D': >> + if (strstr(gpio->direction, "in")) >> + strcpy(atmp, "out"); >> + else if (strstr(gpio->direction, "out")) >> + strcpy(atmp, "in"); >> + else >> + strcpy(atmp, gpio->direction); > > what is the need of this else case?` Yes, I got a new way to do this. > >> + if (!file_write_value(t->path, "direction", "%s", &atmp)) >> + strcpy(gpio->direction, atmp); >> + break; >> + >> + case 'V': >> + if (strstr(gpio->direction, "out")) { >> + tmp = !gpio->value; >> + if (!file_write_value(t->path, "value", "%d", >> &tmp)) >> + gpio->value = tmp; >> + } >> + break; >> + >> +/* It is not good choise to change gpio irq status from userspace. */ >> +/* >> + case 'A': >> + gpio->active_low = !gpio->active_low; > > why you are not writing to file here? > >> + break; >> + >> + case 'E': >> + break; >> +*/ >> + >> + default: >> + return -1; >> + } >> + >> + return 0; >> +} >> + >> static struct display_ops gpio_ops = { >> .display = gpio_display, >> + .change = gpio_change, >> }; >> >> /* >> diff --git a/utils.c b/utils.c >> index e47c58e..5423c49 100644 >> --- a/utils.c >> +++ b/utils.c >> @@ -53,3 +53,39 @@ out_free: >> free(rpath); >> return ret; >> } >> + >> +/* >> + * This functions is a helper to write a specific file content and store >> + * the content inside a variable pointer passed as parameter, the format >> + * parameter gives the variable type to be write to the file. >> + * >> + * @path : directory path containing the file >> + * @name : name of the file to be read >> + * @format : the format of the format >> + * @value : a pointer to a variable to store the content of the file >> + * Returns 0 on success, -1 otherwise >> + */ >> +int file_write_value(const char *path, const char *name, >> + const char *format, void *value) >> +{ >> + FILE *file; >> + char *rpath; >> + int ret; >> + >> + ret = asprintf(&rpath, "%s/%s", path, name); >> + if (ret < 0) >> + return ret; >> + >> + file = fopen(rpath, "wr"); >> + if (!file) { >> + ret = -1; >> + goto out_free; >> + } >> + >> + ret = fprintf(file, format, value) < 0 ? -1 : 0; >> + >> + fclose(file); >> +out_free: >> + free(rpath); >> + return ret; >> +} >> diff --git a/utils.h b/utils.h >> index d4ac65a..73159b9 100644 >> --- a/utils.h >> +++ b/utils.h >> @@ -17,6 +17,8 @@ >> >> extern int file_read_value(const char *path, const char *name, >> const char *format, void *value); >> +extern int file_write_value(const char *path, const char *name, >> + const char *format, void *value); >> >> >> #endif >> >
diff --git a/display.c b/display.c index 41a511d..748cc1b 100644 --- a/display.c +++ b/display.c @@ -164,6 +164,17 @@ static int display_select(void) return 0; } +static int display_change(int keyvalue) +{ + if (!(windata[current_win].nrdata)) + return 0; + + if (windata[current_win].ops && windata[current_win].ops->change) + return windata[current_win].ops->change(keyvalue); + + return 0; +} + static int display_next_panel(void) { size_t array_size = sizeof(windata) / sizeof(windata[0]); @@ -406,6 +417,17 @@ static int display_keystroke(int fd, void *data) display_select(); break; + case 'v': + case 'V': + case 'a': + case 'A': + case 'e': + case 'E': + case 'd': + case 'D': + display_change(toupper(keystroke)); + break; + case EOF: case 'q': case 'Q': diff --git a/display.h b/display.h index 6362a48..b28d26e 100644 --- a/display.h +++ b/display.h @@ -20,6 +20,7 @@ struct display_ops { int (*select)(void); int (*find)(const char *); int (*selectf)(void); + int (*change)(int keyvalue); }; extern int display_print_line(int window, int line, char *str, diff --git a/gpio.c b/gpio.c index 3ecc393..5ab7c2a 100644 --- a/gpio.c +++ b/gpio.c @@ -264,8 +264,56 @@ static int gpio_display(bool refresh) return gpio_print_info(gpio_tree); } +static int gpio_change(int keyvalue) +{ + struct tree *t = display_get_row_data(GPIO); + struct gpio_info *gpio = t->private; + int tmp; + char atmp[MAX_VALUE_BYTE]; + + if (!t || !gpio) + return -1; + + switch (keyvalue) { + case 'D': + if (strstr(gpio->direction, "in")) + strcpy(atmp, "out"); + else if (strstr(gpio->direction, "out")) + strcpy(atmp, "in"); + else + strcpy(atmp, gpio->direction); + if (!file_write_value(t->path, "direction", "%s", &atmp)) + strcpy(gpio->direction, atmp); + break; + + case 'V': + if (strstr(gpio->direction, "out")) { + tmp = !gpio->value; + if (!file_write_value(t->path, "value", "%d", &tmp)) + gpio->value = tmp; + } + break; + +/* It is not good choise to change gpio irq status from userspace. */ +/* + case 'A': + gpio->active_low = !gpio->active_low; + break; + + case 'E': + break; +*/ + + default: + return -1; + } + + return 0; +} + static struct display_ops gpio_ops = { .display = gpio_display, + .change = gpio_change, }; /* diff --git a/utils.c b/utils.c index e47c58e..5423c49 100644 --- a/utils.c +++ b/utils.c @@ -53,3 +53,39 @@ out_free: free(rpath); return ret; } + +/* + * This functions is a helper to write a specific file content and store + * the content inside a variable pointer passed as parameter, the format + * parameter gives the variable type to be write to the file. + * + * @path : directory path containing the file + * @name : name of the file to be read + * @format : the format of the format + * @value : a pointer to a variable to store the content of the file + * Returns 0 on success, -1 otherwise + */ +int file_write_value(const char *path, const char *name, + const char *format, void *value) +{ + FILE *file; + char *rpath; + int ret; + + ret = asprintf(&rpath, "%s/%s", path, name); + if (ret < 0) + return ret; + + file = fopen(rpath, "wr"); + if (!file) { + ret = -1; + goto out_free; + } + + ret = fprintf(file, format, value) < 0 ? -1 : 0; + + fclose(file); +out_free: + free(rpath); + return ret; +} diff --git a/utils.h b/utils.h index d4ac65a..73159b9 100644 --- a/utils.h +++ b/utils.h @@ -17,6 +17,8 @@ extern int file_read_value(const char *path, const char *name, const char *format, void *value); +extern int file_write_value(const char *path, const char *name, + const char *format, void *value); #endif
For power consumption test, we can change gpio direction and value and check that power consumption is falled or not. use 'D' key to change gpio direction. And when gpio direction is "out", use 'V' key to change gpio value. Signed-off-by: Shaojie Sun <shaojie.sun@linaro.com> --- display.c | 22 ++++++++++++++++++++++ display.h | 1 + gpio.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ utils.c | 36 ++++++++++++++++++++++++++++++++++++ utils.h | 2 ++ 5 files changed, 109 insertions(+)