On Fri, 9 Sep 2022 16:57:11 +0200 Lorenzo Pieralisi <lpieralisi@xxxxxxxxxx> wrote: > [+Marc, Thomas - I can't merge this code without them reviewing it, > I am not sure at all you can mix the timer/IRQ code the way you do] > > On Thu, Aug 18, 2022 at 03:51:32PM +0200, Marek Behún wrote: > > From: Pali Rohár <pali@xxxxxxxxxx> > > > > Add support for Data Link Layer State Change in the emulated slot > > registers and hotplug interrupt via the emulated root bridge. > > > > This is mainly useful for when an error causes link down event. With > > this change, drivers can try recovery. > > > > Link down state change can be implemented because Aardvark supports Link > > Down event interrupt. Use it for signaling that Data Link Layer Link is > > not active anymore via Hot-Plug Interrupt on emulated root bridge. > > > > Link up interrupt is not available on Aardvark, but we check for whether > > link is up in the advk_pcie_link_up() function. By triggering Hot-Plug > > Interrupt from this function we achieve Link up event, so long as the > > function is called (which it is after probe and when rescanning). > > Although it is not ideal, it is better than nothing. > > So before even coming to the code review: this patch does two things. > > 1) It adds support for handling the Link down state > 2) It adds some code to emulate a Link-up event > > Now, for (2). IIUC you are adding code to make sure that an HP > event is triggered if advk_pcie_link_up() is called and it > detects a Link-down->Link-up transition, that has to be notified > through an HP event. > > If that's correct, you have to explain to me please what this is > actually achieving and a specific scenario where we want this to be > implemented, in fine details; then we add it to the commit log. Hello Lorenzo, sorry for not replying earlier. Would something like this be sufficient? DLLSC is needed by the pciehp driver, which handles hotplug, but also link state change events. AFAIK no Aardvark devices support hotplug, but link state change events are required for graceful driver unbinding in case of link down event. So with this change we achieve graceful driver unbind for example when WiFi card PCIe link goes down (we've seen this with ath10k and mt76 WiFi cards). Before the WiFi driver started spitting out errors, or even taking the whole system down. Since after link goes down, it can come back up if the WiFi card can recover (or if reset pin is used to reset the card), we need to be able to recognize link up event. Since AFAIK Aardvark does not have interrupt for link up event, the best thing we can do is simulate it - whenever we read the link state, find it is up, and have cached value saying it is down, we trigger the link up event. We read link state whenever the configuration space is read, for example by writing 1 to /sys/bus/pci/rescan. Marek
