Re: [PATCH v4 0/3] PCI: pciehp: Do not turn off slot if presence comes up after link

[Lukas Wunner]

On Tue, Feb 11, 2020 at 08:14:44AM -0600, Bjorn Helgaas wrote:
> I'm a little confused about why pci_hp_initialize()/
> __pci_hp_initialize()/pci_hp_register()/__pci_hp_register() is such a
> rat's nest with hotplug drivers using a mix of them.

This is modeled after device registration, which can be done either
in two steps (device_initialize() + device_add()) or in 1 step
(device_register()).

So it's either pci_hp_initialize() + pci_hp_add() or pci_hp_register().

The rationale is provided in the commit message of 51bbf9bee34f
("PCI: hotplug: Demidlayer registration with the core").


> Feels like sort of a
> double-negative situation, too.  Obviously the hardware bit has to be
> "1 means disabled" to be compatible with previous spec versions, but
> the code is usually easier to read if we test for something being
> *enabled*.

It's a similar situation with the "DisINTx" bit in the Command
register, which, if disabled, is shown as "DisINTx-" in lspci even
though the more intuitive notion is that INTx is *enabled*.  I think
you did the right thing by showing it as "IbPresDis-" because it's
consistent with how it's done elsewhere for similar bits.

Thanks,

Lukas