mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 0/5] commands: gpio: add controller as optional argument
@ 2022-09-05 10:35 Ahmad Fatoum
  2022-09-05 10:35 ` [PATCH 1/5] gpiolib: implement gpio_get_chip_by_dev() Ahmad Fatoum
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2022-09-05 10:35 UTC (permalink / raw)
  To: barebox

gpioinfo currently displays all GPIOs and the rest of the GPIO commands
like gpio_direction_output expects either a GPIO label or a global GPIO
number to act upon.

With this series, the gpio commands now accept an optional argument,
e.g.:

  gpioinfo gpio4
  gpio_direction_output -d gpio4 20 1

Controllers are identified by either device name, full path to the
device tree node backing it or an alias pointing at it.

Ahmad Fatoum (5):
  gpiolib: implement gpio_get_chip_by_dev()
  of: platform: optimize of_find_device_by_node when deep probing
  driver: implement find_device() helper
  commands: gpio: add -d argument to set/get commands
  gpiolib: gpioinfo: add optional CONTROLLER command line argument

 commands/gpio.c        | 51 +++++++++++++++++++++++++++++-------
 drivers/base/driver.c  | 16 ++++++++++++
 drivers/gpio/gpiolib.c | 59 ++++++++++++++++++++++++++++++++----------
 drivers/of/platform.c  |  3 +++
 include/driver.h       |  5 ++++
 include/gpio.h         |  1 +
 6 files changed, 111 insertions(+), 24 deletions(-)

-- 
2.30.2




^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/5] gpiolib: implement gpio_get_chip_by_dev()
  2022-09-05 10:35 [PATCH 0/5] commands: gpio: add controller as optional argument Ahmad Fatoum
@ 2022-09-05 10:35 ` Ahmad Fatoum
  2022-09-05 10:35 ` [PATCH 2/5] of: platform: optimize of_find_device_by_node when deep probing Ahmad Fatoum
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2022-09-05 10:35 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

gpio_of_xlate() already needs the ability to find a struct gpio_chip *
via a struct device_d. Exporting this functionality to elsewhere in the
code will allow restricting commands to manipulate only a single GPIO
bank.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/gpiolib.c | 28 +++++++++++++++++++---------
 include/gpio.h         |  1 +
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 97a99b84e32a..cf61213ca12d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -636,6 +636,18 @@ static int of_gpio_simple_xlate(struct gpio_chip *chip,
 	return chip->base + gpiospec->args[0];
 }
 
+struct gpio_chip *gpio_get_chip_by_dev(struct device_d *dev)
+{
+	struct gpio_chip *chip;
+
+	list_for_each_entry(chip, &chip_list, list) {
+		if (chip->dev == dev)
+			return chip;
+	}
+
+	return NULL;
+}
+
 int gpio_of_xlate(struct device_d *dev, struct of_phandle_args *gpiospec, int *flags)
 {
 	struct gpio_chip *chip;
@@ -643,16 +655,14 @@ int gpio_of_xlate(struct device_d *dev, struct of_phandle_args *gpiospec, int *f
 	if (!dev)
 		return -ENODEV;
 
-	list_for_each_entry(chip, &chip_list, list) {
-		if (chip->dev != dev)
-			continue;
-		if (chip->ops->of_xlate)
-			return chip->ops->of_xlate(chip, gpiospec, flags);
-		else
-			return of_gpio_simple_xlate(chip, gpiospec, flags);
-	}
+	chip = gpio_get_chip_by_dev(dev);
+	if (!chip)
+		return -EPROBE_DEFER;
 
-	return -EPROBE_DEFER;
+	if (chip->ops->of_xlate)
+		return chip->ops->of_xlate(chip, gpiospec, flags);
+	else
+		return of_gpio_simple_xlate(chip, gpiospec, flags);
 }
 
 struct gpio_chip *gpio_get_chip(int gpio)
diff --git a/include/gpio.h b/include/gpio.h
index 8218722dcbf0..82f2eb6b8a34 100644
--- a/include/gpio.h
+++ b/include/gpio.h
@@ -190,5 +190,6 @@ void gpiochip_remove(struct gpio_chip *chip);
 
 int gpio_of_xlate(struct device_d *dev, struct of_phandle_args *gpiospec, int *flags);
 struct gpio_chip *gpio_get_chip(int gpio);
+struct gpio_chip *gpio_get_chip_by_dev(struct device_d *);
 
 #endif /* __GPIO_H */
-- 
2.30.2




^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/5] of: platform: optimize of_find_device_by_node when deep probing
  2022-09-05 10:35 [PATCH 0/5] commands: gpio: add controller as optional argument Ahmad Fatoum
  2022-09-05 10:35 ` [PATCH 1/5] gpiolib: implement gpio_get_chip_by_dev() Ahmad Fatoum
