| Message ID | 1455876982-6743-3-git-send-email-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Under Review |
| Headers | show |
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.
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; };
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(-)