Message ID | 20200925230138.29011-1-ashok.raj@intel.com (mailing list archive) |
---|---|
State | Superseded, archived |
Delegated to: | Bjorn Helgaas |
Headers | show |
Series | [1/1] pci: pciehp: Handle MRL interrupts to enable slot for hotplug. | expand |
Hi Bjorn On Fri, Sep 25, 2020 at 04:01:38PM -0700, Ashok Raj wrote: > When Mechanical Retention Lock (MRL) is present, Linux doesn't process > those change events. > > The following changes need to be enabled when MRL is present. > > 1. Subscribe to MRL change events in SlotControl. > 2. When MRL is closed, > - If there is no ATTN button, then POWER on the slot. > - If there is ATTN button, and an MRL event pending, ignore > Presence Detect. Since we want ATTN button to drive the > hotplug event. > Did you get a chance to review this? Thought i'll just check with you. Seems like there was a lot happening in the error handling and hotplug side, I thought you might have wanted to wait for the dust to settle :-) > > Signed-off-by: Ashok Raj <ashok.raj@intel.com> > Co-developed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > --- > drivers/pci/hotplug/pciehp.h | 1 + > drivers/pci/hotplug/pciehp_ctrl.c | 69 +++++++++++++++++++++++++++++++++++++++ > drivers/pci/hotplug/pciehp_hpc.c | 27 ++++++++++++++- > 3 files changed, 96 insertions(+), 1 deletion(-) >
Hi Ashok, my sincere apologies for the delay. On Fri, Sep 25, 2020 at 04:01:38PM -0700, Ashok Raj wrote: > When Mechanical Retention Lock (MRL) is present, Linux doesn't process > those change events. > > The following changes need to be enabled when MRL is present. > > 1. Subscribe to MRL change events in SlotControl. > 2. When MRL is closed, > - If there is no ATTN button, then POWER on the slot. > - If there is ATTN button, and an MRL event pending, ignore > Presence Detect. Since we want ATTN button to drive the > hotplug event. So I understand you have a hardware platform which has both MRL and Attention Button on its hotplug slots? It may be useful to name the hardware platform in the commit message. If an Attention Button is present, the current behavior is to bring up the hotplug slot as soon as presence or link is detected. We don't wait for a button press. This is intended as a convience feature to bring up slots as quickly as possible, but the behavior can be changed if the button press and 5 second delay is preferred. In any case the behavior in response to an Attention Button press should be the same regardless whether an MRL is present or not. (The spec doesn't seem to say otherwise.) > + if (MRL_SENS(ctrl)) { > + pciehp_get_latch_status(ctrl, &getstatus); > + /* > + * If slot is closed && ATTN button exists > + * don't continue, let the ATTN button > + * drive the hot-plug > + */ > + if (!getstatus && ATTN_BUTTN(ctrl)) > + return; > + } For the sake of readability I'd suggest adding a pciehp_latch_closed() helper similar to pciehp_card_present_or_link_active() which returns true if no MRL is present (i.e. !MRL_SENS(ctrl)), otherwise retrieves and returns the status with pciehp_get_latch_status(). > +void pciehp_handle_mrl_change(struct controller *ctrl) > +{ > + u8 getstatus = 0; > + int present, link_active; > + > + pciehp_get_latch_status(ctrl, &getstatus); > + > + present = pciehp_card_present(ctrl); > + link_active = pciehp_check_link_active(ctrl); > + > + ctrl_info(ctrl, "Slot(%s): Card %spresent\n", > + slot_name(ctrl), present ? "" : "not "); > + > + ctrl_info(ctrl, "Slot(%s): Link %s\n", > + slot_name(ctrl), link_active ? "Up" : "Down"); This duplicates a lot of code from pciehp_handle_presence_or_link_change(), which I think suggests handling MRL events in that function. After all, an MRL event may lead to bringup or bringdown of a slot similar to a link or presence event. Basically pciehp_handle_presence_or_link_change() just needs to be amended to bring down the slot if an MRL event occurred, and afterwards only bring up the slot if MRL is closed. Like this: - if (present <= 0 && link_active <= 0) { + if ((present <= 0 && link_active <= 0) || !latch_closed) { mutex_unlock(&ctrl->state_lock); return; } > + /* > + * Need to handle only MRL Open. When MRL is closed with > + * a Card Present, either the ATTN button, or the PDC indication > + * should power the slot and add the card in the slot > + */ I agree with the second sentence. You may want to refer to PCIe Base Spec r4.0, section 6.7.1.3 either in the commit message or a code comment, which says: "If an MRL Sensor is implemented without a corresponding MRL Sensor input on the Hot-Plug Controller, it is recommended that the MRL Sensor be routed to power fault input of the Hot-Plug Controller. This allows an active adapter to be powered off when the MRL is opened." This seems to suggest that the slot should be brought down as soon as MRL is opened. > + /* > + * If MRL is triggered, if ATTN button exists, ignore the event. > + */ > + if (ATTN_BUTTN(ctrl) && (events & PCI_EXP_SLTSTA_MRLSC)) > + events &= ~PCI_EXP_SLTSTA_PDC; Hm, the spec doesn't seem to imply a connection between presence of an MRL and presence of an Attention Button, so I find this confusing. > + /* > + * If ATTN is present and MRL is triggered > + * ignore the Presence Change Event. > + */ > + if (ATTN_BUTTN(ctrl) && (events & PCI_EXP_SLTSTA_MRLSC)) > + events &= ~PCI_EXP_SLTSTA_PDC; An Attention Button press results in a synthesized PDC event after 5 seconds, which may get lost due to the above if-statement. Thanks, Lukas
On Thu, Nov 19, 2020 at 08:51:20AM +0100, Lukas Wunner wrote: > Hi Ashok, > > my sincere apologies for the delay. No worries.. > > On Fri, Sep 25, 2020 at 04:01:38PM -0700, Ashok Raj wrote: > > When Mechanical Retention Lock (MRL) is present, Linux doesn't process > > those change events. > > > > The following changes need to be enabled when MRL is present. > > > > 1. Subscribe to MRL change events in SlotControl. > > 2. When MRL is closed, > > - If there is no ATTN button, then POWER on the slot. > > - If there is ATTN button, and an MRL event pending, ignore > > Presence Detect. Since we want ATTN button to drive the > > hotplug event. > > So I understand you have a hardware platform which has both MRL and > Attention Button on its hotplug slots? It may be useful to name the > hardware platform in the commit message. I should, let me find out the exact intercept and include it next. > > If an Attention Button is present, the current behavior is to bring up > the hotplug slot as soon as presence or link is detected. We don't wait > for a button press. This is intended as a convience feature to bring up > slots as quickly as possible, but the behavior can be changed if the > button press and 5 second delay is preferred. No personal preference, I thought that is how the code in Linux was earlier. Looks like we still don't subscribe to PDC if ATTN is present? So you don't get an event until someone presses the button to process hotplug right? pciehp_hpc.c:pcie_enable_notification() { .... * Always enable link events: thus link-up and link-down shall * always be treated as hotplug and unplug respectively. Enable * presence detect only if Attention Button is not present. */ cmd = PCI_EXP_SLTCTL_DLLSCE; if (ATTN_BUTTN(ctrl)) cmd |= PCI_EXP_SLTCTL_ABPE; else cmd |= PCI_EXP_SLTCTL_PDCE; .... } Looks like we still wait for button press to process. When you don't have a power control yes the DLLSC would kick in and we would process them. but if you have power control, you won't turn on until the button press? > > In any case the behavior in response to an Attention Button press should > be the same regardless whether an MRL is present or not. (The spec > doesn't seem to say otherwise.) True, Looks like that is a Linux behavior, certainly not a spec recommendation. Our validation teams tell Windows also behaves such. i.e when ATTN exists button press drivers the hot-add. Linux doesn't need to follow suite though. In that case do we need to subscribe to PDC even when ATTN is present? > > > > + if (MRL_SENS(ctrl)) { > > + pciehp_get_latch_status(ctrl, &getstatus); > > + /* > > + * If slot is closed && ATTN button exists > > + * don't continue, let the ATTN button > > + * drive the hot-plug > > + */ > > + if (!getstatus && ATTN_BUTTN(ctrl)) > > + return; > > + } > > For the sake of readability I'd suggest adding a pciehp_latch_closed() > helper similar to pciehp_card_present_or_link_active() which returns > true if no MRL is present (i.e. !MRL_SENS(ctrl)), otherwise retrieves > and returns the status with pciehp_get_latch_status(). Makes sense.. I can add that. > > > > +void pciehp_handle_mrl_change(struct controller *ctrl) > > +{ > > + u8 getstatus = 0; > > + int present, link_active; > > + > > + pciehp_get_latch_status(ctrl, &getstatus); > > + > > + present = pciehp_card_present(ctrl); > > + link_active = pciehp_check_link_active(ctrl); > > + > > + ctrl_info(ctrl, "Slot(%s): Card %spresent\n", > > + slot_name(ctrl), present ? "" : "not "); > > + > > + ctrl_info(ctrl, "Slot(%s): Link %s\n", > > + slot_name(ctrl), link_active ? "Up" : "Down"); > > This duplicates a lot of code from pciehp_handle_presence_or_link_change(), > which I think suggests handling MRL events in that function. After all, > an MRL event may lead to bringup or bringdown of a slot similar to > a link or presence event. > > Basically pciehp_handle_presence_or_link_change() just needs to be > amended to bring down the slot if an MRL event occurred, and > afterwards only bring up the slot if MRL is closed. Like this: > > - if (present <= 0 && link_active <= 0) { > + if ((present <= 0 && link_active <= 0) || !latch_closed) { Certainly looks like good simplication. I'll change and have a test run. > mutex_unlock(&ctrl->state_lock); > return; > } > > > > + /* > > + * Need to handle only MRL Open. When MRL is closed with > > + * a Card Present, either the ATTN button, or the PDC indication > > + * should power the slot and add the card in the slot > > + */ > > I agree with the second sentence. You may want to refer to PCIe Base Spec > r4.0, section 6.7.1.3 either in the commit message or a code comment, > which says: > > "If an MRL Sensor is implemented without a corresponding MRL Sensor input > on the Hot-Plug Controller, it is recommended that the MRL Sensor be > routed to power fault input of the Hot-Plug Controller. > This allows an active adapter to be powered off when the MRL is opened." > > This seems to suggest that the slot should be brought down as soon as MRL > is opened. Good for commit log. I'll add a pointer in code comments too. Rarely people go to commit log :-) > > > > + /* > > + * If MRL is triggered, if ATTN button exists, ignore the event. > > + */ > > + if (ATTN_BUTTN(ctrl) && (events & PCI_EXP_SLTSTA_MRLSC)) > > + events &= ~PCI_EXP_SLTSTA_PDC; > > Hm, the spec doesn't seem to imply a connection between presence of > an MRL and presence of an Attention Button, so I find this confusing. This isn't spec required. But to enforce the earlier behavior that only ATTN drives the hot-add event. Sometimes you close the MRL, even if you didn't subscribe for notification of PDC, the bit is set and un-intentionally acted upon. If the event wasn't subscribed for to start with PDC getting pulled in is a side effect of the MRL. Validation folks look for consistency :-).. so this was mostly to enforce that. > > > > + /* > > + * If ATTN is present and MRL is triggered > > + * ignore the Presence Change Event. > > + */ > > + if (ATTN_BUTTN(ctrl) && (events & PCI_EXP_SLTSTA_MRLSC)) > > + events &= ~PCI_EXP_SLTSTA_PDC; > > An Attention Button press results in a synthesized PDC event after > 5 seconds, which may get lost due to the above if-statement. When its synthesized you don't get the MRLSC? So we won't nuke the PDC then correct? Cheers, Ashok
Hi Ashok, When you update this patch, can you run "git log --oneline drivers/pci/hotplug/pciehp*" and make yours match? It is a severe character defect of mine, but seeing subject lines that are obviously different than the convention is a disincentive to look at the patch. On Thu, Nov 19, 2020 at 02:08:07PM -0800, Raj, Ashok wrote: > On Thu, Nov 19, 2020 at 08:51:20AM +0100, Lukas Wunner wrote: > > Hi Ashok, > > > > my sincere apologies for the delay. > > No worries.. > > > > > On Fri, Sep 25, 2020 at 04:01:38PM -0700, Ashok Raj wrote: > > > When Mechanical Retention Lock (MRL) is present, Linux doesn't process > > > those change events. > > > > > > The following changes need to be enabled when MRL is present. > > > > > > 1. Subscribe to MRL change events in SlotControl. > > > 2. When MRL is closed, > > > - If there is no ATTN button, then POWER on the slot. > > > - If there is ATTN button, and an MRL event pending, ignore > > > Presence Detect. Since we want ATTN button to drive the > > > hotplug event.
On Thu, Nov 19, 2020 at 04:27:41PM -0600, Bjorn Helgaas wrote: > Hi Ashok, > > When you update this patch, can you run "git log --oneline > drivers/pci/hotplug/pciehp*" and make yours match? It is a severe > character defect of mine, but seeing subject lines that are obviously :-) > different than the convention is a disincentive to look at the patch. > Thanks for the reminder. forgot the cap the PCI, and some inadvertant '.' at the end of the line... Will fix when i resend.
Hi, On 11/19/20 2:08 PM, Raj, Ashok wrote: >> If an Attention Button is present, the current behavior is to bring up >> the hotplug slot as soon as presence or link is detected. We don't wait >> for a button press. This is intended as a convience feature to bring up >> slots as quickly as possible, but the behavior can be changed if the >> button press and 5 second delay is preferred. > No personal preference, I thought that is how the code in Linux was > earlier. > > Looks like we still don't subscribe to PDC if ATTN is present? So you don't > get an event until someone presses the button to process hotplug right? > > pciehp_hpc.c:pcie_enable_notification() > { > .... > > * Always enable link events: thus link-up and link-down shall > * always be treated as hotplug and unplug respectively. Enable > * presence detect only if Attention Button is not present. > */ > cmd = PCI_EXP_SLTCTL_DLLSCE; > if (ATTN_BUTTN(ctrl)) > cmd |= PCI_EXP_SLTCTL_ABPE; > else > cmd |= PCI_EXP_SLTCTL_PDCE; > .... > } > > > Looks like we still wait for button press to process. When you don't have a > power control yes the DLLSC would kick in and we would process them. but if > you have power control, you won't turn on until the button press? > Yes, as per current logic, if attention button is present, then we don't subscribe to PDC event changes. we wait for button pressĀ to turn on/off the slot.
On Thu, Nov 19, 2020 at 02:08:07PM -0800, Raj, Ashok wrote: > On Thu, Nov 19, 2020 at 08:51:20AM +0100, Lukas Wunner wrote: > > If an Attention Button is present, the current behavior is to bring up > > the hotplug slot as soon as presence or link is detected. We don't wait > > for a button press. This is intended as a convience feature to bring up > > slots as quickly as possible, but the behavior can be changed if the > > button press and 5 second delay is preferred. > > Looks like we still don't subscribe to PDC if ATTN is present? So you don't > get an event until someone presses the button to process hotplug right? [...] > Looks like we still wait for button press to process. When you don't have a > power control yes the DLLSC would kick in and we would process them. but if > you have power control, you won't turn on until the button press? Right. For hotplug ports without a power controller, we could in principle achieve the same behavior (i.e. not bring up the slot until the button is pressed) by not subscribing to DLLSC events as well (if an Attention Button is present). However there are hotplug ports out there with incorrect data in the Slot Capabilities register: They claim to have an Attention Button but in reality don't have one. In those cases we're still able to bring up and down the slot right now. Whereas if we changed the behavior, those hotplug ports would no longer work. (We'd not bring up the slot until the button is pressed but there is no button.) E.g. this bugzilla contains dmesg output for a 5.4 kernel which is able to bring up and down the slot correctly even though the Slot Capabilities register incorrectly claims to have an Attention Button as well as Command Complete support: https://bugzilla.kernel.org/show_bug.cgi?id=209283 > > In any case the behavior in response to an Attention Button press should > > be the same regardless whether an MRL is present or not. (The spec > > doesn't seem to say otherwise.) > > True, Looks like that is a Linux behavior, certainly not a spec > recommendation. Our validation teams tell Windows also behaves such. i.e > when ATTN exists button press drivers the hot-add. Linux doesn't need to > follow suite though. > > In that case do we need to subscribe to PDC even when ATTN is present? Hm, I'd just leave it the way it is for now if it works. > > > + /* > > > + * If ATTN is present and MRL is triggered > > > + * ignore the Presence Change Event. > > > + */ > > > + if (ATTN_BUTTN(ctrl) && (events & PCI_EXP_SLTSTA_MRLSC)) > > > + events &= ~PCI_EXP_SLTSTA_PDC; > > > > An Attention Button press results in a synthesized PDC event after > > 5 seconds, which may get lost due to the above if-statement. > > When its synthesized you don't get the MRLSC? So we won't nuke the PDC then > correct? I just meant to say, pciehp_queue_pushbutton_work() will synthesize a PDC event after 5 seconds and with the above code snippet, if an MRL event happens simultaneously, that synthesized PDC event would be lost. So I'd just drop the above code snippet. I think you just need to subscribe to MRL events and propagate them to pciehp_handle_presence_or_link_change(). There, you'd bring down the slot if an MRL event has occurred (same as DLLSC or PDC). Then, check whether MRL is closed. If so, and if presence or link is up, try to bring up the slot. If the MRL is open when slot or presence has gone up, the slot is not brought up. But once MRL is closed, there'll be another MRL event and *then* the slot is brought up. Thanks, Lukas
On Sat, Nov 21, 2020 at 12:10:50PM +0100, Lukas Wunner wrote: > > > > > + /* > > > > + * If ATTN is present and MRL is triggered > > > > + * ignore the Presence Change Event. > > > > + */ > > > > + if (ATTN_BUTTN(ctrl) && (events & PCI_EXP_SLTSTA_MRLSC)) > > > > + events &= ~PCI_EXP_SLTSTA_PDC; > > > > > > An Attention Button press results in a synthesized PDC event after > > > 5 seconds, which may get lost due to the above if-statement. > > > > When its synthesized you don't get the MRLSC? So we won't nuke the PDC then > > correct? > > I just meant to say, pciehp_queue_pushbutton_work() will synthesize > a PDC event after 5 seconds and with the above code snippet, if an > MRL event happens simultaneously, that synthesized PDC event would > be lost. So I'd just drop the above code snippet. I think you > just need to subscribe to MRL events and propagate them to > pciehp_handle_presence_or_link_change(). There, you'd bring down > the slot if an MRL event has occurred (same as DLLSC or PDC). > Then, check whether MRL is closed. If so, and if presence or link > is up, try to bring up the slot. > > If the MRL is open when slot or presence has gone up, the slot is not > brought up. But once MRL is closed, there'll be another MRL event and > *then* the slot is brought up. > Sounds good.. I'll send the update patch.
diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h index 4fd200d8b0a9..24a1c9c8ac78 100644 --- a/drivers/pci/hotplug/pciehp.h +++ b/drivers/pci/hotplug/pciehp.h @@ -155,6 +155,7 @@ void pciehp_request(struct controller *ctrl, int action); void pciehp_handle_button_press(struct controller *ctrl); void pciehp_handle_disable_request(struct controller *ctrl); void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events); +void pciehp_handle_mrl_change(struct controller *ctrl); int pciehp_configure_device(struct controller *ctrl); void pciehp_unconfigure_device(struct controller *ctrl, bool presence); void pciehp_queue_pushbutton_work(struct work_struct *work); diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c index 9f85815b4f53..c4310ee3678b 100644 --- a/drivers/pci/hotplug/pciehp_ctrl.c +++ b/drivers/pci/hotplug/pciehp_ctrl.c @@ -227,6 +227,7 @@ void pciehp_handle_disable_request(struct controller *ctrl) void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events) { int present, link_active; + u8 getstatus = 0; /* * If the slot is on and presence or link has changed, turn it off. @@ -275,6 +276,16 @@ void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events) if (link_active) ctrl_info(ctrl, "Slot(%s): Link Up\n", slot_name(ctrl)); + if (MRL_SENS(ctrl)) { + pciehp_get_latch_status(ctrl, &getstatus); + /* + * If slot is closed && ATTN button exists + * don't continue, let the ATTN button + * drive the hot-plug + */ + if (!getstatus && ATTN_BUTTN(ctrl)) + return; + } ctrl->request_result = pciehp_enable_slot(ctrl); break; default: @@ -283,6 +294,64 @@ void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events) } } +void pciehp_handle_mrl_change(struct controller *ctrl) +{ + u8 getstatus = 0; + int present, link_active; + + pciehp_get_latch_status(ctrl, &getstatus); + + present = pciehp_card_present(ctrl); + link_active = pciehp_check_link_active(ctrl); + + ctrl_info(ctrl, "Slot(%s): Card %spresent\n", + slot_name(ctrl), present ? "" : "not "); + + ctrl_info(ctrl, "Slot(%s): Link %s\n", + slot_name(ctrl), link_active ? "Up" : "Down"); + + ctrl_info(ctrl, "Slot(%s): Latch %s\n", + slot_name(ctrl), getstatus ? "Open" : "Closed"); + + /* + * Need to handle only MRL Open. When MRL is closed with + * a Card Present, either the ATTN button, or the PDC indication + * should power the slot and add the card in the slot + */ + if (getstatus) { + /* + * If slot was powered on, time to power off + * and remove the card + */ + mutex_lock(&ctrl->state_lock); + if (ctrl->state == ON_STATE) { + mutex_unlock(&ctrl->state_lock); + pciehp_handle_disable_request(ctrl); + } else + mutex_unlock(&ctrl->state_lock); + } else { + /* + * If latch is closed, and previous state is OFF + * Then enable the slot + */ + mutex_lock(&ctrl->state_lock); + if (ctrl->state == OFF_STATE) { + /* + * Only continue to power on the slot when the + * Attention button is not present. When button + * present, button press event will process the + * hot-add part of the flow. + */ + if ((present || link_active) && !ATTN_BUTTN(ctrl)) { + ctrl->state = POWERON_STATE; + mutex_unlock(&ctrl->state_lock); + ctrl->request_result = pciehp_enable_slot(ctrl); + } else + mutex_unlock(&ctrl->state_lock); + } + } +} + static int __pciehp_enable_slot(struct controller *ctrl) { u8 getstatus = 0; diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 53433b37e181..5a4b5cfbfe48 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -605,7 +605,7 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id) */ status &= PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD | PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_CC | - PCI_EXP_SLTSTA_DLLSC; + PCI_EXP_SLTSTA_DLLSC | PCI_EXP_SLTSTA_MRLSC; /* * If we've already reported a power fault, don't report it again @@ -658,6 +658,12 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id) return IRQ_HANDLED; } + /* + * If MRL is triggered, if ATTN button exists, ignore the event. + */ + if (ATTN_BUTTN(ctrl) && (events & PCI_EXP_SLTSTA_MRLSC)) + events &= ~PCI_EXP_SLTSTA_PDC; + /* Save pending events for consumption by IRQ thread. */ atomic_or(events, &ctrl->pending_events); return IRQ_WAKE_THREAD; @@ -688,6 +694,13 @@ static irqreturn_t pciehp_ist(int irq, void *dev_id) goto out; } + /* + * If ATTN is present and MRL is triggered + * ignore the Presence Change Event. + */ + if (ATTN_BUTTN(ctrl) && (events & PCI_EXP_SLTSTA_MRLSC)) + events &= ~PCI_EXP_SLTSTA_PDC; + /* Check Attention Button Pressed */ if (events & PCI_EXP_SLTSTA_ABP) { ctrl_info(ctrl, "Slot(%s): Attention button pressed\n", @@ -712,6 +725,10 @@ static irqreturn_t pciehp_ist(int irq, void *dev_id) pciehp_handle_disable_request(ctrl); else if (events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC)) pciehp_handle_presence_or_link_change(ctrl, events); + + if (events & PCI_EXP_SLTSTA_MRLSC) + pciehp_handle_mrl_change(ctrl); + up_read(&ctrl->reset_lock); ret = IRQ_HANDLED; @@ -768,6 +785,14 @@ static void pcie_enable_notification(struct controller *ctrl) cmd |= PCI_EXP_SLTCTL_ABPE; else cmd |= PCI_EXP_SLTCTL_PDCE; + + /* + * If MRL sensor is present, then subscribe for MRL + * Changes to be notified as well. + */ + if (MRL_SENS(ctrl)) + cmd |= PCI_EXP_SLTCTL_MRLSCE; + if (!pciehp_poll_mode) cmd |= PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_CCIE;
When Mechanical Retention Lock (MRL) is present, Linux doesn't process those change events. The following changes need to be enabled when MRL is present. 1. Subscribe to MRL change events in SlotControl. 2. When MRL is closed, - If there is no ATTN button, then POWER on the slot. - If there is ATTN button, and an MRL event pending, ignore Presence Detect. Since we want ATTN button to drive the hotplug event. Signed-off-by: Ashok Raj <ashok.raj@intel.com> Co-developed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> --- drivers/pci/hotplug/pciehp.h | 1 + drivers/pci/hotplug/pciehp_ctrl.c | 69 +++++++++++++++++++++++++++++++++++++++ drivers/pci/hotplug/pciehp_hpc.c | 27 ++++++++++++++- 3 files changed, 96 insertions(+), 1 deletion(-)