On 2018-07-03 20:04, Lukas Wunner wrote:
On Mon, Jul 02, 2018 at 06:52:47PM -0400, Sinan Kaya wrote:
@@ -308,8 +310,17 @@ void pcie_do_fatal_recovery(struct pci_dev *dev,
u32 service)
pci_dev_put(pdev);
}
+ hpsvc = pcie_port_find_service(udev, PCIE_PORT_SERVICE_HP);
+ hpdev = pcie_port_find_device(udev, PCIE_PORT_SERVICE_HP);
+
+ if (hpdev && hpsvc)
+ hpsvc->mask_irq(to_pcie_device(hpdev));
+
result = reset_link(udev, service);
+ if (hpdev && hpsvc)
+ hpsvc->unmask_irq(to_pcie_device(hpdev));
+
We've already got the ->reset_slot callback in struct hotplug_slot_ops,
I'm wondering if we really need additional ones for this use case.
If you just do...
if (!pci_probe_reset_slot(dev->slot))
pci_reset_slot(dev->slot)
else {
/* regular code path */
}
would that not be equivalent?
pcie_do_fatal_recovery() calls
reset_link()
which calls dpc_reset_link() or aer_root_reset() depending
on the event.
and dpc_reset_link() and aer_root_reset() do their own cleanup stuffs.
infact, aer_root_reset() is the only function which actually triggers
pci_reset_bridge_secondary_bus().
So if we try to fit in your suggestion:
if (!pci_probe_reset_slot(dev->slot))
{
pci_reset_slot(dev->slot)
result = reset_link(udev, service); >> in this case aer_root_reset
must not call pci_reset_bridge_secondary_bus()
} else
result = reset_link(udev, service); >> in this case aer_root_reset
must call pci_reset_bridge_secondary_bus() [since bridge is not hotplug
capable)
Did I get your suggestion right ?
Regards,
Oza.
(It's worth noting though that pciehp is the only hotplug driver
implementing the ->reset_slot callback. If hotplug is handled by
a different driver or by the platform firmware, devices may still
be removed and re-enumerated twice.)
Thanks,
Lukas