@ 2022-09-05 10:35 ` Ahmad Fatoum
  2022-09-05 10:35 ` [PATCH 3/5] driver: implement find_device() helper Ahmad Fatoum
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2022-09-05 10:35 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

With deep probe enabled, we are guaranteed that struct device_node::dev
is populated whenever a device had been created for a node, so in that
case, skip the iteration over the linked list of registered devices.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/of/platform.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 7f377b8b378a..dc784ea8e550 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -28,6 +28,9 @@ struct device_d *of_find_device_by_node(struct device_node *np)
 	if (ret)
 		return NULL;
 
+	if (deep_probe_is_supported())
+		return np->dev;
+
 	for_each_device(dev)
 		if (dev->device_node == np)
 			return dev;
-- 
2.30.2




^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/5] driver: implement find_device() helper
  2022-09-05 10:35 [PATCH 0/5] commands: gpio: add controller as optional argument Ahmad Fatoum
  2022-09-05 10:35 ` [PATCH 1/5] gpiolib: implement gpio_get_chip_by_dev() Ahmad Fatoum
  2022-09-05 10:35 ` [PATCH 2/5] of: platform: optimize of_find_device_by_node when deep probing Ahmad Fatoum
@ 2022-09-05 10:35 ` Ahmad Fatoum
  2022-09-05 10:35 ` [PATCH 4/5] commands: gpio: add -d argument to set/get commands Ahmad Fatoum
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2022-09-05 10:35 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

For use in commands like done in the follow-up commits, it can be useful
to have a best effort find_device, that not only finds by device name,
but also by device tree path or alias. Add such a helper.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/base/driver.c | 16 ++++++++++++++++
 include/driver.h      |  5 +++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index 072870bea444..cbe1c974f4fe 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -46,6 +46,22 @@ LIST_HEAD(active_device_list);
 EXPORT_SYMBOL(active_device_list);
 static LIST_HEAD(deferred);
 
