RE: [PATCH] PCI: pciehp: Include the Data Link Layer State Changed bit when clearing the Slot Status register's event bits

[Rajat Jain <rajatjain@juniper.net>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2950 bytes --]

Hello,

Sorry, I missed this email.

Please see below. 

> -----Original Message-----
> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-
> owner@vger.kernel.org] On Behalf Of Guenter Roeck
> Sent: Tuesday, June 17, 2014 3:56 PM
> To: Bjorn Helgaas; Myron Stowe
> Cc: linux-pci@vger.kernel.org; Kenji Kaneshige; linux-
> kernel@vger.kernel.org; Rajat Jain
> Subject: Re: [PATCH] PCI: pciehp: Include the Data Link Layer State Changed
> bit when clearing the Slot Status register's event bits
> 
> On 06/17/2014 02:07 PM, Bjorn Helgaas wrote:
> > On Tue, Jun 17, 2014 at 1:27 PM, Myron Stowe
> <myron.stowe@redhat.com> wrote:
> >> During PCIe hot-plug initialization - pciehp_probe - data structures
> >> related to slot capabilities are set up.  As part of this set up,
> >> ISRs are put in place to handle slot events and all event bits are cleared
> out.
> >>
> >> This patch adds the Data Link Layer State Changed
> >> (PCI_EXP_SLTSTA_DLLSC) Slot Status bit to the event bits that are
> >> cleared out during initialization.
> >
> > I think this is related to e48f1b67f668 ("PCI: pciehp: Use link change
> > notifications for hot-plug and removal").  Prior to that, pcie_isr()
> > didn't look at the PCI_EXP_SLTSTA_DLLSC bit.
> >
> > Apparently there's a non-public report of spurious messages like this
> > at boot-time, with no actual hotplug events:
> >
> >    pciehp 0000:82:04.0:pcie24: slot(4): Link Up event
> >    pciehp 0000:82:04.0:pcie24: Device 0000:83:00.0 already exists at
> > 0000:83:00, cannot hot-add
> >    pciehp 0000:82:04.0:pcie24: Cannot add device at 0000:83:00
> >
> 
> I think I have seen that message once in a while in our systems.
> Rajat, didn't we talk about this a while ago ?

Essentially my hiccup was that I was not sure whether the driver should or should not take care of the link change events that have happened BEFORE the driver gets loaded. This has more impact if the pciehp is built as a kernel module.

As an example:

It is common for the platform init code built into the kernel, to take the PCI devices on the board out of reset. And that can happen after the PCI enumeration but before the pciehp driver gets loaded. Thus in this condition, with this patch, the pciehp will ignore the linkup event, thus device will not be visible. The only way (other than a rescan) to do hot-add the device would be to apply-and-remove-reset-signal to the device again. At which point pciehp may give a warning about about an attempt to remove a non-existent card, and then will proceed fine with hot-add.

The patch looks good, only wanted to make sure that we understand and agree that the pciehp should NOT process and link events that have happened before the pciehp was loaded.

Acked-by: Rajat Jain <rajatjain@juniper.net>

Thanks,

Rajat


ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