On Mon, Oct 14, 2019 at 01:21:31PM -0700, Sinan Kaya wrote: > On 10/14/2019 1:20 AM, Thierry Reding wrote: > > I think this makes sense, so: > > > > Reviewed-by: Thierry Reding <treding@xxxxxxxxxx> > > > > However, it looks like Sinan has researched this extensively in the past > > and gave a presentation on this at Plumbers in 2017: > > > > https://blog.linuxplumbersconf.org/2017/ocw/system/presentations/4732/original/crs.pdf > > > > Adding Sinan to see if he has any concerns about this, since resume time > > is explicitly mentioned in the above slides. > > > Thanks for including me. Let me catch up here. > > pci_dev_wait() is supposed to handle this case via pci_pm_reset(). > > /** > * pci_pm_reset - Put device into PCI_D3 and back into PCI_D0. > * @dev: Device to reset. > * @probe: If set, only check if the device can be reset this way. > */ > > Do you know if your execution path hits this function? We might have > missed a use case. > I see only a couple of callers of pci_device_is_present() in the tree, this being from next-20191015: $ git grep -n pci_device_is_present drivers/net/ethernet/broadcom/tg3.c:9070: if (!pci_device_is_present(tp->pdev))drivers/net/ethernet/broadcom/tg3.c:11785: if (pci_device_is_present(tp->pdev)) { drivers/net/ethernet/intel/igb/igb_main.c:8838: if (!pci_device_is_present(pdev)) drivers/nvme/host/pci.c:2866: if (!pci_device_is_present(pdev)) { drivers/pci/hotplug/acpiphp_glue.c:650: alive = pci_device_is_present(dev); drivers/pci/pci.c:935: !pci_device_is_present(dev)) { drivers/pci/pci.c:5902:bool pci_device_is_present(struct pci_dev *pdev) drivers/pci/pci.c:5910:EXPORT_SYMBOL_GPL(pci_device_is_present); drivers/thunderbolt/nhi.c:939: if (!pci_device_is_present(pdev)) { include/linux/pci.h:1206:bool pci_device_is_present(struct pci_dev *pdev); The NVME driver has the call in the ->remove() callback, so I don't think it's relevant here. Both TG3 and IGB ethernet drivers seem to call this during resume and so does Thunderbolt. Vidya, can you clarify for which device you're seeing the issues? Sounds like adding a call to pci_pm_reset() (via pci_reset_function()) at some point. Sinan, it looks as if pci_pm_reset() (or any of its callers) is not used very widely. Is that just because most drivers haven't had a need for it yet? Or am I missing some core functionality that calls this for every device anyway? Thierry
Attachment:
signature.asc
Description: PGP signature