On 18/06/2019 18:50, Bjorn Helgaas wrote:
[+cc Rafael, Mika, Jiang, linux-pci for ACPI host bridge hotplug test question]
Resend with bouncing addresses removed/fixed
On Tue, Jun 18, 2019 at 3:44 AM John Garry <john.garry@xxxxxxxxxx> wrote:
Could you just move the logic_pio_register_range() call farther down
in hisi_lpc_probe()? IIUC, once logic_pio_register_range() returns,
an inb() with the right port number will try to access that port, so
we should be prepared for that, i.e., maybe this in the wrong order to
begin with?
No, unfortunately we can't. The reason is that we need the logical PIO
base for that range before we enumerate the children of that host. We
need that base address for "translating" the child bus addresses to
logical PIO addresses.
Ah, yeah, that makes sense. I think. We do assume that we know all
the MMIO and I/O port translations before enumerating devices. It's
*conceivable* that could be changed someday since we don't actually
need the translations until a driver claims the device,
We actually need them before a driver claims the device.
The reason is that when we create that child platform device we set the
device's IORESOURCE_IO resources according to the translated logic PIO
addresses, and not the host bus address. This is what makes the host
transparent to the child device driver.
I think you need it to set pdev->resource[], which is currently done
long before the driver claims the device (though one could imagine
delaying it even as far as pci_enable_device()-time). I don't think
the translation is actually *used* until the driver claims the device
because only the driver knows how to do any inb/outb to the device.
But of course, that's all speculative and doesn't change what you need
to do now. The current code assumes we know the translations during
enumeration, so you need to do the logic_pio registration before
enumerating.
and it would
gain some flexibility if we didn't have to program the host bridge
windows until we know how much space is required. But I don't see
that happening anytime soon.
My problem is that I need to ensure that the new logical PIO unregister
function works ok for hot-pluggable host bridges. I need to get some way
to test this. Advice?
Hi Bjorn,
Good question. The ACPI host bridge driver (drivers/acpi/pci_root.c)
should support hotplug, but I'm not sure if there's a manual way to
trigger it via sysfs or something similar. If there is, and you have
a machine with more than one host bridge, you might be able to remove
one that leads to non-essential devices.
For one of our earlier boards I don't think that it had any essential
devices on the host bridge. But I need to find out about possibility of
removal. Hmmm.
Bjorn
Further to the topic of supporting hotplug and unregistering IO port
regions, we don't even release IO port regions in the error path of PCI
host enumeration. We have pci_register_io_range(), but no unregister
equivalent.
Looking at the history here, pci_register_io_range() was originally in
OF code. And in the OF code, calling pci_register_io_range() is a
side-effect of parsing the device tree. As such, I can see why there was
no unregister function.
It would be worth noting this discussion, where the same was mentioned:
https://lore.kernel.org/linux-pci/20180403140410.GE27789@ulmo/
The tegra PCI host probe can defer, but, since there is no tidy-up of
pci_register_io_range() when deferring, we need to ensure that the port
IO management code can handle re-attempts to register the same range.
It looks like this can be cleaned up also.
Thanks,
John
.
