irq/irq_sim: try to improve the API
Subsystems (6)
v2 cover irq/irq_sim: try to improve the API
From: Bartosz Golaszewski <[email protected]>
This is my second take at improving the interrupt simulator interface. I marked it as v2 but it actually takes a completely different approach.
The interrupt simulator API exposes a lot of custom data structures and functions and doesn't reuse the interfaces already exposed by the irq subsystem. This series tries to address it.
First, we make irq_domain_reset_irq_data() available to non-V2 domain API users - that'll be used in the subsequent patch. Next we overhaul the public interfaces - we hide all specific data structures and instead rely on the irq_domain struct and virtual interrupt numberspace.
Next four patches simplify the interface even more, but since the change may be a bit more controversial due to modification of the irq_domain I decided to split them out of the second patch.
In patch 3/6 we're adding a new callback to irq_domain_ops that is called right before all the other code in irq_domain_remove(). Next we use it to remove the simulator-specific cleanup function from irq_sim.h - users now can simply use the regular irq_domain_remove().
Last two patches show that the new callback isn't limited to the interrupt simulator and can be used to shrink code in real driver too. We introduce a new helper for a common use case of disposing of all mappings before removing the irq_domain and use it in the keystone irqchip driver.
The end effect is that we limit the interrupt simulator API to two functions (plus one device managed variant) and zero new structures.
v1: https://lkml.org/lkml/2019/8/12/558
v1 -> v2:
- instead of just making the new data structures opaque for users, remove them entirely in favor of irq_domain
- call irq_set_handler() & irq_domain_reset_irq_data() when unmapping the simulated interrupt
- fix a memory leak in error path
- make it possible to use irq_find_matching_fwnode() with the simulator domain
- correctly use irq_create_mapping() and irq_find_mapping(): only use the former at init-time and the latter at interrupt-time
Bartosz Golaszewski (6):
irq: make irq_domain_reset_irq_data() available even for non-V2 users irq/irq_sim: simplify the API
irq/domain: add a new callback to domain ops
irq/irq_sim: remove irq_domain_remove_sim()
irq/domain: provide irq_domain_dispose_mappings() helper
irqchip: keystone: use irq_domain_dispose_mappings()
drivers/gpio/gpio-mockup.c | 47 ++++-- drivers/iio/dummy/iio_dummy_evgen.c | 32 ++-- drivers/irqchip/irq-keystone.c | 5 +- include/linux/irq_sim.h | 33 ++-- include/linux/irqdomain.h | 9 +- kernel/irq/Kconfig | 1 + kernel/irq/irq_sim.c | 225 ++++++++++++++++------------ kernel/irq/irqdomain.c | 44 ++++-- 8 files changed, 238 insertions(+), 158 deletions(-)
Signature · 1 line
2.25.0
PATCH v2 1/6 irq: make irq_domain_reset_irq_data() available even for non-V2 users
From: Bartosz Golaszewski <[email protected]>
irq_domain_reset_irq_data() doesn't modify the parent data, so it can be made available even if irq domain hierarchy is not being built.
--- include/linux/irqdomain.h | 4 ++-- kernel/irq/irqdomain.c | 24 ++++++++++++------------ 2 files changed, 14 insertions(+), 14 deletions(-)
include/linux/irqdomain.h +2 -2
@@ -450,6 +450,7 @@ extern void irq_domain_set_info(struct irq_domain *domain, unsigned int virq, irq_hw_number_t hwirq, struct irq_chip *chip, void *chip_data, irq_flow_handler_t handler, void *handler_data, const char *handler_name); +extern void irq_domain_reset_irq_data (struct irq_data *irq_data);#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY extern struct irq_domain *irq_domain_create_hierarchy(struct irq_domain *parent, unsigned int flags, unsigned int size,
@@ -475,7 +476,6 @@ extern int __irq_domain_alloc_irqs(struct irq_domain *domain, int irq_base,extern void irq_domain_free_irqs (unsigned int virq,unsigned int nr_irqs);extern int irq_domain_activate_irq (struct irq_data *irq_data,bool early);extern void irq_domain_deactivate_irq (struct irq_data *irq_data); -static inline int irq_domain_alloc_irqs (struct irq_domain *domain,unsigned int nr_irqs,int node,void *arg) {
@@ -491,7 +491,7 @@ extern int irq_domain_set_hwirq_and_chip(struct irq_domain *domain,irq_hw_number_t hwirq, struct irq_chip *chip, void *chip_data); -extern void irq_domain_reset_irq_data (struct irq_data *irq_data); +extern void irq_domain_free_irqs_common (struct irq_domain *domain,unsigned int virq,unsigned int nr_irqs);
kernel/irq/irqdomain.c +12 -12
@@ -1047,6 +1047,18 @@ int irq_domain_alloc_descs(int virq, unsigned int cnt, irq_hw_number_t hwirq,return virq; } +/** +* irq_domain_reset_irq_data - Clear hwirq, chip and chip_data in @irq_data +* @irq_data: The pointer to irq_data +*/ +void irq_domain_reset_irq_data (struct irq_data *irq_data) +{ + irq_data->hwirq =0 ; + irq_data->chip = &no_irq_chip; + irq_data->chip_data =NULL ; +} +EXPORT_SYMBOL_GPL (irq_domain_reset_irq_data); +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY /** * irq_domain_create_hierarchy - Add a irqdomain into the hierarchy
@@ -1247,18 +1259,6 @@ void irq_domain_set_info(struct irq_domain *domain, unsigned int virq, }EXPORT_SYMBOL (irq_domain_set_info); -/** -* irq_domain_reset_irq_data - Clear hwirq, chip and chip_data in @irq_data -* @irq_data: The pointer to irq_data -*/ -void irq_domain_reset_irq_data (struct irq_data *irq_data) -{ - irq_data->hwirq =0 ; - irq_data->chip = &no_irq_chip; - irq_data->chip_data =NULL ; -} -EXPORT_SYMBOL_GPL (irq_domain_reset_irq_data); -/** * irq_domain_free_irqs_common - Clear irq_data and free the parent * @domain: Interrupt domain to match
Signature · 1 line
2.25.0
Re: [PATCH v2 1/6] irq: make irq_domain_reset_irq_data() available even for non-V2 users
Quoted 4 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
irq_domain_reset_irq_data()doesn't modify the parent data, so it can be made available even if irq domain hierarchy is not being built.
Yes, it can be made available, but WHY?
Thanks,
tglx
Re: [PATCH v2 1/6] irq: make irq_domain_reset_irq_data() available even for non-V2 users
pt., 14 lut 2020 o 10:02 Thomas Gleixner <[email protected]> napisał(a):
Quoted 4 lines
Quoted 2 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Quoted 8 lines
Quoted 2 lines
irq_domain_reset_irq_data()doesn't modify the parent data, so it can be made available even if irq domain hierarchy is not being built.Yes, it can be made available, but WHY?
Thanks,
tglx
Duh, I forgot to clarify this. After my previous submission, Marc suggested I use irq_domain_reset_irq_data() in the unmap() callback for the simulator domain. I noticed I can't do this because I don't select the V2 irq domain API, so I added this patch.
Bartosz
PATCH v2 2/6 irq/irq_sim: simplify the API
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
The interrupt simulator API exposes a lot of custom data structures and functions and doesn't reuse the interfaces already exposed by the irq subsystem. This patch tries to address it.
We hide all the simulator-related data structures from users and instead rely on the well-known irq domain. When creating the interrupt simulator the user receives a pointer to a newly created irq_domain and can use it to create mappings for simulated interrupts.
It is also possible to pass a handle to fwnode when creating the simulator domain and retrieve it using irq_find_matching_fwnode().
The irq_sim_fire() function now only takes the virtual interrupt number as argument - there's no need anymore to pass it any data structure linked to the simulator.
We modify the two modules that use the simulator at the same time as adding these changes in order to reduce the intermediate bloat that would result when trying to migrate the drivers in separate patches.
--- drivers/gpio/gpio-mockup.c | 47 ++++-- drivers/iio/dummy/iio_dummy_evgen.c | 32 ++-- include/linux/irq_sim.h | 34 ++--- kernel/irq/Kconfig | 1 + kernel/irq/irq_sim.c | 225 +++++++++++++++++----------- 5 files changed, 202 insertions(+), 137 deletions(-)
drivers/gpio/gpio-mockup.c +37 -10
@@ -14,6 +14,7 @@#include <linux/interrupt.h> #include <linux/irq.h> #include <linux/irq_sim.h> +#include <linux/irqdomain.h> #include <linux/module.h> #include <linux/platform_device.h> #include <linux/property.h>
@@ -48,7 +49,7 @@ struct gpio_mockup_line_status {struct gpio_mockup_chip {struct gpio_chip gc;struct gpio_mockup_line_status *lines; -struct irq_sim irqsim; +struct irq_domain *irq_sim_domain;struct dentry *dbg_dir;struct mutex lock; };
@@ -144,14 +145,12 @@ static void gpio_mockup_set_multiple(struct gpio_chip *gc,static int gpio_mockup_apply_pull (struct gpio_mockup_chip *chip,unsigned int offset,int value) { +int curr, irq, irq_type, ret =0 ;struct gpio_desc *desc;struct gpio_chip *gc; -struct irq_sim *sim; -int curr, irq, irq_type; gc = &chip->gc; desc = &gc->gpiodev->descs[offset]; - sim = &chip->irqsim;mutex_lock (&chip->lock);
@@ -161,14 +160,24 @@ static int gpio_mockup_apply_pull(struct gpio_mockup_chip *chip,if (curr == value) goto out; - irq =irq_sim_irqnum (sim, offset); + irq =irq_find_mapping (chip->irq_sim_domain, offset); +if (!irq) +/* +* This is fine - it just means, nobody is listening +* for interrupts on this line, otherwise +* irq_create_mapping() would have been called from +* the to_irq() callback. +*/ + goto set_value; + irq_type =irq_get_trigger_type (irq);if ((value ==1 && (irq_type &IRQ_TYPE_EDGE_RISING )) || (value ==0 && (irq_type &IRQ_TYPE_EDGE_FALLING ))) -irq_sim_fire (sim, offset); +irq_sim_fire (irq); } +set_value:/* Change the value unless we're actively driving the line. */ if (!test_bit (FLAG_REQUESTED , &desc->flags) || !test_bit (FLAG_IS_OUT , &desc->flags))
@@ -177,7 +186,7 @@ static int gpio_mockup_apply_pull(struct gpio_mockup_chip *chip, out: chip->lines[offset].pull = value;mutex_unlock (&chip->lock); -return 0 ; +return ret; } static int gpio_mockup_set_config(struct gpio_chip *gc,
@@ -236,7 +245,7 @@ static int gpio_mockup_to_irq(struct gpio_chip *gc, unsigned int offset) {struct gpio_mockup_chip *chip =gpiochip_get_data (gc); -return irq_sim_irqnum (&chip->irqsim, offset); +return irq_create_mapping (chip->irq_sim_domain, offset); }static void gpio_mockup_free (struct gpio_chip *gc,unsigned int offset)
@@ -389,6 +398,19 @@ static int gpio_mockup_name_lines(struct device *dev,return 0 ; } +static void gpio_mockup_dispose_mappings (void *data) +{ +struct gpio_mockup_chip *chip = data; +struct gpio_chip *gc = &chip->gc; +int i, irq; + +for (i =0 ; i < gc->ngpio; i++) { + irq =irq_find_mapping (chip->irq_sim_domain, i); +if (irq) +irq_dispose_mapping (irq); + } +} +static int gpio_mockup_probe (struct platform_device *pdev) {struct gpio_mockup_chip *chip;
@@ -456,8 +478,13 @@ static int gpio_mockup_probe(struct platform_device *pdev)return rv; } - rv =devm_irq_sim_init (dev, &chip->irqsim, gc->ngpio); -if (rv <0 ) + chip->irq_sim_domain =devm_irq_domain_create_sim (dev,NULL , + gc->ngpio); +if (IS_ERR (chip->irq_sim_domain)) +return PTR_ERR (chip->irq_sim_domain); + + rv =devm_add_action_or_reset (dev, gpio_mockup_dispose_mappings, chip); +if (rv)return rv; rv =devm_gpiochip_add_data (dev, &chip->gc, chip);
drivers/iio/dummy/iio_dummy_evgen.c +18 -14
@@ -37,8 +37,7 @@ struct iio_dummy_eventgen {struct iio_dummy_regs regs[IIO_EVENTGEN_NO ];struct mutex lock;bool inuse[IIO_EVENTGEN_NO ]; -struct irq_sim irq_sim; -int base; +struct irq_domain *irq_sim_domain; };/* We can only ever have one instance of this 'device' */
@@ -46,19 +45,17 @@ static struct iio_dummy_eventgen *iio_evgen;static int iio_dummy_evgen_create (void ) { -int ret; - iio_evgen =kzalloc (sizeof (*iio_evgen),GFP_KERNEL );if (!iio_evgen)return -ENOMEM ; - ret =irq_sim_init (&iio_evgen->irq_sim,IIO_EVENTGEN_NO ); -if (ret <0 ) { + iio_evgen->irq_sim_domain =irq_domain_create_sim (NULL , +IIO_EVENTGEN_NO ); +if (IS_ERR (iio_evgen->irq_sim_domain)) {kfree (iio_evgen); -return ret; +return PTR_ERR (iio_evgen->irq_sim_domain); } - iio_evgen->base =irq_sim_irqnum (&iio_evgen->irq_sim,0 );mutex_init (&iio_evgen->lock);return 0 ;
@@ -80,7 +77,7 @@ int iio_dummy_evgen_get_irq(void)mutex_lock (&iio_evgen->lock);for (i =0 ; i <IIO_EVENTGEN_NO ; i++) {if (!iio_evgen->inuse[i]) { - ret =irq_sim_irqnum (&iio_evgen->irq_sim, i); + ret =irq_create_mapping (iio_evgen->irq_sim_domain, i); iio_evgen->inuse[i] = true;break ; }
@@ -101,21 +98,27 @@ EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_irq);*/ void iio_dummy_evgen_release_irq (int irq) { +struct irq_data *irqd =irq_get_irq_data (irq); +mutex_lock (&iio_evgen->lock); - iio_evgen->inuse[irq - iio_evgen->base] = false; + iio_evgen->inuse[irqd_to_hwirq (irqd)] = false; +irq_dispose_mapping (irq);mutex_unlock (&iio_evgen->lock); }EXPORT_SYMBOL_GPL (iio_dummy_evgen_release_irq);struct iio_dummy_regs *iio_dummy_evgen_get_regs (int irq) { -return &iio_evgen->regs[irq - iio_evgen->base]; +struct irq_data *irqd =irq_get_irq_data (irq); + +return &iio_evgen->regs[irqd_to_hwirq (irqd)]; + }EXPORT_SYMBOL_GPL (iio_dummy_evgen_get_regs);static void iio_dummy_evgen_free (void ) { -irq_sim_fini (&iio_evgen->irq_sim); +irq_domain_remove_sim (iio_evgen->irq_sim_domain);kfree (iio_evgen); }
@@ -131,7 +134,7 @@ static ssize_t iio_evgen_poke(struct device *dev, {struct iio_dev_attr *this_attr =to_iio_dev_attr (attr);unsigned long event; -int ret; +int ret, irq; ret =kstrtoul (buf,10 , &event);if (ret)
@@ -140,7 +143,8 @@ static ssize_t iio_evgen_poke(struct device *dev, iio_evgen->regs[this_attr->address].reg_id = this_attr->address; iio_evgen->regs[this_attr->address].reg_data = event; -irq_sim_fire (&iio_evgen->irq_sim, this_attr->address); + irq =irq_find_mapping (iio_evgen->irq_sim_domain, this_attr->address); +irq_sim_fire (irq);return len; }
include/linux/irq_sim.h +10 -24
@@ -1,41 +1,27 @@/* SPDX-License-Identifier: GPL-2.0+ */ /* * Copyright (C) 2017-2018 Bartosz Golaszewski <[email protected]> +* Copyright (C) 2020 Bartosz Golaszewski <[email protected]> */ #ifndef _LINUX_IRQ_SIM_H#define _LINUX_IRQ_SIM_H -#include <linux/irq_work.h> #include <linux/device.h> +#include <linux/fwnode.h> +#include <linux/irqdomain.h> /* * Provides a framework for allocating simulated interrupts which can be * requested like normal irqs and enqueued from process context. */ -struct irq_sim_work_ctx { -struct irq_work work; -unsigned long *pending; -}; - -struct irq_sim_irq_ctx { -int irqnum; -bool enabled; -}; - -struct irq_sim { -struct irq_sim_work_ctx work_ctx; -int irq_base; -unsigned int irq_count; -struct irq_sim_irq_ctx *irqs; -}; - -int irq_sim_init (struct irq_sim *sim,unsigned int num_irqs); -int devm_irq_sim_init (struct device *dev,struct irq_sim *sim, -unsigned int num_irqs); -void irq_sim_fini (struct irq_sim *sim); -void irq_sim_fire (struct irq_sim *sim,unsigned int offset); -int irq_sim_irqnum (struct irq_sim *sim,unsigned int offset); +struct irq_domain *irq_domain_create_sim (struct fwnode_handle *fwnode, +unsigned int num_irqs); +struct irq_domain *devm_irq_domain_create_sim (struct device *dev, +struct fwnode_handle *fwnode, +unsigned int num_irqs); +void irq_domain_remove_sim (struct irq_domain *domain); +void irq_sim_fire (int virq);#endif /* _LINUX_IRQ_SIM_H */
kernel/irq/Kconfig +1 -0
@@ -68,6 +68,7 @@ config IRQ_DOMAIN config IRQ_SIM bool select IRQ_WORK + select IRQ_DOMAIN # Support for hierarchical irq domains config IRQ_DOMAIN_HIERARCHY
kernel/irq/irq_sim.c +136 -89
298 lines not on this page · raw message
Signature · 1 line
2.25.0
PATCH v2 3/6 irq/domain: add a new callback to domain ops
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
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.
--- include/linux/irqdomain.h | 3 +++ kernel/irq/irqdomain.c | 3 +++ 2 files changed, 6 insertions(+)
include/linux/irqdomain.h +3 -0
@@ -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;
kernel/irq/irqdomain.c +3 -0
@@ -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);
Signature · 1 line
2.25.0
Re: [PATCH v2 3/6] irq/domain: add a new callback to domain ops
Quoted 5 lines · On Tue, 11 Feb 2020 14:12:37 +0100 Bartosz Golaszewski <[email protected]> wrote:
From: Bartosz Golaszewski <[email protected]>
Add the
remove()callback toirq_domain_opswhich can be used to automatically dispose of any host data associated with the domain whenirq_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.
Signature · 1 line
Jazz is not dead. It just smells funny...
Re: [PATCH v2 3/6] irq/domain: add a new callback to domain ops
niedz., 8 mar 2020 o 14:51 Marc Zyngier <[email protected]> napisał(a):
Quoted 14 lines
Quoted 5 lines · On Tue, 11 Feb 2020 14:12:37 +0100 Bartosz Golaszewski <[email protected]> wrote:
From: Bartosz Golaszewski <[email protected]>
Add the
remove()callback toirq_domain_opswhich can be used to automatically dispose of any host data associated with the domain whenirq_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.
Quoted 6 lines
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
Re: [PATCH v2 3/6] irq/domain: add a new callback to domain ops
niedz., 8 mar 2020 o 18:59 Bartosz Golaszewski <[email protected]> napisał(a):
Quoted 23 lines
niedz., 8 mar 2020 o 14:51 Marc Zyngier <[email protected]> napisał(a):
Quoted 14 lines
Quoted 5 lines · On Tue, 11 Feb 2020 14:12:37 +0100 Bartosz Golaszewski <[email protected]> wrote:
From: Bartosz Golaszewski <[email protected]>
Add the
remove()callback toirq_domain_opswhich can be used to automatically dispose of any host data associated with the domain whenirq_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.
Quoted 6 lines
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.
Quoted 8 lines
I'm mainly trying to remove
irq_domain_remove_sim()and make it possible to destroy the interrupt simulator domain with regularirq_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
Re: [PATCH v2 3/6] irq/domain: add a new callback to domain ops
czw., 12 mar 2020 o 09:15 Bartosz Golaszewski <[email protected]> napisał(a):
Quoted 32 lines
niedz., 8 mar 2020 o 18:59 Bartosz Golaszewski <[email protected]> napisał(a):
Quoted 30 lines
niedz., 8 mar 2020 o 14:51 Marc Zyngier <[email protected]> napisał(a):
Quoted 14 lines
Quoted 5 lines · On Tue, 11 Feb 2020 14:12:37 +0100 Bartosz Golaszewski <[email protected]> wrote:
From: Bartosz Golaszewski <[email protected]>
Add the
remove()callback toirq_domain_opswhich can be used to automatically dispose of any host data associated with the domain whenirq_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.
Quoted 6 lines
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 regularirq_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.
Quoted 6 lines
Quoted 1 line
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 v2 4/6 irq/irq_sim: remove irq_domain_remove_sim()
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Use the new remove() callback in irq_domain_ops to dispose of any private data associated with the sim domain. This allows us to drop the dedicated irq_domain_remove_sim() function and instead just use the regular irq_domain_remove().
--- drivers/iio/dummy/iio_dummy_evgen.c | 2 +- include/linux/irq_sim.h | 1 - kernel/irq/irq_sim.c | 32 ++++++++++++----------------- 3 files changed, 14 insertions(+), 21 deletions(-)
drivers/iio/dummy/iio_dummy_evgen.c +1 -1
@@ -118,7 +118,7 @@ EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_regs);static void iio_dummy_evgen_free (void ) { -irq_domain_remove_sim (iio_evgen->irq_sim_domain); +irq_domain_remove (iio_evgen->irq_sim_domain);kfree (iio_evgen); }
include/linux/irq_sim.h +0 -1
@@ -21,7 +21,6 @@ struct irq_domain *irq_domain_create_sim(struct fwnode_handle *fwnode,struct irq_domain *devm_irq_domain_create_sim (struct device *dev,struct fwnode_handle *fwnode,unsigned int num_irqs); -void irq_domain_remove_sim (struct irq_domain *domain);void irq_sim_fire (int virq);#endif /* _LINUX_IRQ_SIM_H */
kernel/irq/irq_sim.c +13 -19
@@ -108,9 +108,19 @@ static void irq_sim_domain_unmap(struct irq_domain *domain, unsigned int virq)kfree (irq_ctx); } +static void irq_sim_domain_remove (struct irq_domain *domain) +{ +struct irq_sim_work_ctx *work_ctx = domain->host_data; + +irq_work_sync (&work_ctx->work); +bitmap_free (work_ctx->pending); +kfree (work_ctx); +} +static const struct irq_domain_ops irq_sim_domain_ops = { .map = irq_sim_domain_map, .unmap = irq_sim_domain_unmap, + .remove = irq_sim_domain_remove, };/**
@@ -122,6 +132,8 @@ static const struct irq_domain_ops irq_sim_domain_ops = {* * On success: return a new irq_domain object. * On failure: a negative errno wrapped with ERR_PTR(). +* +* The returned domain can be freed using irq_domain_remove(). */ struct irq_domain *irq_domain_create_sim (struct fwnode_handle *fwnode,unsigned int num_irqs)
@@ -156,29 +168,11 @@ struct irq_domain *irq_domain_create_sim(struct fwnode_handle *fwnode, }EXPORT_SYMBOL_GPL (irq_domain_create_sim); -/** -* irq_domain_remove_sim - Deinitialize the interrupt simulator domain: free -* the interrupt descriptors and allocated memory. -* -* @domain: The interrupt simulator domain to tear down. -*/ -void irq_domain_remove_sim (struct irq_domain *domain) -{ -struct irq_sim_work_ctx *work_ctx = domain->host_data; - -irq_work_sync (&work_ctx->work); -bitmap_free (work_ctx->pending); -kfree (work_ctx); - -irq_domain_remove (domain); -} -EXPORT_SYMBOL_GPL (irq_domain_remove_sim); -static void devm_irq_domain_release_sim (struct device *dev,void *res) {struct irq_sim_devres *this = res; -irq_domain_remove_sim (this->domain); +irq_domain_remove (this->domain); }/**
Signature · 1 line
2.25.0
PATCH v2 5/6 irq/domain: provide irq_domain_dispose_mappings() helper
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Unmapping all interrupts before removing an irq_domain is a common use-case. Provide an appropriate helper that can also be used with the remove() callback in irq_domain ops.
--- include/linux/irqdomain.h | 2 ++ kernel/irq/irqdomain.c | 17 +++++++++++++++++ 2 files changed, 19 insertions(+)
include/linux/irqdomain.h +2 -0
@@ -390,6 +390,7 @@ extern unsigned int irq_create_mapping(struct irq_domain *host, irq_hw_number_t hwirq);extern unsigned int irq_create_fwspec_mapping (struct irq_fwspec *fwspec);extern void irq_dispose_mapping (unsigned int virq); +extern void irq_domain_dispose_mappings (struct irq_domain *domain);/** * irq_linear_revmap() - Find a linux irq from a hw irq number.
@@ -594,6 +595,7 @@ irq_domain_hierarchical_is_msi_remap(struct irq_domain *domain)#else /* CONFIG_IRQ_DOMAIN */ static inline void irq_dispose_mapping (unsigned int virq) { } +static inline void irq_domain_dispose_mappings (struct irq_domain *domain) { }static inline struct irq_domain *irq_find_matching_fwnode (struct fwnode_handle *fwnode,enum irq_domain_bus_token bus_token) {
kernel/irq/irqdomain.c +17 -0
@@ -888,6 +888,23 @@ void irq_dispose_mapping(unsigned int virq) }EXPORT_SYMBOL_GPL (irq_dispose_mapping); +/** +* irq_domain_dispose_mappings() - Unmap all interrupts +* @domain: domain for which to unmap all interrupts +*/ +void irq_domain_dispose_mappings (struct irq_domain *domain) +{ +unsigned int virq; +int i; + +for (i =0 ; i < domain->hwirq_max; i++) { + virq =irq_find_mapping (domain, i); +if (virq) +irq_dispose_mapping (virq); + } +} +EXPORT_SYMBOL_GPL (irq_domain_dispose_mappings); +/** * irq_find_mapping() - Find a linux irq from a hw irq number. * @domain: domain owning this hardware interrupt
Signature · 1 line
2.25.0
PATCH v2 6/6 irqchip: keystone: use irq_domain_dispose_mappings()
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Shrink the code a bit by using the new irq_domain_dispose_mappings() helper.
--- drivers/irqchip/irq-keystone.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/irqchip/irq-keystone.c +1 -4
@@ -135,6 +135,7 @@ static int keystone_irq_map(struct irq_domain *h, unsigned int virq,static const struct irq_domain_ops keystone_irq_ops = { .map = keystone_irq_map, .xlate = irq_domain_xlate_onecell, + .remove = irq_domain_dispose_mappings, };static int keystone_irq_probe (struct platform_device *pdev)
@@ -203,13 +204,9 @@ static int keystone_irq_probe(struct platform_device *pdev)static int keystone_irq_remove (struct platform_device *pdev) {struct keystone_irq_device *kirq =platform_get_drvdata (pdev); -int hwirq;free_irq (kirq->irq, kirq); -for (hwirq =0 ; hwirq <KEYSTONE_N_IRQ ; hwirq++) -irq_dispose_mapping (irq_find_mapping (kirq->irqd, hwirq)); -irq_domain_remove (kirq->irqd);return 0 ; }
Signature · 1 line
2.25.0
wt., 11 lut 2020 o 14:12 Bartosz Golaszewski <[email protected]> napisał(a):
Quoted 34 lines
From: Bartosz Golaszewski <[email protected]>
This is my second take at improving the interrupt simulator interface. I marked it as v2 but it actually takes a completely different approach.
The interrupt simulator API exposes a lot of custom data structures and functions and doesn't reuse the interfaces already exposed by the irq subsystem. This series tries to address it.
First, we make
irq_domain_reset_irq_data()available to non-V2 domain API users - that'll be used in the subsequent patch. Next we overhaul the public interfaces - we hide all specific data structures and instead rely on theirq_domainstruct and virtual interrupt numberspace.Next four patches simplify the interface even more, but since the change may be a bit more controversial due to modification of the
irq_domainI decided to split them out of the second patch.In patch 3/6 we're adding a new callback to
irq_domain_opsthat is called right before all the other code inirq_domain_remove(). Next we use it to remove the simulator-specific cleanup function fromirq_sim.h- users now can simply use the regularirq_domain_remove().Last two patches show that the new callback isn't limited to the interrupt simulator and can be used to shrink code in real driver too. We introduce a new helper for a common use case of disposing of all mappings before removing the
irq_domainand use it in the keystone irqchip driver.The end effect is that we limit the interrupt simulator API to two functions (plus one device managed variant) and zero new structures.
Quoted 11 lines
v1 -> v2:
- instead of just making the new data structures opaque for users, remove them entirely in favor ofirq_domain
- callirq_set_handler()&irq_domain_reset_irq_data()when unmapping the simulated interrupt
- fix a memory leak in error path
- make it possible to useirq_find_matching_fwnode()with the simulator domain
- correctly useirq_create_mapping()andirq_find_mapping(): only use the former at init-time and the latter at interrupt-time
Hi,
it's been three weeks, so gentle ping on that.
Or should I resend the entire series with a more elaborate commit message for patch 1/6?
Bartosz
Jonathan Cameron
maintainerQuoted 47 lines · On Tue, 3 Mar 2020 08:57:43 +0100 Bartosz Golaszewski <[email protected]> wrote:
wt., 11 lut 2020 o 14:12 Bartosz Golaszewski <[email protected]> napisał(a):
Quoted 34 lines
From: Bartosz Golaszewski <[email protected]>
This is my second take at improving the interrupt simulator interface. I marked it as v2 but it actually takes a completely different approach.
The interrupt simulator API exposes a lot of custom data structures and functions and doesn't reuse the interfaces already exposed by the irq subsystem. This series tries to address it.
First, we make
irq_domain_reset_irq_data()available to non-V2 domain API users - that'll be used in the subsequent patch. Next we overhaul the public interfaces - we hide all specific data structures and instead rely on theirq_domainstruct and virtual interrupt numberspace.Next four patches simplify the interface even more, but since the change may be a bit more controversial due to modification of the
irq_domainI decided to split them out of the second patch.In patch 3/6 we're adding a new callback to
irq_domain_opsthat is called right before all the other code inirq_domain_remove(). Next we use it to remove the simulator-specific cleanup function fromirq_sim.h- users now can simply use the regularirq_domain_remove().Last two patches show that the new callback isn't limited to the interrupt simulator and can be used to shrink code in real driver too. We introduce a new helper for a common use case of disposing of all mappings before removing the
irq_domainand use it in the keystone irqchip driver.The end effect is that we limit the interrupt simulator API to two functions (plus one device managed variant) and zero new structures.
Quoted 11 lines
v1 -> v2:
- instead of just making the new data structures opaque for users, remove them entirely in favor ofirq_domain
- callirq_set_handler()&irq_domain_reset_irq_data()when unmapping the simulated interrupt
- fix a memory leak in error path
- make it possible to useirq_find_matching_fwnode()with the simulator domain
- correctly useirq_create_mapping()andirq_find_mapping(): only use the former at init-time and the latter at interrupt-time
Quoted 6 lines
Hi,
it's been three weeks, so gentle ping on that.
Or should I resend the entire series with a more elaborate commit message for patch 1/6?
I'd resend it now if nothing has happened that I missed. Merge window having just closed, it's the perfect time for a rebase on rc1.
thanks,
Jonathan
Quoted 2 lines
Bartosz
pon., 13 kwi 2020 o 19:07 Jonathan Cameron <[email protected]> napisał(a):
Quoted 11 lines
Quoted 7 lines
Hi,
it's been three weeks, so gentle ping on that.
Or should I resend the entire series with a more elaborate commit message for patch 1/6?
I'd resend it now if nothing has happened that I missed. Merge window having just closed, it's the perfect time for a rebase on rc1.
Yeah, I'll resend the first two patches which are less controversial and then see with the rest of them.
Bart