diff mbox

POWERDEBUG: Add modify gpio function in powerdebug tool.

Message ID 1375099906-12144-1-git-send-email-shaojie.sun@linaro.com
State New
Headers show

Commit Message

sunshaojie July 29, 2013, 12:11 p.m. UTC
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>
---
 README    |    8 +++++++-
 display.c |   18 ++++++++++++++++++
 display.h |    1 +
 gpio.c    |   45 +++++++++++++++++++++++++++++++++++++++++++++
 utils.c   |   36 ++++++++++++++++++++++++++++++++++++
 utils.h   |    2 ++
 6 files changed, 109 insertions(+), 1 deletion(-)

Comments

Sanjay Singh Rawat July 30, 2013, 5:10 a.m. UTC | #1
please use "powerdebug" and version number in prefix of the patch

On Monday 29 July 2013 05:41 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>
> ---
>   README    |    8 +++++++-
>   display.c |   18 ++++++++++++++++++
>   display.h |    1 +
>   gpio.c    |   45 +++++++++++++++++++++++++++++++++++++++++++++
>   utils.c   |   36 ++++++++++++++++++++++++++++++++++++
>   utils.h   |    2 ++
>   6 files changed, 109 insertions(+), 1 deletion(-)
>
> diff --git a/README b/README
> index fa997f6..3bf1659 100644
> --- a/README
> +++ b/README
> @@ -7,4 +7,10 @@ information.
>   Current version only displays regulator information and clock tree from
>   debugfs. Support will be added for sensors later.
>
> - -- Amit Arora <amit.arora@linaro.org>  Fri, 08 Sep 2010
> + -- Amit Arora <amit.arora@linaro.org>  Fri, 08 Sep 2010
> +
> +Now we add gpio-change function for power consumption need.
> +When gpio is not in interrupt mode. use 'D' key to change gpio direction.
> +And when gpio direction is "out", use 'V' key to change gpio value.
> +
> + -- Shaojie Sun <shaojie.sun@linaro.org> Mon, 29 Jul 2013
> diff --git a/display.c b/display.c
> index 41a511d..c0afe03 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,13 @@ static int display_keystroke(int fd, void *data)
>   		display_select();
>   		break;
>
> +	case 'v':
> +	case 'V':
> +	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..84f150f 100644
> --- a/gpio.c
> +++ b/gpio.c
> @@ -264,8 +264,53 @@ 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;
> +
> +	if (!t || !gpio)
> +		return -1;
> +
> +	switch (keyvalue) {
> +	case 'D':
> +		/* Only change direction when gpio interrupt not set.*/
> +		if (!strstr(gpio->edge, "none"))
> +			return 0;
> +
> +		if (strstr(gpio->direction, "in"))
> +			strcpy(gpio->direction, "out");
> +		else if (strstr(gpio->direction, "out"))
> +			strcpy(gpio->direction, "in");
> +		file_write_value(t->path, "direction", "%s", &gpio->direction);
> +		file_read_value(t->path, "direction", "%s", &gpio->direction);
> +
> +		break;
> +
> +	case 'V':
> +		/* Only change value when gpio direction is out. */
> +		if (!strstr(gpio->edge, "none")
> +			 || !strstr(gpio->direction, "out"))
> +			return 0;
> +
> +		if (gpio->value)
> +			file_write_value(t->path, "direction", "%s", &"low");
> +		else
> +			file_write_value(t->path, "direction", "%s", &"high");
we are not setting _value_ here. Did you meant "value", instead of 
direction?
also we have to set "value" as 1/0 right? hight/low is for direction. 
Disi you tested this patch?
> +		file_read_value(t->path, "value", "%d", &gpio->value);
> +
> +		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..4d4b780 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, "w");
> +	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
>
sunshaojie July 30, 2013, 5:53 a.m. UTC | #2
On 30 July 2013 13:10, Sanjay Singh Rawat <sanjay.rawat@linaro.org> wrote:
> please use "powerdebug" and version number in prefix of the patch
I don't know how to change. please tell me. Thanks.

