[PATCH 0/4] gpiolib debug and led trigger fixes

[PATCH 0/4] gpiolib debug and led trigger fixes

[Sebastian Hesselbarth]

This small set of patches first fixes led triggers, where the led
should be disabled, before installing a trigger function. Otherwise,
the led may stay enabled from previous bootloaders messing with it.

The second part of the patches impoves gpiolib command verbosity by
first adding an (optional) callback to get_direction of a gpiochip
pin. Based on that new callback, it adds some more printf information
to reflect gpiochip/gpio pin relation and also print current pin
direction and value, if the corresponding callbacks are available.
Finally, the get_direction callback is added to the Synopsys DW gpio
driver.

Sebastian Hesselbarth (4):
  led: trigger: disable LEDs with trigger before installing it
  gpiolib: add get_direction callback
  gpiolib: make gpiolib command more verbose
  gpio: dw: add get_direction callback

 drivers/gpio/gpio-dw.c |  9 +++++++++
 drivers/gpio/gpiolib.c | 24 +++++++++++++++++++++---
 drivers/led/led-gpio.c |  4 +++-
 include/gpio.h         |  4 ++++
 4 files changed, 37 insertions(+), 4 deletions(-)

---
Cc: barebox@lists.infradead.org
-- 
1.8.4.rc3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/4] led: trigger: disable LEDs with trigger before installing it

[Sebastian Hesselbarth]

This disables LEDs that have a trigger function assigned right before
the trigger is installed. As the trigger was parsed before the LED has
been registered, also swap LED registration and trigger parsing.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: barebox@lists.infradead.org
---
 drivers/led/led-gpio.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/led/led-gpio.c b/drivers/led/led-gpio.c
index 69db70f..7a5ef47 100644
--- a/drivers/led/led-gpio.c
+++ b/drivers/led/led-gpio.c
@@ -225,6 +225,8 @@ static void led_of_parse_trigger(struct led *led, struct device_node *np)
 	for (i = 0; i < ARRAY_SIZE(triggers); i++) {
 		struct led_trg *trg = &triggers[i];
 		if (!strcmp(trg->str, trigger)) {
+			/* disable LED before installing trigger */
+			led_set(led, 0);
 			led_set_trigger(trg->trg, led);
 			return;
 		}
@@ -252,8 +254,8 @@ static int led_gpio_of_probe(struct device_d *dev)
 		dev_dbg(dev, "register led %s on gpio%d, active_low = %d\n",
 			gled->led.name, gled->gpio, gled->active_low);
 
-		led_of_parse_trigger(&gled->led, child);
 		led_gpio_register(gled);
+		led_of_parse_trigger(&gled->led, child);
 	}
 
 	return 0;
-- 
1.8.4.rc3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/4] gpiolib: add get_direction callback

[Sebastian Hesselbarth]

At least for debugging purposes it is helpful to determine the current
direction for a given GPIO. Add a callback to gpiochip, to allow to
get it.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: barebox@lists.infradead.org
---
 include/gpio.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/gpio.h b/include/gpio.h
index 140d53c..708b2aa 100644
--- a/include/gpio.h
+++ b/include/gpio.h
@@ -3,6 +3,9 @@
 
 #include <asm/gpio.h>
 
+#define GPIO_DIR_OUT	(0 << 0)
+#define GPIO_DIR_IN	(1 << 0)
+
 #ifndef CONFIG_GPIOLIB
 static inline int gpio_request(unsigned gpio, const char *label)
 {
@@ -24,6 +27,7 @@ struct gpio_ops {
 	void (*free)(struct gpio_chip *chip, unsigned offset);
 	int (*direction_input)(struct gpio_chip *chip, unsigned offset);
 	int (*direction_output)(struct gpio_chip *chip, unsigned offset, int value);
+	int (*get_direction)(struct gpio_chip *chip, unsigned offset);
 	int (*get)(struct gpio_chip *chip, unsigned offset);
 	void (*set)(struct gpio_chip *chip, unsigned offset, int value);
 };
-- 
1.8.4.rc3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/4] gpiolib: make gpiolib command more verbose

[Sebastian Hesselbarth]

This adds some more printf information to gpiolib command, like the
gpiochip handling a specific gpio. Also, current direction and value
of the gpio are printed, if the gpiochip provides the corresponding
callbacks.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: barebox@lists.infradead.org
---
 drivers/gpio/gpiolib.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index ca6e8ad..c12ebe6 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -228,16 +228,34 @@ static int do_gpiolib(int argc, char *argv[])
 	int i;
 
 	printf("gpiolib: gpio lists\n");
