| Message ID | 20220523174238.28942-4-prabhakar.mahadev-lad.rj@bp.renesas.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Renesas RZ/G2L IRQC support | expand |
On Mon, May 23, 2022 at 7:43 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > Allow free() callback to be overridden from irq_domain_ops for > hierarchical chips. > > This allows drivers to free up resources which are allocated during > child_to_parent_hwirq()/populate_parent_alloc_arg() callbacks. > > On Renesas RZ/G2L platform a bitmap is maintained for TINT slots, a slot > is allocated in child_to_parent_hwirq() callback which is freed up in free > callback hence this override. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> So that function today looks like this: static void gpiochip_hierarchy_setup_domain_ops(struct irq_domain_ops *ops) { ops->activate = gpiochip_irq_domain_activate; ops->deactivate = gpiochip_irq_domain_deactivate; ops->alloc = gpiochip_hierarchy_irq_domain_alloc; ops->free = irq_domain_free_irqs_common; /* * We only allow overriding the translate() function for * hierarchical chips, and this should only be done if the user * really need something other than 1:1 translation. */ if (!ops->translate) ops->translate = gpiochip_hierarchy_irq_domain_translate; } (...) - ops->free = irq_domain_free_irqs_common; (...) > + if (!ops->free) > + ops->free = irq_domain_free_irqs_common; Marc Z is working on cleaning up the way that gpiolib is (ab)using irqchips. We definitely need his ACK if we do things like this. This doesn't look like one of the big offenders to me, but I want to make sure we don't create new problems while Marc is trying to solve the old ones. Yours, Linus Walleij
Hi Linus, Thank you for the feedback. On Tue, May 24, 2022 at 9:54 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Mon, May 23, 2022 at 7:43 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > Allow free() callback to be overridden from irq_domain_ops for > > hierarchical chips. > > > > This allows drivers to free up resources which are allocated during > > child_to_parent_hwirq()/populate_parent_alloc_arg() callbacks. > > > > On Renesas RZ/G2L platform a bitmap is maintained for TINT slots, a slot > > is allocated in child_to_parent_hwirq() callback which is freed up in free > > callback hence this override. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > So that function today looks like this: > > static void gpiochip_hierarchy_setup_domain_ops(struct irq_domain_ops *ops) > { > ops->activate = gpiochip_irq_domain_activate; > ops->deactivate = gpiochip_irq_domain_deactivate; > ops->alloc = gpiochip_hierarchy_irq_domain_alloc; > ops->free = irq_domain_free_irqs_common; > > /* > * We only allow overriding the translate() function for > * hierarchical chips, and this should only be done if the user > * really need something other than 1:1 translation. > */ > if (!ops->translate) > ops->translate = gpiochip_hierarchy_irq_domain_translate; > } > > (...) > - ops->free = irq_domain_free_irqs_common; > (...) > > + if (!ops->free) > > + ops->free = irq_domain_free_irqs_common; > > Marc Z is working on cleaning up the way that gpiolib is (ab)using > irqchips. We definitely need his ACK if we do things like this. > This doesn't look like one of the big offenders to me, but I want > to make sure we don't create new problems while Marc is trying > to solve the old ones. > Agreed, I had a discussion with Marc on v3 series [0]. [0] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220511183210.5248-4-prabhakar.mahadev-lad.rj@bp.renesas.com/ Cheers, Prabhakar
On Tue, May 24, 2022 at 11:07 AM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > On Tue, May 24, 2022 at 9:54 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > > > On Mon, May 23, 2022 at 7:43 PM Lad Prabhakar > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > > > Allow free() callback to be overridden from irq_domain_ops for > > > hierarchical chips. > > > > > > This allows drivers to free up resources which are allocated during > > > child_to_parent_hwirq()/populate_parent_alloc_arg() callbacks. > > > > > > On Renesas RZ/G2L platform a bitmap is maintained for TINT slots, a slot > > > is allocated in child_to_parent_hwirq() callback which is freed up in free > > > callback hence this override. > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > So that function today looks like this: > > > > static void gpiochip_hierarchy_setup_domain_ops(struct irq_domain_ops *ops) > > { > > ops->activate = gpiochip_irq_domain_activate; > > ops->deactivate = gpiochip_irq_domain_deactivate; > > ops->alloc = gpiochip_hierarchy_irq_domain_alloc; > > ops->free = irq_domain_free_irqs_common; > > > > /* > > * We only allow overriding the translate() function for > > * hierarchical chips, and this should only be done if the user > > * really need something other than 1:1 translation. > > */ > > if (!ops->translate) > > ops->translate = gpiochip_hierarchy_irq_domain_translate; > > } > > > > (...) > > - ops->free = irq_domain_free_irqs_common; > > (...) > > > + if (!ops->free) > > > + ops->free = irq_domain_free_irqs_common; > > > > Marc Z is working on cleaning up the way that gpiolib is (ab)using > > irqchips. We definitely need his ACK if we do things like this. > > This doesn't look like one of the big offenders to me, but I want > > to make sure we don't create new problems while Marc is trying > > to solve the old ones. > > > Agreed, I had a discussion with Marc on v3 series [0]. Hm yeah I guess I am just stepping on Marc's toes with all my mails :( I'll try to just wait for Marc's Reviewed-by instead and not add to the noise, I'm probably just wrong. Yours, Linus Walleij
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 690035124faa..8fcb9d23fea5 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1187,15 +1187,18 @@ static void gpiochip_hierarchy_setup_domain_ops(struct irq_domain_ops *ops) ops->activate = gpiochip_irq_domain_activate; ops->deactivate = gpiochip_irq_domain_deactivate; ops->alloc = gpiochip_hierarchy_irq_domain_alloc; - ops->free = irq_domain_free_irqs_common; /* - * We only allow overriding the translate() function for + * We only allow overriding the translate() and free() functions for * hierarchical chips, and this should only be done if the user - * really need something other than 1:1 translation. + * really need something other than 1:1 translation for translate() + * callback and free if user wants to free up any resources which + * were allocated during callbacks, for example populate_parent_alloc_arg. */ if (!ops->translate) ops->translate = gpiochip_hierarchy_irq_domain_translate; + if (!ops->free) + ops->free = irq_domain_free_irqs_common; } static int gpiochip_hierarchy_add_domain(struct gpio_chip *gc)