On Fri, Mar 21, 2025 at 07:07:47PM +0100, Lukas Wunner wrote:
> On Fri, Mar 21, 2025 at 12:09:19PM -0500, Bjorn Helgaas wrote:
> ...
> > - It's annoying that pcie_enable_interrupt() and
> > pcie_disable_interrupt() are global symbols, a consequence of
> > pciehp being split across five files instead of being one, which
> > is also a nuisance for code browsing.
>
> Roughly,
> pciehp_core.c contains the interface to the PCI hotplug core
> (registering the hotplug_slot_ops etc),
> pciehp_hpc.c contains the interaction with hardware registers,
> pciehp_core.c contains the state machine,
> pciehp_pci.c contains the interaction with the PCI core
> (enumeration / de-enumeration of devices on slot bringup / bringdown).
>
> The only reason I've refrained from making major adjustments to this
> structure in the past was that it would make "git blame" a little more
> difficult and applying fixes to stable kernels would also become somewhat
> more painful as it would require backporting.
Yeah, that's the main reason I haven't tried to do anything either.
On the other hand, the browsing nuisance is an everyday thing forever
if we leave it as-is. I did consolidate portdrv.c a couple years ago
and don't regret it. But moving things definitely makes "git blame" a
bit of a hassle; my notes are full of things like this:
a1ccd3d91138 ("PCI/portdrv: Squash into portdrv.c")
squash drivers/pci/pcie/portdrv_pci.c and portdrv_core.c into portdrv.c
950bf6388bc2 ("PCI: Move DesignWare IP support to new drivers/pci/dwc/ directory")
mv drivers/pci/host/pci-imx6.c drivers/pci/dwc/pci-imx6.c
6e0832fa432e ("PCI: Collect all native drivers under drivers/pci/controller/")
mv drivers/pci/dwc/pci-imx6.c drivers/pci/controller/dwc/pci-imx6.c
> > - I forgot why we have both pcie_write_cmd() and
> > pcie_write_cmd_nowait() and how to decide which to use.
>
> pcie_write_cmd_nowait() is the "fire and forget" variant,
> whereas pcie_write_cmd() can be thought of as the "_sync" variant,
> i.e. the control flow doesn't continue until the command has been
> processed by the slot.
>
> E.g. pciehp_power_on_slot() waits for the slot command to complete
> before making sure the Link Disable bit is clear. It wouldn't make
> much sense to do the latter when the former hasn't been completed yet.
Right, I know what the difference is; I guess I just don't know how to
figure out when pcie_write_cmd_nowait() is safe.
Bjorn
