On 22/07/15 10:30, Zhou Wang wrote:
> On 2015/7/13 18:43, 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 arch 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 relate 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 arch specific pcibios_msi_controller callback
>> and the related pci_sys_data msi_ctrl pointer.
>>
>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
>> Cc: Pratyush Anand <pratyush.anand@xxxxxxxxx>
>> Cc: Arnd Bergmann <arnd@xxxxxxxx>
>> Cc: Jingoo Han <jingoohan1@xxxxxxxxx>
>> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
>> Cc: Simon Horman <horms@xxxxxxxxxxxx>
>> Cc: Russell King <linux@xxxxxxxxxxxxxxxx>
>> Cc: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx>
>> Cc: Thierry Reding <thierry.reding@xxxxxxxxx>
>> Cc: Michal Simek <michal.simek@xxxxxxxxxx>
>> Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
>> ---
>> Bjorn, all,
>>
>> I am posting this patch to sound out if that's a reasonable approach.
>> xgene implements MSI look-up this way at present and this would represent
>> another step forward towards having common drivers for ARM/ARM64, comments
>> and testing welcome.
>>
>> Thanks,
>> Lorenzo
>>
>> arch/arm/include/asm/mach/pci.h | 3 ---
>> arch/arm/kernel/bios32.c | 35 +++++++++++++++++------------------
>> drivers/pci/host/pcie-designware.c | 9 +++++++--
>> drivers/pci/host/pcie-xilinx.c | 12 ++++++++++--
>> 4 files changed, 34 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
>> index 28b9bb3..32abc0c 100644
>> --- a/arch/arm/include/asm/mach/pci.h
>> +++ b/arch/arm/include/asm/mach/pci.h
>> @@ -42,9 +42,6 @@ struct hw_pci {
>> * Per-controller structure
>> */
>> struct pci_sys_data {
>> -#ifdef CONFIG_PCI_MSI
>> - struct msi_controller *msi_ctrl;
>> -#endif
>> struct list_head node;
>> int busnr; /* primary bus number */
>> u64 mem_offset; /* bus->cpu memory mapping offset */
>> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
>> index fcbbbb1..c841b33 100644
>> --- a/arch/arm/kernel/bios32.c
>> +++ b/arch/arm/kernel/bios32.c
>> @@ -18,15 +18,6 @@
>>
>> static int debug_pci;
>>
>> -#ifdef CONFIG_PCI_MSI
>> -struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
>> -{
>> - struct pci_sys_data *sysdata = dev->bus->sysdata;
>> -
>> - return sysdata->msi_ctrl;
>> -}
>> -#endif
>> -
>> /*
>> * We can't use pci_get_device() here since we are
>> * called from interrupt context.
>> @@ -462,9 +453,6 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
>> if (!sys)
>> panic("PCI: unable to allocate sys data!");
>>
>> -#ifdef CONFIG_PCI_MSI
>> - sys->msi_ctrl = hw->msi_ctrl;
>> -#endif
>> sys->busnr = busnr;
>> sys->swizzle = hw->swizzle;
>> sys->map_irq = hw->map_irq;
>> @@ -483,14 +471,25 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
>> break;
>> }
>>
>> - if (hw->scan)
>> + if (hw->scan) {
>> sys->bus = hw->scan(nr, sys);
>> - else
>> - sys->bus = pci_scan_root_bus(parent, sys->busnr,
>> - hw->ops, sys, &sys->resources);
>> + if (!sys->bus)
>> + panic("PCI: unable to scan bus!");
>> + } else {
>> + sys->bus = pci_create_root_bus(parent,
>> + sys->busnr,
>> + hw->ops, sys,
>> + &sys->resources);
>> + if (WARN_ON(!sys->bus)) {
>> + kfree(sys);
>> + break;
>> + }
>> +#ifdef CONFIG_PCI_MSI
>> + sys->bus->msi = hw->msi_ctrl;
>> +#endif
>>
>> - if (!sys->bus)
>> - panic("PCI: unable to scan bus!");
>> + pci_scan_child_bus(sys->bus);
>> + }
>>
>> busnr = sys->bus->busn_res.end + 1;
>>
>> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
>> index 69486be..e584dfa 100644
>> --- a/drivers/pci/host/pcie-designware.c
>> +++ b/drivers/pci/host/pcie-designware.c
>> @@ -526,7 +526,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
>>
>> #ifdef CONFIG_PCI_MSI
>> dw_pcie_msi_chip.dev = pp->dev;
>> - dw_pci.msi_ctrl = &dw_pcie_msi_chip;
>> #endif
>>
>> dw_pci.nr_controllers = 1;
>> @@ -708,11 +707,17 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
>> struct pcie_port *pp = sys_to_pcie(sys);
>>
>> pp->root_bus_nr = sys->busnr;
>> - bus = pci_scan_root_bus(pp->dev, sys->busnr,
>> + bus = pci_create_root_bus(pp->dev, sys->busnr,
>> &dw_pcie_ops, sys, &sys->resources);
>> if (!bus)
>> return NULL;
>>
>> +#ifdef CONFIG_PCI_MSI
>> + bus->msi = &dw_pcie_msi_chip;
>> +#endif
>> +
>> + pci_scan_child_bus(bus);
>> +
>> if (bus && pp->ops->scan_bus)
>> pp->ops->scan_bus(pp);
>>
>> diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
>> index f1a06a0..b21eb7d 100644
>> --- a/drivers/pci/host/pcie-xilinx.c
>> +++ b/drivers/pci/host/pcie-xilinx.c
>> @@ -647,9 +647,18 @@ static struct pci_bus *xilinx_pcie_scan_bus(int nr, struct pci_sys_data *sys)
>> struct pci_bus *bus;
>>
>> port->root_busno = sys->busnr;
>> - bus = pci_scan_root_bus(port->dev, sys->busnr, &xilinx_pcie_ops,
>> + bus = pci_create_root_bus(port->dev, sys->busnr, &xilinx_pcie_ops,
>> sys, &sys->resources);
>>
>> + if (!bus)
>> + return NULL;
>> +
>> +#ifdef CONFIG_PCI_MSI
>> + bus->msi = &xilinx_pcie_msi_chip;
>> +#endif
>> +
>> + pci_scan_child_bus(bus);
>> +
>> return bus;
>> }
>>
>> @@ -847,7 +856,6 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
>>
>> #ifdef CONFIG_PCI_MSI
>> xilinx_pcie_msi_chip.dev = port->dev;
>> - hw.msi_ctrl = &xilinx_pcie_msi_chip;
>> #endif
>> pci_common_init_dev(dev, &hw);
>>
>>
>
> Hi Lorenzo,
>
> I wonder if we can delete weak pcibios_msi_controller and the call in
> pci_msi_controller in drivers/pci/msi.c
Yeah, I think we can replace it with a straight "return NULL;" in
pci_msi_controller, as ARM is the only arch to implement this function
(which Lorenzo now removes).
Thanks,
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
