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