>
> On Monday 29 July 2013 05:41 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>
>> ---
>>   README    |    8 +++++++-
>>   display.c |   18 ++++++++++++++++++
>>   display.h |    1 +
>>   gpio.c    |   45 +++++++++++++++++++++++++++++++++++++++++++++
>>   utils.c   |   36 ++++++++++++++++++++++++++++++++++++
>>   utils.h   |    2 ++
>>   6 files changed, 109 insertions(+), 1 deletion(-)
>>
>> diff --git a/README b/README
>> index fa997f6..3bf1659 100644
>> --- a/README
>> +++ b/README
>> @@ -7,4 +7,10 @@ information.
>>   Current version only displays regulator information and clock tree from
>>   debugfs. Support will be added for sensors later.
>>
>> - -- Amit Arora <amit.arora@linaro.org>  Fri, 08 Sep 2010
>> + -- Amit Arora <amit.arora@linaro.org>  Fri, 08 Sep 2010
>> +
>> +Now we add gpio-change function for power consumption need.
>> +When gpio is not in interrupt mode. use 'D' key to change gpio direction.
>> +And when gpio direction is "out", use 'V' key to change gpio value.
>> +
>> + -- Shaojie Sun <shaojie.sun@linaro.org> Mon, 29 Jul 2013
>> diff --git a/display.c b/display.c
>> index 41a511d..c0afe03 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,13 @@ static int display_keystroke(int fd, void *data)
>>                 display_select();
>>                 break;
>>
>> +       case 'v':
>> +       case 'V':
>> +       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..84f150f 100644
>> --- a/gpio.c
>> +++ b/gpio.c
>> @@ -264,8 +264,53 @@ 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;
>> +
>> +       if (!t || !gpio)
>> +               return -1;
>> +
>> +       switch (keyvalue) {
>> +       case 'D':
>> +               /* Only change direction when gpio interrupt not set.*/
>> +               if (!strstr(gpio->edge, "none"))
>> +                       return 0;
>> +
>> +               if (strstr(gpio->direction, "in"))
>> +                       strcpy(gpio->direction, "out");
>> +               else if (strstr(gpio->direction, "out"))
>> +                       strcpy(gpio->direction, "in");
>> +               file_write_value(t->path, "direction", "%s",
>> &gpio->direction);
>> +               file_read_value(t->path, "direction", "%s",
>> &gpio->direction);
>> +
>> +               break;
>> +
>> +       case 'V':
>> +               /* Only change value when gpio direction is out. */
>> +               if (!strstr(gpio->edge, "none")
>> +                        || !strstr(gpio->direction, "out"))
>> +                       return 0;
>> +
>> +               if (gpio->value)
>> +                       file_write_value(t->path, "direction", "%s",
>> &"low");
>> +               else
>> +                       file_write_value(t->path, "direction", "%s",
>> &"high");
>
> we are not setting _value_ here. Did you meant "value", instead of
> direction?
> also we have to set "value" as 1/0 right? hight/low is for direction. Disi
> you tested this patch?
Yes, I test it. In gpio sysfs, value file is a read only file. We must
change value through
writing hight/low to direction file.
Last time I didn't read value from the value file after write
operation. So I made a mistake.

>
>> +               file_read_value(t->path, "value", "%d", &gpio->value);
>> +
>> +               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..4d4b780 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, "w");
>> +       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
>>
>
Mike Turquette July 30, 2013, 6 a.m. UTC | #3
On Mon, Jul 29, 2013 at 10:53 PM, Shaojie Sun <shaojie.sun@linaro.org> wrote:
> On 30 July 2013 13:10, Sanjay Singh Rawat <sanjay.rawat@linaro.org> wrote:
>> please use "powerdebug" and version number in prefix of the patch
> I don't know how to change. please tell me. Thanks.

