[AMD Public Use] > -----Original Message----- > From: Bjorn Helgaas <helgaas@xxxxxxxxxx> > Sent: Sunday, September 13, 2020 11:43 AM > To: Lukas Wunner <lukas@xxxxxxxxx> > Cc: Huacai Chen <chenhc@xxxxxxxxxx>; Bjorn Helgaas > <bhelgaas@xxxxxxxxxx>; Deucher, Alexander > <Alexander.Deucher@xxxxxxx>; linux-pci@xxxxxxxxxxxxxxx; Huacai Chen > <chenhuacai@xxxxxxxxx>; Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx> > Subject: Re: [PATCH 1/2] PCI/portdrv: Remove the .remove() method in > pcie_portdriver > > On Sun, Sep 13, 2020 at 07:01:29AM +0200, Lukas Wunner wrote: > > On Fri, Sep 11, 2020 at 06:09:36PM +0800, Huacai Chen wrote: > > > As Bjorn Helgaas said, portdrv can only be built statically (not as > > > a module), so the .remove() method in pcie_portdriver is useless. So > > > just remove it. > > > > No, PCIe switches (containing upstream and downstream PCIe ports) can > > be hot-plugged and hot-removed at runtime. Every Thunderbolt device > > contains a PCIe switch and is hot-pluggable. We do want to clean up a > > hot-removed PCIe port properly. > > Right, sorry, I was thinking only of driver unbinding, not of device removal. > Sorry to have wasted your time. > FWIW, our newer GPUs have both upstream and downstream ports that are part of the device. Slightly off topic, but does the current pm code handle these cases correctly? ACPI related power handling doesn't seem to work correctly for these devices in laptops where the GPU power control is handled by ACPI. Alex > > > --- a/drivers/pci/pcie/portdrv_pci.c > > > +++ b/drivers/pci/pcie/portdrv_pci.c > > > @@ -134,7 +134,7 @@ static int pcie_portdrv_probe(struct pci_dev *dev, > > > return 0; > > > } > > > > > > -static void pcie_portdrv_remove(struct pci_dev *dev) > > > +static void pcie_portdrv_shutdown(struct pci_dev *dev) > > > { > > > if (pci_bridge_d3_possible(dev)) { > > > pm_runtime_forbid(&dev->dev); > > > @@ -210,8 +210,7 @@ static struct pci_driver pcie_portdriver = { > > > .id_table = &port_pci_ids[0], > > > > > > .probe = pcie_portdrv_probe, > > > - .remove = pcie_portdrv_remove, > > > - .shutdown = pcie_portdrv_remove, > > > + .shutdown = pcie_portdrv_shutdown, > > > > > > .err_handler = &pcie_portdrv_err_handler,