[v2,3/6] irq/domain: add a new callback to domain ops
diff mbox series

Message ID 20200211131240.15853-4-brgl@bgdev.pl
State New
Headers show
Series
  • irq/irq_sim: try to improve the API
Related show

Commit Message

Bartosz Golaszewski Feb. 11, 2020, 1:12 p.m. UTC
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Add the remove() callback to irq_domain_ops which can be used to
automatically dispose of any host data associated with the domain when
irq_domain_remove() is called.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 include/linux/irqdomain.h | 3 +++
 kernel/irq/irqdomain.c    | 3 +++
 2 files changed, 6 insertions(+)

Comments

Marc Zyngier March 8, 2020, 1:51 p.m. UTC | #1
On Tue, 11 Feb 2020 14:12:37 +0100
Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Add the remove() callback to irq_domain_ops which can be used to
> automatically dispose of any host data associated with the domain when
> irq_domain_remove() is called.

I have a hard time buying this. Whatever data that is associated to the
domain is already owned known by whoever created the domain the first
place.

Since the expected use case is that whoever created the domain also
destroys it, the caller is already in a position to do its own cleanup,
and we don't need any of this.

So please explain what you are trying to achieve here.

Thanks,

	M.
Bartosz Golaszewski March 8, 2020, 5:59 p.m. UTC | #2
niedz., 8 mar 2020 o 14:51 Marc Zyngier <maz@kernel.org> napisał(a):
>
> On Tue, 11 Feb 2020 14:12:37 +0100
> Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Add the remove() callback to irq_domain_ops which can be used to
> > automatically dispose of any host data associated with the domain when
> > irq_domain_remove() is called.
>
> I have a hard time buying this. Whatever data that is associated to the
> domain is already owned known by whoever created the domain the first
> place.
>
> Since the expected use case is that whoever created the domain also
> destroys it, the caller is already in a position to do its own cleanup,
> and we don't need any of this.
>
> So please explain what you are trying to achieve here.
>

I'm mainly trying to remove irq_domain_remove_sim() and make it
possible to destroy the interrupt simulator domain with regular
irq_domain_remove(). If you prefer that we retain this routine as is,
I can limit this series to the first two patches, but I assumed the
fewer functions in the interface, the better. If you have a different
idea on how to do this - please let me know too.

Bartosz
Bartosz Golaszewski March 12, 2020, 8:15 a.m. UTC | #3
niedz., 8 mar 2020 o 18:59 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a):
>
> niedz., 8 mar 2020 o 14:51 Marc Zyngier <maz@kernel.org> napisał(a):
> >
> > On Tue, 11 Feb 2020 14:12:37 +0100
> > Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > >
> > > Add the remove() callback to irq_domain_ops which can be used to
> > > automatically dispose of any host data associated with the domain when
> > > irq_domain_remove() is called.
> >
> > I have a hard time buying this. Whatever data that is associated to the
> > domain is already owned known by whoever created the domain the first
> > place.
> >
> > Since the expected use case is that whoever created the domain also
> > destroys it, the caller is already in a position to do its own cleanup,
> > and we don't need any of this.
> >
> > So please explain what you are trying to achieve here.
> >
>
> I'm mainly trying to remove irq_domain_remove_sim() and make it
> possible to destroy the interrupt simulator domain with regular
> irq_domain_remove(). If you prefer that we retain this routine as is,
> I can limit this series to the first two patches, but I assumed the
> fewer functions in the interface, the better. If you have a different
> idea on how to do this - please let me know too.
>
> Bartosz

While this is being discussed - are the first two patches
uncontroversial enough to make it into v5.7?

Bartosz
Bartosz Golaszewski March 20, 2020, 9:38 a.m. UTC | #4
czw., 12 mar 2020 o 09:15 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a):
>
> niedz., 8 mar 2020 o 18:59 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a):
> >
> > niedz., 8 mar 2020 o 14:51 Marc Zyngier <maz@kernel.org> napisał(a):
> > >
> > > On Tue, 11 Feb 2020 14:12:37 +0100
> > > Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > >
> > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > >
> > > > Add the remove() callback to irq_domain_ops which can be used to
> > > > automatically dispose of any host data associated with the domain when
> > > > irq_domain_remove() is called.
> > >
> > > I have a hard time buying this. Whatever data that is associated to the
> > > domain is already owned known by whoever created the domain the first
> > > place.
> > >
> > > Since the expected use case is that whoever created the domain also
> > > destroys it, the caller is already in a position to do its own cleanup,
> > > and we don't need any of this.
> > >
> > > So please explain what you are trying to achieve here.
> > >
> >
> > I'm mainly trying to remove irq_domain_remove_sim() and make it
> > possible to destroy the interrupt simulator domain with regular
> > irq_domain_remove(). If you prefer that we retain this routine as is,
> > I can limit this series to the first two patches, but I assumed the
> > fewer functions in the interface, the better. If you have a different
> > idea on how to do this - please let me know too.
> >
> > Bartosz
>
> While this is being discussed - are the first two patches
> uncontroversial enough to make it into v5.7?
>
> Bartosz

Gentle ping after another week.

Bart

Patch
diff mbox series

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 20d38621e2f8..fbc25f464f62 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -95,6 +95,8 @@  enum irq_domain_bus_token {
  * @unmap: Dispose of such a mapping
  * @xlate: Given a device tree node and interrupt specifier, decode
  *         the hardware irq number and linux irq type value.
+ * @remove: Free any custom resources associated with this domain. This is
+ *          called from irq_domain_remove() before any other code.
  *
  * Functions below are provided by the driver and called whenever a new mapping
  * is created or an old mapping is disposed. The driver can then proceed to
@@ -126,6 +128,7 @@  struct irq_domain_ops {
 	void (*debug_show)(struct seq_file *m, struct irq_domain *d,
 			   struct irq_data *irqd, int ind);
 #endif
+	void (*remove)(struct irq_domain *d);
 };
 
 extern struct irq_domain_ops irq_generic_chip_ops;
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 039427c98af8..b391d2e65bdd 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -242,6 +242,9 @@  EXPORT_SYMBOL_GPL(__irq_domain_add);
  */
 void irq_domain_remove(struct irq_domain *domain)
 {
+	if (domain->ops->remove)
+		domain->ops->remove(domain);
+
 	mutex_lock(&irq_domain_mutex);
 	debugfs_remove_domain_dir(domain);