Hello, > -----Original Message----- > From: linux-pci-owner@xxxxxxxxxxxxxxx [mailto:linux-pci- > owner@xxxxxxxxxxxxxxx] On Behalf Of Ethan Zhao > Sent: Sunday, December 08, 2013 6:10 PM > To: Rajat Jain > Cc: Bjorn Helgaas; Yinghai Lu; linux-pci@xxxxxxxxxxxxxxx; Kenji > Kaneshige; stable@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] PCI, pciehp: Turn on link a while to workaround > presense detection > > Rajat, > > Do you have a draft to extend the hotplug spec ? seem you wanna > introduce link change interrupt to the your chip or just poll the link > status register ? This is for the systems that do not implement elements like attention button: http://www.spinics.net/lists/hotplug/msg05802.html (Inspiration from the thread: http://www.spinics.net/lists/linux-pci/msg05783.html) Here was the proposed minimal patch: http://marc.info/?l=linux-pci&m=138610993912921&w=2 http://marc.info/?t=138611014300001&r=1&w=2 Would appreciate any review comments. Thanks, Rajat > > Thanks, > Ethan > > On Mon, Dec 9, 2013 at 5:29 AM, Rajat Jain <rajatjain@xxxxxxxxxxx> > wrote: > > Hello, > > > >> -----Original Message----- > >> From: linux-pci-owner@xxxxxxxxxxxxxxx [mailto:linux-pci- > >> owner@xxxxxxxxxxxxxxx] On Behalf Of Bjorn Helgaas > >> Sent: Saturday, December 07, 2013 10:36 AM > >> To: Yinghai Lu > >> Cc: linux-pci@xxxxxxxxxxxxxxx; Kenji Kaneshige; > >> stable@xxxxxxxxxxxxxxx > >> Subject: Re: [PATCH] PCI, pciehp: Turn on link a while to workaround > >> presense detection > >> > >> On Fri, Dec 6, 2013 at 1:22 PM, Yinghai Lu <yinghai@xxxxxxxxxx> > wrote: > >> > During hotplug test on platform, found slot status register does > >> > not report present status correctly. That present bit does not get > >> > cleared even after that card is removed. > >> > > >> > That problem is caused by commit: > >> > | commit 2debd9289997fc5d1c0043b41201a8b40d5e11d0 > >> > | > >> > | PCI: pciehp: Disable/enable link during slot power off/on > >> > > >> > It looks like chipset bug, that PresDet bit is "OR" operation > >> > between sideband input from FPGA, and chipset inband detection from > pcie link. > >> > >> This doesn't sound like a chipset bug. It sounds like exactly what > >> the spec describes: "Presence Detect State - This bit indicates the > >> presence of an adapter in the slot, reflected by the logical "OR" of > >> the Physical Layer in-band presence detect mechanism and, if present, > >> any out-of-band presence detect mechanism defined for the slot's > >> corresponding form factor" (PCIe 3.0, sec 7.8.11). > >> > >> So I want to fix this pciehp problem, but between 2debd9289997 and > >> this patch, pciehp_power_off_slot() is accumulating warts that make > >> it look like we're just reacting to things that break rather than > >> making a robust design to start with. > > > > Yes. > > > > In fact things do get more complicated if we introduce link state > based hot-plug, because then that requires that we do NOT disable the > link permanently during power off. (Because we would want to receive > future hot-plug link-up events). > > > >> > >> > It does not like if we disable pcie link at first and power off > >> > other side device, and it has input from inband detection always 1. > >> > > >> > Workaround: Try turn on link a while after power off. > >> > > >> > After this patch, PresDet report correct status when removing or > >> > adding card later. > >> > > >> > Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx> > >> > Cc: Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx> > >> > Cc: <stable@xxxxxxxxxxxxxxx> 3.4+ > >> > > >> > --- > >> > drivers/pci/hotplug/pciehp_hpc.c | 9 +++++++++ > >> > 1 file changed, 9 insertions(+) > >> > > >> > Index: linux-2.6/drivers/pci/hotplug/pciehp_hpc.c > >> > =================================================================== > >> > --- linux-2.6.orig/drivers/pci/hotplug/pciehp_hpc.c > >> > +++ linux-2.6/drivers/pci/hotplug/pciehp_hpc.c > >> > @@ -637,6 +637,15 @@ int pciehp_power_off_slot(struct slot * > >> > } > >> > ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__, > >> > pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, > >> > slot_cmd); > >> > + > >> > + /* > >> > + * Enable link for a while so chipset could settle down > >> > + * inband presence detection logic > >> > + */ > >> > + pciehp_link_enable(ctrl); > >> > + msleep(20); > >> > + pciehp_link_disable(ctrl); > >> > + > >> > return 0; > >> > } > >> > > >> -- > >> To unsubscribe from this list: send the line "unsubscribe linux-pci" > >> in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo > >> info at http://vger.kernel.org/majordomo-info.html > >> > > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-pci" > > in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo > > info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info > at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html