| Message ID | 20220703194020.78701-4-prabhakar.mahadev-lad.rj@bp.renesas.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v7,1/5] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller | expand |
On Sun, Jul 3, 2022 at 9:43 PM Lad Prabhakar <prabhakar.csengg@gmail.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. Hmm... To me this sounds asymmetrical. We alloc something in another callback, which is not what this free is for. Perhaps it should be an optional free_populated_parent_arg() or alike?
On Wed, Jul 6, 2022 at 8:01 AM Marc Zyngier <maz@kernel.org> wrote: > > On Tue, 05 Jul 2022 05:53:03 +0100, > "Lad, Prabhakar" <prabhakar.csengg@gmail.com> wrote: > > > > On Mon, Jul 4, 2022 at 5:16 PM Andy Shevchenko > > <andy.shevchenko@gmail.com> wrote: > > > > > > On Sun, Jul 3, 2022 at 9:43 PM Lad Prabhakar <prabhakar.csengg@gmail.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. > > > > > > Hmm... To me this sounds asymmetrical. We alloc something in another > > > callback, which is not what this free is for. Perhaps it should be an > > > optional > > > > > > free_populated_parent_arg() or alike? > > > > > @Marc your thoughts? > > I think there are enough optional callbacks, and we don't currently > have a clear picture of how this may be used in the future, specially > based on a sample of *one*. > > Let's get it in as is, and tweak things as we go, should another user > require a slightly different behaviour. This also saves us the debate > around the naming, which is always pretty useless. > Thanks, I will repost it as is. Cheers, Prabhakar
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index bfde94243752..68d9f95d7799 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1181,15 +1181,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)