On Sat, Sep 01, 2018 at 05:18:36PM +0200, Lukas Wunner wrote: > * In pcie_enable_notification() there's a code comment: > "TBD: Power fault detected software notification support" > Is there really anything left to be done or can this code comment be > deleted? AFAICS, a power fault *is* detected and acted upon, then > has to be cleared by the user, either through enablement via sysfs > or by physically replacing the card. > > * In pciehp_ist(), the if-condition to check for a power fault also > checks for "&& !ctrl->power_fault_detected". The check seems > superfluous to me because pciehp_isr() clears the PCI_EXP_SLTSTA_PFD > bit in the pending events if ctrl->power_fault_detected is true. > Can the additional check be removed from the if-condition in > pciehp_ist()? The slot is supposed to have PFD set while the controller detects a power fault. The host acknowledging the event doesn't actually make the power fault stop exisiting, so we have to make the observation sticky until it goes away. > * In pciehp_ist(), we call pciehp_green_led_off() if a power fault is > detected. However my (limited) understanding is that a power fault > always results in a Link Down, and when bringing down the slot in > response to that, we already call pciehp_green_led_off() in > remove_board(). Can the call to pciehp_green_led_off() be removed > from pciehp_ist()? Or does the link not necessarily go down on a > power fault and we should rather bring the slot down when a power > fault is detected? Is that the "TBD" mentioned in the code comment? > Or is this just an intentional cosmetic behavior that we turn off > the green LED as early as possible once we detect the slot is off?