-	printf("%*crequested  label\n", 11, ' ');
 
 	for (i = 0; i < ARCH_NR_GPIOS; i++) {
 		struct gpio_info *gi = &gpio_desc[i];
+		int val = -1, dir = -1;
 
 		if (!gi->chip)
 			continue;
 
-		printf("gpio %*d: %*s  %s\n", 4,
-			i, 9, gi->requested ? "true" : "false",
+		/* print chip information and header on first gpio */
+		if (gi->chip->base == i) {
+			printf("\ngpios %u-%u, chip %s:\n",
+				gi->chip->base,
+				gi->chip->base + gi->chip->ngpio,
+				gi->chip->dev->name);
+			printf("%*cdir val requested  label\n", 13, ' ');
+		}
+
+		if (gi->chip->ops->get_direction)
+			dir = gi->chip->ops->get_direction(gi->chip,
+						i - gi->chip->base);
+		if (gi->chip->ops->get)
+			val = gi->chip->ops->get(gi->chip,
+						i - gi->chip->base);
+
+		printf("  gpio %*d: %*s %*s %*s  %s\n", 4, i,
+			3, (dir < 0) ? "unk" : ((dir == GPIO_DIR_IN) ? "in" : "out"),
+			3, (val < 0) ? "unk" : ((val == 0) ? "lo" : "hi"),
+			9, gi->requested ? "true" : "false",
 			gi->label ? gi->label : "");
 	}
 
-- 
1.8.4.rc3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/4] gpio: dw: add get_direction callback

[Sebastian Hesselbarth]

This adds a callback function to read the current state of a GPIOs
in/out direction.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: barebox@lists.infradead.org
---
 drivers/gpio/gpio-dw.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpio/gpio-dw.c b/drivers/gpio/gpio-dw.c
index 791488a..6577042 100644
--- a/drivers/gpio/gpio-dw.c
+++ b/drivers/gpio/gpio-dw.c
@@ -90,9 +90,18 @@ static int dw_gpio_direction_output(struct gpio_chip *gc,
 	return 0;
 }
 
+static int dw_gpio_get_direction(struct gpio_chip *gc, unsigned offset)
+{
+	struct dw_gpio_instance *chip = to_dw_gpio(gc);
+
+	return (readl(chip->regs + DW_GPIO_DDR) & (1 << offset)) ?
+		GPIO_DIR_OUT : GPIO_DIR_IN;
+}
+
 static struct gpio_ops imx_gpio_ops = {
 	.direction_input = dw_gpio_direction_input,
 	.direction_output = dw_gpio_direction_output,
+	.get_direction = dw_gpio_get_direction,
 	.get = dw_gpio_get,
 	.set = dw_gpio_set,
 };
-- 
1.8.4.rc3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 3/4] gpiolib: make gpiolib command more verbose

[Alexander Aring]

Hi Sebastian,

