| Message ID | 20200211131240.15853-4-brgl@bgdev.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | irq/irq_sim: try to improve the API | expand |
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.
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
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
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
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);