On 05/16/2018 07:49 AM, Linus Walleij wrote:
On Fri, May 4, 2018 at 12:11 AM, Chris Lesiak <chris.lesiak@xxxxxxxxx> wrote:Fix incorrect logic in gpiochip_irqchip_add_key(). A driver needs to override both irq_request_resources and irq_request_resources, otherwise gpiochip_irq_reqres and gpiochip_irq_relres should be used. Signed-off-by: Chris Lesiak <chris.lesiak@xxxxxxxxx> --- drivers/gpio/gpiolib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index bdd68ff197dc..3e441879fea7 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1831,7 +1831,7 @@ int gpiochip_irqchip_add_key(struct gpio_chip *gpiochip, * It is possible for a driver to override this, but only if the * alternative functions are both implemented. */ - if (!irqchip->irq_request_resources && + if (!irqchip->irq_request_resources || !irqchip->irq_release_resources) { irqchip->irq_request_resources = gpiochip_irq_reqres; irqchip->irq_release_resources = gpiochip_irq_relres;This code was refactored by Thierry, but the logic is from Rabin's patch commit 8b67a1f0ad1f260f1a4032d5f7b032ac113bfa7d "gpio: don't override irq_*_resources() callbacks" I think the intention was to only allow gpiolib's implementation to kick in if the driver provided no callbacks at all.
I agree that is what Rabin's patch implemented, but it doesn't match the patches comment.
The code in kernel/irq/manage.c is certainly happy of one (or both) of irqchip->irq_request_resources and irqchip->irq_release_resources are null. But it seems likely that a driver, if needing to override at all, would need to override the pair. The existing drivers follow that pattern.
As such, I think Rabin's comment was correct, but his code wrong. -- Chris Lesiak Principal Design Engineer, Software LI-COR Biosciences chris.lesiak@xxxxxxxxx Any opinions expressed are those of the author and do not necessarily represent those of his employer. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
