[PATCH v2 2/2] ARM: pci: kill pcibios_msi_controller

[Lorenzo Pieralisi]

On Sun, Jul 26, 2015 at 04:25:29PM +0100, Jayachandran C. wrote:
> On Fri, Jul 24, 2015 at 05:13:03PM +0100, Lorenzo Pieralisi wrote:
> > On ARM PCI systems relying on the pcibios API to initialize PCI host
> > controllers, the pcibios_msi_controller weak callback is used to look-up
> > the msi_controller pointer, through pci_sys_data msi_ctrl pointer.
> > 
> > pci_sys_data is an ARM specific structure, which prevents using the
> > same mechanism (so same PCI host controller drivers) on ARM64 systems.
> > 
> > Since the struct pci_bus already contains an msi_controller pointer and
> > the kernel already uses it to look-up the msi controller,
> > this patch converts ARM host controller and related pcibios/host bridges
> > initialization routines so that the msi_controller pointer look-up can be
> > carried out by PCI core code through the struct pci_bus msi pointer,
> > removing the need for the arch specific pcibios_msi_controller callback
> > and the related pci_sys_data msi_ctrl pointer.
> > 
> > ARM is the only arch relying on the pcibios_msi_controller() weak
> > function, hence this patch removes the default weak implementation
> > from PCI core code since it becomes of no use.
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi at arm.com>
> > Acked-by: Marc Zyngier <marc.zyngier at arm.com>
> > Cc: Pratyush Anand <pratyush.anand at gmail.com>
> > Cc: Arnd Bergmann <arnd at arndb.de>
> > Cc: Jingoo Han <jingoohan1 at gmail.com>
> > Cc: Bjorn Helgaas <bhelgaas at google.com>
> > Cc: Simon Horman <horms at verge.net.au>
> > Cc: Russell King <linux at arm.linux.org.uk>
> > Cc: Thomas Petazzoni <thomas.petazzoni at free-electrons.com>
> > Cc: Thierry Reding <thierry.reding at gmail.com>
> > Cc: Michal Simek <michal.simek at xilinx.com>
> > Cc: Marc Zyngier <marc.zyngier at arm.com>
> > ---
> > v1->v2
> > 
> > - Added patch to replace panic statements with WARN
> > - Removed unused pcibios_msi_controller() and pci_msi_controller() from
> >   core code
> > - Dropped RFT status
> > 
> > v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/356028.html
> > 
> >  arch/arm/include/asm/mach/pci.h    |  3 ---
> >  arch/arm/kernel/bios32.c           | 29 +++++++++++++----------------
> >  drivers/pci/host/pcie-designware.c |  9 +++++++--
> >  drivers/pci/host/pcie-xilinx.c     | 12 ++++++++++--
> >  drivers/pci/msi.c                  | 17 +----------------
> >  5 files changed, 31 insertions(+), 39 deletions(-)
> > 
> [...]
> > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> > index f66be86..0d20142 100644
> > --- a/drivers/pci/msi.c
> > +++ b/drivers/pci/msi.c
> > @@ -77,24 +77,9 @@ static void pci_msi_teardown_msi_irqs(struct pci_dev *dev)
> >  
> >  /* Arch hooks */
> >  
> > -struct msi_controller * __weak pcibios_msi_controller(struct pci_dev *dev)
> > -{
> > -	return NULL;
> > -}
> > -
> > -static struct msi_controller *pci_msi_controller(struct pci_dev *dev)
> > -{
> > -	struct msi_controller *msi_ctrl = dev->bus->msi;
> > -
> > -	if (msi_ctrl)
> > -		return msi_ctrl;
> > -
> > -	return pcibios_msi_controller(dev);
> > -}
> > -
> >  int __weak arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
> >  {
> > -	struct msi_controller *chip = pci_msi_controller(dev);
> > +	struct msi_controller *chip = dev->bus->msi;
> >  	int err;
> >  
> >  	if (!chip || !chip->setup_irq)
> 
> Don't you have to go to the top level bus and get the ->msi pointer? Something
> like:
> 
> 	for (bus = dev->bus; bus != NULL; bus = bus->parent)
> 		if (bus->msi)
> 			return bus->msi;
> 
> I have not been following this closely, so I may have missed some patches.

The msi pointer is initialized from parent to child in
pci_alloc_child_bus(), so PCI core does that for us,
that's my understanding.

It works the same as sysdata pointer, which is why
pcibios_msi_controller works in the current kernel.

On a side note, are you able to prepare a new version
of your set to enable the PCI generic host on ARM64
(and remove pci_sys_data and related ifdef CONFIG_ARM
from it) ?

We are not far from removing the pci_sys_data dependency,
if you can't prepare a new version of your series let me
know I can do it on your behalf.

Thanks !
Lorenzo