LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v2 0/4] Let leds use named gpios
@ 2015-01-21 21:33 Olliver Schinagl
  2015-01-21 21:33 ` [PATCH v2 1/4] gpio: use sizeof() instead of hardcoded values Olliver Schinagl
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Olliver Schinagl @ 2015-01-21 21:33 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu,
	Richard Purdie, Olliver Schinagl, Robin Gong, Mika Westerberg,
	Aaron Lu, Grant Likely, Jingoo Han, Alexander Shiyan,
	Wolfram Sang
  Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-leds,
	Olliver Schinagl

The gpio binding docs ask to use named-gpios wherever possible, however 40b73183 probably forgot that. This patch makes the new devm_get_gpiod_from_child use named gpios.

Changed since v1:
  Fix a few typo's
  Fix of_find_gpio to remove hardcoded length of string
  Check both for leds-gpios and if that fails, fall back to standard gpios for the users of devm_get_gpiod_from_child.

Olliver Schinagl (4):
  gpio: use sizeof() instead of hardcoded values
  gpio: add parameter to allow the use named gpios
  leds: Let the binding document example for leds-gpio follow the gpio
    bindings
  leds: no longer use unnamed gpios

 Documentation/devicetree/bindings/leds/leds-gpio.txt | 14 ++++++++------
 drivers/gpio/devres.c                                | 18 +++++++++++++++++-
 drivers/gpio/gpiolib.c                               |  6 ++++--
 drivers/input/keyboard/gpio_keys_polled.c            | 20 ++++++++++++--------
 drivers/leds/leds-gpio.c                             |  2 +-
 include/linux/gpio/consumer.h                        |  1 +
 6 files changed, 43 insertions(+), 18 deletions(-)

-- 
2.1.4


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

* [PATCH v2 1/4] gpio: use sizeof() instead of hardcoded values
  2015-01-21 21:33 [PATCH v2 0/4] Let leds use named gpios Olliver Schinagl
@ 2015-01-21 21:33 ` Olliver Schinagl
  2015-01-30 13:43   ` Linus Walleij
  2015-01-21 21:33 ` [PATCH v2 2/4] gpio: add parameter to allow the use named gpios Olliver Schinagl
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Olliver Schinagl @ 2015-01-21 21:33 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu,
	Richard Purdie, Olliver Schinagl, Robin Gong, Mika Westerberg,
	Aaron Lu, Grant Likely, Jingoo Han, Alexander Shiyan,
	Wolfram Sang
  Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-leds

From: Olliver Schinagl <oliver@schinagl.nl>

