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 ?
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
