[RFC PATCH 1/2] irqchip: cdx-bus: add cdx-MSI domain with gic-its domain as parent
Marc Zyngier
maz at kernel.org
Thu Aug 4 03:38:41 PDT 2022
On Thu, 04 Aug 2022 10:18:19 +0100,
"Gupta, Nipun" <Nipun.Gupta at amd.com> wrote:
> > > +static const struct of_device_id cdx_device_id[] = {
> > > + {.compatible = "xlnx,cdx-controller-1.0", },
> >
> > What is this? If this is supposed to represent am ITS, it really
> > should say so.
>
> This is a CDX bus controller, not an ITS. This will be added as a part of
> device-tree documentation when we add formal CDX bus patches.
> CDX is an upcoming AMD bus, supporting dynamically discovered
> FPGA devices.
Right. So that's yet another reason to not have this glue code at all.
>
> >
> > > + {},
> > > +};
> > > +
> > > +struct irq_domain *get_parent(struct fwnode_handle *handle)
> > > +{
> > > + return irq_find_matching_fwnode(handle, DOMAIN_BUS_NEXUS);
> > > +}
> > > +
> > > +static void __init its_cdx_msi_init_one(struct device_node *np,
> > > + const char *name)
> > > +{
> > > + struct irq_domain *parent;
> > > + struct irq_domain *cdx_msi_domain;
> > > + struct fwnode_handle *fwnode_handle;
> > > + struct device_node *parent_node;
> > > +
> > > + parent_node = of_parse_phandle(np, "msi-parent", 0);
> > > +
> > > + parent = get_parent(of_node_to_fwnode(parent_node));
> > > + if (!parent || !msi_get_domain_info(parent)) {
> > > + pr_err("%s: unable to locate ITS domain\n", name);
> > > + return;
> > > + }
> > > +
> > > + fwnode_handle = of_node_to_fwnode(np);
> > > + cdx_msi_domain = platform_msi_create_irq_domain(fwnode_handle,
> > > + &its_cdx_msi_domain_info,
> > > + parent);
> > > + if (!cdx_msi_domain) {
> > > + pr_err("%s: unable to create cdx bus domain\n", name);
> > > + return;
> > > + }
> > > +
> > > + pr_info("cdx bus MSI: %s domain created\n", name);
> > > +}
> > > +
> > > +static void __init its_cdx_of_msi_init(void)
> > > +{
> > > + struct device_node *np;
> > > +
> > > + for (np = of_find_matching_node(NULL, cdx_device_id); np;
> > > + np = of_find_matching_node(np, cdx_device_id)) {
> > > + if (!of_device_is_available(np))
> > > + continue;
> > > +
> > > + its_cdx_msi_init_one(np, np->full_name);
> > > + }
> > > +}
> > > +
> > > +static int __init its_cdx_msi_init(void)
> > > +{
> > > + its_cdx_of_msi_init();
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +early_initcall(its_cdx_msi_init);
> >
> > I really don't think we should have any more of this muck. Yes, the
> > other busses are also doing that, but here's our chance to do
> > something better.
> >
> > Why can't the *bus* driver (I assume there is one) perform these
> > tasks? It would really help if this patch was shown in context,
> > because I have no idea how this fits in the grand scheme of things.
>
> Agree, bus driver could do this stuff, and I was considering it too,
> but as this was the norm so added these changes in this IRQ chip :).
Well, let's not miss this opportunity to improve things! :-)
> Bus driver is not out yet, and we have pushed these as RFC changes
> for early feedback (mentioned in the cover letter), but yes, these
> patches would be clubbed as a part of overall changes.
>
> And we can move it as a part of bus driver itself. I hope that suits?
It certainly sounds better than the above looks! Please Cc me on the
submission of this infrastructure.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
More information about the linux-arm-kernel
mailing list