git format-patch --subject-prefix="POWERDEBUG v4" ...

Regards,
Mike

>
>>
>> On Monday 29 July 2013 05:41 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>
>>> ---
>>>   README    |    8 +++++++-
>>>   display.c |   18 ++++++++++++++++++
>>>   display.h |    1 +
>>>   gpio.c    |   45 +++++++++++++++++++++++++++++++++++++++++++++
>>>   utils.c   |   36 ++++++++++++++++++++++++++++++++++++
>>>   utils.h   |    2 ++
>>>   6 files changed, 109 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/README b/README
>>> index fa997f6..3bf1659 100644
>>> --- a/README
>>> +++ b/README
>>> @@ -7,4 +7,10 @@ information.
>>>   Current version only displays regulator information and clock tree from
>>>   debugfs. Support will be added for sensors later.
>>>
>>> - -- Amit Arora <amit.arora@linaro.org>  Fri, 08 Sep 2010
>>> + -- Amit Arora <amit.arora@linaro.org>  Fri, 08 Sep 2010
>>> +
>>> +Now we add gpio-change function for power consumption need.
>>> +When gpio is not in interrupt mode. use 'D' key to change gpio direction.
>>> +And when gpio direction is "out", use 'V' key to change gpio value.
>>> +
>>> + -- Shaojie Sun <shaojie.sun@linaro.org> Mon, 29 Jul 2013
>>> diff --git a/display.c b/display.c
>>> index 41a511d..c0afe03 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,13 @@ static int display_keystroke(int fd, void *data)
>>>                 display_select();
>>>                 break;
>>>
>>> +       case 'v':
>>> +       case 'V':
>>> +       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..84f150f 100644
>>> --- a/gpio.c
>>> +++ b/gpio.c
>>> @@ -264,8 +264,53 @@ 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;
>>> +
>>> +       if (!t || !gpio)
>>> +               return -1;
>>> +
>>> +       switch (keyvalue) {
>>> +       case 'D':
>>> +               /* Only change direction when gpio interrupt not set.*/
>>> +               if (!strstr(gpio->edge, "none"))
>>> +                       return 0;
>>> +
>>> +               if (strstr(gpio->direction, "in"))
>>> +                       strcpy(gpio->direction, "out");
>>> +               else if (strstr(gpio->direction, "out"))
>>> +                       strcpy(gpio->direction, "in");
>>> +               file_write_value(t->path, "direction", "%s",
>>> &gpio->direction);
>>> +               file_read_value(t->path, "direction", "%s",
>>> &gpio->direction);
>>> +
>>> +               break;
>>> +
>>> +       case 'V':
>>> +               /* Only change value when gpio direction is out. */
>>> +               if (!strstr(gpio->edge, "none")
>>> +                        || !strstr(gpio->direction, "out"))
>>> +                       return 0;
>>> +
>>> +               if (gpio->value)
>>> +                       file_write_value(t->path, "direction", "%s",
>>> &"low");
>>> +               else
>>> +                       file_write_value(t->path, "direction", "%s",
>>> &"high");
>>
>> we are not setting _value_ here. Did you meant "value", instead of
>> direction?
>> also we have to set "value" as 1/0 right? hight/low is for direction. Disi
>> you tested this patch?
> Yes, I test it. In gpio sysfs, value file is a read only file. We must
> change value through
> writing hight/low to direction file.
> Last time I didn't read value from the value file after write
> operation. So I made a mistake.
>
>>
>>> +               file_read_value(t->path, "value", "%d", &gpio->value);
>>> +
>>> +               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..4d4b780 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, "w");
>>> +       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
>>>
>>
Sanjay Singh Rawat July 30, 2013, 7:04 a.m. UTC | #4
On Tuesday 30 July 2013 11:23 AM, Shaojie Sun wrote:
> On 30 July 2013 13:10, Sanjay Singh Rawat <sanjay.rawat@linaro.org> wrote:
>> please use "powerdebug" and version number in prefix of the patch
> I don't know how to change. please tell me. Thanks.
>
>>
>> On Monday 29 July 2013 05:41 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>
>>> ---
>>>    README    |    8 +++++++-
>>>    display.c |   18 ++++++++++++++++++
>>>    display.h |    1 +
>>>    gpio.c    |   45 +++++++++++++++++++++++++++++++++++++++++++++
>>>    utils.c   |   36 ++++++++++++++++++++++++++++++++++++
>>>    utils.h   |    2 ++
>>>    6 files changed, 109 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/README b/README
>>> index fa997f6..3bf1659 100644
>>> --- a/README
>>> +++ b/README
>>> @@ -7,4 +7,10 @@ information.
>>>    Current version only displays regulator information and clock tree from
>>>    debugfs. Support will be added for sensors later.
>>>
>>> - -- Amit Arora <amit.arora@linaro.org>  Fri, 08 Sep 2010
>>> + -- Amit Arora <amit.arora@linaro.org>  Fri, 08 Sep 2010
>>> +
>>> +Now we add gpio-change function for power consumption need.
>>> +When gpio is not in interrupt mode. use 'D' key to change gpio direction.
>>> +And when gpio direction is "out", use 'V' key to change gpio value.
>>> +
>>> + -- Shaojie Sun <shaojie.sun@linaro.org> Mon, 29 Jul 2013
>>> diff --git a/display.c b/display.c
>>> index 41a511d..c0afe03 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,13 @@ static int display_keystroke(int fd, void *data)
>>>                  display_select();
>>>                  break;
>>>
>>> +       case 'v':
>>> +       case 'V':
>>> +       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..84f150f 100644
>>> --- a/gpio.c
>>> +++ b/gpio.c
>>> @@ -264,8 +264,53 @@ 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;
>>> +
>>> +       if (!t || !gpio)
>>> +               return -1;
>>> +
>>> +       switch (keyvalue) {
>>> +       case 'D':
>>> +               /* Only change direction when gpio interrupt not set.*/
>>> +               if (!strstr(gpio->edge, "none"))
>>> +                       return 0;
>>> +
>>> +               if (strstr(gpio->direction, "in"))
>>> +                       strcpy(gpio->direction, "out");
>>> +               else if (strstr(gpio->direction, "out"))
>>> +                       strcpy(gpio->direction, "in");
>>> +               file_write_value(t->path, "direction", "%s",
>>> &gpio->direction);
>>> +               file_read_value(t->path, "direction", "%s",
>>> &gpio->direction);
>>> +
>>> +               break;
>>> +
>>> +       case 'V':
>>> +               /* Only change value when gpio direction is out. */
>>> +               if (!strstr(gpio->edge, "none")
>>> +                        || !strstr(gpio->direction, "out"))
>>> +                       return 0;
>>> +
>>> +               if (gpio->value)
>>> +                       file_write_value(t->path, "direction", "%s",
>>> &"low");
>>> +               else
>>> +                       file_write_value(t->path, "direction", "%s",
>>> &"high");
>>
>> we are not setting _value_ here. Did you meant "value", instead of
>> direction?
>> also we have to set "value" as 1/0 right? hight/low is for direction. Disi
>> you tested this patch?
> Yes, I test it. In gpio sysfs, value file is a read only file. We must
the permission for all attributes are *644* and anyway we have to run it 
in superuser mode everytime
> change value through
> writing hight/low to direction file.
- writing high/low will change the direction to in/out respectively, i 
was expecting will pull down the line(by setting value=0) to save power. 
Not sure about your hardware.
- here as per the implementation, if the *value==1* we are setting the 
direction to low(OUT), which is the _case-'V'_ we ckecked already; so 
effectively we are doing nothing. Is this right?

> Last time I didn't read value from the value file after write
> operation. So I made a mistake.
>
>>
>>> +               file_read_value(t->path, "value", "%d", &gpio->value);
>>> +
>>> +               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..4d4b780 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, "w");
>>> +       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
>>>
>>
sunshaojie July 30, 2013, 7:25 a.m. UTC | #5
I tried to write value=0 when value file is 1. But I find nothing
change that the gpio value still is 1.

So I read the driver of gpio. And I found the annotation about value
file. So I think the value couldn't be change in some platform.

You can find this code from kernel/driver/gpio/gpiolib.c

 *   /value
 *      * always readable, subject to hardware behavior
 *      * may be writable, as zero/nonzero

I also find when we set "high" to direction file. It will change value
to 1 first, then change the direction to out.
and set "low" to direction file. It will change value to 0 first, then
change the direction to out.

So if the *value==1* we are setting the direction to low(OUT), which
is the _case-'V'_ we changed value to 0 and direction was also OUT.

static ssize_t gpio_direction_store(struct device *dev,
                struct device_attribute *attr, const char *buf, size_t size)
{
        struct gpio_desc        *desc = dev_get_drvdata(dev);
        ssize_t                 status;

        mutex_lock(&sysfs_lock);

        if (!test_bit(FLAG_EXPORT, &desc->flags))
                status = -EIO;
        else if (sysfs_streq(buf, "high"))
                status = gpiod_direction_output(desc, 1);
        else if (sysfs_streq(buf, "out") || sysfs_streq(buf, "low"))
                status = gpiod_direction_output(desc, 0);
        else if (sysfs_streq(buf, "in"))
                status = gpiod_direction_input(desc);
        else
                status = -EINVAL;