+struct device_d *find_device(const char *str)
+{
+	struct device_d *dev;
+	struct device_node *np;
+
+	dev = get_device_by_name(str);
+	if (dev)
+		return dev;
+
+	np = of_find_node_by_path_or_alias(NULL, str);
+	if (np)
+		return of_find_device_by_node(np);
+
+	return NULL;
+}
+
 struct device_d *get_device_by_name(const char *name)
 {
 	struct device_d *dev;
diff --git a/include/driver.h b/include/driver.h
index 67ffbba6be80..543287a27638 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -169,6 +169,11 @@ struct device_d *get_device_by_type(ulong type, struct device_d *last);
 struct device_d *get_device_by_id(const char *id);
 struct device_d *get_device_by_name(const char *name);
 
+/* Find a device by name and if not found look up by device tree path
+ * or alias
+ */
+struct device_d *find_device(const char *str);
+
 /* Find a free device id from the given template. This is archieved by
  * appending a number to the template. Dynamically created devices should
  * use this function rather than filling the id field themselves.
-- 
2.30.2




^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 4/5] commands: gpio: add -d argument to set/get commands
  2022-09-05 10:35 [PATCH 0/5] commands: gpio: add controller as optional argument Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2022-09-05 10:35 ` [PATCH 3/5] driver: implement find_device() helper Ahmad Fatoum
@ 2022-09-05 10:35 ` Ahmad Fatoum
  2022-09-28 13:37   ` Enrico Scholz
  2022-09-05 10:35 ` [PATCH 5/5] gpiolib: gpioinfo: add optional CONTROLLER command line argument Ahmad Fatoum
  2022-09-12  9:13 ` [PATCH 0/5] commands: gpio: add controller as optional argument Sascha Hauer
  5 siblings, 1 reply; 12+ messages in thread
From: Ahmad Fatoum @ 2022-09-05 10:35 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

For debugging, it can be useful to reference GPIOs relative to a
controller, e.g.:

  gpio_direction_output -d gpio4 20 1

instead of calculating the global gpio number. Extend the GPIO
functions to support that. The argument to -d may be either a device
name, a full path to a device tree node or an alias as in the example
above.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 commands/gpio.c | 51 +++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 10 deletions(-)

diff --git a/commands/gpio.c b/commands/gpio.c
index 955b60e91bce..d04fd65bc8e0 100644
--- a/commands/gpio.c
+++ b/commands/gpio.c
@@ -4,27 +4,58 @@
 #include <command.h>
 #include <errno.h>
 #include <gpio.h>
+#include <getopt.h>
 
 static int get_gpio_and_value(int argc, char *argv[],
 			      int *gpio, int *value)
 {
-	const int count = value ? 3 : 2;
+	struct gpio_chip *chip = NULL;
+	struct device_d *dev;
+	int count = 2;
 	int ret = 0;
+	int opt;
+
+	while ((opt = getopt(argc, argv, "d:")) > 0) {
+		switch (opt) {
+		case 'd':
+			dev = find_device(optarg);
+			if (!dev)
+				return -ENODEV;
+
+			chip = gpio_get_chip_by_dev(dev);
+			if (!chip)
+				return -EINVAL;
+			break;
+		default:
+			return COMMAND_ERROR_USAGE;
+		}
+	}
+
+	if (value)
+		count++;
 
-	if (argc < count)
+	if (optind < count)
 		return COMMAND_ERROR_USAGE;
 
-	*gpio = gpio_find_by_name(argv[1]);
+	*gpio = gpio_find_by_name(argv[optind]);
 	if (*gpio < 0)
-		*gpio = gpio_find_by_label(argv[1]);
+		*gpio = gpio_find_by_label(argv[optind]);
 	if (*gpio < 0) {
-		ret = kstrtoint(argv[1], 0, gpio);
+		ret = kstrtoint(argv[optind], 0, gpio);
 		if (ret < 0)
 			return ret;
+
+		if (chip)
+			*gpio += chip->base;
+	} else if (chip) {
+		if (gpio_get_chip(*gpio) != chip) {
+			printf("%s: not exporting pin %u\n", dev_name(chip->dev), *gpio);
+			return -EINVAL;
+		}
 	}
 
 	if (value)
-		ret = kstrtoint(argv[2], 0, value);
+		ret = kstrtoint(argv[optind + 1], 0, value);
 
 	return ret;
 }
@@ -47,7 +78,7 @@ static int do_gpio_get_value(int argc, char *argv[])
 BAREBOX_CMD_START(gpio_get_value)
 	.cmd		= do_gpio_get_value,
 	BAREBOX_CMD_DESC("return value of a GPIO pin")
-	BAREBOX_CMD_OPTS("GPIO")
+	BAREBOX_CMD_OPTS("[-d CONTROLLER] GPIO")
 	BAREBOX_CMD_GROUP(CMD_GRP_HWMANIP)
 BAREBOX_CMD_END
 
@@ -67,7 +98,7 @@ static int do_gpio_set_value(int argc, char *argv[])
 BAREBOX_CMD_START(gpio_set_value)
 	.cmd		= do_gpio_set_value,
 	BAREBOX_CMD_DESC("set a GPIO's output value")
-	BAREBOX_CMD_OPTS("GPIO VALUE")
+	BAREBOX_CMD_OPTS("[-d CONTROLLER] GPIO VALUE")
 	BAREBOX_CMD_GROUP(CMD_GRP_HWMANIP)
 BAREBOX_CMD_END
 
@@ -89,7 +120,7 @@ static int do_gpio_direction_input(int argc, char *argv[])
 BAREBOX_CMD_START(gpio_direction_input)
 	.cmd		= do_gpio_direction_input,
 	BAREBOX_CMD_DESC("set direction of a GPIO pin to input")
-	BAREBOX_CMD_OPTS("GPIO")
+	BAREBOX_CMD_OPTS("[-d CONTROLLER] GPIO")
 	BAREBOX_CMD_GROUP(CMD_GRP_HWMANIP)
 BAREBOX_CMD_END
 
@@ -111,6 +142,6 @@ static int do_gpio_direction_output(int argc, char *argv[])
 BAREBOX_CMD_START(gpio_direction_output)
 	.cmd		= do_gpio_direction_output,
 	BAREBOX_CMD_DESC("set direction of a GPIO pin to output")
-	BAREBOX_CMD_OPTS("GPIO VALUE")
+	BAREBOX_CMD_OPTS("[-d CONTROLLER] GPIO VALUE")
 	BAREBOX_CMD_GROUP(CMD_GRP_HWMANIP)
 BAREBOX_CMD_END
-- 
2.30.2




^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 5/5] gpiolib: gpioinfo: add optional CONTROLLER command line argument
  2022-09-05 10:35 [PATCH 0/5] commands: gpio: add controller as optional argument Ahmad Fatoum
                   ` (3 preceding siblings ...)
  2022-09-05 10:35 ` [PATCH 4/5] commands: gpio: add -d argument to set/get commands Ahmad Fatoum
@ 2022-09-05 10:35 ` Ahmad Fatoum
  2022-09-28 13:22   ` Enrico Scholz
  2022-09-12  9:13 ` [PATCH 0/5] commands: gpio: add controller as optional argument Sascha Hauer
  5 siblings, 1 reply; 12+ messages in thread
From: Ahmad Fatoum @ 2022-09-05 10:35 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Like done with gpio setter/getter functions in the previous commit,
extend the gpioinfo command to accept an optional argument that
restricts output to the supplied gpio controller instead of printing
all GPIOs at once.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/gpiolib.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index cf61213ca12d..7f2070903501 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -675,15 +675,35 @@ struct gpio_chip *gpio_get_chip(int gpio)
 #ifdef CONFIG_CMD_GPIO
 static int do_gpiolib(int argc, char *argv[])
 {
+	struct gpio_chip *chip = NULL;
 	int i;
 
+	if (argc > 2)
+		return COMMAND_ERROR_USAGE;
+
+	if (argc == 1) {
+		struct device_d *dev;
+
+		dev = find_device(argv[1]);
+		if (!dev)
+			return -ENODEV;
+
+		chip = gpio_get_chip_by_dev(dev);
+		if (!chip)
+			return -EINVAL;
+	}
+
 	for (i = 0; i < ARCH_NR_GPIOS; i++) {
 		struct gpio_info *gi = &gpio_desc[i];
 		int val = -1, dir = -1;
+		int idx;
 
 		if (!gi->chip)
 			continue;
 
+		if (chip && chip != gi->chip)
+			continue;
+
 		/* print chip information and header on first gpio */
 		if (gi->chip->base == i) {
 			printf("\nGPIOs %u-%u, chip %s:\n",
@@ -693,14 +713,14 @@ static int do_gpiolib(int argc, char *argv[])
 			printf("             %-3s %-3s %-9s %-20s %-20s\n", "dir", "val", "requested", "name", "label");
 		}
 
+		idx = i - gi->chip->base;
+
 		if (gi->chip->ops->get_direction)
-			dir = gi->chip->ops->get_direction(gi->chip,
-						i - gi->chip->base);
+			dir = gi->chip->ops->get_direction(gi->chip, idx);
 		if (gi->chip->ops->get)
-			val = gi->chip->ops->get(gi->chip,
-						i - gi->chip->base);
+			val = gi->chip->ops->get(gi->chip, idx);
 
-		printf("  GPIO %4d: %-3s %-3s %-9s %-20s %-20s\n", i,
+		printf("  GPIO %4d: %-3s %-3s %-9s %-20s %-20s\n", chip ? idx : i,
 			(dir < 0) ? "unk" : ((dir == GPIOF_DIR_IN) ? "in" : "out"),
 			(val < 0) ? "unk" : ((val == 0) ? "lo" : "hi"),
 		        gi->requested ? (gi->active_low ? "active low" : "true") : "false",
@@ -714,6 +734,7 @@ static int do_gpiolib(int argc, char *argv[])
 BAREBOX_CMD_START(gpioinfo)
 	.cmd		= do_gpiolib,
 	BAREBOX_CMD_DESC("list registered GPIOs")
+	BAREBOX_CMD_OPTS("[CONTROLLER]")
 	BAREBOX_CMD_GROUP(CMD_GRP_INFO)
 	BAREBOX_CMD_COMPLETE(empty_complete)
 BAREBOX_CMD_END
-- 
2.30.2




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/5] commands: gpio: add controller as optional argument
  2022-09-05 10:35 [PATCH 0/5] commands: gpio: add controller as optional argument Ahmad Fatoum
                   ` (4 preceding siblings ...)
  2022-09-05 10:35 ` [PATCH 5/5] gpiolib: gpioinfo: add optional CONTROLLER command line argument Ahmad Fatoum
@ 2022-09-12  9:13 ` Sascha Hauer
  5 siblings, 0 replies; 12+ messages in thread
From: Sascha Hauer @ 2022-09-12  9:13 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Mon, Sep 05, 2022 at 12:35:41PM +0200, Ahmad Fatoum wrote:
> gpioinfo currently displays all GPIOs and the rest of the GPIO commands
> like gpio_direction_output expects either a GPIO label or a global GPIO
> number to act upon.
> 
> With this series, the gpio commands now accept an optional argument,
> e.g.:
> 
>   gpioinfo gpio4
>   gpio_direction_output -d gpio4 20 1
> 
> Controllers are identified by either device name, full path to the
> device tree node backing it or an alias pointing at it.
> 
> Ahmad Fatoum (5):
>   gpiolib: implement gpio_get_chip_by_dev()
>   of: platform: optimize of_find_device_by_node when deep probing
>   driver: implement find_device() helper
>   commands: gpio: add -d argument to set/get commands
>   gpiolib: gpioinfo: add optional CONTROLLER command line argument

Applied, thanks

Sascha

> 
>  commands/gpio.c        | 51 +++++++++++++++++++++++++++++-------
>  drivers/base/driver.c  | 16 ++++++++++++
>  drivers/gpio/gpiolib.c | 59 ++++++++++++++++++++++++++++++++----------
>  drivers/of/platform.c  |  3 +++
>  include/driver.h       |  5 ++++
>  include/gpio.h         |  1 +
>  6 files changed, 111 insertions(+), 24 deletions(-)
> 
> -- 
> 2.30.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] gpiolib: gpioinfo: add optional CONTROLLER command line argument
  2022-09-05 10:35 ` [PATCH 5/5] gpiolib: gpioinfo: add optional CONTROLLER command line argument Ahmad Fatoum
@ 2022-09-28 13:22   ` Enrico Scholz
  2022-09-28 14:24     ` Sascha Hauer
  0 siblings, 1 reply; 12+ messages in thread
From: Enrico Scholz @ 2022-09-28 13:22 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

Ahmad Fatoum <a.fatoum@pengutronix.de> writes:

>  static int do_gpiolib(int argc, char *argv[])
>  {
> +	if (argc == 1) {
> +		dev = find_device(argv[1]);

'gpioinfo' without arguments will crash because this calls
'find_device(NULL)'

| :/ gpioinfo 
| BUG: failure at lib/string.c:226/strcmp()!
| BUG!



Enrico



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/5] commands: gpio: add -d argument to set/get commands
  2022-09-05 10:35 ` [PATCH 4/5] commands: gpio: add -d argument to set/get commands Ahmad Fatoum
@ 2022-09-28 13:37   ` Enrico Scholz
  2022-09-28 14:36     ` Sascha Hauer
  2022-09-29  9:50     ` Ahmad Fatoum
  0 siblings, 2 replies; 12+ messages in thread
From: Enrico Scholz @ 2022-09-28 13:37 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

Ahmad Fatoum <a.fatoum@pengutronix.de> writes:

> For debugging, it can be useful to reference GPIOs relative to a
> controller, e.g.:
>
>   gpio_direction_output -d gpio4 20 1
>
> -	if (argc < count)
> +	if (optind < count)
>  		return COMMAND_ERROR_USAGE;

This change seems to make '-d ...' mandatory.  E.g.

| :/ gpio_get_value 65
| 
| gpio_get_value - return value of a GPIO pin
| 
| Usage: gpio_get_value [-d CONTROLLER] GPIO


Enrico



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] gpiolib: gpioinfo: add optional CONTROLLER command line argument
  2022-09-28 13:22   ` Enrico Scholz
@ 2022-09-28 14:24     ` Sascha Hauer
  0 siblings, 0 replies; 12+ messages in thread
From: Sascha Hauer @ 2022-09-28 14:24 UTC (permalink / raw)
  To: Enrico Scholz; +Cc: Ahmad Fatoum, barebox

On Wed, Sep 28, 2022 at 03:22:46PM +0200, Enrico Scholz wrote:
> Ahmad Fatoum <a.fatoum@pengutronix.de> writes:
> 
> >  static int do_gpiolib(int argc, char *argv[])
> >  {
> > +	if (argc == 1) {
> > +		dev = find_device(argv[1]);
> 
> 'gpioinfo' without arguments will crash because this calls
> 'find_device(NULL)'
> 
> | :/ gpioinfo 
> | BUG: failure at lib/string.c:226/strcmp()!
> | BUG!

I fixed it with the obvious change

Sascha

----------------------------8<--------------------------

>From 252c76cdf8c9d516589d72ac228ac0d19fcb2021 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Wed, 28 Sep 2022 16:20:48 +0200
Subject: [PATCH] gpiolib: Fix gpioinfo without args

Since a30ae2921a the gpioinfo command crashes when called without
arguments. Fix it by using argv[1] when it actually exists, not when
it doesn't exist.

Fixes: a30ae2921a ("gpiolib: gpioinfo: add optional CONTROLLER command line argument")
Reported-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/gpio/gpiolib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 7f20709035..2503262d65 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -681,7 +681,7 @@ static int do_gpiolib(int argc, char *argv[])
 	if (argc > 2)
 		return COMMAND_ERROR_USAGE;
 
-	if (argc == 1) {
+	if (argc > 1) {
 		struct device_d *dev;
 
 		dev = find_device(argv[1]);
-- 
2.30.2

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/5] commands: gpio: add -d argument to set/get commands
  2022-09-28 13:37   ` Enrico Scholz
@ 2022-09-28 14:36     ` Sascha Hauer
  2022-09-29  9:50     ` Ahmad Fatoum
  1 sibling, 0 replies; 12+ messages in thread
From: Sascha Hauer @ 2022-09-28 14:36 UTC (permalink / raw)
  To: Enrico Scholz; +Cc: Ahmad Fatoum, barebox

Hi Enrico,

On Wed, Sep 28, 2022 at 03:37:30PM +0200, Enrico Scholz wrote:
> Ahmad Fatoum <a.fatoum@pengutronix.de> writes:
> 
> > For debugging, it can be useful to reference GPIOs relative to a
> > controller, e.g.:
> >
> >   gpio_direction_output -d gpio4 20 1
> >
> > -	if (argc < count)
> > +	if (optind < count)
> >  		return COMMAND_ERROR_USAGE;
> 
> This change seems to make '-d ...' mandatory.  E.g.
> 
> | :/ gpio_get_value 65
> | 
> | gpio_get_value - return value of a GPIO pin
> | 
> | Usage: gpio_get_value [-d CONTROLLER] GPIO
> 

Thanks for reporting. Here is a fix for that

Sascha

-----------------------------8<---------------------------

>From 7c797eb87385522ce1ec1ba44315ba386d82978d Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Wed, 28 Sep 2022 16:33:21 +0200
Subject: [PATCH] gpio: Fix gpio commands called without -d option

04443dc5fc breaks the calculation of arguments needed which effectively
makes the -d option mandatory. Fix this.

Fixes: 04443dc5fc ("commands: gpio: add -d argument to set/get commands")
Reported-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/gpio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/commands/gpio.c b/commands/gpio.c
index d04fd65bc8..5e5eb20583 100644
--- a/commands/gpio.c
+++ b/commands/gpio.c
@@ -11,7 +11,7 @@ static int get_gpio_and_value(int argc, char *argv[],
 {
 	struct gpio_chip *chip = NULL;
 	struct device_d *dev;
-	int count = 2;
+	int count = 1;
 	int ret = 0;
 	int opt;
 
@@ -34,7 +34,7 @@ static int get_gpio_and_value(int argc, char *argv[],
 	if (value)
 		count++;
 
-	if (optind < count)
+	if (argc < optind + count)
 		return COMMAND_ERROR_USAGE;
 
 	*gpio = gpio_find_by_name(argv[optind]);
-- 
2.30.2

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/5] commands: gpio: add -d argument to set/get commands
  2022-09-28 13:37   ` Enrico Scholz
  2022-09-28 14:36     ` Sascha Hauer
@ 2022-09-29  9:50     ` Ahmad Fatoum
  1 sibling, 0 replies; 12+ messages in thread
From: Ahmad Fatoum @ 2022-09-29  9:50 UTC (permalink / raw)
  To: Enrico Scholz; +Cc: barebox

On 28.09.22 14:37, Enrico Scholz wrote:
> Ahmad Fatoum <a.fatoum@pengutronix.de> writes:
> 
>> For debugging, it can be useful to reference GPIOs relative to a
>> controller, e.g.:
>>
>>   gpio_direction_output -d gpio4 20 1
>>
>> -	if (argc < count)
>> +	if (optind < count)
>>  		return COMMAND_ERROR_USAGE;
> 
> This change seems to make '-d ...' mandatory.  E.g.
> 
> | :/ gpio_get_value 65
> | 
> | gpio_get_value - return value of a GPIO pin
> | 
> | Usage: gpio_get_value [-d CONTROLLER] GPIO

Seems I had the blinders on when I drafted this patch
series. Thanks for reporting and thanks Sascha for
the fixes.


> 
> 
> Enrico
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-09-29  9:52 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-05 10:35 [PATCH 0/5] commands: gpio: add controller as optional argument Ahmad Fatoum
2022-09-05 10:35 ` [PATCH 1/5] gpiolib: implement gpio_get_chip_by_dev() Ahmad Fatoum
2022-09-05 10:35 ` [PATCH 2/5] of: platform: optimize of_find_device_by_node when deep probing Ahmad Fatoum
2022-09-05 10:35 ` [PATCH 3/5] driver: implement find_device() helper Ahmad Fatoum
2022-09-05 10:35 ` [PATCH 4/5] commands: gpio: add -d argument to set/get commands Ahmad Fatoum
2022-09-28 13:37   ` Enrico Scholz
2022-09-28 14:36     ` Sascha Hauer
2022-09-29  9:50     ` Ahmad Fatoum
2022-09-05 10:35 ` [PATCH 5/5] gpiolib: gpioinfo: add optional CONTROLLER command line argument Ahmad Fatoum
2022-09-28 13:22   ` Enrico Scholz
2022-09-28 14:24     ` Sascha Hauer
2022-09-12  9:13 ` [PATCH 0/5] commands: gpio: add controller as optional argument Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox