Re: [PATCH v2] bus: hisi_lpc: Don't use devm_kzalloc() to allocate logical PIO range

[Bjorn Helgaas]

[+cc Rafael, Mika, Jiang, linux-pci for ACPI host bridge hotplug test question]

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?

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.

Bjorn