gpiolib uses a fixed string for the suffixes and defines it at 32 bytes.
Later in the code snprintf is used with this fixed value of 32. Using
sizeof() is safer in case the size for the suffixes is ever changed.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
---
 drivers/gpio/gpiolib.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index c926641..bf6016d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1665,9 +1665,11 @@ static struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 
 	for (i = 0; i < ARRAY_SIZE(suffixes); i++) {
 		if (con_id)
-			snprintf(prop_name, 32, "%s-%s", con_id, suffixes[i]);
+			snprintf(prop_name, sizeof(prop_name), "%s-%s", con_id,
+							       suffixes[i]);
 		else
-			snprintf(prop_name, 32, "%s", suffixes[i]);
+			snprintf(prop_name, sizeof(prop_name), "%s",
+							       suffixes[i]);
 
 		desc = of_get_named_gpiod_flags(dev->of_node, prop_name, idx,
 						&of_flags);
-- 
2.1.4


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

* [PATCH v2 2/4] gpio: add parameter to allow the use named gpios
  2015-01-21 21:33 [PATCH v2 0/4] Let leds use named gpios Olliver Schinagl
  2015-01-21 21:33 ` [PATCH v2 1/4] gpio: use sizeof() instead of hardcoded values Olliver Schinagl
@ 2015-01-21 21:33 ` Olliver Schinagl
  2015-01-30 13:46   ` Linus Walleij
  2015-03-04 13:00   ` Linus Walleij
  2015-01-21 21:33 ` [PATCH v2 3/4] leds: Let the binding document example for leds-gpio follow the gpio bindings Olliver Schinagl
  2015-01-21 21:33 ` [PATCH v2 4/4] leds: no longer use unnamed gpios Olliver Schinagl
  3 siblings, 2 replies; 17+ messages in thread
From: Olliver Schinagl @ 2015-01-21 21:33 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu,
	Richard Purdie, Olliver Schinagl, Robin Gong, Mika Westerberg,
	Aaron Lu, Grant Likely, Jingoo Han, Alexander Shiyan,
	Wolfram Sang
  Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-leds

From: Olliver Schinagl <oliver@schinagl.nl>

The gpio binding document says that new code should always use named
gpios. Patch 40b73183 added support to parse a list of gpios from child
nodes, but does not make it possible to use named gpios. This patch adds
the con_id property and implements it is done in gpiolib.c, where the
old-style of using unnamed gpios still works.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
---
 drivers/gpio/devres.c                     | 18 +++++++++++++++++-
 drivers/input/keyboard/gpio_keys_polled.c |  2 +-
 drivers/leds/leds-gpio.c                  |  2 +-
 include/linux/gpio/consumer.h             |  1 +
 4 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/devres.c b/drivers/gpio/devres.c
index 13dbd3d..12c2082 100644
--- a/drivers/gpio/devres.c
+++ b/drivers/gpio/devres.c
@@ -111,23 +111,39 @@ EXPORT_SYMBOL(__devm_gpiod_get_index);
 /**
  * devm_get_gpiod_from_child - get a GPIO descriptor from a device's child node
  * @dev:	GPIO consumer
+ * @con_id:	function within the GPIO consumer
  * @child:	firmware node (child of @dev)
  *
  * GPIO descriptors returned from this function are automatically disposed on
  * driver detach.
  */
 struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
+					    const char *con_id,
 					    struct fwnode_handle *child)
 {
+	static const char const *suffixes[] = { "gpios", "gpio" };
+	char prop_name[32]; /* 32 is max size of property name */
 	struct gpio_desc **dr;
 	struct gpio_desc *desc;
+	unsigned int i;
 
 	dr = devres_alloc(devm_gpiod_release, sizeof(struct gpio_desc *),
 			  GFP_KERNEL);
 	if (!dr)
 		return ERR_PTR(-ENOMEM);
 
-	desc = fwnode_get_named_gpiod(child, "gpios");
+	for (i = 0; i < ARRAY_SIZE(suffixes); i++) {
+		if (con_id)
+			snprintf(prop_name, sizeof(prop_name), "%s-%s",
+					    con_id, suffixes[i]);
+		else
+			snprintf(prop_name, sizeof(prop_name), "%s",
+							       suffixes[i]);
+
+		desc = fwnode_get_named_gpiod(child, prop_name);
+		if (!IS_ERR(desc) || (PTR_ERR(desc) == -EPROBE_DEFER))
+			break;
+	}
 	if (IS_ERR(desc)) {
 		devres_free(dr);
 		return desc;
diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 90df4df..097d721 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -125,7 +125,7 @@ static struct gpio_keys_platform_data *gpio_keys_polled_get_devtree_pdata(struct
 	device_for_each_child_node(dev, child) {
 		struct gpio_desc *desc;
 
-		desc = devm_get_gpiod_from_child(dev, child);
+		desc = devm_get_gpiod_from_child(dev, NULL, child);
 		if (IS_ERR(desc)) {
 			error = PTR_ERR(desc);
 			if (error != -EPROBE_DEFER)
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 7ea1ea42..8e5af69 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -184,7 +184,7 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
 		struct gpio_led led = {};
 		const char *state = NULL;
 
-		led.gpiod = devm_get_gpiod_from_child(dev, child);
+		led.gpiod = devm_get_gpiod_from_child(dev, NULL, child);
 		if (IS_ERR(led.gpiod)) {
 			fwnode_handle_put(child);
 			goto err;
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 45afc2d..ed20759 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -110,6 +110,7 @@ struct fwnode_handle;
 struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 					 const char *propname);
 struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
+					    const char *con_id,
 					    struct fwnode_handle *child);
 #else /* CONFIG_GPIOLIB */
 
-- 
2.1.4


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

* [PATCH v2 3/4] leds: Let the binding document example for leds-gpio follow the gpio bindings
  2015-01-21 21:33 [PATCH v2 0/4] Let leds use named gpios Olliver Schinagl
  2015-01-21 21:33 ` [PATCH v2 1/4] gpio: use sizeof() instead of hardcoded values Olliver Schinagl
  2015-01-21 21:33 ` [PATCH v2 2/4] gpio: add parameter to allow the use named gpios Olliver Schinagl
@ 2015-01-21 21:33 ` Olliver Schinagl
  2015-03-02 11:24   ` Linus Walleij
  2015-01-21 21:33 ` [PATCH v2 4/4] leds: no longer use unnamed gpios Olliver Schinagl
  3 siblings, 1 reply; 17+ messages in thread
From: Olliver Schinagl @ 2015-01-21 21:33 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu,
	Richard Purdie, Olliver Schinagl, Robin Gong, Mika Westerberg,
	Aaron Lu, Grant Likely, Jingoo Han, Alexander Shiyan,
	Wolfram Sang
  Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-leds

From: Olliver Schinagl <oliver@schinagl.nl>

In the gpio bindings documents it is requested to use the marco's in
include/dt-bindings/gpio/gpio.h whenever possible. The gpios in the led
drivers don't seem to form an exception, so update the example in the
document bindings.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 Documentation/devicetree/bindings/leds/leds-gpio.txt | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-gpio.txt b/Documentation/devicetree/bindings/leds/leds-gpio.txt
index f77148f..fea1ebf 100644
--- a/Documentation/devicetree/bindings/leds/leds-gpio.txt
+++ b/Documentation/devicetree/bindings/leds/leds-gpio.txt
@@ -26,16 +26,18 @@ LED sub-node properties:
 
 Examples:
 
+#include <dt-bindings/gpio/gpio.h>
+
 leds {
 	compatible = "gpio-leds";
 	hdd {
 		label = "IDE Activity";
-		gpios = <&mcu_pio 0 1>; /* Active low */
+		gpios = <&mcu_pio 0 GPIO_ACTIVE_LOW>;
 		linux,default-trigger = "ide-disk";
 	};
 
 	fault {
-		gpios = <&mcu_pio 1 0>;
+		gpios = <&mcu_pio 1 GPIO_ACTIVE_HIGH>;
 		/* Keep LED on if BIOS detected hardware fault */
 		default-state = "keep";
 	};
@@ -44,11 +46,11 @@ leds {
 run-control {
 	compatible = "gpio-leds";
 	red {
-		gpios = <&mpc8572 6 0>;
+		gpios = <&mpc8572 6 GPIO_ACTIVE_HIGH>;
 		default-state = "off";
 	};
 	green {
-		gpios = <&mpc8572 7 0>;
+		gpios = <&mpc8572 7 GPIO_ACTIVE_HIGH>;
 		default-state = "on";
 	};
 };
@@ -57,7 +59,7 @@ leds {
 	compatible = "gpio-leds";
 
 	charger-led {
-		gpios = <&gpio1 2 0>;
+		gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
 		linux,default-trigger = "max8903-charger-charging";
 		retain-state-suspended;
 	};
-- 
2.1.4


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

* [PATCH v2 4/4] leds: no longer use unnamed gpios
  2015-01-21 21:33 [PATCH v2 0/4] Let leds use named gpios Olliver Schinagl
                   ` (2 preceding siblings ...)
  2015-01-21 21:33 ` [PATCH v2 3/4] leds: Let the binding document example for leds-gpio follow the gpio bindings Olliver Schinagl
@ 2015-01-21 21:33 ` Olliver Schinagl
  2015-01-22  9:32   ` Rojhalat Ibrahim
  2015-01-22 16:33   ` Dmitry Torokhov
  3 siblings, 2 replies; 17+ messages in thread
From: Olliver Schinagl @ 2015-01-21 21:33 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu,
	Richard Purdie, Olliver Schinagl, Robin Gong, Mika Westerberg,
	Aaron Lu, Grant Likely, Jingoo Han, Alexander Shiyan,
	Wolfram Sang
  Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-leds

From: Olliver Schinagl <oliver@schinagl.nl>

The gpio document says we should not use unnamed bindings for gpios.
This patch uses the 'led-' prefix to the gpios and updates code and
documents. Because the devm_get_gpiod_from_child() falls back to using
old-style unnamed gpios, we can update the code first, and update
dts files as time allows.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
---
 Documentation/devicetree/bindings/leds/leds-gpio.txt | 12 ++++++------
 drivers/input/keyboard/gpio_keys_polled.c            | 20 ++++++++++++--------
 drivers/leds/leds-gpio.c                             |  2 +-
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-gpio.txt b/Documentation/devicetree/bindings/leds/leds-gpio.txt
index fea1ebf..1ac7ccb5 100644
--- a/Documentation/devicetree/bindings/leds/leds-gpio.txt
+++ b/Documentation/devicetree/bindings/leds/leds-gpio.txt
@@ -7,7 +7,7 @@ Each LED is represented as a sub-node of the gpio-leds device.  Each
 node's name represents the name of the corresponding LED.
 
 LED sub-node properties:
-- gpios :  Should specify the LED's GPIO, see "gpios property" in
+- led-gpios :  Should specify the LED's GPIO, see "gpios property" in
   Documentation/devicetree/bindings/gpio/gpio.txt.  Active low LEDs should be
   indicated using flags in the GPIO specifier.
 - label :  (optional)
@@ -32,12 +32,12 @@ leds {
 	compatible = "gpio-leds";
 	hdd {
 		label = "IDE Activity";
-		gpios = <&mcu_pio 0 GPIO_ACTIVE_LOW>;
+		led-gpios = <&mcu_pio 0 GPIO_ACTIVE_LOW>;
 		linux,default-trigger = "ide-disk";
 	};
 
 	fault {
-		gpios = <&mcu_pio 1 GPIO_ACTIVE_HIGH>;
+		led-gpios = <&mcu_pio 1 GPIO_ACTIVE_HIGH>;
 		/* Keep LED on if BIOS detected hardware fault */
 		default-state = "keep";
 	};
@@ -46,11 +46,11 @@ leds {
 run-control {
 	compatible = "gpio-leds";
 	red {
-		gpios = <&mpc8572 6 GPIO_ACTIVE_HIGH>;
+		led-gpios = <&mpc8572 6 GPIO_ACTIVE_HIGH>;
 		default-state = "off";
 	};
 	green {
-		gpios = <&mpc8572 7 GPIO_ACTIVE_HIGH>;
+		led-gpios = <&mpc8572 7 GPIO_ACTIVE_HIGH>;
 		default-state = "on";
 	};
 };
@@ -59,7 +59,7 @@ leds {
 	compatible = "gpio-leds";
 
 	charger-led {
-		gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
+		led-gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
 		linux,default-trigger = "max8903-charger-charging";
 		retain-state-suspended;
 	};
diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 097d721..eadb472 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -125,15 +125,19 @@ static struct gpio_keys_platform_data *gpio_keys_polled_get_devtree_pdata(struct
 	device_for_each_child_node(dev, child) {
 		struct gpio_desc *desc;
 
-		desc = devm_get_gpiod_from_child(dev, NULL, child);
+		desc = devm_get_gpiod_from_child(dev, "gpio_keys_polled",
+						 child);
 		if (IS_ERR(desc)) {
-			error = PTR_ERR(desc);
-			if (error != -EPROBE_DEFER)
-				dev_err(dev,
-					"Failed to get gpio flags, error: %d\n",
-					error);
-			fwnode_handle_put(child);
-			return ERR_PTR(error);
+			desc = devm_get_gpiod_from_child(dev, NULL, child);
+			if (IS_ERR(desc)) {
+				error = PTR_ERR(desc);
+				if (error != -EPROBE_DEFER)
+					dev_err(dev,
+						"Failed to get gpio flags, error: %d\n",
+						error);
+				fwnode_handle_put(child);
+				return ERR_PTR(error);
+			}
 		}
 
 		button = &pdata->buttons[pdata->nbuttons++];
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 8e5af69..5b7bc84 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -184,7 +184,7 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
 		struct gpio_led led = {};
 		const char *state = NULL;
 
-		led.gpiod = devm_get_gpiod_from_child(dev, NULL, child);
+		led.gpiod = devm_get_gpiod_from_child(dev, "led", child);
 		if (IS_ERR(led.gpiod)) {
 			fwnode_handle_put(child);
 			goto err;
-- 
2.1.4


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

* Re: [PATCH v2 4/4] leds: no longer use unnamed gpios
  2015-01-21 21:33 ` [PATCH v2 4/4] leds: no longer use unnamed gpios Olliver Schinagl
@ 2015-01-22  9:32   ` Rojhalat Ibrahim
  2015-01-22  9:37     ` Olliver Schinagl
  2015-01-22 16:33   ` Dmitry Torokhov
  1 sibling, 1 reply; 17+ messages in thread
From: Rojhalat Ibrahim @ 2015-01-22  9:32 UTC (permalink / raw)
  To: Olliver Schinagl
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu,
	Richard Purdie, Olliver Schinagl, Robin Gong, Mika Westerberg,
	Aaron Lu, Grant Likely, Jingoo Han, Alexander Shiyan,
	Wolfram Sang, devicetree, linux-kernel, linux-gpio, linux-input,
	linux-leds

On Wednesday 21 January 2015 22:33:48 Olliver Schinagl wrote:
> From: Olliver Schinagl <oliver@schinagl.nl>
> 
> The gpio document says we should not use unnamed bindings for gpios.
> This patch uses the 'led-' prefix to the gpios and updates code and
> documents. Because the devm_get_gpiod_from_child() falls back to using
> old-style unnamed gpios, we can update the code first, and update
> dts files as time allows.
> 
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>

Where does devm_get_gpiod_from_child() fall back "to using old-style
unnamed gpios"?

After applying this patch the leds defined in my devicetree do not
work anymore.

   Rojhalat



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

* Re: [PATCH v2 4/4] leds: no longer use unnamed gpios
  2015-01-22  9:32   ` Rojhalat Ibrahim
@ 2015-01-22  9:37     ` Olliver Schinagl
  0 siblings, 0 replies; 17+ messages in thread
From: Olliver Schinagl @ 2015-01-22  9:37 UTC (permalink / raw)
  To: Rojhalat Ibrahim
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu,
	Richard Purdie, Olliver Schinagl, Robin Gong, Mika Westerberg,
	Aaron Lu, Grant Likely, Jingoo Han, Alexander Shiyan,
	Wolfram Sang, devicetree, linux-kernel, linux-gpio, linux-input,
	linux-leds

Hey Rojhalat,

On 22-01-15 10:32, Rojhalat Ibrahim wrote:
> On Wednesday 21 January 2015 22:33:48 Olliver Schinagl wrote:
>> From: Olliver Schinagl <oliver@schinagl.nl>
>>
>> The gpio document says we should not use unnamed bindings for gpios.
>> This patch uses the 'led-' prefix to the gpios and updates code and
>> documents. Because the devm_get_gpiod_from_child() falls back to using
>> old-style unnamed gpios, we can update the code first, and update
>> dts files as time allows.
>>
>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> Where does devm_get_gpiod_from_child() fall back "to using old-style
> unnamed gpios"?
Your absolutly right, I accidentally forgot a patch that was supposed to 
get squashed into this patchset. The idea is to do the same as in 
gpio-keys-polled.c. I'll make sure it sits in v3 of the set! My appologies!

Olliver
>
> After applying this patch the leds defined in my devicetree do not
> work anymore.
>
>     Rojhalat
>
>
>

-- 
Met vriendelijke groeten, Kind regards, 与亲切的问候

Olliver Schinagl
Research & Development
Ultimaker B.V.
http://www.ultimaker.com


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

* Re: [PATCH v2 4/4] leds: no longer use unnamed gpios
  2015-01-21 21:33 ` [PATCH v2 4/4] leds: no longer use unnamed gpios Olliver Schinagl
  2015-01-22  9:32   ` Rojhalat Ibrahim
@ 2015-01-22 16:33   ` Dmitry Torokhov
  1 sibling, 0 replies; 17+ messages in thread
From: Dmitry Torokhov @ 2015-01-22 16:33 UTC (permalink / raw)
  To: Olliver Schinagl
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Linus Walleij, Alexandre Courbot, Bryan Wu, Richard Purdie,
	Olliver Schinagl, Robin Gong, Mika Westerberg, Aaron Lu,
	Grant Likely, Jingoo Han, Alexander Shiyan, Wolfram Sang,
	devicetree, linux-kernel, linux-gpio, linux-input, linux-leds

Hi Olliver,

On Wed, Jan 21, 2015 at 10:33:48PM +0100, Olliver Schinagl wrote:
> diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
> index 097d721..eadb472 100644
> --- a/drivers/input/keyboard/gpio_keys_polled.c
> +++ b/drivers/input/keyboard/gpio_keys_polled.c
> @@ -125,15 +125,19 @@ static struct gpio_keys_platform_data *gpio_keys_polled_get_devtree_pdata(struct
>  	device_for_each_child_node(dev, child) {
>  		struct gpio_desc *desc;
>  
> -		desc = devm_get_gpiod_from_child(dev, NULL, child);
> +		desc = devm_get_gpiod_from_child(dev, "gpio_keys_polled",
> +						 child);
>  		if (IS_ERR(desc)) {
> -			error = PTR_ERR(desc);
> -			if (error != -EPROBE_DEFER)
> -				dev_err(dev,
> -					"Failed to get gpio flags, error: %d\n",
> -					error);
> -			fwnode_handle_put(child);
> -			return ERR_PTR(error);
> +			desc = devm_get_gpiod_from_child(dev, NULL, child);
> +			if (IS_ERR(desc)) {
> +				error = PTR_ERR(desc);
> +				if (error != -EPROBE_DEFER)
> +					dev_err(dev,
> +						"Failed to get gpio flags, error: %d\n",
> +						error);
> +				fwnode_handle_put(child);
> +				return ERR_PTR(error);
> +			}
>  		}
>  

I do not think this is correct. If devm_get_gpiod_from_child(dev,
"gpio_keys_polled", child) retruns -EPROBE_DEFER we'll try
devm_get_gpiod_from_child(dev, NULL, child). If that returns some other
error we'll fail probing entire driver.

Did I mention how *I HATE* the -EPROBE_DEFER (the error that is not an
error)?

Thanks.

-- 
Dmitry

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

* Re: [PATCH v2 1/4] gpio: use sizeof() instead of hardcoded values
  2015-01-21 21:33 ` [PATCH v2 1/4] gpio: use sizeof() instead of hardcoded values Olliver Schinagl
@ 2015-01-30 13:43   ` Linus Walleij
  0 siblings, 0 replies; 17+ messages in thread
From: Linus Walleij @ 2015-01-30 13:43 UTC (permalink / raw)
  To: Olliver Schinagl
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Richard Purdie,
	Olliver Schinagl, Robin Gong, Mika Westerberg, Aaron Lu,
	Grant Likely, Jingoo Han, Alexander Shiyan, Wolfram Sang,
	devicetree, linux-kernel, linux-gpio, Linux Input, linux-leds

On Wed, Jan 21, 2015 at 10:33 PM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:

> From: Olliver Schinagl <oliver@schinagl.nl>
>
> gpiolib uses a fixed string for the suffixes and defines it at 32 bytes.
> Later in the code snprintf is used with this fixed value of 32. Using
> sizeof() is safer in case the size for the suffixes is ever changed.
>
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>

OK looks nice.

Patch applied.

Yours,
Linus Walleij

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

* Re: [PATCH v2 2/4] gpio: add parameter to allow the use named gpios
  2015-01-21 21:33 ` [PATCH v2 2/4] gpio: add parameter to allow the use named gpios Olliver Schinagl
@ 2015-01-30 13:46   ` Linus Walleij
  2015-01-30 19:12     ` Bryan Wu
  2015-02-09  5:24     ` Alexandre Courbot
  2015-03-04 13:00   ` Linus Walleij
  1 sibling, 2 replies; 17+ messages in thread
From: Linus Walleij @ 2015-01-30 13:46 UTC (permalink / raw)
  To: Olliver Schinagl, Alexandre Courbot, Bryan Wu, Dmitry Torokhov
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Richard Purdie, Olliver Schinagl, Robin Gong, Mika Westerberg,
	Aaron Lu, Grant Likely, Jingoo Han, Alexander Shiyan,
	Wolfram Sang, devicetree, linux-kernel, linux-gpio, Linux Input,
	linux-leds

On Wed, Jan 21, 2015 at 10:33 PM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:

> From: Olliver Schinagl <oliver@schinagl.nl>
>
> The gpio binding document says that new code should always use named
> gpios. Patch 40b73183 added support to parse a list of gpios from child
> nodes, but does not make it possible to use named gpios. This patch adds
> the con_id property and implements it is done in gpiolib.c, where the
> old-style of using unnamed gpios still works.
>
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> ---
>  drivers/gpio/devres.c                     | 18 +++++++++++++++++-
>  drivers/input/keyboard/gpio_keys_polled.c |  2 +-
>  drivers/leds/leds-gpio.c                  |  2 +-
>  include/linux/gpio/consumer.h             |  1 +

Alexandre: does this match your vision of how it should work, i.e. ACK?

Bryan/Dmitry: can you ACK the oneliners in your subsystems?

Yours,
Linus Walleij

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

* Re: [PATCH v2 2/4] gpio: add parameter to allow the use named gpios
  2015-01-30 13:46   ` Linus Walleij
@ 2015-01-30 19:12     ` Bryan Wu
  2015-01-30 22:16       ` Dmitry Torokhov
  2015-02-09  5:24     ` Alexandre Courbot
  1 sibling, 1 reply; 17+ messages in thread
From: Bryan Wu @ 2015-01-30 19:12 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Olliver Schinagl, Alexandre Courbot, Dmitry Torokhov,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Richard Purdie, Olliver Schinagl, Robin Gong, Mika Westerberg,
	Aaron Lu, Grant Likely, Jingoo Han, Alexander Shiyan,
	Wolfram Sang, devicetree, linux-kernel, linux-gpio, Linux Input,
	linux-leds

On Fri, Jan 30, 2015 at 5:46 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Jan 21, 2015 at 10:33 PM, Olliver Schinagl
> <o.schinagl@ultimaker.com> wrote:
>
>> From: Olliver Schinagl <oliver@schinagl.nl>
>>
>> The gpio binding document says that new code should always use named
>> gpios. Patch 40b73183 added support to parse a list of gpios from child
>> nodes, but does not make it possible to use named gpios. This patch adds
>> the con_id property and implements it is done in gpiolib.c, where the
>> old-style of using unnamed gpios still works.
>>
>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>> ---
>>  drivers/gpio/devres.c                     | 18 +++++++++++++++++-
>>  drivers/input/keyboard/gpio_keys_polled.c |  2 +-
>>  drivers/leds/leds-gpio.c                  |  2 +-
>>  include/linux/gpio/consumer.h             |  1 +
>
> Alexandre: does this match your vision of how it should work, i.e. ACK?
>
> Bryan/Dmitry: can you ACK the oneliners in your subsystems?

Sure, please take my Ack
Acked-by: Bryan Wu <cooloney@gmail.com>

>
> Yours,
> Linus Walleij

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

* Re: [PATCH v2 2/4] gpio: add parameter to allow the use named gpios
  2015-01-30 19:12     ` Bryan Wu
@ 2015-01-30 22:16       ` Dmitry Torokhov
  2015-01-30 22:22         ` Dmitry Torokhov
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Torokhov @ 2015-01-30 22:16 UTC (permalink / raw)
  To: Bryan Wu
  Cc: Linus Walleij, Olliver Schinagl, Alexandre Courbot, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Richard Purdie, Olliver Schinagl, Robin Gong, Mika Westerberg,
	Aaron Lu, Grant Likely, Jingoo Han, Alexander Shiyan,
	Wolfram Sang, devicetree, linux-kernel, linux-gpio, Linux Input,
	linux-leds

On Fri, Jan 30, 2015 at 11:12:53AM -0800, Bryan Wu wrote:
> On Fri, Jan 30, 2015 at 5:46 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Wed, Jan 21, 2015 at 10:33 PM, Olliver Schinagl
> > <o.schinagl@ultimaker.com> wrote:
> >
> >> From: Olliver Schinagl <oliver@schinagl.nl>
> >>
> >> The gpio binding document says that new code should always use named
> >> gpios. Patch 40b73183 added support to parse a list of gpios from child
> >> nodes, but does not make it possible to use named gpios. This patch adds
> >> the con_id property and implements it is done in gpiolib.c, where the
> >> old-style of using unnamed gpios still works.
> >>
> >> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> >> ---
> >>  drivers/gpio/devres.c                     | 18 +++++++++++++++++-
> >>  drivers/input/keyboard/gpio_keys_polled.c |  2 +-
> >>  drivers/leds/leds-gpio.c                  |  2 +-
> >>  include/linux/gpio/consumer.h             |  1 +
> >
> > Alexandre: does this match your vision of how it should work, i.e. ACK?
> >
> > Bryan/Dmitry: can you ACK the oneliners in your subsystems?
> 
> Sure, please take my Ack
> Acked-by: Bryan Wu <cooloney@gmail.com>

Mine as well:

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks.

-- 
Dmitry

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

* Re: [PATCH v2 2/4] gpio: add parameter to allow the use named gpios
  2015-01-30 22:16       ` Dmitry Torokhov
@ 2015-01-30 22:22         ` Dmitry Torokhov
  0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Torokhov @ 2015-01-30 22:22 UTC (permalink / raw)
  To: Bryan Wu
  Cc: Linus Walleij, Olliver Schinagl, Alexandre Courbot, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Richard Purdie, Olliver Schinagl, Robin Gong, Mika Westerberg,
	Aaron Lu, Grant Likely, Jingoo Han, Alexander Shiyan,
	Wolfram Sang, devicetree, linux-kernel, linux-gpio, Linux Input,
	linux-leds

On Fri, Jan 30, 2015 at 02:16:00PM -0800, Dmitry Torokhov wrote:
> On Fri, Jan 30, 2015 at 11:12:53AM -0800, Bryan Wu wrote:
> > On Fri, Jan 30, 2015 at 5:46 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> > > On Wed, Jan 21, 2015 at 10:33 PM, Olliver Schinagl
> > > <o.schinagl@ultimaker.com> wrote:
> > >
> > >> From: Olliver Schinagl <oliver@schinagl.nl>
> > >>
> > >> The gpio binding document says that new code should always use named
> > >> gpios. Patch 40b73183 added support to parse a list of gpios from child
> > >> nodes, but does not make it possible to use named gpios. This patch adds
> > >> the con_id property and implements it is done in gpiolib.c, where the
> > >> old-style of using unnamed gpios still works.
> > >>
> > >> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> > >> ---
> > >>  drivers/gpio/devres.c                     | 18 +++++++++++++++++-
> > >>  drivers/input/keyboard/gpio_keys_polled.c |  2 +-
> > >>  drivers/leds/leds-gpio.c                  |  2 +-
> > >>  include/linux/gpio/consumer.h             |  1 +
> > >
> > > Alexandre: does this match your vision of how it should work, i.e. ACK?
> > >
> > > Bryan/Dmitry: can you ACK the oneliners in your subsystems?
> > 
> > Sure, please take my Ack
> > Acked-by: Bryan Wu <cooloney@gmail.com>
> 
> Mine as well:
> 
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Forgot to mention: the ack is for this patch only; the patch #4 is
NAKed because:

1. The logic of handling old and new name AFAICS is broken and
2. gpio_keys_polled-gpios as name is plain ugly.

Thanks.

-- 
Dmitry

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

* Re: [PATCH v2 2/4] gpio: add parameter to allow the use named gpios
  2015-01-30 13:46   ` Linus Walleij
  2015-01-30 19:12     ` Bryan Wu
@ 2015-02-09  5:24     ` Alexandre Courbot
  1 sibling, 0 replies; 17+ messages in thread
From: Alexandre Courbot @ 2015-02-09  5:24 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Olliver Schinagl, Bryan Wu, Dmitry Torokhov, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Richard Purdie, Olliver Schinagl, Robin Gong, Mika Westerberg,
	Aaron Lu, Grant Likely, Jingoo Han, Alexander Shiyan,
	Wolfram Sang, devicetree, linux-kernel, linux-gpio, Linux Input,
	linux-leds

On Fri, Jan 30, 2015 at 10:46 PM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Wed, Jan 21, 2015 at 10:33 PM, Olliver Schinagl
> <o.schinagl@ultimaker.com> wrote:
>
>> From: Olliver Schinagl <oliver@schinagl.nl>
>>
>> The gpio binding document says that new code should always use named
>> gpios. Patch 40b73183 added support to parse a list of gpios from child
>> nodes, but does not make it possible to use named gpios. This patch adds
>> the con_id property and implements it is done in gpiolib.c, where the
>> old-style of using unnamed gpios still works.
>>
>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>> ---
>>  drivers/gpio/devres.c                     | 18 +++++++++++++++++-
>>  drivers/input/keyboard/gpio_keys_polled.c |  2 +-
>>  drivers/leds/leds-gpio.c                  |  2 +-
>>  include/linux/gpio/consumer.h             |  1 +
>
> Alexandre: does this match your vision of how it should work, i.e. ACK?

Pretty much, yes - as I mentioned in the previous versions there may
be shortcomings for ACPI, but we need a refactor of the whole thing -
nothing that this patch should address by itself.

So this patch:

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

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

* Re: [PATCH v2 3/4] leds: Let the binding document example for leds-gpio follow the gpio bindings
  2015-01-21 21:33 ` [PATCH v2 3/4] leds: Let the binding document example for leds-gpio follow the gpio bindings Olliver Schinagl
@ 2015-03-02 11:24   ` Linus Walleij
  2015-03-02 20:03     ` Bryan Wu
  0 siblings, 1 reply; 17+ messages in thread
From: Linus Walleij @ 2015-03-02 11:24 UTC (permalink / raw)
  To: Olliver Schinagl, Bryan Wu
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Alexandre Courbot, Dmitry Torokhov, Richard Purdie,
	Olliver Schinagl, Robin Gong, Mika Westerberg, Aaron Lu,
	Grant Likely, Jingoo Han, Alexander Shiyan, Wolfram Sang,
	devicetree, linux-kernel, linux-gpio, Linux Input, linux-leds

On Wed, Jan 21, 2015 at 10:33 PM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:

> From: Olliver Schinagl <oliver@schinagl.nl>
>
> In the gpio bindings documents it is requested to use the marco's in
> include/dt-bindings/gpio/gpio.h whenever possible. The gpios in the led
> drivers don't seem to form an exception, so update the example in the
> document bindings.
>
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> Acked-by: Rob Herring <robh@kernel.org>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

Bryan: please merge this patch to the LED git tree.

Yours,
Linus Walleij

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

* Re: [PATCH v2 3/4] leds: Let the binding document example for leds-gpio follow the gpio bindings
  2015-03-02 11:24   ` Linus Walleij
@ 2015-03-02 20:03     ` Bryan Wu
  0 siblings, 0 replies; 17+ messages in thread
From: Bryan Wu @ 2015-03-02 20:03 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Olliver Schinagl, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Alexandre Courbot, Dmitry Torokhov,
	Richard Purdie, Olliver Schinagl, Robin Gong, Mika Westerberg,
	Aaron Lu, Grant Likely, Jingoo Han, Alexander Shiyan,
	Wolfram Sang, devicetree, linux-kernel, linux-gpio, Linux Input,
	linux-leds

On Mon, Mar 2, 2015 at 3:24 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Jan 21, 2015 at 10:33 PM, Olliver Schinagl
> <o.schinagl@ultimaker.com> wrote:
>
>> From: Olliver Schinagl <oliver@schinagl.nl>
>>
>> In the gpio bindings documents it is requested to use the marco's in
>> include/dt-bindings/gpio/gpio.h whenever possible. The gpios in the led
>> drivers don't seem to form an exception, so update the example in the
>> document bindings.
>>
>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>> Acked-by: Rob Herring <robh@kernel.org>
>> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>
> Bryan: please merge this patch to the LED git tree.
>

Merged, thanks

-Bryan

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

* Re: [PATCH v2 2/4] gpio: add parameter to allow the use named gpios
  2015-01-21 21:33 ` [PATCH v2 2/4] gpio: add parameter to allow the use named gpios Olliver Schinagl
  2015-01-30 13:46   ` Linus Walleij
@ 2015-03-04 13:00   ` Linus Walleij
  1 sibling, 0 replies; 17+ messages in thread
From: Linus Walleij @ 2015-03-04 13:00 UTC (permalink / raw)
  To: Olliver Schinagl
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Richard Purdie,
	Olliver Schinagl, Robin Gong, Mika Westerberg, Aaron Lu,
	Grant Likely, Jingoo Han, Alexander Shiyan, Wolfram Sang,
	devicetree, linux-kernel, linux-gpio, Linux Input, linux-leds

On Wed, Jan 21, 2015 at 10:33 PM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:

> From: Olliver Schinagl <oliver@schinagl.nl>
>
> The gpio binding document says that new code should always use named
> gpios. Patch 40b73183 added support to parse a list of gpios from child
> nodes, but does not make it possible to use named gpios. This patch adds
> the con_id property and implements it is done in gpiolib.c, where the
> old-style of using unnamed gpios still works.
>
> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>

Patch applied with the ACKs and tags.

Yours,
Linus Walleij

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

end of thread, other threads:[~2015-03-04 13:00 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-21 21:33 [PATCH v2 0/4] Let leds use named gpios Olliver Schinagl
2015-01-21 21:33 ` [PATCH v2 1/4] gpio: use sizeof() instead of hardcoded values Olliver Schinagl
2015-01-30 13:43   ` Linus Walleij
2015-01-21 21:33 ` [PATCH v2 2/4] gpio: add parameter to allow the use named gpios Olliver Schinagl
2015-01-30 13:46   ` Linus Walleij
2015-01-30 19:12     ` Bryan Wu
2015-01-30 22:16       ` Dmitry Torokhov
2015-01-30 22:22         ` Dmitry Torokhov
2015-02-09  5:24     ` Alexandre Courbot
2015-03-04 13:00   ` Linus Walleij
2015-01-21 21:33 ` [PATCH v2 3/4] leds: Let the binding document example for leds-gpio follow the gpio bindings Olliver Schinagl
2015-03-02 11:24   ` Linus Walleij
2015-03-02 20:03     ` Bryan Wu
2015-01-21 21:33 ` [PATCH v2 4/4] leds: no longer use unnamed gpios Olliver Schinagl
2015-01-22  9:32   ` Rojhalat Ibrahim
2015-01-22  9:37     ` Olliver Schinagl
2015-01-22 16:33   ` Dmitry Torokhov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).