        mutex_unlock(&sysfs_lock);
        return status ? : size;
}

On 30 July 2013 15:04, Sanjay Singh Rawat <sanjay.rawat@linaro.org> wrote:
> On Tuesday 30 July 2013 11:23 AM, Shaojie Sun wrote:
>>
>> On 30 July 2013 13:10, Sanjay Singh Rawat <sanjay.rawat@linaro.org> wrote:
>>>
>>> please use "powerdebug" and version number in prefix of the patch
>>
>> I don't know how to change. please tell me. Thanks.
>>
>>>
>>> On Monday 29 July 2013 05:41 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>
>>>> ---
>>>>    README    |    8 +++++++-
>>>>    display.c |   18 ++++++++++++++++++
>>>>    display.h |    1 +
>>>>    gpio.c    |   45 +++++++++++++++++++++++++++++++++++++++++++++
>>>>    utils.c   |   36 ++++++++++++++++++++++++++++++++++++
>>>>    utils.h   |    2 ++
>>>>    6 files changed, 109 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/README b/README
>>>> index fa997f6..3bf1659 100644
>>>> --- a/README
>>>> +++ b/README
>>>> @@ -7,4 +7,10 @@ information.
>>>>    Current version only displays regulator information and clock tree
>>>> from
>>>>    debugfs. Support will be added for sensors later.
>>>>
>>>> - -- Amit Arora <amit.arora@linaro.org>  Fri, 08 Sep 2010
>>>> + -- Amit Arora <amit.arora@linaro.org>  Fri, 08 Sep 2010
>>>> +
>>>> +Now we add gpio-change function for power consumption need.
>>>> +When gpio is not in interrupt mode. use 'D' key to change gpio
>>>> direction.
>>>> +And when gpio direction is "out", use 'V' key to change gpio value.
>>>> +
>>>> + -- Shaojie Sun <shaojie.sun@linaro.org> Mon, 29 Jul 2013
>>>> diff --git a/display.c b/display.c
>>>> index 41a511d..c0afe03 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,13 @@ static int display_keystroke(int fd, void *data)
>>>>                  display_select();
>>>>                  break;
>>>>
>>>> +       case 'v':
>>>> +       case 'V':
>>>> +       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..84f150f 100644
>>>> --- a/gpio.c
>>>> +++ b/gpio.c
>>>> @@ -264,8 +264,53 @@ 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;
>>>> +
>>>> +       if (!t || !gpio)
>>>> +               return -1;
>>>> +
>>>> +       switch (keyvalue) {
>>>> +       case 'D':
>>>> +               /* Only change direction when gpio interrupt not set.*/
>>>> +               if (!strstr(gpio->edge, "none"))
>>>> +                       return 0;
>>>> +
>>>> +               if (strstr(gpio->direction, "in"))
>>>> +                       strcpy(gpio->direction, "out");
>>>> +               else if (strstr(gpio->direction, "out"))
>>>> +                       strcpy(gpio->direction, "in");
>>>> +               file_write_value(t->path, "direction", "%s",
>>>> &gpio->direction);
>>>> +               file_read_value(t->path, "direction", "%s",
>>>> &gpio->direction);
>>>> +
>>>> +               break;
>>>> +
>>>> +       case 'V':
>>>> +               /* Only change value when gpio direction is out. */
>>>> +               if (!strstr(gpio->edge, "none")
>>>> +                        || !strstr(gpio->direction, "out"))
>>>> +                       return 0;
>>>> +
>>>> +               if (gpio->value)
>>>> +                       file_write_value(t->path, "direction", "%s",
>>>> &"low");
>>>> +               else
>>>> +                       file_write_value(t->path, "direction", "%s",
>>>> &"high");
>>>
>>>
>>> we are not setting _value_ here. Did you meant "value", instead of
>>> direction?
>>> also we have to set "value" as 1/0 right? hight/low is for direction.
>>> Disi
>>> you tested this patch?
>>
>> Yes, I test it. In gpio sysfs, value file is a read only file. We must
>
> the permission for all attributes are *644* and anyway we have to run it in
> superuser mode everytime
>
>> change value through
>> writing hight/low to direction file.
>
> - writing high/low will change the direction to in/out respectively, i was
> expecting will pull down the line(by setting value=0) to save power. Not
> sure about your hardware.
> - here as per the implementation, if the *value==1* we are setting the
> direction to low(OUT), which is the _case-'V'_ we ckecked already; so
> effectively we are doing nothing. Is this right?
>
>
>> Last time I didn't read value from the value file after write
>> operation. So I made a mistake.
>>
>>>
>>>> +               file_read_value(t->path, "value", "%d", &gpio->value);
>>>> +
>>>> +               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..4d4b780 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, "w");
>>>> +       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
>>>>
>>>
>
Sanjay Singh Rawat July 30, 2013, 8:12 a.m. UTC | #6
On Tuesday 30 July 2013 12:55 PM, Shaojie Sun wrote:
> I tried to write value=0 when value file is 1. But I find nothing
> change that the gpio value still is 1.
>
> So I read the driver of gpio. And I found the annotation about value
> file. So I think the value couldn't be change in some platform.
>
> You can find this code from kernel/driver/gpio/gpiolib.c
>
>   *   /value
>   *      * always readable, subject to hardware behavior
>   *      * may be writable, as zero/nonzero
>
> I also find when we set "high" to direction file. It will change value
> to 1 first, then change the direction to out.
> and set "low" to direction file. It will change value to 0 first, then
> change the direction to out.
>
> So if the *value==1* we are setting the direction to low(OUT), which
> is the _case-'V'_ we changed value to 0 and direction was also OUT.
Ack, thanks for the explanation.
>
> static ssize_t gpio_direction_store(struct device *dev,
>                  struct device_attribute *attr, const char *buf, size_t size)
> {
>          struct gpio_desc        *desc = dev_get_drvdata(dev);
>          ssize_t                 status;
>
>          mutex_lock(&sysfs_lock);
>
>          if (!test_bit(FLAG_EXPORT, &desc->flags))
>                  status = -EIO;
>          else if (sysfs_streq(buf, "high"))
>                  status = gpiod_direction_output(desc, 1);
>          else if (sysfs_streq(buf, "out") || sysfs_streq(buf, "low"))
>                  status = gpiod_direction_output(desc, 0);
>          else if (sysfs_streq(buf, "in"))
>                  status = gpiod_direction_input(desc);
>          else
>                  status = -EINVAL;
>
>          mutex_unlock(&sysfs_lock);
>          return status ? : size;
> }
>
> On 30 July 2013 15:04, Sanjay Singh Rawat <sanjay.rawat@linaro.org> wrote:
>> On Tuesday 30 July 2013 11:23 AM, Shaojie Sun wrote:
>>>
>>> On 30 July 2013 13:10, Sanjay Singh Rawat <sanjay.rawat@linaro.org> wrote:
>>>>
>>>> please use "powerdebug" and version number in prefix of the patch
>>>
>>> I don't know how to change. please tell me. Thanks.
>>>
>>>>
>>>> On Monday 29 July 2013 05:41 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>
>>>>> ---
>>>>>     README    |    8 +++++++-
>>>>>     display.c |   18 ++++++++++++++++++
>>>>>     display.h |    1 +
>>>>>     gpio.c    |   45 +++++++++++++++++++++++++++++++++++++++++++++
>>>>>     utils.c   |   36 ++++++++++++++++++++++++++++++++++++
>>>>>     utils.h   |    2 ++
>>>>>     6 files changed, 109 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/README b/README
>>>>> index fa997f6..3bf1659 100644
>>>>> --- a/README
>>>>> +++ b/README
>>>>> @@ -7,4 +7,10 @@ information.
>>>>>     Current version only displays regulator information and clock tree
>>>>> from
>>>>>     debugfs. Support will be added for sensors later.
>>>>>
>>>>> - -- Amit Arora <amit.arora@linaro.org>  Fri, 08 Sep 2010
>>>>> + -- Amit Arora <amit.arora@linaro.org>  Fri, 08 Sep 2010
>>>>> +
>>>>> +Now we add gpio-change function for power consumption need.
>>>>> +When gpio is not in interrupt mode. use 'D' key to change gpio
>>>>> direction.
>>>>> +And when gpio direction is "out", use 'V' key to change gpio value.
>>>>> +
>>>>> + -- Shaojie Sun <shaojie.sun@linaro.org> Mon, 29 Jul 2013
>>>>> diff --git a/display.c b/display.c
>>>>> index 41a511d..c0afe03 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,13 @@ static int display_keystroke(int fd, void *data)
>>>>>                   display_select();
>>>>>                   break;
>>>>>
>>>>> +       case 'v':
>>>>> +       case 'V':
>>>>> +       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..84f150f 100644
>>>>> --- a/gpio.c
>>>>> +++ b/gpio.c
>>>>> @@ -264,8 +264,53 @@ 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;
>>>>> +
>>>>> +       if (!t || !gpio)
>>>>> +               return -1;
>>>>> +
>>>>> +       switch (keyvalue) {
>>>>> +       case 'D':
>>>>> +               /* Only change direction when gpio interrupt not set.*/
>>>>> +               if (!strstr(gpio->edge, "none"))
>>>>> +                       return 0;
>>>>> +
>>>>> +               if (strstr(gpio->direction, "in"))
>>>>> +                       strcpy(gpio->direction, "out");
>>>>> +               else if (strstr(gpio->direction, "out"))
>>>>> +                       strcpy(gpio->direction, "in");
>>>>> +               file_write_value(t->path, "direction", "%s",
>>>>> &gpio->direction);
>>>>> +               file_read_value(t->path, "direction", "%s",
>>>>> &gpio->direction);
>>>>> +
>>>>> +               break;
>>>>> +
>>>>> +       case 'V':
>>>>> +               /* Only change value when gpio direction is out. */
>>>>> +               if (!strstr(gpio->edge, "none")
>>>>> +                        || !strstr(gpio->direction, "out"))
>>>>> +                       return 0;
>>>>> +
>>>>> +               if (gpio->value)
>>>>> +                       file_write_value(t->path, "direction", "%s",
>>>>> &"low");
>>>>> +               else
>>>>> +                       file_write_value(t->path, "direction", "%s",
>>>>> &"high");
>>>>
>>>>
>>>> we are not setting _value_ here. Did you meant "value", instead of
>>>> direction?
>>>> also we have to set "value" as 1/0 right? hight/low is for direction.
>>>> Disi
>>>> you tested this patch?
>>>
>>> Yes, I test it. In gpio sysfs, value file is a read only file. We must
>>
>> the permission for all attributes are *644* and anyway we have to run it in
>> superuser mode everytime
>>
>>> change value through
>>> writing hight/low to direction file.
>>
>> - writing high/low will change the direction to in/out respectively, i was
>> expecting will pull down the line(by setting value=0) to save power. Not
>> sure about your hardware.
>> - here as per the implementation, if the *value==1* we are setting the
>> direction to low(OUT), which is the _case-'V'_ we ckecked already; so
>> effectively we are doing nothing. Is this right?
>>
>>
>>> Last time I didn't read value from the value file after write
>>> operation. So I made a mistake.
>>>
>>>>
>>>>> +               file_read_value(t->path, "value", "%d", &gpio->value);
>>>>> +
>>>>> +               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..4d4b780 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, "w");
>>>>> +       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 mbox

