Re: [PATCH 1/4] arm64: pci: acpi: Use pci_assign_unassigned_root_bus_resources()

Bjorn Helgaas
2019-06-21T20:06:45Z

Match the subject line convention, e.g.,

arm64: PCI: Use pci_assign_unassigned_root_bus_resources()

But the function name doesn't really tell us anything unless we already
know how everything works. I think the point is that
pci_assign_unassigned_root_bus_resources() gives us the possibility of
reallocating things if necessary. A subject that hints at that would be
good.

On Sat, Jun 15, 2019 at 10:23:56AM +1000, Benjamin Herrenschmidt wrote:
> Instead of the simpler
>
> pci_bus_size_bridges(bus);
> pci_bus_assign_resources(bus);
>
> Use pci_assign_unassigned_root_bus_resources(). This should have no
> effect as long as we are reassigning everything.

pci_bus_size_bridges(bus) == __pci_bus_size_bridges(bus, NULL)
pci_bus_assign_resources(bus) == __pci_bus_assign_resources(bus, NULL, NULL)

and we have:

pci_assign_unassigned_root_bus_resources()
{
...
__pci_bus_size_bridges(bus, add_list);
__pci_bus_assign_resources(bus, add_list, &fail_head);

so I guess this should have no effect as long as we were able to
assign everything. If we were unable to assign something, previously
we did nothing and left it unassigned, but after this patch, we will
attempt to do some reallocation. Right?

> Once we start honoring FW resource allocations, this will bring up
> the "reallocation" feature which can help making room for SR-IOV
> when necessary.

I think this should be reordered so it's immediately before the patch
that checks hb->preserve_config, i.e., the patch that honors FW
assignments.

> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/arm64/kernel/pci.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index bb85e2f4603f..1419b1b4e9b9 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -193,8 +193,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
> if (!bus)
> return NULL;
>
> - pci_bus_size_bridges(bus);
> - pci_bus_assign_resources(bus);
> + pci_assign_unassigned_root_bus_resources(bus);
>
> list_for_each_entry(child, &bus->children, node)
> pcie_bus_configure_settings(child);
> --
> 2.17.1
>