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

[Bjorn Helgaas]

On Mon, Jun 17, 2019 at 3:47 AM John Garry <john.garry@xxxxxxxxxx> wrote:
>
> >>
> >> For the logical PIO framework, it was written to match what was done
> >> originally for PCI IO port management in pci_register_io_range(), cf
> >> https://elixir.bootlin.com/linux/v4.4.180/source/drivers/of/address.c#L691
> >>
> >> That is, no method to unregister ranges. As such, it leaks IO port
> >> ranges. I can come up with a few guesses why the original PCI IO port
> >> management author did not add an unregistration method.
> >
>
> Hi Bjorn,
>
> > I think that was written before the era of support for hot-pluggable
> > host bridges and loadable drivers for them.
>
> I see that the original support was added in 41f8bba7f555. I don't know
> how this coincides with hot-pluggable host bridges and their loadable
> drivers support.
>
> >
> >> Anyway, we can work on adding support to unregister regions, at least at
> >> probe time. It may become more tricky to do this once the host children
> >> have probed and are accessing the IO port regions.
> >
> > I think we *do* need support for unregistering regions because we do
> > claim to support hot-pluggable host bridges, and the I/O port regions
> > below them should go away when the host bridge does.
>
> It's now on my todo list.
>
> I'll need advice on how to test this for hot-pluggable host bridges.
>
> >
> > 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, 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.

Bjorn