[2/3] input: gpio_keys: Switch from irq_of_parse_and_map() to platform_get_irq()
diff mbox

Message ID 1455876982-6743-3-git-send-email-geert+renesas@glider.be
State Under Review
Headers show

Commit Message

Geert Uytterhoeven Feb. 19, 2016, 10:16 a.m. UTC
Note that irq_of_parse_and_map() returns 0 on failure, while
platform_get_irq() returns -ENXIO on failure, so we have to make irq
signed, and update all error checks.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/input/keyboard/gpio_keys.c | 16 ++++++++--------
 include/linux/gpio_keys.h          |  2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

Comments

Dmitry Torokhov Feb. 23, 2016, 7:35 p.m. UTC | #1
On Fri, Feb 19, 2016 at 11:16:21AM +0100, Geert Uytterhoeven wrote:
> Note that irq_of_parse_and_map() returns 0 on failure, while
> platform_get_irq() returns -ENXIO on failure, so we have to make irq
> signed, and update all error checks.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/input/keyboard/gpio_keys.c | 16 ++++++++--------
>  include/linux/gpio_keys.h          |  2 +-
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index d2b6c3acd9c32f1d..b6262d94aff19f70 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -30,7 +30,6 @@
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
>  #include <linux/of_gpio.h>
> -#include <linux/of_irq.h>
>  #include <linux/spinlock.h>
>  
>  struct gpio_button_data {
> @@ -511,7 +510,7 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
>  						button->debounce_interval;
>  		}
>  
> -		if (button->irq) {
> +		if (button->irq >= 0) {
>  			bdata->irq = button->irq;
>  		} else {
>  			irq = gpiod_to_irq(button->gpiod);
> @@ -531,7 +530,7 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
>  		irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
>  
>  	} else {
> -		if (!button->irq) {
> +		if (button->irq < 0) {
>  			dev_err(dev, "No IRQ specified\n");
>  			return -EINVAL;
>  		}
> @@ -631,8 +630,9 @@ static void gpio_keys_close(struct input_dev *input)
>   * Translate OpenFirmware node properties into platform_data
>   */
>  static struct gpio_keys_platform_data *
> -gpio_keys_get_devtree_pdata(struct device *dev)
> +gpio_keys_get_devtree_pdata(struct platform_device *pdev)
>  {
> +	struct device *dev = &pdev->dev;
>  	struct device_node *node, *pp;
>  	struct gpio_keys_platform_data *pdata;
>  	struct gpio_keys_button *button;
> @@ -681,9 +681,9 @@ gpio_keys_get_devtree_pdata(struct device *dev)
>  			button->active_low = flags & OF_GPIO_ACTIVE_LOW;
>  		}
>  
> -		button->irq = irq_of_parse_and_map(pp, 0);
> +		button->irq = platform_get_irq(pdev, 0);

There are a lot of current users of platform data that do not specify
button IRQ (and leave it as 0). If we start treating 0 as valid IRQ
number then we may break them.

I do not think that any real setup will actually use IRQ 0 as anything
but timer, so can we do:

		int irq;

		...

		irq = platform_get_irq(pdev, 0);
		if (irq < 0) {
			if (irq == -EPROBE_DEFER)
				return ERR_PTR(-EPROBE_DEFER);

			/* The driver treats IRQ 0 as invalid */
			irq = 0;
		}

		button->irq = irq;

and leave the rest of the code and users as is?


Or maybe inner condition should be:

			if (irq != -ENXIO)
				return ERR_PTR(irq);

Thanks.

Patch
diff mbox

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index d2b6c3acd9c32f1d..b6262d94aff19f70 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -30,7 +30,6 @@ 
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/of_gpio.h>
-#include <linux/of_irq.h>
 #include <linux/spinlock.h>
 
 struct gpio_button_data {
@@ -511,7 +510,7 @@  static int gpio_keys_setup_key(struct platform_device *pdev,
 						button->debounce_interval;
 		}
 
-		if (button->irq) {
+		if (button->irq >= 0) {
 			bdata->irq = button->irq;
 		} else {
 			irq = gpiod_to_irq(button->gpiod);
@@ -531,7 +530,7 @@  static int gpio_keys_setup_key(struct platform_device *pdev,
 		irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
 
 	} else {
-		if (!button->irq) {
+		if (button->irq < 0) {
 			dev_err(dev, "No IRQ specified\n");
 			return -EINVAL;
 		}
@@ -631,8 +630,9 @@  static void gpio_keys_close(struct input_dev *input)
  * Translate OpenFirmware node properties into platform_data
  */
 static struct gpio_keys_platform_data *
-gpio_keys_get_devtree_pdata(struct device *dev)
+gpio_keys_get_devtree_pdata(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct device_node *node, *pp;
 	struct gpio_keys_platform_data *pdata;
 	struct gpio_keys_button *button;
@@ -681,9 +681,9 @@  gpio_keys_get_devtree_pdata(struct device *dev)
 			button->active_low = flags & OF_GPIO_ACTIVE_LOW;
 		}
 
-		button->irq = irq_of_parse_and_map(pp, 0);
+		button->irq = platform_get_irq(pdev, 0);
 
-		if (!gpio_is_valid(button->gpio) && !button->irq) {
+		if (!gpio_is_valid(button->gpio) && button->irq < 0) {
 			dev_err(dev, "Found button without gpios or irqs\n");
 			return ERR_PTR(-EINVAL);
 		}
@@ -725,7 +725,7 @@  MODULE_DEVICE_TABLE(of, gpio_keys_of_match);
 #else
 
 static inline struct gpio_keys_platform_data *
-gpio_keys_get_devtree_pdata(struct device *dev)
+gpio_keys_get_devtree_pdata(struct platform_device *pdev)
 {
 	return ERR_PTR(-ENODEV);
 }
@@ -743,7 +743,7 @@  static int gpio_keys_probe(struct platform_device *pdev)
 	int wakeup = 0;
 
 	if (!pdata) {
-		pdata = gpio_keys_get_devtree_pdata(dev);
+		pdata = gpio_keys_get_devtree_pdata(pdev);
 		if (IS_ERR(pdata))
 			return PTR_ERR(pdata);
 	}
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index ee2d8c6f91300db7..0c04d38069535b86 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -30,7 +30,7 @@  struct gpio_keys_button {
 	int debounce_interval;
 	bool can_disable;
 	int value;
-	unsigned int irq;
+	int irq;
 	struct gpio_desc *gpiod;
 };