| Message ID | 20241204124549.607054-3-maz@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | irqchip: MSI parent cleanup and PCI host driver conversion | expand |
On Wed, Dec 04 2024 at 12:45, Marc Zyngier wrote: > Creating an irq domain that serves as an MSI parent requires > a substantial amount of esoteric boiler-plate code, some of > which is often provided twice (such as the bus token). > > To make things a bit simpler for the unsuspecting MSI tinkerer, > provide a helper that does it for them, and serves as documentation > of what needs to be provided. > > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > include/linux/msi.h | 7 +++++++ > kernel/irq/msi.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 47 insertions(+) > > diff --git a/include/linux/msi.h b/include/linux/msi.h > index b10093c4d00ea..f08d14cf07103 100644 > --- a/include/linux/msi.h > +++ b/include/linux/msi.h > @@ -594,6 +594,13 @@ struct irq_domain *msi_create_irq_domain(struct fwnode_handle *fwnode, > struct msi_domain_info *info, > struct irq_domain *parent); > > +struct irq_domain *msi_create_parent_irq_domain(struct fwnode_handle *fwnode, > + const struct msi_parent_ops *msi_parent_ops, > + const struct irq_domain_ops *ops, > + unsigned long flags, unsigned long size, > + void *host_data, > + struct irq_domain *parent); Can we please make this a template based interface similar to msi_create_device_irq_domain()? > +/** > + * msi_create_parent_irq_domain - Create an MSI-parent interrupt domain > + * @fwnode: Optional fwnode of the interrupt controller > + * @msi_parent_ops: MSI parent callbacks and configuration > + * @ops: Interrupt domain ballbacks > + * @flags: Interrupt domain flags > + * @size: Interrupt domain size (0 if arbitrarily large) > + * @host_data: Interrupt domain private data > + * @parent: Parent irq domain > + * > + * Return: pointer to the created &struct irq_domain or %NULL on failure > + */ > +struct irq_domain *msi_create_parent_irq_domain(struct fwnode_handle *fwnode, > + const struct msi_parent_ops *msi_parent_ops, > + const struct irq_domain_ops *ops, > + unsigned long flags, unsigned long size, > + void *host_data, > + struct irq_domain *parent) > +{ > + struct irq_domain_info info = { > + .fwnode = fwnode, > + .size = size, > + .hwirq_max = size, > + .ops = ops, > + .host_data = host_data, > + .domain_flags = flags | IRQ_DOMAIN_FLAG_MSI_PARENT, > + .parent = parent, > + .bus_token = msi_parent_ops->bus_select_token, > + }; Instead of hiding the template in the function? We've been burnt with interfaces which might require extensions over time before and I just converted the GIC patch (3/11) over to a template at call site model. It results in the same code size reduction at the call sites, but allows us to expand the template without touching any existing driver in the future. See below. It might be a good idea to have a specific msi_irq_domain_info template which only contains the information required instead of reusing and expanding irq_domain_info. Hmm? Thanks, tglx --- --- a/drivers/irqchip/irq-gic-v2m.c +++ b/drivers/irqchip/irq-gic-v2m.c @@ -263,24 +263,25 @@ static struct msi_parent_ops gicv2m_msi_ static __init int gicv2m_allocate_domains(struct irq_domain *parent) { - struct irq_domain *inner_domain; struct v2m_data *v2m; + struct irq_domain_info info = { + .ops = &gic2m_domain_ops, + .parent = parent, + .msi_parent_ops = &gicv2m_msi_parent_ops, + }; v2m = list_first_entry_or_null(&v2m_nodes, struct v2m_data, entry); if (!v2m) return 0; - inner_domain = irq_domain_create_hierarchy(parent, 0, 0, v2m->fwnode, - &gicv2m_domain_ops, v2m); - if (!inner_domain) { - pr_err("Failed to create GICv2m domain\n"); - return -ENOMEM; - } - - irq_domain_update_bus_token(inner_domain, DOMAIN_BUS_NEXUS); - inner_domain->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT; - inner_domain->msi_parent_ops = &gicv2m_msi_parent_ops; - return 0; + info->fwnode = v2m->fwnode; + info->host_data = v2m; + + if (msi_create_parent_irq_domain(&info)) + return 0; + + pr_err("Failed to create GICv2m domain\n"); + return -ENOMEM; } static int __init gicv2m_init_one(struct fwnode_handle *fwnode, --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -5076,31 +5076,27 @@ static void __init __iomem *its_map_one( static int its_init_domain(struct its_node *its) { - struct irq_domain *inner_domain; - struct msi_domain_info *info; + struct msi_domain_info *msi_info; + struct irq_domain_info info = { + .fwnode = its->fwnode_handle, + .ops = &its_domain_ops, + .parent = its_parent, + .msi_parent_ops = &gic_v3_its_msi_parent_ops, + .flags = its->msi_domain_flags, + }; - info = kzalloc(sizeof(*info), GFP_KERNEL); - if (!info) + msi_info = kzalloc(sizeof(*info), GFP_KERNEL); + if (!msi_info) return -ENOMEM; - info->ops = &its_msi_domain_ops; - info->data = its; - - inner_domain = irq_domain_create_hierarchy(its_parent, - its->msi_domain_flags, 0, - its->fwnode_handle, &its_domain_ops, - info); - if (!inner_domain) { - kfree(info); - return -ENOMEM; - } - - irq_domain_update_bus_token(inner_domain, DOMAIN_BUS_NEXUS); - - inner_domain->msi_parent_ops = &gic_v3_its_msi_parent_ops; - inner_domain->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT; - - return 0; + msi_info->ops = &its_msi_domain_ops; + msi_info->data = its; + info.host_data = msi_info; + + if (msi_create_parent_irq_domain(&info)) + return 0; + kfree(info); + return -ENOMEM; } static int its_init_vpe_domain(void) --- a/drivers/irqchip/irq-gic-v3-mbi.c +++ b/drivers/irqchip/irq-gic-v3-mbi.c @@ -209,17 +209,14 @@ static const struct msi_parent_ops gic_v static int mbi_allocate_domain(struct irq_domain *parent) { - struct irq_domain *nexus_domain; + struct irq_domain_info info = { + .fwnode = parent->fwnode, + .ops = &mbi_domain_ops, + .parent = parent, + .msi_parent_ops = &gic_v3_mbi_msi_parent_ops, + }; - nexus_domain = irq_domain_create_hierarchy(parent, 0, 0, parent->fwnode, - &mbi_domain_ops, NULL); - if (!nexus_domain) - return -ENOMEM; - - irq_domain_update_bus_token(nexus_domain, DOMAIN_BUS_NEXUS); - nexus_domain->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT; - nexus_domain->msi_parent_ops = &gic_v3_mbi_msi_parent_ops; - return 0; + return msi_create_parent_irq_domain(&info) ? 0 : -ENOMEM; } int __init mbi_init(struct fwnode_handle *fwnode, struct irq_domain *parent)
diff --git a/include/linux/msi.h b/include/linux/msi.h index b10093c4d00ea..f08d14cf07103 100644 --- a/include/linux/msi.h +++ b/include/linux/msi.h @@ -594,6 +594,13 @@ struct irq_domain *msi_create_irq_domain(struct fwnode_handle *fwnode, struct msi_domain_info *info, struct irq_domain *parent); +struct irq_domain *msi_create_parent_irq_domain(struct fwnode_handle *fwnode, + const struct msi_parent_ops *msi_parent_ops, + const struct irq_domain_ops *ops, + unsigned long flags, unsigned long size, + void *host_data, + struct irq_domain *parent); + bool msi_create_device_irq_domain(struct device *dev, unsigned int domid, const struct msi_domain_template *template, unsigned int hwsize, void *domain_data, diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c index 396a067a8a56b..037d85cf0b21c 100644 --- a/kernel/irq/msi.c +++ b/kernel/irq/msi.c @@ -885,6 +885,46 @@ struct irq_domain *msi_create_irq_domain(struct fwnode_handle *fwnode, return __msi_create_irq_domain(fwnode, info, 0, parent); } +/** + * msi_create_parent_irq_domain - Create an MSI-parent interrupt domain + * @fwnode: Optional fwnode of the interrupt controller + * @msi_parent_ops: MSI parent callbacks and configuration + * @ops: Interrupt domain ballbacks + * @flags: Interrupt domain flags + * @size: Interrupt domain size (0 if arbitrarily large) + * @host_data: Interrupt domain private data + * @parent: Parent irq domain + * + * Return: pointer to the created &struct irq_domain or %NULL on failure + */ +struct irq_domain *msi_create_parent_irq_domain(struct fwnode_handle *fwnode, + const struct msi_parent_ops *msi_parent_ops, + const struct irq_domain_ops *ops, + unsigned long flags, unsigned long size, + void *host_data, + struct irq_domain *parent) +{ + struct irq_domain_info info = { + .fwnode = fwnode, + .size = size, + .hwirq_max = size, + .ops = ops, + .host_data = host_data, + .domain_flags = flags | IRQ_DOMAIN_FLAG_MSI_PARENT, + .parent = parent, + .bus_token = msi_parent_ops->bus_select_token, + }; + struct irq_domain *d; + + d = irq_domain_instantiate(&info); + if (IS_ERR(d)) + return NULL; + + d->msi_parent_ops = msi_parent_ops; + return d; +} +EXPORT_SYMBOL_GPL(msi_create_parent_irq_domain); + /** * msi_parent_init_dev_msi_info - Delegate initialization of device MSI info down * in the domain hierarchy
Creating an irq domain that serves as an MSI parent requires a substantial amount of esoteric boiler-plate code, some of which is often provided twice (such as the bus token). To make things a bit simpler for the unsuspecting MSI tinkerer, provide a helper that does it for them, and serves as documentation of what needs to be provided. Signed-off-by: Marc Zyngier <maz@kernel.org> --- include/linux/msi.h | 7 +++++++ kernel/irq/msi.c | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+)