At Tue, 11 Feb 2014 16:18:35 -0700, Bjorn Helgaas wrote: > > On Tue, Feb 11, 2014 at 06:52:30PM +0000, Rajat Jain wrote: > > Hello Takashi, > > > > > -----Original Message----- > > > From: [email protected] [mailto:linux-pci- > > > [email protected]] On Behalf Of Takashi Iwai > > > Sent: Tuesday, February 11, 2014 3:39 AM > > > To: Bjorn Helgaas > > > Cc: Alex Williamson; [email protected]; linux- > > > [email protected] > > > Subject: [PATCH] PCI: pciehp: Remove surprise bit checks > > > > > > Currently pciehp driver checks the surprise removal bit and handles > > > the hotplug event only when the bit is set. But there are devices > > > that don't set that bit but yet expect the hotplug working, e.g. the > > > PCI card reader on HP ProBook 445 and 455 laptops appears only when > > > you insert a card, and it needs the hotplug event handling. > > > > > > For fixing this, basically we may ignore the surprise bit and always > > > handle the event. > > > > Some of this has been taken care of in the recent changes to pciehcp (last > > night). > > > > Please see > > > > https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/pciehp&id=a3eba3887960d00abb912fe3703cd3a058c721b8 > > > > Also, link state hotplug has been added as part of this patch set. Can you > > please try with Bjorn's latest pci tree? > > > > With that, I think you should not see the issue you are describing, because > > if the PCIe Link comes up when you insert the card, it shall be added (now) > > irrespective of whether or not you have the surprise bit set. > > Takashi's patch removed more HP_SUPR_RM() usage than yours, and I think > those additional changes are probably necessary. I rebased Takashi's patch > on top of my pci/pciehp branch, resulting in the patch below. > > I pushed this, so you can try the whole thing out at: > > http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/pciehp&id=237e0218bcad52d3df6746cd8d15a0d353bae84f
Thanks, I confirmed that the tree works. Takashi > > Bjorn > > > > PCI: pciehp: Remove surprise bit checks > > From: Takashi Iwai <[email protected]> > > Currently pciehp driver checks the surprise removal bit and handles the > hotplug event only when the bit is set. But there are devices that don't > set that bit, yet expect hotplug to work, e.g., the SD/MMC card reader on > HP ProBook 445 and 455 laptops appears only when you insert a card, and it > needs the hotplug event handling. > > For fixing this, basically we may ignore the surprise bit and always handle > the event. The only big concern in the past was the KVM device assignment, > but this has been fixed in KVM side (ignoring hotplug events during > secondary bus resets), there should be no obstacle ahead. > > The earlier discussion thread is found at: > https://lkml.kernel.org/r/s5h38vqjhk6.wl%[email protected] > > [bhelgaas: rebase on Rajat's changes, remove HP_SUPR_RM() completely] > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=70261 > Signed-off-by: Takashi Iwai <[email protected]> > Signed-off-by: Bjorn Helgaas <[email protected]> > --- > drivers/pci/hotplug/pciehp.h | 1 - > drivers/pci/hotplug/pciehp_ctrl.c | 2 -- > drivers/pci/hotplug/pciehp_hpc.c | 11 +++++------ > 3 files changed, 5 insertions(+), 9 deletions(-) > > diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h > index 8a66866b8cf1..d2a67aa0e49d 100644 > --- a/drivers/pci/hotplug/pciehp.h > +++ b/drivers/pci/hotplug/pciehp.h > @@ -124,7 +124,6 @@ struct controller { > #define MRL_SENS(ctrl) ((ctrl)->slot_cap & > PCI_EXP_SLTCAP_MRLSP) > #define ATTN_LED(ctrl) ((ctrl)->slot_cap & PCI_EXP_SLTCAP_AIP) > #define PWR_LED(ctrl) ((ctrl)->slot_cap & PCI_EXP_SLTCAP_PIP) > -#define HP_SUPR_RM(ctrl) ((ctrl)->slot_cap & PCI_EXP_SLTCAP_HPS) > #define EMI(ctrl) ((ctrl)->slot_cap & PCI_EXP_SLTCAP_EIP) > #define NO_CMD_CMPL(ctrl) ((ctrl)->slot_cap & PCI_EXP_SLTCAP_NCCS) > #define PSN(ctrl) ((ctrl)->slot_cap >> 19) > diff --git a/drivers/pci/hotplug/pciehp_ctrl.c > b/drivers/pci/hotplug/pciehp_ctrl.c > index fec99a164d93..a4d29d24057d 100644 > --- a/drivers/pci/hotplug/pciehp_ctrl.c > +++ b/drivers/pci/hotplug/pciehp_ctrl.c > @@ -535,8 +535,6 @@ static void interrupt_event_handler(struct work_struct > *work) > break; > case INT_PRESENCE_ON: > case INT_PRESENCE_OFF: > - if (!HP_SUPR_RM(ctrl)) > - break; > ctrl_dbg(ctrl, "Surprise Removal\n"); > handle_surprise_event(p_slot); > break; > diff --git a/drivers/pci/hotplug/pciehp_hpc.c > b/drivers/pci/hotplug/pciehp_hpc.c > index da4b0204b4f7..1076a51623b5 100644 > --- a/drivers/pci/hotplug/pciehp_hpc.c > +++ b/drivers/pci/hotplug/pciehp_hpc.c > @@ -628,17 +628,18 @@ int pciehp_reset_slot(struct slot *slot, int probe) > { > struct controller *ctrl = slot->ctrl; > struct pci_dev *pdev = ctrl_dev(ctrl); > - u16 stat_mask = 0, ctrl_mask = 0; > + u16 stat_mask, ctrl_mask; > > if (probe) > return 0; > > - if (HP_SUPR_RM(ctrl) && !ATTN_BUTTN(ctrl)) { > + ctrl_mask = PCI_EXP_SLTCTL_DLLSCE; > + stat_mask = PCI_EXP_SLTSTA_DLLSC; > + > + if (!ATTN_BUTTN(ctrl)) { /* as in pcie_enable_notification() */ > ctrl_mask |= PCI_EXP_SLTCTL_PDCE; > stat_mask |= PCI_EXP_SLTSTA_PDC; > } > - ctrl_mask |= PCI_EXP_SLTCTL_DLLSCE; > - stat_mask |= PCI_EXP_SLTSTA_DLLSC; > > pcie_write_cmd(ctrl, 0, ctrl_mask); > if (pciehp_poll_mode) > @@ -741,8 +742,6 @@ static inline void dbg_ctrl(struct controller *ctrl) > ATTN_LED(ctrl) ? "yes" : "no"); > ctrl_info(ctrl, " Power Indicator : %3s\n", > PWR_LED(ctrl) ? "yes" : "no"); > - ctrl_info(ctrl, " Hot-Plug Surprise : %3s\n", > - HP_SUPR_RM(ctrl) ? "yes" : "no"); > ctrl_info(ctrl, " EMI Present : %3s\n", > EMI(ctrl) ? "yes" : "no"); > ctrl_info(ctrl, " Command Completed : %3s\n", > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
