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

[Marc Zyngier]

On 26/07/15 19:43, Jayachandran C. wrote:
> On Sun, Jul 26, 2015 at 07:16:46PM +0100, Lorenzo Pieralisi wrote:
>> 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.
> 
> Thanks.
> 
>> 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.
> 
> I will post an updated patch with a minor fix. I will also
> add a patch for parsing msi-parent and setting ->msi on
> the root bus.

That shouldn't be necessary if you base your series on this one:

http://lwn.net/Articles/652151/

Patch 5 in this series does it for you as part of the core PCI code.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...