diff mbox

[2/3] POWERDEBUG: show gpio direction and egde in string.

Message ID 1374049468-3990-1-git-send-email-shaojie.sun@linaro.com
State Deferred
Headers show

Commit Message

sunshaojie July 17, 2013, 8:24 a.m. UTC
In gpio sysfs, direction and egde is shown in string. dierction value is "in" or "out".
And egde value is "none", "falling", "rising" or "both".
So strings must be shown in powerdebug tool.

Signed-off-by: sunshaojie <shaojie.sun@linaro.com>
---
 gpio.c |   20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

Comments

Sanjay Singh Rawat July 19, 2013, 1:17 p.m. UTC | #1
Sunshaojie,

I don't have the gpio configuration functions for panda. Have you test 
this code?

On Wednesday 17 July 2013 01:54 PM, sunshaojie wrote:
> In gpio sysfs, direction and egde is shown in string. dierction value is "in" or "out".
> And egde value is "none", "falling", "rising" or "both".
> So strings must be shown in powerdebug tool.
>
> Signed-off-by: sunshaojie <shaojie.sun@linaro.com>
> ---
>   gpio.c |   20 +++++++++++---------
>   1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/gpio.c b/gpio.c
> index f7d2a10..9501e42 100644
> --- a/gpio.c
> +++ b/gpio.c
> @@ -33,12 +33,14 @@
>
>   #define SYSFS_GPIO "/sys/class/gpio"
>
> +#define MAX_VALUE_BYTE	10
> +
>   struct gpio_info {
>   	bool expanded;
>   	int active_low;
>   	int value;
> -	int direction;
> -	int edge;
> +	char direction[MAX_VALUE_BYTE];
> +	char edge[MAX_VALUE_BYTE];
>   	char *prefix;
>   } *gpios_info;
>
> @@ -89,8 +91,8 @@ static inline int read_gpio_cb(struct tree *t, void *data)
>
>   	file_read_value(t->path, "active_low", "%d", &gpio->active_low);
>   	file_read_value(t->path, "value", "%d", &gpio->value);
> -	file_read_value(t->path, "edge", "%d", &gpio->edge);
> -	file_read_value(t->path, "direction", "%d", &gpio->direction);
> +	file_read_value(t->path, "edge", "%8s", &gpio->edge);
what is the need of width(8) here? for display?
> +	file_read_value(t->path, "direction", "%4s", &gpio->direction);
same as above
>
>   	return 0;
>   }
> @@ -150,11 +152,11 @@ static int dump_gpio_cb(struct tree *t, void *data)
>   	if (gpio->value != -1)
>   		printf(", value:%d", gpio->value);
>
> -	if (gpio->edge != -1)
> -		printf(", edge:%d", gpio->edge);
> +	if (*(gpio->edge) != -1)
Is this check fine? As edge is an array now, earlier it was "int". iiuc 
the first index will be set to -1 by fscanf code.
> +		printf(", edge:%s", gpio->edge);
>
> -	if (gpio->direction != -1)
> -		printf(", direction:%d", gpio->direction);
> +	if (*(gpio->direction) != -1)
same as above
> +		printf(", direction:%s", gpio->direction);
>
>   	printf(" )\n");
>
> @@ -183,7 +185,7 @@ static char *gpio_line(struct tree *t)
>   	struct gpio_info *gpio = t->private;
>   	char *gpioline;
>
> -	if (asprintf(&gpioline, "%-20s %-10d %-10d %-10d %-10d", t-> name,
> +	if (asprintf(&gpioline, "%-20s %-10d %-10d %-10s %-10s", t->name,
>   		     gpio->value, gpio->active_low, gpio->edge, gpio->direction) < 0)
>   		return NULL;
>
>
sunshaojie July 22, 2013, 6:45 a.m. UTC | #2
On 19 July 2013 21:17, Sanjay Singh Rawat <sanjay.rawat@linaro.org> wrote:
> Sunshaojie,
>
> I don't have the gpio configuration functions for panda. Have you test this
> code?

In /sys/class/gpio folder.
you can try "echo 2 > export" when your gpio number begin from 0.
or "echo 254 > export" begin from 255.

Maybe you can see gpio2 or gpio254 folder.

>what is the need of width(8) here? for display?

>> +       file_read_value(t->path, "direction", "%4s", &gpio->direction);

>same as above

trigger_types[] = {
         { "none",    0 },
         { "falling", BIT(FLAG_TRIG_FALL) },
         { "rising",  BIT(FLAG_TRIG_RISE) },
         { "both",    BIT(FLAG_TRIG_FALL) | BIT(FLAG_TRIG_RISE) },
 };

The longest number of edge file is strlen("falling"). So i use 8 bytes
which equals  strlen("falling") +1.

And I alloc 10 bytes for edge and direction. So even gpio driver would
be changed, the array of edge
couldn't overwrite.
And I tested on our platform. It runs well.

>>
>>         return 0;
>>   }
>> @@ -150,11 +152,11 @@ static int dump_gpio_cb(struct tree *t, void *data)
>>         if (gpio->value != -1)
>>                 printf(", value:%d", gpio->value);
>>
>> -       if (gpio->edge != -1)
>> -               printf(", edge:%d", gpio->edge);
>> +       if (*(gpio->edge) != -1)

>Is this check fine? As edge is an array now, earlier it was "int". iiuc the first index will be set to -1 by fscanf code.

>> +               printf(", edge:%s", gpio->edge);
>>
>> -       if (gpio->direction != -1)
>> -               printf(", direction:%d", gpio->direction);
>> +       if (*(gpio->direction) != -1)

>same as above

There are same warning about it, I will change and send a new patch.
diff mbox

Patch

diff --git a/gpio.c b/gpio.c
index f7d2a10..9501e42 100644
--- a/gpio.c
+++ b/gpio.c
@@ -33,12 +33,14 @@ 
 
 #define SYSFS_GPIO "/sys/class/gpio"
 
+#define MAX_VALUE_BYTE	10
+
 struct gpio_info {
 	bool expanded;
 	int active_low;
 	int value;
-	int direction;
-	int edge;
+	char direction[MAX_VALUE_BYTE];
+	char edge[MAX_VALUE_BYTE];
 	char *prefix;
 } *gpios_info;
 
@@ -89,8 +91,8 @@  static inline int read_gpio_cb(struct tree *t, void *data)
 
 	file_read_value(t->path, "active_low", "%d", &gpio->active_low);
 	file_read_value(t->path, "value", "%d", &gpio->value);
-	file_read_value(t->path, "edge", "%d", &gpio->edge);
-	file_read_value(t->path, "direction", "%d", &gpio->direction);
+	file_read_value(t->path, "edge", "%8s", &gpio->edge);
+	file_read_value(t->path, "direction", "%4s", &gpio->direction);
 
 	return 0;
 }
@@ -150,11 +152,11 @@  static int dump_gpio_cb(struct tree *t, void *data)
 	if (gpio->value != -1)
 		printf(", value:%d", gpio->value);
 
-	if (gpio->edge != -1)
-		printf(", edge:%d", gpio->edge);
+	if (*(gpio->edge) != -1)
+		printf(", edge:%s", gpio->edge);
 
-	if (gpio->direction != -1)
-		printf(", direction:%d", gpio->direction);
+	if (*(gpio->direction) != -1)
+		printf(", direction:%s", gpio->direction);
 
 	printf(" )\n");
 
@@ -183,7 +185,7 @@  static char *gpio_line(struct tree *t)
 	struct gpio_info *gpio = t->private;
 	char *gpioline;
 
-	if (asprintf(&gpioline, "%-20s %-10d %-10d %-10d %-10d", t-> name,
+	if (asprintf(&gpioline, "%-20s %-10d %-10d %-10s %-10s", t->name,
 		     gpio->value, gpio->active_low, gpio->edge, gpio->direction) < 0)
 		return NULL;