On Sat, Nov 09, 2013 at 02:24:08PM +0100, Sebastian Hesselbarth wrote:
> This adds some more printf information to gpiolib command, like the
> gpiochip handling a specific gpio. Also, current direction and value
> of the gpio are printed, if the gpiochip provides the corresponding
> callbacks.
> 
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> ---
> Cc: barebox@lists.infradead.org
> ---
>  drivers/gpio/gpiolib.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index ca6e8ad..c12ebe6 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -228,16 +228,34 @@ static int do_gpiolib(int argc, char *argv[])
>  	int i;
>  
>  	printf("gpiolib: gpio lists\n");
> -	printf("%*crequested  label\n", 11, ' ');
>  
>  	for (i = 0; i < ARCH_NR_GPIOS; i++) {
>  		struct gpio_info *gi = &gpio_desc[i];
> +		int val = -1, dir = -1;
>  
>  		if (!gi->chip)
>  			continue;
>  
> -		printf("gpio %*d: %*s  %s\n", 4,
> -			i, 9, gi->requested ? "true" : "false",
> +		/* print chip information and header on first gpio */
> +		if (gi->chip->base == i) {
> +			printf("\ngpios %u-%u, chip %s:\n",
> +				gi->chip->base,
> +				gi->chip->base + gi->chip->ngpio,

I think this should be "gi->chip->base + gi->chip->ngpio - 1", because
we starting at zero on base.

- Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 3/4] gpiolib: make gpiolib command more verbose

[Sascha Hauer]

On Mon, Nov 11, 2013 at 02:30:20AM +0100, Alexander Aring wrote:
> Hi Sebastian,
> 
> On Sat, Nov 09, 2013 at 02:24:08PM +0100, Sebastian Hesselbarth wrote:
> > This adds some more printf information to gpiolib command, like the
> > gpiochip handling a specific gpio. Also, current direction and value
> > of the gpio are printed, if the gpiochip provides the corresponding
> > callbacks.
> > 
> > Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> > ---
> > Cc: barebox@lists.infradead.org
> > ---
> >  drivers/gpio/gpiolib.c | 24 +++++++++++++++++++++---
> >  1 file changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index ca6e8ad..c12ebe6 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -228,16 +228,34 @@ static int do_gpiolib(int argc, char *argv[])
> >  	int i;
> >  
> >  	printf("gpiolib: gpio lists\n");
> > -	printf("%*crequested  label\n", 11, ' ');
> >  
> >  	for (i = 0; i < ARCH_NR_GPIOS; i++) {
> >  		struct gpio_info *gi = &gpio_desc[i];
> > +		int val = -1, dir = -1;
> >  
> >  		if (!gi->chip)
> >  			continue;
> >  
> > -		printf("gpio %*d: %*s  %s\n", 4,
> > -			i, 9, gi->requested ? "true" : "false",
> > +		/* print chip information and header on first gpio */
> > +		if (gi->chip->base == i) {
> > +			printf("\ngpios %u-%u, chip %s:\n",
> > +				gi->chip->base,
> > +				gi->chip->base + gi->chip->ngpio,
> 
> I think this should be "gi->chip->base + gi->chip->ngpio - 1", because
> we starting at zero on base.

Indeed. Otherwise I get the following for my 32bit gpio bank:

gpios 0-31, chip 209c000.gpio:
             dir val requested  label
  gpio    0: unk  lo     false  
  gpio    1: unk  lo     false  
...

Fixed this while applying.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 3/4] gpiolib: make gpiolib command more verbose

[Alexander Aring]

On Mon, Nov 11, 2013 at 09:07:27AM +0100, Sascha Hauer wrote:
> On Mon, Nov 11, 2013 at 02:30:20AM +0100, Alexander Aring wrote:
> > Hi Sebastian,
> > 
> > On Sat, Nov 09, 2013 at 02:24:08PM +0100, Sebastian Hesselbarth wrote:
> > > This adds some more printf information to gpiolib command, like the
> > > gpiochip handling a specific gpio. Also, current direction and value
> > > of the gpio are printed, if the gpiochip provides the corresponding
> > > callbacks.
> > > 
> > > Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> > > ---
> > > Cc: barebox@lists.infradead.org
> > > ---
> > >  drivers/gpio/gpiolib.c | 24 +++++++++++++++++++++---
> > >  1 file changed, 21 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > > index ca6e8ad..c12ebe6 100644
> > > --- a/drivers/gpio/gpiolib.c
> > > +++ b/drivers/gpio/gpiolib.c
> > > @@ -228,16 +228,34 @@ static int do_gpiolib(int argc, char *argv[])
> > >  	int i;
> > >  
> > >  	printf("gpiolib: gpio lists\n");
> > > -	printf("%*crequested  label\n", 11, ' ');
> > >  
> > >  	for (i = 0; i < ARCH_NR_GPIOS; i++) {
> > >  		struct gpio_info *gi = &gpio_desc[i];
> > > +		int val = -1, dir = -1;
> > >  
> > >  		if (!gi->chip)
> > >  			continue;
> > >  
> > > -		printf("gpio %*d: %*s  %s\n", 4,
> > > -			i, 9, gi->requested ? "true" : "false",
> > > +		/* print chip information and header on first gpio */
> > > +		if (gi->chip->base == i) {
> > > +			printf("\ngpios %u-%u, chip %s:\n",
> > > +				gi->chip->base,
> > > +				gi->chip->base + gi->chip->ngpio,
> > 
> > I think this should be "gi->chip->base + gi->chip->ngpio - 1", because
> > we starting at zero on base.
> 
> Indeed. Otherwise I get the following for my 32bit gpio bank:
> 
> gpios 0-31, chip 209c000.gpio:
>              dir val requested  label
>   gpio    0: unk  lo     false  
>   gpio    1: unk  lo     false  
> ...
> 
> Fixed this while applying.

detected by starring, watching and thinking. Yea and maybe I used the
force. :-)

Thanks Sascha.

- Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox