[v2,4/4] leds: no longer use unnamed gpios
diff mbox

Message ID 1421876028-22799-5-git-send-email-o.schinagl@ultimaker.com
State New
Headers show

Commit Message

Olliver Schinagl Jan. 21, 2015, 9:33 p.m. UTC
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(-)

Comments

Rojhalat Ibrahim Jan. 22, 2015, 9:32 a.m. UTC | #1
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


--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Olliver Schinagl Jan. 22, 2015, 9:37 a.m. UTC | #2
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
>
>
>
Dmitry Torokhov Jan. 22, 2015, 4:33 p.m. UTC | #3
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.

Patch
diff mbox

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;