On Wed, 22 Jan 2025 17:34:51 +0000,
Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> wrote:
>
> + Marc (for the IRQCHIP implementation review)
>
> On Wed, Jan 22, 2025 at 09:28:12PM +0800, Chen Wang wrote:
> >
> > > > +static int sg2042_pcie_setup_msi(struct sg2042_pcie *pcie,
> > > > + struct device_node *msi_node)
> > > > +{
> > > > + struct device *dev = pcie->cdns_pcie->dev;
> > > > + struct fwnode_handle *fwnode = of_node_to_fwnode(dev->of_node);
> > > > + struct irq_domain *parent_domain;
> > > > + int ret = 0;
> > > > +
> > > > + if (!of_property_read_bool(msi_node, "msi-controller"))
> > > > + return -ENODEV;
> > > > +
> > > > + ret = of_irq_get_byname(msi_node, "msi");
> > > > + if (ret <= 0) {
> > > > + dev_err(dev, "%pOF: failed to get MSI irq\n", msi_node);
> > > > + return ret;
> > > > + }
> > > > + pcie->msi_irq = ret;
> > > > +
> > > > + irq_set_chained_handler_and_data(pcie->msi_irq,
> > > > + sg2042_pcie_msi_chained_isr, pcie);
> > > > +
> > > > + parent_domain = irq_domain_create_linear(fwnode, MSI_DEF_NUM_VECTORS,
> > > > + &sg2042_pcie_msi_domain_ops, pcie);
> > > > + if (!parent_domain) {
> > > > + dev_err(dev, "%pfw: Failed to create IRQ domain\n", fwnode);
> > > > + return -ENOMEM;
> > > > + }
> > > > + irq_domain_update_bus_token(parent_domain, DOMAIN_BUS_NEXUS);
> > > > +
> > > The MSI controller is wired to PLIC isn't it? If so, why can't you use
> > > hierarchial MSI domain implementation as like other controller drivers?
> >
> > The method used here is somewhat similar to dw_pcie_allocate_domains() in
> > drivers/pci/controller/dwc/pcie-designware-host.c. This MSI controller is
> > about Method A, the PCIe controller implements an MSI interrupt controller
> > inside, and connect to PLIC upward through only ONE interrupt line. Because
> > MSI to PLIC is multiple to one, I use linear mode here and use chained ISR
> > to handle the interrupts.
> >
>
> Hmm, ok. I'm not an IRQCHIP expert, but I'll defer to Marc to review the IRQCHIP
> implementation part.
I don't offer this service anymore, I'm afraid.
As for the "I create my own non-hierarchical IRQ domain", this is
something that happens for all completely mis-designed interrupt
controllers, MSI or not, that multiplex interrupts.
These implementations are stuck in the previous century, and seeing
this on modern designs, for a "server SoC", is really pathetic.
maybe you now understand why I don't offer this sort of reviewing
service anymore.
M.
--
Without deviation from the norm, progress is not possible.
