Re: [patch V2 34/46] PCI/MSI: Make arch_.*_msi_irq[s] fallbacks selectable

From: Marc Zyngier
Date: Fri Aug 28 2020 - 08:50:16 EST


Hi Jason,

On 2020-08-28 13:19, Jason Gunthorpe wrote:
On Fri, Aug 28, 2020 at 12:21:42PM +0100, Lorenzo Pieralisi wrote:
On Thu, Aug 27, 2020 at 01:20:40PM -0500, Bjorn Helgaas wrote:

[...]

> And I can't figure out what's special about tegra, rcar, and xilinx
> that makes them need it as well. Is there something I could grep for
> to identify them? Is there a way to convert them so they don't need
> it?

I think DT binding and related firmware support are needed to setup the
MSI IRQ domains correctly, there is nothing special about tegra, rcar
and xilinx AFAIK (well, all native host controllers MSI handling is
*special* just to be polite but let's gloss over this for the time
being).

struct msi_controller, to answer the first question.

I have doubts about pci_mvebu too, they do allocate an msi_controller
but without methods so it looks pretty much useless.

Oh, I did once know things about mvebu..

I suspect the msi controller pointer assignment is dead code at this
point. The only implementation of MSI with that PCI root port is
drivers/irqchip/irq-armada-370-xp.c which looks like it uses
irq_domain.

Actually looks like things are very close to eliminating
msi_controller.

This is dead code, can't find a setter for hw_pci->msi_ctrl:

arch/arm/include/asm/mach/pci.h: struct msi_controller *msi_ctrl;
arch/arm/kernel/bios32.c: bridge->msi =
hw->msi_ctrl;

This is probably just copying NULL from one place to another:

drivers/pci/controller/pci-mvebu.c: struct msi_controller *msi;

These need conversion to irq_domain (right?):

drivers/pci/controller/pci-hyperv.c: struct msi_controller msi_chip;
drivers/pci/controller/pci-tegra.c: struct msi_controller chip;
drivers/pci/controller/pcie-rcar-host.c: struct msi_controller chip;
drivers/pci/controller/pcie-xilinx.c:static struct msi_controller
xilinx_pcie_msi_chip = {

Then the stuff in drivers/pci/msi.c can go away.

So the arch_setup_msi_irq/etc is not really an arch hook, but some
infrastructure to support those 4 PCI root port drivers.

I happen to have a *really old* patch addressing Tegra [1], which
I was never able to test (no HW). Rebasing it shouldn't be too hard,
and maybe you can find someone internally willing to give it a spin?

That'd be a good start towards the removal of this cruft.

Thanks,

M.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=irq/kill-msi-controller&id=83b3602fcee7972b9d549ed729b56ec28de16081
--
Jazz is not dead. It just smells funny...