Patch

diff --git a/README b/README
index fa997f6..3bf1659 100644
--- a/README
+++ b/README
@@ -7,4 +7,10 @@  information.
 Current version only displays regulator information and clock tree from
 debugfs. Support will be added for sensors later.
 
- -- Amit Arora <amit.arora@linaro.org>  Fri, 08 Sep 2010 
+ -- Amit Arora <amit.arora@linaro.org>  Fri, 08 Sep 2010
+
+Now we add gpio-change function for power consumption need.
+When gpio is not in interrupt mode. use 'D' key to change gpio direction.
+And when gpio direction is "out", use 'V' key to change gpio value.
+
+ -- Shaojie Sun <shaojie.sun@linaro.org> Mon, 29 Jul 2013
diff --git a/display.c b/display.c
index 41a511d..c0afe03 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,13 @@  static int display_keystroke(int fd, void *data)
 		display_select();
 		break;
 
+	case 'v':
+	case 'V':
+	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..84f150f 100644
--- a/gpio.c
+++ b/gpio.c
@@ -264,8 +264,53 @@  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;
+
+	if (!t || !gpio)
+		return -1;
+
+	switch (keyvalue) {
+	case 'D':
+		/* Only change direction when gpio interrupt not set.*/
+		if (!strstr(gpio->edge, "none"))
+			return 0;
+
+		if (strstr(gpio->direction, "in"))
+			strcpy(gpio->direction, "out");
+		else if (strstr(gpio->direction, "out"))
+			strcpy(gpio->direction, "in");
+		file_write_value(t->path, "direction", "%s", &gpio->direction);
+		file_read_value(t->path, "direction", "%s", &gpio->direction);
+
+		break;
+
+	case 'V':
+		/* Only change value when gpio direction is out. */
+		if (!strstr(gpio->edge, "none")
+			 || !strstr(gpio->direction, "out"))
+			return 0;
+
+		if (gpio->value)
+			file_write_value(t->path, "direction", "%s", &"low");
+		else
+			file_write_value(t->path, "direction", "%s", &"high");
+		file_read_value(t->path, "value", "%d", &gpio->value);
+
+		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..4d4b780 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, "w");
+	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