| Message ID | 20240527161450.326615-11-herve.codina@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Add support for the LAN966x PCI device using a DT overlay | expand |
Herve!
On Mon, May 27 2024 at 18:14, Herve Codina wrote:
Sorry I missed V1 somehow. I'll review this tomorrow.
Thanks,
tglx
On Mon, May 27 2024 at 18:14, Herve Codina wrote: > The irq_domain_add_*() family functions create an irq_domain and also > publish this newly created to domain. Once an irq_domain is published, > consumers can request IRQ in order to use them. > > Some interrupt controller drivers have to perform some more operations > with the created irq_domain in order to have it ready to be used. > For instance: > - Allocate generic irq chips with irq_alloc_domain_generic_chips() > - Retrieve the generic irq chips with irq_get_domain_generic_chip() > - Initialize retrieved chips: set register base address and offsets, > set several hooks such as irq_mask, irq_unmask, ... > > To avoid a window where the domain is published but not yet ready to be I can see the point, but why is this suddenly a problem? There are tons of interrupt chip drivers which have exactly that pattern. Also why is all of this burried in a series which aims to add a network driver and touches the world and some more. If you had sent the two irq domain patches seperately w/o spamming 100 people on CC then this would have been solved long ago. That's documented clearly, no? > void irq_domain_free_fwnode(struct fwnode_handle *fwnode); > +struct irq_domain *irq_domain_alloc(struct fwnode_handle *fwnode, unsigned int size, > + irq_hw_number_t hwirq_max, int direct_max, > + const struct irq_domain_ops *ops, > + void *host_data); > + > +static inline struct irq_domain *irq_domain_alloc_linear(struct fwnode_handle *fwnode, > + unsigned int size, > + const struct irq_domain_ops *ops, > + void *host_data) > +{ > + return irq_domain_alloc(fwnode, size, size, 0, ops, host_data); > +} So this creates exactly one wrapper, which means we'll grow another ton of wrappers if that becomes popular for whatever reason. We have already too many of variants for creating domains. But what's worse is that this does not work for hierarchical domains and is just an ad hoc scratch my itch solution. Also looking at the irq chip drivers which use generic interrupt chips. There are 24 instances of irq_alloc_domain_generic_chips() and most of this code is just boilerplate. So what we really want is a proper solution to get rid of this mess instead of creating interfaces which just proliferate and extend it. Something like the uncompiled below allows to convert all the boilerplate into a template based setup/remove. I just converted a random driver over to it and the result is pretty neutral in terms of lines, but the amount of code to get wrong is significantly smaller. I'm sure that more complex drivers will benefit even more and your problem should be completely solved by that. The below is just an initial sketch which allows further consolidation in the irqdomain space. You get the idea. Thanks, tglx --- --- a/include/linux/irq.h +++ b/include/linux/irq.h @@ -1106,6 +1106,7 @@ enum irq_gc_flags { * @irq_flags_to_set: IRQ* flags to set on irq setup * @irq_flags_to_clear: IRQ* flags to clear on irq setup * @gc_flags: Generic chip specific setup flags + * FIXME * @gc: Array of pointers to generic interrupt chips */ struct irq_domain_chip_generic { @@ -1114,9 +1115,26 @@ struct irq_domain_chip_generic { unsigned int irq_flags_to_clear; unsigned int irq_flags_to_set; enum irq_gc_flags gc_flags; + void (*destroy)(struct irq_chip_generic *c); struct irq_chip_generic *gc[]; }; +/** + * irq_domain_chip_generic_info - Init structure + * FIXME + */ +struct irq_domain_chip_generic_info { + const char *name; + irq_flow_handler_t handler; + void (*init)(struct irq_chip_generic *d); + void (*destroy)(struct irq_chip_generic *c); + unsigned int irqs_per_chip; + unsigned int num_chips; + unsigned int irq_flags_to_clear; + unsigned int irq_flags_to_set; + enum irq_gc_flags gc_flags; +}; + /* Generic chip callback functions */ void irq_gc_noop(struct irq_data *d); void irq_gc_mask_disable_reg(struct irq_data *d); @@ -1153,6 +1171,9 @@ int devm_irq_setup_generic_chip(struct d struct irq_chip_generic *irq_get_domain_generic_chip(struct irq_domain *d, unsigned int hw_irq); +int irq_domain_alloc_generic_chips(struct irq_domain *d, struct irq_domain_chip_generic_info *info); +void irq_domain_remove_generic_chips(struct irq_domain *d); + int __irq_alloc_domain_generic_chips(struct irq_domain *d, int irqs_per_chip, int num_ct, const char *name, irq_flow_handler_t handler, --- a/include/linux/irqdomain.h +++ b/include/linux/irqdomain.h @@ -41,13 +41,13 @@ struct device_node; struct fwnode_handle; struct irq_domain; struct irq_chip; +struct irq_chip_generic; struct irq_data; struct irq_desc; struct cpumask; struct seq_file; struct irq_affinity_desc; struct msi_parent_ops; - #define IRQ_DOMAIN_IRQ_SPEC_PARAMS 16 /** @@ -169,6 +169,7 @@ struct irq_domain { #ifdef CONFIG_GENERIC_MSI_IRQ const struct msi_parent_ops *msi_parent_ops; #endif + void (*destroy)(struct irq_domain *d); /* reverse map data. The linear map gets appended to the irq_domain */ irq_hw_number_t hwirq_max; @@ -208,6 +209,9 @@ enum { /* Irq domain is a MSI device domain */ IRQ_DOMAIN_FLAG_MSI_DEVICE = (1 << 9), + /* Irq domain must destroy generic chips when removed */ + IRQ_DOMAIN_FLAG_DESTROY_GC = (1 << 10), + /* * Flags starting from IRQ_DOMAIN_FLAG_NONCORE are reserved * for implementation specific purposes and ignored by the @@ -257,6 +261,28 @@ static inline struct fwnode_handle *irq_ } void irq_domain_free_fwnode(struct fwnode_handle *fwnode); + +struct irq_domain_chip_generic_info; + +/** + * irq_domain_info - Init structure + * FIXME + */ +struct irq_domain_info { + struct fwnode_handle *fwnode; + unsigned int domain_flags; + unsigned int size; + irq_hw_number_t hwirq_max; + int direct_max; + enum irq_domain_bus_token bus_token; + const struct irq_domain_ops *ops; + void *host_data; + struct irq_domain_chip_generic_info *gc_info; + void (*init)(struct irq_domain *d); +}; + +struct irq_domain *irq_domain_instantiate(struct irq_domain_info *info); + struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int size, irq_hw_number_t hwirq_max, int direct_max, const struct irq_domain_ops *ops, --- a/kernel/irq/generic-chip.c +++ b/kernel/irq/generic-chip.c @@ -274,23 +274,11 @@ irq_gc_init_mask_cache(struct irq_chip_g *mskptr = irq_reg_readl(gc, mskreg); } } - /** - * __irq_alloc_domain_generic_chips - Allocate generic chips for an irq domain - * @d: irq domain for which to allocate chips - * @irqs_per_chip: Number of interrupts each chip handles (max 32) - * @num_ct: Number of irq_chip_type instances associated with this - * @name: Name of the irq chip - * @handler: Default flow handler associated with these chips - * @clr: IRQ_* bits to clear in the mapping function - * @set: IRQ_* bits to set in the mapping function - * @gcflags: Generic chip specific setup flags + * irq_domain_alloc_generic_chips - Allocate generic chips for an irq domain + * FIXME */ -int __irq_alloc_domain_generic_chips(struct irq_domain *d, int irqs_per_chip, - int num_ct, const char *name, - irq_flow_handler_t handler, - unsigned int clr, unsigned int set, - enum irq_gc_flags gcflags) +int irq_domain_alloc_generic_chips(struct irq_domain *d, struct irq_domain_chip_generic_info *info) { struct irq_domain_chip_generic *dgc; struct irq_chip_generic *gc; @@ -304,23 +292,24 @@ int __irq_alloc_domain_generic_chips(str if (d->gc) return -EBUSY; - numchips = DIV_ROUND_UP(d->revmap_size, irqs_per_chip); + numchips = DIV_ROUND_UP(d->revmap_size, info->irqs_per_chip); if (!numchips) return -EINVAL; /* Allocate a pointer, generic chip and chiptypes for each chip */ - gc_sz = struct_size(gc, chip_types, num_ct); + gc_sz = struct_size(gc, chip_types, info->num_ct); dgc_sz = struct_size(dgc, gc, numchips); sz = dgc_sz + numchips * gc_sz; tmp = dgc = kzalloc(sz, GFP_KERNEL); if (!dgc) return -ENOMEM; - dgc->irqs_per_chip = irqs_per_chip; + dgc->irqs_per_chip = info->irqs_per_chip; dgc->num_chips = numchips; - dgc->irq_flags_to_set = set; - dgc->irq_flags_to_clear = clr; - dgc->gc_flags = gcflags; + dgc->irq_flags_to_set = info->irq_flags_to_set; + dgc->irq_flags_to_clear = info->irq_flags_to_clear; + dgc->gc_flags = info->gcflags; + dgc->destroy = info->destroy; d->gc = dgc; /* Calc pointer to the first generic chip */ @@ -337,6 +326,9 @@ int __irq_alloc_domain_generic_chips(str gc->reg_writel = &irq_writel_be; } + if (info->init) + info->init(gc); + raw_spin_lock_irqsave(&gc_lock, flags); list_add_tail(&gc->list, &gc_list); raw_spin_unlock_irqrestore(&gc_lock, flags); @@ -345,6 +337,56 @@ int __irq_alloc_domain_generic_chips(str } return 0; } + +/** + * irq_domain_remove_generic_chips - Remove generic chips from an interrupt domain + * FIXME + */ +void irq_domain_remove_generic_chips(struct irq_domain *d) +{ + struct irq_domain_chip_generic *dgc = d->gc; + struct irq_domain_ops *ops = d->ops; + + if (!dgc) + return; + + for (unsigned int i = 0; i < dgc->num_chips, i++) { + if (dgc->destroy) + dgc->destroy_gc(dgc->gc + i); + irq_remove_generic_chip(dgc->gc + i, ~0U, 0, 0); + } + d->dgc = NULL; + kfree(dgc); +} + +/** + * __irq_alloc_domain_generic_chips - Allocate generic chips for an irq domain + * @d: irq domain for which to allocate chips + * @irqs_per_chip: Number of interrupts each chip handles (max 32) + * @num_ct: Number of irq_chip_type instances associated with this + * @name: Name of the irq chip + * @handler: Default flow handler associated with these chips + * @clr: IRQ_* bits to clear in the mapping function + * @set: IRQ_* bits to set in the mapping function + * @gcflags: Generic chip specific setup flags + */ +int __irq_alloc_domain_generic_chips(struct irq_domain *d, int irqs_per_chip, + int num_ct, const char *name, + irq_flow_handler_t handler, + unsigned int clr, unsigned int set, + enum irq_gc_flags gcflags) +{ + struct irq_domain_chip_generic_info info = { + .name = name, + .handler = handler, + .irqs_per_chip = irqs_per_chip, + .irq_flags_to_set = set, + .irq_flags_to_clear = clr, + .gc_flags = gcflags, + }; + + return irq_domain_alloc_generic_chips(d, &info); +} EXPORT_SYMBOL_GPL(__irq_alloc_domain_generic_chips); static struct irq_chip_generic * --- a/kernel/irq/irqdomain.c +++ b/kernel/irq/irqdomain.c @@ -240,6 +240,47 @@ static void __irq_domain_publish(struct pr_debug("Added domain %s\n", domain->name); } +static void irq_domain_free(struct irq_domain *domain) +{ + fwnode_dev_initialized(domain->fwnode, false); + fwnode_handle_put(domain->fwnode); + if (domain->flags & IRQ_DOMAIN_NAME_ALLOCATED) + kfree(domain->name); + kfree(domain); +} + +/** + * irq_domain_instantiate() - Instantiate a new irq_domain data structure + * FIXME + */ +struct irq_domain *irq_domain_instantiate(struct irq_domain_info *info) +{ + struct irq_domain *domain; + + // FIXME: Convert irq_domain_create() to use @info + domain = __irq_domain_create(info->fwnode, info->size, info->hwirq_max, info->direct_max, + info->ops, info->host_data); + if (!domain) + return NULL; + + domain->flags |= info->domain_flags; + + if (info->gc_info) { + if (!irq_domain_alloc_generic_chips(domain, info->gc_info)) { + irq_domain_remove(domain); + return NULL; + } + } + if (info->init) + info->init(domain); + __irq_domain_publish(domain); + + // FIXME: Make this part of irq_domain_create() + if (info->bus_token) + irq_domain_update_bus_token(domain, info->bus_token); + return domain; +} + /** * __irq_domain_add() - Allocate a new irq_domain data structure * @fwnode: firmware node for the interrupt controller @@ -279,6 +320,9 @@ EXPORT_SYMBOL_GPL(__irq_domain_add); */ void irq_domain_remove(struct irq_domain *domain) { + if (domain->destroy) + domain->destroy(domain); + mutex_lock(&irq_domain_mutex); debugfs_remove_domain_dir(domain); @@ -294,13 +338,11 @@ void irq_domain_remove(struct irq_domain mutex_unlock(&irq_domain_mutex); - pr_debug("Removed domain %s\n", domain->name); + if (domain->flags & IRQ_DOMAIN_FLAG_DESTROY_GC) + irq_domain_remove_generic_chips(domain); - fwnode_dev_initialized(domain->fwnode, false); - fwnode_handle_put(domain->fwnode); - if (domain->flags & IRQ_DOMAIN_NAME_ALLOCATED) - kfree(domain->name); - kfree(domain); + pr_debug("Removed domain %s\n", domain->name); + irq_domain_free(domain); } EXPORT_SYMBOL_GPL(irq_domain_remove); --- a/drivers/irqchip/irq-al-fic.c +++ b/drivers/irqchip/irq-al-fic.c @@ -133,32 +133,10 @@ static int al_fic_irq_retrigger(struct i return 1; } -static int al_fic_register(struct device_node *node, - struct al_fic *fic) +static void al_fic_gc_init(struct irq_chip_generic *gc) { - struct irq_chip_generic *gc; - int ret; + struct al_fic *fic = gc->domain->host_data; - fic->domain = irq_domain_add_linear(node, - NR_FIC_IRQS, - &irq_generic_chip_ops, - fic); - if (!fic->domain) { - pr_err("fail to add irq domain\n"); - return -ENOMEM; - } - - ret = irq_alloc_domain_generic_chips(fic->domain, - NR_FIC_IRQS, - 1, fic->name, - handle_level_irq, - 0, 0, IRQ_GC_INIT_MASK_CACHE); - if (ret) { - pr_err("fail to allocate generic chip (%d)\n", ret); - goto err_domain_remove; - } - - gc = irq_get_domain_generic_chip(fic->domain, 0); gc->reg_base = fic->base; gc->chip_types->regs.mask = AL_FIC_MASK; gc->chip_types->regs.ack = AL_FIC_CAUSE; @@ -169,16 +147,37 @@ static int al_fic_register(struct device gc->chip_types->chip.irq_retrigger = al_fic_irq_retrigger; gc->chip_types->chip.flags = IRQCHIP_SKIP_SET_WAKE; gc->private = fic; +} + +static void al_fic_domain_init(struct irq_domain *d) +{ + struct al_fic *fic = d->host_data; - irq_set_chained_handler_and_data(fic->parent_irq, - al_fic_irq_handler, - fic); - return 0; + irq_set_chained_handler_and_data(fic->parent_irq, al_fic_irq_handler, fic); +} -err_domain_remove: - irq_domain_remove(fic->domain); +static int al_fic_register(struct device_node *node, struct al_fic *fic) +{ + struct irq_domain_chip_generic_info gc_info = { + .irqs_per_chip = NR_FIC_IRQS, + .num_chips = 1, + .name = fic->name, + .handler = handle_level_irq, + .gc_flags = IRQ_GC_INIT_MASK_CACHE, + .init = al_fic_gc_init, + }; + struct irq_domain_info info = { + .fwnode = of_node_to_fwnode(node), + .size = NR_FIC_IRQS, + .hwirq_max = NR_FIC_IRQS, + .ops = &irq_generic_chip_ops, + .host_data = fic, + .gc_info = &gc_info, + .init = al_fic_domain_init, + }; - return ret; + fic->domain = irq_domain_instantiate(&info); + return fic->domain ? 0 : -ENOMEM; } /*
Hi Thomas, On Wed, 05 Jun 2024 15:02:46 +0200 Thomas Gleixner <tglx@linutronix.de> wrote: > On Mon, May 27 2024 at 18:14, Herve Codina wrote: > > The irq_domain_add_*() family functions create an irq_domain and also > > publish this newly created to domain. Once an irq_domain is published, > > consumers can request IRQ in order to use them. > > > > Some interrupt controller drivers have to perform some more operations > > with the created irq_domain in order to have it ready to be used. > > For instance: > > - Allocate generic irq chips with irq_alloc_domain_generic_chips() > > - Retrieve the generic irq chips with irq_get_domain_generic_chip() > > - Initialize retrieved chips: set register base address and offsets, > > set several hooks such as irq_mask, irq_unmask, ... > > > > To avoid a window where the domain is published but not yet ready to be > > I can see the point, but why is this suddenly a problem? There are tons > of interrupt chip drivers which have exactly that pattern. > I thing the issue was not triggered because these interrupt chip driver are usually builtin compiled and the probe sequence is the linear one done at boot time. Consumers/supplier are probe sequentially without any parallel execution issues. In the LAN966x PCI device driver use case, the drivers were built as modules. Modules loading and drivers .probe() calls for the irqs supplier and irqs consumers are done in parallel. This reveals the race condition. > Also why is all of this burried in a series which aims to add a network > driver and touches the world and some more. If you had sent the two irq > domain patches seperately w/o spamming 100 people on CC then this would > have been solved long ago. That's documented clearly, no? Yes, the main idea of the series, as mentioned in the cover letter, is to give the big picture of the LAN966x PCI device use case in order to have all the impacted subsystems and drivers maintainers be aware of the global use case: DT overlay on top of PCI device. Of course, the plan is to split this series into smaller ones once parts get discussed in the DT overlay on top of PCI use case and reach some kind of maturity at least on the way to implement a solution. Thomas, do you prefer to have all the IRQ related patches extracted right now from this big picture series ? > > > void irq_domain_free_fwnode(struct fwnode_handle *fwnode); > > +struct irq_domain *irq_domain_alloc(struct fwnode_handle *fwnode, unsigned int size, > > + irq_hw_number_t hwirq_max, int direct_max, > > + const struct irq_domain_ops *ops, > > + void *host_data); > > + > > +static inline struct irq_domain *irq_domain_alloc_linear(struct fwnode_handle *fwnode, > > + unsigned int size, > > + const struct irq_domain_ops *ops, > > + void *host_data) > > +{ > > + return irq_domain_alloc(fwnode, size, size, 0, ops, host_data); > > +} > > So this creates exactly one wrapper, which means we'll grow another ton > of wrappers if that becomes popular for whatever reason. We have already > too many of variants for creating domains. > > But what's worse is that this does not work for hierarchical domains and > is just an ad hoc scratch my itch solution. > > Also looking at the irq chip drivers which use generic interrupt > chips. There are 24 instances of irq_alloc_domain_generic_chips() and > most of this code is just boilerplate. > > So what we really want is a proper solution to get rid of this mess > instead of creating interfaces which just proliferate and extend it. > > Something like the uncompiled below allows to convert all the > boilerplate into a template based setup/remove. > > I just converted a random driver over to it and the result is pretty > neutral in terms of lines, but the amount of code to get wrong is > significantly smaller. I'm sure that more complex drivers will benefit > even more and your problem should be completely solved by that. > > The below is just an initial sketch which allows further consolidation > in the irqdomain space. You get the idea. Got it, thanks a lot for the idea, the sketch and the way to use it in drivers. I will rework my patches in that way. Thanks, Hervé
Herve! On Thu, Jun 06 2024 at 17:52, Herve Codina wrote: > On Wed, 05 Jun 2024 15:02:46 +0200 > Thomas Gleixner <tglx@linutronix.de> wrote: >> On Mon, May 27 2024 at 18:14, Herve Codina wrote: >> > To avoid a window where the domain is published but not yet ready to be >> >> I can see the point, but why is this suddenly a problem? There are tons >> of interrupt chip drivers which have exactly that pattern. > > I thing the issue was not triggered because these interrupt chip driver > are usually builtin compiled and the probe sequence is the linear one > done at boot time. Consumers/supplier are probe sequentially without any > parallel execution issues. > > In the LAN966x PCI device driver use case, the drivers were built as > modules. Modules loading and drivers .probe() calls for the irqs supplier > and irqs consumers are done in parallel. This reveals the race condition. So how is that supposed to work? There is clearly a requirement that the interrupt controller is ready to use when the network driver is probed, no? >> Also why is all of this burried in a series which aims to add a network >> driver and touches the world and some more. If you had sent the two irq >> domain patches seperately w/o spamming 100 people on CC then this would >> have been solved long ago. That's documented clearly, no? > > Yes, the main idea of the series, as mentioned in the cover letter, is to > give the big picture of the LAN966x PCI device use case in order to have > all the impacted subsystems and drivers maintainers be aware of the global > use case: DT overlay on top of PCI device. > Of course, the plan is to split this series into smaller ones once parts > get discussed in the DT overlay on top of PCI use case and reach some kind > of maturity at least on the way to implement a solution. Fair enough. > Thomas, do you prefer to have all the IRQ related patches extracted right > now from this big picture series ? I think the interrupt controller problem is completely orthogonal to the PCI/DT issue. So yes, please split them out as preparatory work which is probably also not that interesting for the PCI/DT/net folks. If the template approach holds, then the infrastructure change is definitely worth it on its own and the actual driver just falls in place and is off your backlog list. Thanks, tglx
On Thu, 06 Jun 2024 20:11:23 +0200 Thomas Gleixner <tglx@linutronix.de> wrote: > Herve! > > On Thu, Jun 06 2024 at 17:52, Herve Codina wrote: > > On Wed, 05 Jun 2024 15:02:46 +0200 > > Thomas Gleixner <tglx@linutronix.de> wrote: > >> On Mon, May 27 2024 at 18:14, Herve Codina wrote: > >> > To avoid a window where the domain is published but not yet ready to be > >> > >> I can see the point, but why is this suddenly a problem? There are tons > >> of interrupt chip drivers which have exactly that pattern. > > > > I thing the issue was not triggered because these interrupt chip driver > > are usually builtin compiled and the probe sequence is the linear one > > done at boot time. Consumers/supplier are probe sequentially without any > > parallel execution issues. > > > > In the LAN966x PCI device driver use case, the drivers were built as > > modules. Modules loading and drivers .probe() calls for the irqs supplier > > and irqs consumers are done in parallel. This reveals the race condition. > > So how is that supposed to work? There is clearly a requirement that the > interrupt controller is ready to use when the network driver is probed, no? Yes, EPROBE_DEFER mecanism. The race condition window leads to an other error code instead of the expected EPROBE_DEFER. > > >> Also why is all of this burried in a series which aims to add a network > >> driver and touches the world and some more. If you had sent the two irq > >> domain patches seperately w/o spamming 100 people on CC then this would > >> have been solved long ago. That's documented clearly, no? > > > > Yes, the main idea of the series, as mentioned in the cover letter, is to > > give the big picture of the LAN966x PCI device use case in order to have > > all the impacted subsystems and drivers maintainers be aware of the global > > use case: DT overlay on top of PCI device. > > Of course, the plan is to split this series into smaller ones once parts > > get discussed in the DT overlay on top of PCI use case and reach some kind > > of maturity at least on the way to implement a solution. > > Fair enough. > > > Thomas, do you prefer to have all the IRQ related patches extracted right > > now from this big picture series ? > > I think the interrupt controller problem is completely orthogonal to the > PCI/DT issue. > > So yes, please split them out as preparatory work which is probably also > not that interesting for the PCI/DT/net folks. > Will do that. Best regards, Hervé
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h index 21ecf582a0fe..86203e7e6659 100644 --- a/include/linux/irqdomain.h +++ b/include/linux/irqdomain.h @@ -257,6 +257,22 @@ static inline struct fwnode_handle *irq_domain_alloc_fwnode(phys_addr_t *pa) } void irq_domain_free_fwnode(struct fwnode_handle *fwnode); +struct irq_domain *irq_domain_alloc(struct fwnode_handle *fwnode, unsigned int size, + irq_hw_number_t hwirq_max, int direct_max, + const struct irq_domain_ops *ops, + void *host_data); + +static inline struct irq_domain *irq_domain_alloc_linear(struct fwnode_handle *fwnode, + unsigned int size, + const struct irq_domain_ops *ops, + void *host_data) +{ + return irq_domain_alloc(fwnode, size, size, 0, ops, host_data); +} + +void irq_domain_free(struct irq_domain *domain); +void irq_domain_publish(struct irq_domain *domain); +void irq_domain_unpublish(struct irq_domain *domain); struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int size, irq_hw_number_t hwirq_max, int direct_max, const struct irq_domain_ops *ops, diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index 86f8b91b0d3a..06c3e1b03a1d 100644 --- a/kernel/irq/irqdomain.c +++ b/kernel/irq/irqdomain.c @@ -231,7 +231,38 @@ static struct irq_domain *__irq_domain_create(struct fwnode_handle *fwnode, return domain; } -static void __irq_domain_publish(struct irq_domain *domain) +struct irq_domain *irq_domain_alloc(struct fwnode_handle *fwnode, unsigned int size, + irq_hw_number_t hwirq_max, int direct_max, + const struct irq_domain_ops *ops, + void *host_data) +{ + return __irq_domain_create(fwnode, size, hwirq_max, direct_max, ops, + host_data); +} +EXPORT_SYMBOL_GPL(irq_domain_alloc); + +/** + * irq_domain_free() - Free an irq domain. + * @domain: domain to free + * + * This routine is used to free an irq domain. The caller must ensure + * that the domain is not published. + */ +void irq_domain_free(struct irq_domain *domain) +{ + fwnode_dev_initialized(domain->fwnode, false); + fwnode_handle_put(domain->fwnode); + if (domain->flags & IRQ_DOMAIN_NAME_ALLOCATED) + kfree(domain->name); + kfree(domain); +} +EXPORT_SYMBOL_GPL(irq_domain_free); + +/** + * irq_domain_publish() - Publish an irq domain. + * @domain: domain to publish + */ +void irq_domain_publish(struct irq_domain *domain) { mutex_lock(&irq_domain_mutex); debugfs_add_domain_dir(domain); @@ -240,6 +271,36 @@ static void __irq_domain_publish(struct irq_domain *domain) pr_debug("Added domain %s\n", domain->name); } +EXPORT_SYMBOL_GPL(irq_domain_publish); + +/** + * irq_domain_unpublish() - Unpublish an irq domain. + * @domain: domain to unpublish + * + * This routine is used to unpublish an irq domain. The caller must ensure + * that all mappings within the domain have been disposed of prior to + * use, depending on the revmap type. + */ +void irq_domain_unpublish(struct irq_domain *domain) +{ + mutex_lock(&irq_domain_mutex); + debugfs_remove_domain_dir(domain); + + WARN_ON(!radix_tree_empty(&domain->revmap_tree)); + + list_del(&domain->link); + + /* + * If the going away domain is the default one, reset it. + */ + if (unlikely(irq_default_domain == domain)) + irq_set_default_host(NULL); + + mutex_unlock(&irq_domain_mutex); + + pr_debug("Removed domain %s\n", domain->name); +} +EXPORT_SYMBOL_GPL(irq_domain_unpublish); /** * __irq_domain_add() - Allocate a new irq_domain data structure @@ -264,7 +325,7 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int s domain = __irq_domain_create(fwnode, size, hwirq_max, direct_max, ops, host_data); if (domain) - __irq_domain_publish(domain); + irq_domain_publish(domain); return domain; } @@ -280,28 +341,8 @@ EXPORT_SYMBOL_GPL(__irq_domain_add); */ void irq_domain_remove(struct irq_domain *domain) { - mutex_lock(&irq_domain_mutex); - debugfs_remove_domain_dir(domain); - - WARN_ON(!radix_tree_empty(&domain->revmap_tree)); - - list_del(&domain->link); - - /* - * If the going away domain is the default one, reset it. - */ - if (unlikely(irq_default_domain == domain)) - irq_set_default_host(NULL); - - mutex_unlock(&irq_domain_mutex); - - pr_debug("Removed domain %s\n", domain->name); - - fwnode_dev_initialized(domain->fwnode, false); - fwnode_handle_put(domain->fwnode); - if (domain->flags & IRQ_DOMAIN_NAME_ALLOCATED) - kfree(domain->name); - kfree(domain); + irq_domain_unpublish(domain); + irq_domain_free(domain); } EXPORT_SYMBOL_GPL(irq_domain_remove); @@ -1184,7 +1225,7 @@ struct irq_domain *irq_domain_create_hierarchy(struct irq_domain *parent, domain->parent = parent; domain->flags |= flags; - __irq_domain_publish(domain); + irq_domain_publish(domain); } return domain;
The irq_domain_add_*() family functions create an irq_domain and also publish this newly created to domain. Once an irq_domain is published, consumers can request IRQ in order to use them. Some interrupt controller drivers have to perform some more operations with the created irq_domain in order to have it ready to be used. For instance: - Allocate generic irq chips with irq_alloc_domain_generic_chips() - Retrieve the generic irq chips with irq_get_domain_generic_chip() - Initialize retrieved chips: set register base address and offsets, set several hooks such as irq_mask, irq_unmask, ... To avoid a window where the domain is published but not yet ready to be used, introduce irq_domain_alloc_*() family functions to create the irq_domain and irq_domain_publish() to publish the irq_domain. With this new functions, any additional initialisation can then be done between the call creating the irq_domain and the call publishing it. Signed-off-by: Herve Codina <herve.codina@bootlin.com> --- include/linux/irqdomain.h | 16 +++++++ kernel/irq/irqdomain.c | 91 ++++++++++++++++++++++++++++----------- 2 files changed, 82 insertions(+), 25 deletions(-)