Message ID | 20230621174943.30302-5-andriy.shevchenko@linux.intel.com |
---|---|
State | New |
Headers | show |
Series | [v1,1/5] gpiolib: Make gpiochip_hierarchy_add_domain() return domain | expand |
On Wed, Jun 21, 2023 at 7:49 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > Replace open coded variant of gpiochip_irqchip_add_allocated_domain() > in gpiochip_add_irqchip(). > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> and this concludes patches 4,5 very nicely as well. Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Fri, Jun 30, 2023 at 2:52 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Wed, Jun 21, 2023 at 7:49 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > Replace open coded variant of gpiochip_irqchip_add_allocated_domain() > > in gpiochip_add_irqchip(). > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > and this concludes patches 4,5 very nicely as well. Yep! > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Thank you for the review! While at it, I would like to ask on ->to_irq() callback. IIUC assigning it with an IRQ chip makes a dead code in the driver. Am I correct? If not, can somebody shed some light on how the RT5677 driver, for example, works with GPIO IRQ?
On Fri, Jun 30, 2023 at 3:58 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > While at it, I would like to ask on ->to_irq() callback. IIUC > assigning it with an IRQ chip makes a dead code in the driver. Am I > correct? It's fine to assign it with an IRQ chip but not with GPIOLIB_IRQCHIP, i.e. gc->irq better be NULL. > If not, can somebody shed some light on how the RT5677 > driver, for example, works with GPIO IRQ? That is theoretically fine (I don't know this HW in particular). It looks a bit fragile... It's just a helper to translate a GPIO line to the corresponding Linux IRQ number and when using GPIOLIB_IRQCHIP the GPIO core will do this using the irqdomain, else it is up to the driver. Yours, Linus Walleij
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 59d87e60b108..bc8b9d6afe0e 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1642,6 +1642,9 @@ static int gpiochip_irqchip_add_allocated_domain(struct gpio_chip *gc, if (!domain) return -EINVAL; + if (gc->to_irq) + chip_warn(gc, "to_irq is redefined in %s and you shouldn't rely on it\n", __func__); + gc->to_irq = gpiochip_to_irq; gc->irq.domain = domain; gc->irq.domain_is_allocated_externally = allocated_externally; @@ -1672,6 +1675,7 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc, struct irq_domain *domain; unsigned int type; unsigned int i; + int ret; if (!irqchip) return 0; @@ -1692,10 +1696,6 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc, "%pfw: Ignoring %u default trigger\n", fwnode, type)) type = IRQ_TYPE_NONE; - if (gc->to_irq) - chip_warn(gc, "to_irq is redefined in %s and you shouldn't rely on it\n", __func__); - - gc->to_irq = gpiochip_to_irq; gc->irq.default_type = type; gc->irq.lock_key = lock_key; gc->irq.request_key = request_key; @@ -1708,7 +1708,6 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc, } if (IS_ERR(domain)) return PTR_ERR(domain); - gc->irq.domain = domain; if (gc->irq.parent_handler) { for (i = 0; i < gc->irq.num_parents; i++) { @@ -1732,14 +1731,9 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc, gpiochip_set_irq_hooks(gc); - /* - * Using barrier() here to prevent compiler from reordering - * gc->irq.initialized before initialization of above - * GPIO chip irq members. - */ - barrier(); - - gc->irq.initialized = true; + ret = gpiochip_irqchip_add_allocated_domain(gc, domain, false); + if (ret) + return ret; acpi_gpiochip_request_interrupts(gc);
Replace open coded variant of gpiochip_irqchip_add_allocated_domain() in gpiochip_add_irqchip(). Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/gpio/gpiolib.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)