RE: [PATCH] PCI, pciehp: Turn on link a while to workaround presense detection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]