diff mbox series

[v7,3/5] gpio: gpiolib: Allow free() callback to be overridden

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

Commit Message

Lad, Prabhakar July 3, 2022, 7:40 p.m. UTC
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>
---
 drivers/gpio/gpiolib.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Andy Shevchenko July 4, 2022, 4:15 p.m. UTC | #1
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?
Lad, Prabhakar July 7, 2022, 5:51 p.m. UTC | #2
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 mbox series

Patch

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)