| Message ID | 1429630951-27082-4-git-send-email-johan@kernel.org |
|---|---|
| State | New |
| Headers | show |
On Tue, Apr 21, 2015 at 5:42 PM, Johan Hovold <johan@kernel.org> wrote: > Drop redundant lock-as-irq in gpio_setup_irq, which has already been > handled when requesting and releasing the irq (i.e. in the irq chip > irq_request_resources and irq_release_resources callbacks). Well we would hope they all do that. And I hope for the vast majority that is true, but there is a TODO to go over all gpiochip drivers (some which are elsewhere in the kernel than drivers/gpio) and make sure they actually do so. Right now it's a bit arbitrary if so happens, and in not marked by the driver as IRQ then this kicks in and provides an additional protection. But maybe that's overzealous, what do people say? Yours, Linus Walleij -- 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
On Wed, Apr 29, 2015 at 11:48:57PM +0200, Linus Walleij wrote: > On Tue, Apr 21, 2015 at 5:42 PM, Johan Hovold <johan@kernel.org> wrote: > > > Drop redundant lock-as-irq in gpio_setup_irq, which has already been > > handled when requesting and releasing the irq (i.e. in the irq chip > > irq_request_resources and irq_release_resources callbacks). > > Well we would hope they all do that. And I hope for the vast majority > that is true, but there is a TODO to go over all gpiochip drivers > (some which are elsewhere in the kernel than drivers/gpio) and > make sure they actually do so. > > Right now it's a bit arbitrary if so happens, and in not marked by > the driver as IRQ then this kicks in and provides an additional > protection. > > But maybe that's overzealous, what do people say? No, you're right. The drivers that fail to do this needs to be fixed, but the "redundant" lock-as-irq in the sysfs interface should not be removed before that. I'll respin the series and add it back with a comment explaining why gpiochip_lock_as_irq is currently called twice. Thanks, Johan -- 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
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c index af3bc7a8033b..d87f595a02ce 100644 --- a/drivers/gpio/gpiolib-sysfs.c +++ b/drivers/gpio/gpiolib-sysfs.c @@ -161,7 +161,6 @@ static int gpio_setup_irq(struct gpio_desc *desc, struct device *dev, desc->flags &= ~GPIO_TRIGGER_MASK; if (!gpio_flags) { - gpiochip_unlock_as_irq(desc->chip, gpio_chip_hwgpio(desc)); ret = 0; goto free_id; } @@ -200,12 +199,6 @@ static int gpio_setup_irq(struct gpio_desc *desc, struct device *dev, if (ret < 0) goto free_id; - ret = gpiochip_lock_as_irq(desc->chip, gpio_chip_hwgpio(desc)); - if (ret < 0) { - gpiod_warn(desc, "failed to flag the GPIO for IRQ\n"); - goto free_id; - } - desc->flags |= gpio_flags; return 0;
Drop redundant lock-as-irq in gpio_setup_irq, which has already been handled when requesting and releasing the irq (i.e. in the irq chip irq_request_resources and irq_release_resources callbacks). Note that we would be leaking the irq in the gpiochip_lock_as_irq error path if the (second) explicit call ever failed. Signed-off-by: Johan Hovold <johan@kernel.org> --- drivers/gpio/gpiolib-sysfs.c | 7 ------- 1 file changed, 7 deletions(-)