| Message ID | 20140203160724.949a0ee5.izumi.taku@jp.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
[+cc Rajat] On Mon, Feb 03, 2014 at 04:07:24PM +0900, Taku Izumi wrote: > > I found the problem that the present bit does not be cleared > though the slot is empty. > > Step to reproduce: > > # cd /sys/bus/pci/slots/66 > # cat adapter > 0 > --- (insert PCIe card on slot#66) --- > # cat adapter > 1 > # echo 1 > power > # echo 0 > power > --- (eject PCIe card on slot#66) --- > # cat adapter > 1 > ==> should be 0. > > According to git-bisect this is caused by: > 2debd9289997fc5d1c0043b41201a8b40d5e11d0 > PCI: pciehp: Disable/enable link during slot power off/on > is the first bad commit > > By reverting the above patch, this bug can be solved. > And I also found this can be fixed by changing the timing of link-disable > during power off the slot. > So, this patch changes the timing of link-disable during power off. Hi Taku, Thanks a lot for finding and bisecting this problem. I recently merged some patches from Rajat that partially revert 2debd9289997. Can you try out my pci/pciehp branch and see whether it is enough to fix the problem for you? Here's the branch: http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/pciehp and the specific change Rajat made is: http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/pciehp&id=b1811d2455f32754cc3d8725bf2e961c5eda2a72 Let me know if that isn't enough to fix the problem you're seeing, and we can work on it some more. Bjorn > > Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> > --- > drivers/pci/hotplug/pciehp_hpc.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c > index 14acfcc..163f0b4 100644 > --- a/drivers/pci/hotplug/pciehp_hpc.c > +++ b/drivers/pci/hotplug/pciehp_hpc.c > @@ -508,7 +508,12 @@ void pciehp_power_off_slot(struct slot * slot) > { > struct controller *ctrl = slot->ctrl; > > - /* Disable the link at first */ > + pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PWR_OFF, PCI_EXP_SLTCTL_PCC); > + ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__, > + pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, > + PCI_EXP_SLTCTL_PWR_OFF); > + > + /* Disable the link */ > pciehp_link_disable(ctrl); > /* wait the link is down */ > if (ctrl->link_active_reporting) > @@ -516,10 +521,6 @@ void pciehp_power_off_slot(struct slot * slot) > else > msleep(1000); > > - pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PWR_OFF, PCI_EXP_SLTCTL_PCC); > - ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__, > - pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, > - PCI_EXP_SLTCTL_PWR_OFF); > } > > static irqreturn_t pcie_isr(int irq, void *dev_id) > -- > 1.8.3.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bjorn, > Hi Taku, > > Thanks a lot for finding and bisecting this problem. I recently merged > some patches from Rajat that partially revert 2debd9289997. Can you try > out my pci/pciehp branch and see whether it is enough to fix the problem > for you? > > Here's the branch: > http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/pciehp > > and the specific change Rajat made is: > http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/pciehp&id=b1811d2455f32754cc3d8725bf2e961c > 5eda2a72 > > Let me know if that isn't enough to fix the problem you're seeing, and we > can work on it some more. I tested the kernel checkouted from remotes/origin/pci/pciehp. The original problem I reported was fixed. However the sequence of hot-plug operation got changed. The slot power becomes ON the moment when I insert PCIe card on the slot. This behavior is unacceptable to me. (before) - hot-add 1. insert PCIe card 2. echo 1 > power - hot-remove 1. echo 0 > power 2. eject PCIe card (after) - hot-add 1. insert PCIe card (and automatically power-on) - hot-remove 1. echo 0 > power 2. eject PCIe card Best regards, Taku Izumi -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 12, 2014 at 1:00 AM, Izumi, Taku <izumi.taku@jp.fujitsu.com> wrote: > Hi Bjorn, > >> Hi Taku, >> >> Thanks a lot for finding and bisecting this problem. I recently merged >> some patches from Rajat that partially revert 2debd9289997. Can you try >> out my pci/pciehp branch and see whether it is enough to fix the problem >> for you? >> >> Here's the branch: >> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/pciehp >> >> and the specific change Rajat made is: >> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/pciehp&id=b1811d2455f32754cc3d8725bf2e961c >> 5eda2a72 >> >> Let me know if that isn't enough to fix the problem you're seeing, and we >> can work on it some more. > > I tested the kernel checkouted from remotes/origin/pci/pciehp. > The original problem I reported was fixed. > However the sequence of hot-plug operation got changed. > The slot power becomes ON the moment when I insert PCIe card on the slot. > This behavior is unacceptable to me. > > (before) > - hot-add > 1. insert PCIe card > 2. echo 1 > power > - hot-remove > 1. echo 0 > power > 2. eject PCIe card > > (after) > - hot-add > 1. insert PCIe card (and automatically power-on) > > - hot-remove > 1. echo 0 > power > 2. eject PCIe card Huh, that doesn't sound good. Does your slot have an attention button? Can you collect the "lspci -vvv" output for the Downstream Port leading to this slot? I think that for ExpressCard and similar devices, we want to turn on the device and attach a driver as soon as the card is inserted. But in your case, I assume we want a model like that in Table 2-7 of the "PCI Standard Hot-Plug Controller and Subsystem Specification," rev 1.0, i.e., "Hot Insertion Initiated via Software UI." So there must be some way to differentiate an ExpressCard slot from a slot like yours. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On Wed, Feb 12, 2014 at 8:56 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Wed, Feb 12, 2014 at 1:00 AM, Izumi, Taku <izumi.taku@jp.fujitsu.com> wrote: >> Hi Bjorn, >> >>> Hi Taku, >>> >>> Thanks a lot for finding and bisecting this problem. I recently merged >>> some patches from Rajat that partially revert 2debd9289997. Can you try >>> out my pci/pciehp branch and see whether it is enough to fix the problem >>> for you? >>> >>> Here's the branch: >>> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/pciehp >>> >>> and the specific change Rajat made is: >>> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/pciehp&id=b1811d2455f32754cc3d8725bf2e961c >>> 5eda2a72 >>> >>> Let me know if that isn't enough to fix the problem you're seeing, and we >>> can work on it some more. >> >> I tested the kernel checkouted from remotes/origin/pci/pciehp. >> The original problem I reported was fixed. >> However the sequence of hot-plug operation got changed. >> The slot power becomes ON the moment when I insert PCIe card on the slot. >> This behavior is unacceptable to me. >> >> (before) >> - hot-add >> 1. insert PCIe card >> 2. echo 1 > power >> - hot-remove >> 1. echo 0 > power >> 2. eject PCIe card >> >> (after) >> - hot-add >> 1. insert PCIe card (and automatically power-on) >> >> - hot-remove >> 1. echo 0 > power >> 2. eject PCIe card > > Huh, that doesn't sound good. Does your slot have an attention > button? Can you collect the "lspci -vvv" output for the Downstream > Port leading to this slot? If there is no attention button, then the behavior seen, matches with what I expected of the code. Seemingly, the HW does not have a power controller (and attention button), thus the power is automatically turned on (by HW) when card is inserted and the link comes up. Thus prior to link state based hot-plug, "echo 1 > power" used to do a little else than initiate a rescan for this particular HW. > > I think that for ExpressCard and similar devices, we want to turn on > the device and attach a driver as soon as the card is inserted. But > in your case, I assume we want a model like that in Table 2-7 of the > "PCI Standard Hot-Plug Controller and Subsystem Specification," rev > 1.0, i.e., "Hot Insertion Initiated via Software UI." So there must > be some way to differentiate an ExpressCard slot from a slot like > yours. Yes, that would help if there was some way. Thanks, Rajat > > Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
SGkgQmpvcm4sIFJhamF0LA0KDQo+ID4gSHVoLCB0aGF0IGRvZXNuJ3Qgc291bmQgZ29vZC4gIERv ZXMgeW91ciBzbG90IGhhdmUgYW4gYXR0ZW50aW9uDQo+ID4gYnV0dG9uPyAgQ2FuIHlvdSBjb2xs ZWN0IHRoZSAibHNwY2kgLXZ2diIgb3V0cHV0IGZvciB0aGUgRG93bnN0cmVhbQ0KPiA+IFBvcnQg bGVhZGluZyB0byB0aGlzIHNsb3Q/DQoNCiAgIFRoZSBzbG90cyBoYXZlIG5vIEF0dGVudGlvbiBi dXR0b24gYW5kIGFyZSBub3QgZm9yIEV4cHJlc3NDYXJkLg0KICAgbHNwY2kgLXZ2diBvdXRwdXQg aXM6DQoNCiAgICAgICAgQ29udHJvbDogSS9PKyBNZW0rIEJ1c01hc3RlcisgU3BlY0N5Y2xlLSBN ZW1XSU5WLSBWR0FTbm9vcC0gUGFyRXJyKyBTdGVwcGluZy0gU0VSUisgRmFzdEIyQi0gRGlzSU5U eCsNCiAgICAgICAgU3RhdHVzOiBDYXArIDY2TUh6LSBVREYtIEZhc3RCMkItIFBhckVyci0gREVW U0VMPWZhc3QgPlRBYm9ydC0gPFRBYm9ydC0gPE1BYm9ydC0gPlNFUlItIDxQRVJSLSBJTlR4LQ0K ICAgICAgICBMYXRlbmN5OiAwDQogICAgICAgIEJ1czogcHJpbWFyeT0yYywgc2Vjb25kYXJ5PTJk LCBzdWJvcmRpbmF0ZT0yZiwgc2VjLWxhdGVuY3k9MA0KICAgICAgICBJL08gYmVoaW5kIGJyaWRn ZTogMDAwMGYwMDAtMDAwMDBmZmYNCiAgICAgICAgTWVtb3J5IGJlaGluZCBicmlkZ2U6IGIxODAw MDAwLWIxZmZmZmZmDQogICAgICAgIFByZWZldGNoYWJsZSBtZW1vcnkgYmVoaW5kIGJyaWRnZTog MDAwMDAwMDBhZTgwMDAwMC0wMDAwMDAwMGFlZmZmZmZmDQogICAgICAgIFNlY29uZGFyeSBzdGF0 dXM6IDY2TUh6LSBGYXN0QjJCLSBQYXJFcnItIERFVlNFTD1mYXN0ID5UQWJvcnQtIDxUQWJvcnQt IDxNQWJvcnQtIDxTRVJSLSA8UEVSUi0NCiAgICAgICAgQnJpZGdlQ3RsOiBQYXJpdHktIFNFUlIr IE5vSVNBLSBWR0EtIE1BYm9ydC0gPlJlc2V0LSBGYXN0QjJCLQ0KICAgICAgICAgICAgICAgIFBy aURpc2NUbXItIFNlY0Rpc2NUbXItIERpc2NUbXJTdGF0LSBEaXNjVG1yU0VSUkVuLQ0KICAgICAg ICBDYXBhYmlsaXRpZXM6IFs0MF0gUG93ZXIgTWFuYWdlbWVudCB2ZXJzaW9uIDMNCiAgICAgICAg ICAgICAgICBGbGFnczogUE1FQ2xrLSBEU0ktIEQxLSBEMi0gQXV4Q3VycmVudD0wbUEgUE1FKEQw KyxEMS0sRDItLEQzaG90KyxEM2NvbGQrKQ0KICAgICAgICAgICAgICAgIFN0YXR1czogRDAgTm9T b2Z0UnN0KyBQTUUtRW5hYmxlLSBEU2VsPTAgRFNjYWxlPTAgUE1FLQ0KICAgICAgICBDYXBhYmls aXRpZXM6IFs0OF0gTVNJOiBFbmFibGUrIENvdW50PTEvOCBNYXNrYWJsZSsgNjRiaXQrDQogICAg ICAgICAgICAgICAgQWRkcmVzczogMDAwMDAwMDBmZWUwMDUxOCAgRGF0YTogMDAwMA0KICAgICAg ICAgICAgICAgIE1hc2tpbmc6IDAwMDAwMGZlICBQZW5kaW5nOiAwMDAwMDAwMA0KICAgICAgICBD YXBhYmlsaXRpZXM6IFs2OF0gRXhwcmVzcyAodjIpIERvd25zdHJlYW0gUG9ydCAoU2xvdCspLCBN U0kgMDANCiAgICAgICAgICAgICAgICBEZXZDYXA6IE1heFBheWxvYWQgMjA0OCBieXRlcywgUGhh bnRGdW5jIDAsIExhdGVuY3kgTDBzIDw2NG5zLCBMMSA8MXVzDQogICAgICAgICAgICAgICAgICAg ICAgICBFeHRUYWctIFJCRSsgRkxSZXNldC0NCiAgICAgICAgICAgICAgICBEZXZDdGw6IFJlcG9y dCBlcnJvcnM6IENvcnJlY3RhYmxlKyBOb24tRmF0YWwrIEZhdGFsKyBVbnN1cHBvcnRlZC0NCiAg ICAgICAgICAgICAgICAgICAgICAgIFJseGRPcmQrIEV4dFRhZy0gUGhhbnRGdW5jLSBBdXhQd3It IE5vU25vb3ArDQogICAgICAgICAgICAgICAgICAgICAgICBNYXhQYXlsb2FkIDEyOCBieXRlcywg TWF4UmVhZFJlcSAxMjggYnl0ZXMNCiAgICAgICAgICAgICAgICBEZXZTdGE6IENvcnJFcnIrIFVu Y29yckVyci0gRmF0YWxFcnItIFVuc3VwcFJlcSsgQXV4UHdyLSBUcmFuc1BlbmQtDQogICAgICAg ICAgICAgICAgTG5rQ2FwOiBQb3J0ICMxLCBTcGVlZCA4R1QvcywgV2lkdGggeDgsIEFTUE0gTDEs IExhdGVuY3kgTDAgPDR1cywgTDEgPDR1cw0KICAgICAgICAgICAgICAgICAgICAgICAgQ2xvY2tQ TS0gU3VycHJpc2UrIExMQWN0UmVwKyBCd05vdCsNCiAgICAgICAgICAgICAgICBMbmtDdGw6IEFT UE0gRGlzYWJsZWQ7IERpc2FibGVkLSBSZXRyYWluLSBDb21tQ2xrLQ0KICAgICAgICAgICAgICAg ICAgICAgICAgRXh0U3luY2gtIENsb2NrUE0tIEF1dFdpZERpcy0gQldJbnQtIEF1dEJXSW50LQ0K ICAgICAgICAgICAgICAgIExua1N0YTogU3BlZWQgMi41R1QvcywgV2lkdGggeDAsIFRyRXJyLSBU cmFpbi0gU2xvdENsaysgRExBY3RpdmUtIEJXTWdtdC0gQUJXTWdtdC0NCiAgICAgICAgICAgICAg ICBTbHRDYXA6IEF0dG5CdG4tIFB3ckN0cmwrIE1STCsgQXR0bkluZCsgUHdySW5kKyBIb3RQbHVn KyBTdXJwcmlzZS0NCiAgICAgICAgICAgICAgICAgICAgICAgIFNsb3QgIzY3LCBQb3dlckxpbWl0 IDI1LjAwMFc7IEludGVybG9jay0gTm9Db21wbC0NCiAgICAgICAgICAgICAgICBTbHRDdGw6IEVu YWJsZTogQXR0bkJ0bi0gUHdyRmx0LSBNUkwrIFByZXNEZXQrIENtZENwbHQrIEhQSXJxKyBMaW5r Q2hnKw0KICAgICAgICAgICAgICAgICAgICAgICAgQ29udHJvbDogQXR0bkluZCBPZmYsIFB3cklu ZCBPZmYsIFBvd2VyKyBJbnRlcmxvY2stDQogICAgICAgICAgICAgICAgU2x0U3RhOiBTdGF0dXM6 IEF0dG5CdG4tIFBvd2VyRmx0LSBNUkwtIENtZENwbHQtIFByZXNEZXQtIEludGVybG9jay0NCiAg ICAgICAgICAgICAgICAgICAgICAgIENoYW5nZWQ6IE1STC0gUHJlc0RldC0gTGlua1N0YXRlLQ0K ICAgICAgICAgICAgICAgIERldkNhcDI6IENvbXBsZXRpb24gVGltZW91dDogTm90IFN1cHBvcnRl ZCwgVGltZW91dERpcy0sIExUUissIE9CRkYgVmlhIG1lc3NhZ2UgQVJJRndkKw0KICAgICAgICAg ICAgICAgIERldkN0bDI6IENvbXBsZXRpb24gVGltZW91dDogNTB1cyB0byA1MG1zLCBUaW1lb3V0 RGlzLSwgTFRSLSwgT0JGRiBEaXNhYmxlZCBBUklGd2QtDQogICAgICAgICAgICAgICAgTG5rQ3Rs MjogVGFyZ2V0IExpbmsgU3BlZWQ6IDhHVC9zLCBFbnRlckNvbXBsaWFuY2UtIFNwZWVkRGlzLSwg U2VsZWN0YWJsZSBEZS1lbXBoYXNpczogLTZkQg0KICAgICAgICAgICAgICAgICAgICAgICAgIFRy YW5zbWl0IE1hcmdpbjogTm9ybWFsIE9wZXJhdGluZyBSYW5nZSwgRW50ZXJNb2RpZmllZENvbXBs aWFuY2UtIENvbXBsaWFuY2VTT1MtDQogICAgICAgICAgICAgICAgICAgICAgICAgQ29tcGxpYW5j ZSBEZS1lbXBoYXNpczogLTZkQg0KICAgICAgICAgICAgICAgIExua1N0YTI6IEN1cnJlbnQgRGUt ZW1waGFzaXMgTGV2ZWw6IC0zLjVkQiwgRXF1YWxpemF0aW9uQ29tcGxldGUtLCBFcXVhbGl6YXRp b25QaGFzZTEtDQogICAgICAgICAgICAgICAgICAgICAgICAgRXF1YWxpemF0aW9uUGhhc2UyLSwg RXF1YWxpemF0aW9uUGhhc2UzLSwgTGlua0VxdWFsaXphdGlvblJlcXVlc3QtDQogICAgICAgIENh cGFiaWxpdGllczogW2E0XSBTdWJzeXN0ZW06IEZ1aml0c3UgTGltaXRlZC4gRGV2aWNlIDE4MDQN CiAgICAgICAgQ2FwYWJpbGl0aWVzOiBbMTAwIHYxXSBEZXZpY2UgU2VyaWFsIE51bWJlciBhYS04 Ny0wMC0xMC1iNS1kZi0wZS0wMA0KICAgICAgICBDYXBhYmlsaXRpZXM6IFtmYjQgdjFdIEFkdmFu Y2VkIEVycm9yIFJlcG9ydGluZw0KICAgICAgICAgICAgICAgIFVFU3RhOiAgRExQLSBTREVTLSBU TFAtIEZDUC0gQ21wbHRUTy0gQ21wbHRBYnJ0LSBVbnhDbXBsdC0gUnhPRi0gTWFsZlRMUC0gRUNS Qy0gVW5zdXBSZXEtIEFDU1Zpb2wtDQogICAgICAgICAgICAgICAgVUVNc2s6ICBETFAtIFNERVMt IFRMUC0gRkNQLSBDbXBsdFRPLSBDbXBsdEFicnQtIFVueENtcGx0LSBSeE9GLSBNYWxmVExQLSBF Q1JDKyBVbnN1cFJlcSsgQUNTVmlvbC0NCiAgICAgICAgICAgICAgICBVRVN2cnQ6IERMUCsgU0RF UysgVExQKyBGQ1ArIENtcGx0VE8tIENtcGx0QWJydCsgVW54Q21wbHQrIFJ4T0YrIE1hbGZUTFAr IEVDUkMrIFVuc3VwUmVxLSBBQ1NWaW9sKw0KICAgICAgICAgICAgICAgIENFU3RhOiAgUnhFcnIt IEJhZFRMUC0gQmFkRExMUC0gUm9sbG92ZXItIFRpbWVvdXQtIE5vbkZhdGFsRXJyKw0KICAgICAg ICAgICAgICAgIENFTXNrOiAgUnhFcnItIEJhZFRMUC0gQmFkRExMUC0gUm9sbG92ZXItIFRpbWVv dXQtIE5vbkZhdGFsRXJyKw0KICAgICAgICAgICAgICAgIEFFUkNhcDogRmlyc3QgRXJyb3IgUG9p bnRlcjogMWYsIEdlbkNhcCsgQ0dlbkVuLSBDaGtDYXArIENoa0VuLQ0KICAgICAgICBDYXBhYmls aXRpZXM6IFsxMzggdjFdIFBvd2VyIEJ1ZGdldGluZyA8Pz4NCiAgICAgICAgQ2FwYWJpbGl0aWVz OiBbMTBjIHYxXSAjMTkNCiAgICAgICAgQ2FwYWJpbGl0aWVzOiBbMTQ4IHYxXSBWaXJ0dWFsIENo YW5uZWwNCiAgICAgICAgICAgICAgICBDYXBzOiAgIExQRVZDPTAgUmVmQ2xrPTEwMG5zIFBBVEVu dHJ5Qml0cz0xDQogICAgICAgICAgICAgICAgQXJiOiAgICBGaXhlZC0gV1JSMzItIFdSUjY0LSBX UlIxMjgtDQogICAgICAgICAgICAgICAgQ3RybDogICBBcmJTZWxlY3Q9Rml4ZWQNCiAgICAgICAg ICAgICAgICBTdGF0dXM6IEluUHJvZ3Jlc3MtDQogICAgICAgICAgICAgICAgVkMwOiAgICBDYXBz OiAgIFBBVE9mZnNldD0wMCBNYXhUaW1lU2xvdHM9MSBSZWpTbm9vcFRyYW5zLQ0KICAgICAgICAg ICAgICAgICAgICAgICAgQXJiOiAgICBGaXhlZCsgV1JSMzItIFdSUjY0LSBXUlIxMjgtIFRXUlIx MjgtIFdSUjI1Ni0NCiAgICAgICAgICAgICAgICAgICAgICAgIEN0cmw6ICAgRW5hYmxlKyBJRD0w IEFyYlNlbGVjdD1GaXhlZCBUQy9WQz1mZg0KICAgICAgICAgICAgICAgICAgICAgICAgU3RhdHVz OiBOZWdvUGVuZGluZysgSW5Qcm9ncmVzcy0NCiAgICAgICAgQ2FwYWJpbGl0aWVzOiBbZTAwIHYx XSAjMTINCiAgICAgICAgQ2FwYWJpbGl0aWVzOiBbZjI0IHYxXSBBY2Nlc3MgQ29udHJvbCBTZXJ2 aWNlcw0KICAgICAgICAgICAgICAgIEFDU0NhcDogU3JjVmFsaWQrIFRyYW5zQmxrKyBSZXFSZWRp cisgQ21wbHRSZWRpcisgVXBzdHJlYW1Gd2QrIEVncmVzc0N0cmwrIERpcmVjdFRyYW5zKw0KICAg ICAgICAgICAgICAgIEFDU0N0bDogU3JjVmFsaWQtIFRyYW5zQmxrLSBSZXFSZWRpci0gQ21wbHRS ZWRpci0gVXBzdHJlYW1Gd2QtIEVncmVzc0N0cmwtIERpcmVjdFRyYW5zLQ0KICAgICAgICBDYXBh YmlsaXRpZXM6IFtiNzAgdjFdIFZlbmRvciBTcGVjaWZpYyBJbmZvcm1hdGlvbjogSUQ9MDAwMSBS ZXY9MCBMZW49MDEwIDw/Pg0KICAgICAgICBLZXJuZWwgZHJpdmVyIGluIHVzZTogcGNpZXBvcnQN Cg0KDQo+IElmIHRoZXJlIGlzIG5vIGF0dGVudGlvbiBidXR0b24sIHRoZW4gdGhlIGJlaGF2aW9y IHNlZW4sIG1hdGNoZXMgd2l0aA0KPiB3aGF0IEkgZXhwZWN0ZWQgb2YgdGhlIGNvZGUuDQo+IA0K PiBTZWVtaW5nbHksIHRoZSBIVyBkb2VzIG5vdCBoYXZlIGEgcG93ZXIgY29udHJvbGxlciAoYW5k IGF0dGVudGlvbg0KPiBidXR0b24pLCB0aHVzIHRoZSBwb3dlciBpcyBhdXRvbWF0aWNhbGx5IHR1 cm5lZCBvbiAoYnkgSFcpIHdoZW4gY2FyZA0KPiBpcyBpbnNlcnRlZCBhbmQgdGhlIGxpbmsgY29t ZXMgdXAuIFRodXMgcHJpb3IgdG8gbGluayBzdGF0ZSBiYXNlZA0KPiBob3QtcGx1ZywgImVjaG8g MSA+IHBvd2VyIiB1c2VkIHRvIGRvIGEgbGl0dGxlIGVsc2UgdGhhbiBpbml0aWF0ZSBhDQo+IHJl c2NhbiBmb3IgdGhpcyBwYXJ0aWN1bGFyIEhXLg0KDQo+IA0KPiA+DQo+ID4gSSB0aGluayB0aGF0 IGZvciBFeHByZXNzQ2FyZCBhbmQgc2ltaWxhciBkZXZpY2VzLCB3ZSB3YW50IHRvIHR1cm4gb24N Cj4gPiB0aGUgZGV2aWNlIGFuZCBhdHRhY2ggYSBkcml2ZXIgYXMgc29vbiBhcyB0aGUgY2FyZCBp cyBpbnNlcnRlZC4gIEJ1dA0KPiA+IGluIHlvdXIgY2FzZSwgSSBhc3N1bWUgd2Ugd2FudCBhIG1v ZGVsIGxpa2UgdGhhdCBpbiBUYWJsZSAyLTcgb2YgdGhlDQo+ID4gIlBDSSBTdGFuZGFyZCBIb3Qt UGx1ZyBDb250cm9sbGVyIGFuZCBTdWJzeXN0ZW0gU3BlY2lmaWNhdGlvbiwiIHJldg0KPiA+IDEu MCwgaS5lLiwgIkhvdCBJbnNlcnRpb24gSW5pdGlhdGVkIHZpYSBTb2Z0d2FyZSBVSS4iICBTbyB0 aGVyZSBtdXN0DQo+ID4gYmUgc29tZSB3YXkgdG8gZGlmZmVyZW50aWF0ZSBhbiBFeHByZXNzQ2Fy ZCBzbG90IGZyb20gYSBzbG90IGxpa2UNCj4gPiB5b3Vycy4NCj4gDQo+IFllcywgdGhhdCB3b3Vs ZCBoZWxwIGlmIHRoZXJlIHdhcyBzb21lIHdheS4NCj4gDQoNCkJlc3QgcmVnYXJkcywNClRha3Ug SXp1bWkNCg0K -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[+Takashi] Hello, > > > > Huh, that doesn't sound good. Does your slot have an attention > > > button? Can you collect the "lspci -vvv" output for the Downstream > > > Port leading to this slot? > > The slots have no Attention button and are not for ExpressCard. > lspci -vvv output is: > > Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- > ParErr+ Stepping- SERR+ FastB2B- DisINTx+ > Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- > <TAbort- <MAbort- >SERR- <PERR- INTx- > Latency: 0 > Bus: primary=2c, secondary=2d, subordinate=2f, sec-latency=0 > I/O behind bridge: 0000f000-00000fff > Memory behind bridge: b1800000-b1ffffff > Prefetchable memory behind bridge: 00000000ae800000- > 00000000aeffffff > Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- > <TAbort- <MAbort- <SERR- <PERR- > BridgeCtl: Parity- SERR+ NoISA- VGA- MAbort- >Reset- FastB2B- > PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- > Capabilities: [40] Power Management version 3 > Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1- > ,D2-,D3hot+,D3cold+) > Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- > Capabilities: [48] MSI: Enable+ Count=1/8 Maskable+ 64bit+ > Address: 00000000fee00518 Data: 0000 > Masking: 000000fe Pending: 00000000 > Capabilities: [68] Express (v2) Downstream Port (Slot+), MSI 00 > DevCap: MaxPayload 2048 bytes, PhantFunc 0, Latency L0s > <64ns, L1 <1us > ExtTag- RBE+ FLReset- > DevCtl: Report errors: Correctable+ Non-Fatal+ Fatal+ > Unsupported- > RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+ > MaxPayload 128 bytes, MaxReadReq 128 bytes > DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq+ AuxPwr- > TransPend- > LnkCap: Port #1, Speed 8GT/s, Width x8, ASPM L1, Latency > L0 <4us, L1 <4us > ClockPM- Surprise+ LLActRep+ BwNot+ > LnkCtl: ASPM Disabled; Disabled- Retrain- CommClk- > ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt- > LnkSta: Speed 2.5GT/s, Width x0, TrErr- Train- SlotClk+ > DLActive- BWMgmt- ABWMgmt- > SltCap: AttnBtn- PwrCtrl+ MRL+ AttnInd+ PwrInd+ HotPlug+ > Surprise- > Slot #67, PowerLimit 25.000W; Interlock- > NoCompl- Hmm... I see that the Slot has a power controller. Which means that the power to the slot shall not be turned on automatically (by HW) when the card is plugged in. Also meaning that the link will not come up automatically - so this does not seem like the Link state based hotplug kicking in. What I suspect is this one: f02d1843d83b "PCI: pciehp: Remove surprise bit checks" Since this patch removes *all* the surprise bit checks causing all the presence detect changes (including 0->1) to be initiate hotplug. I think it may be worthwhile to try it out while removing this particular hunk: @@ -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; May be we should remove the surprise check for INT_PRESENCE_OFF only (and let it stay for INT_PRESEWNCE_ON)? Thanks, Rajat > SltCtl: Enable: AttnBtn- PwrFlt- MRL+ PresDet+ CmdCplt+ > HPIrq+ LinkChg+ > Control: AttnInd Off, PwrInd Off, Power+ > Interlock- > SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- > PresDet- Interlock- > Changed: MRL- PresDet- LinkState- > DevCap2: Completion Timeout: Not Supported, TimeoutDis-, > LTR+, OBFF Via message ARIFwd+ > DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, > LTR-, OBFF Disabled ARIFwd- > LnkCtl2: Target Link Speed: 8GT/s, EnterCompliance- > SpeedDis-, Selectable De-emphasis: -6dB > Transmit Margin: Normal Operating Range, > EnterModifiedCompliance- ComplianceSOS- > Compliance De-emphasis: -6dB > LnkSta2: Current De-emphasis Level: -3.5dB, > EqualizationComplete-, EqualizationPhase1- > EqualizationPhase2-, EqualizationPhase3-, > LinkEqualizationRequest- > Capabilities: [a4] Subsystem: Fujitsu Limited. Device 1804 > Capabilities: [100 v1] Device Serial Number aa-87-00-10-b5-df- > 0e-00 > Capabilities: [fb4 v1] Advanced Error Reporting > UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- > UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- > UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- > UnxCmplt- RxOF- MalfTLP- ECRC+ UnsupReq+ ACSViol- > UESvrt: DLP+ SDES+ TLP+ FCP+ CmpltTO- CmpltAbrt+ > UnxCmplt+ RxOF+ MalfTLP+ ECRC+ UnsupReq- ACSViol+ > CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- > NonFatalErr+ > CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- > NonFatalErr+ > AERCap: First Error Pointer: 1f, GenCap+ CGenEn- ChkCap+ > ChkEn- > Capabilities: [138 v1] Power Budgeting <?> > Capabilities: [10c v1] #19 > Capabilities: [148 v1] Virtual Channel > Caps: LPEVC=0 RefClk=100ns PATEntryBits=1 > Arb: Fixed- WRR32- WRR64- WRR128- > Ctrl: ArbSelect=Fixed > Status: InProgress- > VC0: Caps: PATOffset=00 MaxTimeSlots=1 > RejSnoopTrans- > Arb: Fixed+ WRR32- WRR64- WRR128- TWRR128- > WRR256- > Ctrl: Enable+ ID=0 ArbSelect=Fixed TC/VC=ff > Status: NegoPending+ InProgress- > Capabilities: [e00 v1] #12 > Capabilities: [f24 v1] Access Control Services > ACSCap: SrcValid+ TransBlk+ ReqRedir+ CmpltRedir+ > UpstreamFwd+ EgressCtrl+ DirectTrans+ > ACSCtl: SrcValid- TransBlk- ReqRedir- CmpltRedir- > UpstreamFwd- EgressCtrl- DirectTrans- > Capabilities: [b70 v1] Vendor Specific Information: ID=0001 > Rev=0 Len=010 <?> > Kernel driver in use: pcieport > > > > If there is no attention button, then the behavior seen, matches with > > what I expected of the code. > > > > Seemingly, the HW does not have a power controller (and attention > > button), thus the power is automatically turned on (by HW) when card > > is inserted and the link comes up. Thus prior to link state based > > hot-plug, "echo 1 > power" used to do a little else than initiate a > > rescan for this particular HW. > > > > > > > > > I think that for ExpressCard and similar devices, we want to turn on > > > the device and attach a driver as soon as the card is inserted. But > > > in your case, I assume we want a model like that in Table 2-7 of the > > > "PCI Standard Hot-Plug Controller and Subsystem Specification," rev > > > 1.0, i.e., "Hot Insertion Initiated via Software UI." So there must > > > be some way to differentiate an ExpressCard slot from a slot like > > > yours. > > > > Yes, that would help if there was some way. > > > > Best regards, > Taku Izumi
At Thu, 13 Feb 2014 16:03:30 +0000, Rajat Jain wrote: > > [+Takashi] > > Hello, > > > > > > Huh, that doesn't sound good. Does your slot have an attention > > > > button? Can you collect the "lspci -vvv" output for the Downstream > > > > Port leading to this slot? > > > > The slots have no Attention button and are not for ExpressCard. > > lspci -vvv output is: > > > > Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- > > ParErr+ Stepping- SERR+ FastB2B- DisINTx+ > > Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- > > <TAbort- <MAbort- >SERR- <PERR- INTx- > > Latency: 0 > > Bus: primary=2c, secondary=2d, subordinate=2f, sec-latency=0 > > I/O behind bridge: 0000f000-00000fff > > Memory behind bridge: b1800000-b1ffffff > > Prefetchable memory behind bridge: 00000000ae800000- > > 00000000aeffffff > > Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- > > <TAbort- <MAbort- <SERR- <PERR- > > BridgeCtl: Parity- SERR+ NoISA- VGA- MAbort- >Reset- FastB2B- > > PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- > > Capabilities: [40] Power Management version 3 > > Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1- > > ,D2-,D3hot+,D3cold+) > > Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- > > Capabilities: [48] MSI: Enable+ Count=1/8 Maskable+ 64bit+ > > Address: 00000000fee00518 Data: 0000 > > Masking: 000000fe Pending: 00000000 > > Capabilities: [68] Express (v2) Downstream Port (Slot+), MSI 00 > > DevCap: MaxPayload 2048 bytes, PhantFunc 0, Latency L0s > > <64ns, L1 <1us > > ExtTag- RBE+ FLReset- > > DevCtl: Report errors: Correctable+ Non-Fatal+ Fatal+ > > Unsupported- > > RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+ > > MaxPayload 128 bytes, MaxReadReq 128 bytes > > DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq+ AuxPwr- > > TransPend- > > LnkCap: Port #1, Speed 8GT/s, Width x8, ASPM L1, Latency > > L0 <4us, L1 <4us > > ClockPM- Surprise+ LLActRep+ BwNot+ > > LnkCtl: ASPM Disabled; Disabled- Retrain- CommClk- > > ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt- > > LnkSta: Speed 2.5GT/s, Width x0, TrErr- Train- SlotClk+ > > DLActive- BWMgmt- ABWMgmt- > > SltCap: AttnBtn- PwrCtrl+ MRL+ AttnInd+ PwrInd+ HotPlug+ > > Surprise- > > Slot #67, PowerLimit 25.000W; Interlock- > > NoCompl- > > Hmm... I see that the Slot has a power controller. Which means that the power to the slot shall not be turned on automatically (by HW) when the card is plugged in. Also meaning that the link will not come up automatically - so this does not seem like the Link state based hotplug kicking in. > > What I suspect is this one: > > f02d1843d83b "PCI: pciehp: Remove surprise bit checks" > > Since this patch removes *all* the surprise bit checks causing all the presence detect changes (including 0->1) to be initiate hotplug. I think it may be worthwhile to try it out while removing this particular hunk: > > @@ -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; > > May be we should remove the surprise check for INT_PRESENCE_OFF only (and let it stay for INT_PRESEWNCE_ON)? I checked my test devices, and they seem to have no power controller bit involved. So I double-checked the recent commits, and indeed Rajat's recent work already fixed the detection (presumed that the commit [e48f1b67: PCI: pciehp: Use link change notifications for hot-plug and removal] does it) without fiddling with the surprise bit. That said, my patch can be reverted completely. Can anyone check whether it cures? thanks, Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
SGVsbG8sDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogVGFrYXNoaSBJ d2FpIFttYWlsdG86dGl3YWlAc3VzZS5kZV0NCj4gU2VudDogVGh1cnNkYXksIEZlYnJ1YXJ5IDEz LCAyMDE0IDg6MzcgQU0NCj4gVG86IFJhamF0IEphaW4NCj4gQ2M6IEl6dW1pLCBUYWt1OyByYWph dHhqYWluQGdtYWlsLmNvbTsgQmpvcm4gSGVsZ2FhczsgbGludXgtDQo+IHBjaUB2Z2VyLmtlcm5l bC5vcmc7IHlpbmdoYWlAa2VybmVsLm9yZzsgR3VlbnRlciBSb2Vjaw0KPiBTdWJqZWN0OiBSZTog W1BBVENIXSBQQ0k6IHBjaWVocDogRml4IHRoZSBwcm9ibGVtIHRoYXQgdGhlIHByZXNlbnQgYml0 DQo+IGlzIG5vdCBjbGVhcmVkIHRob3VnaCBzbG90IGlzIGVtcHR5DQo+IA0KPiBBdCBUaHUsIDEz IEZlYiAyMDE0IDE2OjAzOjMwICswMDAwLA0KPiBSYWphdCBKYWluIHdyb3RlOg0KPiA+DQo+ID4g WytUYWthc2hpXQ0KPiA+DQo+ID4gSGVsbG8sDQo+ID4gPg0KPiA+ID4gPiA+IEh1aCwgdGhhdCBk b2Vzbid0IHNvdW5kIGdvb2QuICBEb2VzIHlvdXIgc2xvdCBoYXZlIGFuIGF0dGVudGlvbg0KPiA+ ID4gPiA+IGJ1dHRvbj8gIENhbiB5b3UgY29sbGVjdCB0aGUgImxzcGNpIC12dnYiIG91dHB1dCBm b3IgdGhlDQo+ID4gPiA+ID4gRG93bnN0cmVhbSBQb3J0IGxlYWRpbmcgdG8gdGhpcyBzbG90Pw0K PiA+ID4NCj4gPiA+ICAgIFRoZSBzbG90cyBoYXZlIG5vIEF0dGVudGlvbiBidXR0b24gYW5kIGFy ZSBub3QgZm9yIEV4cHJlc3NDYXJkLg0KPiA+ID4gICAgbHNwY2kgLXZ2diBvdXRwdXQgaXM6DQo+ ID4gPg0KPiA+ID4gICAgICAgICBDb250cm9sOiBJL08rIE1lbSsgQnVzTWFzdGVyKyBTcGVjQ3lj bGUtIE1lbVdJTlYtIFZHQVNub29wLQ0KPiA+ID4gUGFyRXJyKyBTdGVwcGluZy0gU0VSUisgRmFz dEIyQi0gRGlzSU5UeCsNCj4gPiA+ICAgICAgICAgU3RhdHVzOiBDYXArIDY2TUh6LSBVREYtIEZh c3RCMkItIFBhckVyci0gREVWU0VMPWZhc3QNCj4gPiA+ID5UQWJvcnQtDQo+ID4gPiA8VEFib3J0 LSA8TUFib3J0LSA+U0VSUi0gPFBFUlItIElOVHgtDQo+ID4gPiAgICAgICAgIExhdGVuY3k6IDAN Cj4gPiA+ICAgICAgICAgQnVzOiBwcmltYXJ5PTJjLCBzZWNvbmRhcnk9MmQsIHN1Ym9yZGluYXRl PTJmLCBzZWMtbGF0ZW5jeT0wDQo+ID4gPiAgICAgICAgIEkvTyBiZWhpbmQgYnJpZGdlOiAwMDAw ZjAwMC0wMDAwMGZmZg0KPiA+ID4gICAgICAgICBNZW1vcnkgYmVoaW5kIGJyaWRnZTogYjE4MDAw MDAtYjFmZmZmZmYNCj4gPiA+ICAgICAgICAgUHJlZmV0Y2hhYmxlIG1lbW9yeSBiZWhpbmQgYnJp ZGdlOiAwMDAwMDAwMGFlODAwMDAwLQ0KPiA+ID4gMDAwMDAwMDBhZWZmZmZmZg0KPiA+ID4gICAg ICAgICBTZWNvbmRhcnkgc3RhdHVzOiA2Nk1Iei0gRmFzdEIyQi0gUGFyRXJyLSBERVZTRUw9ZmFz dA0KPiA+ID4gPlRBYm9ydC0NCj4gPiA+IDxUQWJvcnQtIDxNQWJvcnQtIDxTRVJSLSA8UEVSUi0N Cj4gPiA+ICAgICAgICAgQnJpZGdlQ3RsOiBQYXJpdHktIFNFUlIrIE5vSVNBLSBWR0EtIE1BYm9y dC0gPlJlc2V0LQ0KPiBGYXN0QjJCLQ0KPiA+ID4gICAgICAgICAgICAgICAgIFByaURpc2NUbXIt IFNlY0Rpc2NUbXItIERpc2NUbXJTdGF0LSBEaXNjVG1yU0VSUkVuLQ0KPiA+ID4gICAgICAgICBD YXBhYmlsaXRpZXM6IFs0MF0gUG93ZXIgTWFuYWdlbWVudCB2ZXJzaW9uIDMNCj4gPiA+ICAgICAg ICAgICAgICAgICBGbGFnczogUE1FQ2xrLSBEU0ktIEQxLSBEMi0gQXV4Q3VycmVudD0wbUENCj4g PiA+IFBNRShEMCssRDEtDQo+ID4gPiAsRDItLEQzaG90KyxEM2NvbGQrKQ0KPiA+ID4gICAgICAg ICAgICAgICAgIFN0YXR1czogRDAgTm9Tb2Z0UnN0KyBQTUUtRW5hYmxlLSBEU2VsPTAgRFNjYWxl PTANCj4gUE1FLQ0KPiA+ID4gICAgICAgICBDYXBhYmlsaXRpZXM6IFs0OF0gTVNJOiBFbmFibGUr IENvdW50PTEvOCBNYXNrYWJsZSsgNjRiaXQrDQo+ID4gPiAgICAgICAgICAgICAgICAgQWRkcmVz czogMDAwMDAwMDBmZWUwMDUxOCAgRGF0YTogMDAwMA0KPiA+ID4gICAgICAgICAgICAgICAgIE1h c2tpbmc6IDAwMDAwMGZlICBQZW5kaW5nOiAwMDAwMDAwMA0KPiA+ID4gICAgICAgICBDYXBhYmls aXRpZXM6IFs2OF0gRXhwcmVzcyAodjIpIERvd25zdHJlYW0gUG9ydCAoU2xvdCspLCBNU0kNCj4g MDANCj4gPiA+ICAgICAgICAgICAgICAgICBEZXZDYXA6IE1heFBheWxvYWQgMjA0OCBieXRlcywg UGhhbnRGdW5jIDAsIExhdGVuY3kNCj4gPiA+IEwwcyA8NjRucywgTDEgPDF1cw0KPiA+ID4gICAg ICAgICAgICAgICAgICAgICAgICAgRXh0VGFnLSBSQkUrIEZMUmVzZXQtDQo+ID4gPiAgICAgICAg ICAgICAgICAgRGV2Q3RsOiBSZXBvcnQgZXJyb3JzOiBDb3JyZWN0YWJsZSsgTm9uLUZhdGFsKw0K PiA+ID4gRmF0YWwrDQo+ID4gPiBVbnN1cHBvcnRlZC0NCj4gPiA+ICAgICAgICAgICAgICAgICAg ICAgICAgIFJseGRPcmQrIEV4dFRhZy0gUGhhbnRGdW5jLSBBdXhQd3ItIE5vU25vb3ArDQo+ID4g PiAgICAgICAgICAgICAgICAgICAgICAgICBNYXhQYXlsb2FkIDEyOCBieXRlcywgTWF4UmVhZFJl cSAxMjggYnl0ZXMNCj4gPiA+ICAgICAgICAgICAgICAgICBEZXZTdGE6IENvcnJFcnIrIFVuY29y ckVyci0gRmF0YWxFcnItIFVuc3VwcFJlcSsNCj4gPiA+IEF1eFB3ci0NCj4gPiA+IFRyYW5zUGVu ZC0NCj4gPiA+ICAgICAgICAgICAgICAgICBMbmtDYXA6IFBvcnQgIzEsIFNwZWVkIDhHVC9zLCBX aWR0aCB4OCwgQVNQTSBMMSwNCj4gPiA+IExhdGVuY3kNCj4gPiA+IEwwIDw0dXMsIEwxIDw0dXMN Cj4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAgIENsb2NrUE0tIFN1cnByaXNlKyBMTEFjdFJl cCsgQndOb3QrDQo+ID4gPiAgICAgICAgICAgICAgICAgTG5rQ3RsOiBBU1BNIERpc2FibGVkOyBE aXNhYmxlZC0gUmV0cmFpbi0gQ29tbUNsay0NCj4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAg IEV4dFN5bmNoLSBDbG9ja1BNLSBBdXRXaWREaXMtIEJXSW50LQ0KPiBBdXRCV0ludC0NCj4gPiA+ ICAgICAgICAgICAgICAgICBMbmtTdGE6IFNwZWVkIDIuNUdUL3MsIFdpZHRoIHgwLCBUckVyci0g VHJhaW4tDQo+ID4gPiBTbG90Q2xrKw0KPiA+ID4gRExBY3RpdmUtIEJXTWdtdC0gQUJXTWdtdC0N Cj4gPiA+ICAgICAgICAgICAgICAgICBTbHRDYXA6IEF0dG5CdG4tIFB3ckN0cmwrIE1STCsgQXR0 bkluZCsgUHdySW5kKw0KPiA+ID4gSG90UGx1ZysNCj4gPiA+IFN1cnByaXNlLQ0KPiA+ID4gICAg ICAgICAgICAgICAgICAgICAgICAgU2xvdCAjNjcsIFBvd2VyTGltaXQgMjUuMDAwVzsgSW50ZXJs b2NrLQ0KPiA+ID4gTm9Db21wbC0NCj4gPg0KPiA+IEhtbS4uLiBJIHNlZSB0aGF0IHRoZSBTbG90 IGhhcyBhIHBvd2VyIGNvbnRyb2xsZXIuIFdoaWNoIG1lYW5zIHRoYXQNCj4gdGhlIHBvd2VyIHRv IHRoZSBzbG90IHNoYWxsIG5vdCBiZSB0dXJuZWQgb24gYXV0b21hdGljYWxseSAoYnkgSFcpIHdo ZW4NCj4gdGhlIGNhcmQgaXMgcGx1Z2dlZCBpbi4gQWxzbyBtZWFuaW5nIHRoYXQgdGhlIGxpbmsg d2lsbCBub3QgY29tZSB1cA0KPiBhdXRvbWF0aWNhbGx5IC0gc28gdGhpcyBkb2VzIG5vdCBzZWVt IGxpa2UgdGhlIExpbmsgc3RhdGUgYmFzZWQgaG90cGx1Zw0KPiBraWNraW5nIGluLg0KPiA+DQo+ ID4gV2hhdCBJIHN1c3BlY3QgaXMgdGhpcyBvbmU6DQo+ID4NCj4gPiBmMDJkMTg0M2Q4M2IgIlBD STogcGNpZWhwOiBSZW1vdmUgc3VycHJpc2UgYml0IGNoZWNrcyINCj4gPg0KPiA+IFNpbmNlIHRo aXMgcGF0Y2ggcmVtb3ZlcyAqYWxsKiB0aGUgc3VycHJpc2UgYml0IGNoZWNrcyBjYXVzaW5nIGFs bCB0aGUNCj4gcHJlc2VuY2UgZGV0ZWN0IGNoYW5nZXMgKGluY2x1ZGluZyAwLT4xKSB0byBiZSBp bml0aWF0ZSBob3RwbHVnLiBJIHRoaW5rDQo+IGl0IG1heSBiZSB3b3J0aHdoaWxlIHRvIHRyeSBp dCBvdXQgd2hpbGUgcmVtb3ZpbmcgdGhpcyBwYXJ0aWN1bGFyIGh1bms6DQo+ID4NCj4gPiBAQCAt NTM1LDggKzUzNSw2IEBAIHN0YXRpYyB2b2lkIGludGVycnVwdF9ldmVudF9oYW5kbGVyKHN0cnVj dA0KPiB3b3JrX3N0cnVjdCAqd29yaykNCj4gPiAgCQlicmVhazsNCj4gPiAgCWNhc2UgSU5UX1BS RVNFTkNFX09OOg0KPiA+ICAJY2FzZSBJTlRfUFJFU0VOQ0VfT0ZGOg0KPiA+IC0JCWlmICghSFBf U1VQUl9STShjdHJsKSkNCj4gPiAtCQkJYnJlYWs7DQo+ID4gIAkJY3RybF9kYmcoY3RybCwgIlN1 cnByaXNlIFJlbW92YWxcbiIpOw0KPiA+ICAJCWhhbmRsZV9zdXJwcmlzZV9ldmVudChwX3Nsb3Qp Ow0KPiA+ICAJCWJyZWFrOw0KPiA+DQo+ID4gTWF5IGJlIHdlIHNob3VsZCByZW1vdmUgdGhlIHN1 cnByaXNlIGNoZWNrIGZvciBJTlRfUFJFU0VOQ0VfT0ZGIG9ubHkNCj4gKGFuZCBsZXQgaXQgc3Rh eSBmb3IgSU5UX1BSRVNFV05DRV9PTik/DQo+IA0KPiBJIGNoZWNrZWQgbXkgdGVzdCBkZXZpY2Vz LCBhbmQgdGhleSBzZWVtIHRvIGhhdmUgbm8gcG93ZXIgY29udHJvbGxlciBiaXQNCj4gaW52b2x2 ZWQuICBTbyBJIGRvdWJsZS1jaGVja2VkIHRoZSByZWNlbnQgY29tbWl0cywgYW5kIGluZGVlZCBS YWphdCdzDQo+IHJlY2VudCB3b3JrIGFscmVhZHkgZml4ZWQgdGhlIGRldGVjdGlvbiAocHJlc3Vt ZWQgdGhhdCB0aGUgY29tbWl0DQo+IFtlNDhmMWI2NzogUENJOiBwY2llaHA6IFVzZSBsaW5rIGNo YW5nZSBub3RpZmljYXRpb25zIGZvciBob3QtcGx1ZyBhbmQNCj4gcmVtb3ZhbF0gZG9lcyBpdCkg d2l0aG91dCBmaWRkbGluZyB3aXRoIHRoZSBzdXJwcmlzZSBiaXQuDQoNClRoYW5rcyBhIGxvdCBm b3IgdGVzdGluZyENCg0KPiANCj4gVGhhdCBzYWlkLCBteSBwYXRjaCBjYW4gYmUgcmV2ZXJ0ZWQg Y29tcGxldGVseS4gIENhbiBhbnlvbmUgY2hlY2sNCj4gd2hldGhlciBpdCBjdXJlcz8NCg0KSSBz dGlsbCB0aGluayB0aGF0IHBhcnQgb2YgeW91ciBwYXRjaCBpcyBuZWVkZWQuIEF0IGxlYXN0IHRo ZSBvbmUgdGhhdCByZW1vdmVzIHRoZSBzdXJwcmlzZSBiaXQgY2hlY2sgaWYgdGhlIGNhcmQgaXMg c3VkZGVubHkgeWFua2VkIG91dC4NCg0KVGhhbmtzLA0KDQpSYWphdA0KDQo+IA0KPiANCj4gdGhh bmtzLA0KPiANCj4gVGFrYXNoaQ0KPiANCg0K -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rajat, > [+Takashi] > > Hello, > > > > > > Huh, that doesn't sound good. Does your slot have an attention > > > > button? Can you collect the "lspci -vvv" output for the Downstream > > > > Port leading to this slot? > > > > The slots have no Attention button and are not for ExpressCard. > > lspci -vvv output is: > > > > Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- > > ParErr+ Stepping- SERR+ FastB2B- DisINTx+ > > Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- > > <TAbort- <MAbort- >SERR- <PERR- INTx- > > Latency: 0 > > Bus: primary=2c, secondary=2d, subordinate=2f, sec-latency=0 > > I/O behind bridge: 0000f000-00000fff > > Memory behind bridge: b1800000-b1ffffff > > Prefetchable memory behind bridge: 00000000ae800000- > > 00000000aeffffff > > Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- > > <TAbort- <MAbort- <SERR- <PERR- > > BridgeCtl: Parity- SERR+ NoISA- VGA- MAbort- >Reset- FastB2B- > > PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- > > Capabilities: [40] Power Management version 3 > > Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1- > > ,D2-,D3hot+,D3cold+) > > Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- > > Capabilities: [48] MSI: Enable+ Count=1/8 Maskable+ 64bit+ > > Address: 00000000fee00518 Data: 0000 > > Masking: 000000fe Pending: 00000000 > > Capabilities: [68] Express (v2) Downstream Port (Slot+), MSI 00 > > DevCap: MaxPayload 2048 bytes, PhantFunc 0, Latency L0s > > <64ns, L1 <1us > > ExtTag- RBE+ FLReset- > > DevCtl: Report errors: Correctable+ Non-Fatal+ Fatal+ > > Unsupported- > > RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+ > > MaxPayload 128 bytes, MaxReadReq 128 bytes > > DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq+ AuxPwr- > > TransPend- > > LnkCap: Port #1, Speed 8GT/s, Width x8, ASPM L1, Latency > > L0 <4us, L1 <4us > > ClockPM- Surprise+ LLActRep+ BwNot+ > > LnkCtl: ASPM Disabled; Disabled- Retrain- CommClk- > > ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt- > > LnkSta: Speed 2.5GT/s, Width x0, TrErr- Train- SlotClk+ > > DLActive- BWMgmt- ABWMgmt- > > SltCap: AttnBtn- PwrCtrl+ MRL+ AttnInd+ PwrInd+ HotPlug+ > > Surprise- > > Slot #67, PowerLimit 25.000W; Interlock- > > NoCompl- > > Hmm... I see that the Slot has a power controller. Which means that the power to the slot shall not be turned on automatically > (by HW) when the card is plugged in. Also meaning that the link will not come up automatically - so this does not seem > like the Link state based hotplug kicking in. > > What I suspect is this one: > > f02d1843d83b "PCI: pciehp: Remove surprise bit checks" You are right. In case of omitting comit-f02d1843, it worked as expected. Slot power doesn't become ON automatically when PCIe card is inserted. Best regards, Taku Izumi > > Since this patch removes *all* the surprise bit checks causing all the presence detect changes (including 0->1) to be > initiate hotplug. I think it may be worthwhile to try it out while removing this particular hunk: > > @@ -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; > > May be we should remove the surprise check for INT_PRESENCE_OFF only (and let it stay for INT_PRESEWNCE_ON)? > > Thanks, > > Rajat > > > > > SltCtl: Enable: AttnBtn- PwrFlt- MRL+ PresDet+ CmdCplt+ > > HPIrq+ LinkChg+ > > Control: AttnInd Off, PwrInd Off, Power+ > > Interlock- > > SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- > > PresDet- Interlock- > > Changed: MRL- PresDet- LinkState- > > DevCap2: Completion Timeout: Not Supported, TimeoutDis-, > > LTR+, OBFF Via message ARIFwd+ > > DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, > > LTR-, OBFF Disabled ARIFwd- > > LnkCtl2: Target Link Speed: 8GT/s, EnterCompliance- > > SpeedDis-, Selectable De-emphasis: -6dB > > Transmit Margin: Normal Operating Range, > > EnterModifiedCompliance- ComplianceSOS- > > Compliance De-emphasis: -6dB > > LnkSta2: Current De-emphasis Level: -3.5dB, > > EqualizationComplete-, EqualizationPhase1- > > EqualizationPhase2-, EqualizationPhase3-, > > LinkEqualizationRequest- > > Capabilities: [a4] Subsystem: Fujitsu Limited. Device 1804 > > Capabilities: [100 v1] Device Serial Number aa-87-00-10-b5-df- > > 0e-00 > > Capabilities: [fb4 v1] Advanced Error Reporting > > UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- > > UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- > > UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- > > UnxCmplt- RxOF- MalfTLP- ECRC+ UnsupReq+ ACSViol- > > UESvrt: DLP+ SDES+ TLP+ FCP+ CmpltTO- CmpltAbrt+ > > UnxCmplt+ RxOF+ MalfTLP+ ECRC+ UnsupReq- ACSViol+ > > CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- > > NonFatalErr+ > > CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- > > NonFatalErr+ > > AERCap: First Error Pointer: 1f, GenCap+ CGenEn- ChkCap+ > > ChkEn- > > Capabilities: [138 v1] Power Budgeting <?> > > Capabilities: [10c v1] #19 > > Capabilities: [148 v1] Virtual Channel > > Caps: LPEVC=0 RefClk=100ns PATEntryBits=1 > > Arb: Fixed- WRR32- WRR64- WRR128- > > Ctrl: ArbSelect=Fixed > > Status: InProgress- > > VC0: Caps: PATOffset=00 MaxTimeSlots=1 > > RejSnoopTrans- > > Arb: Fixed+ WRR32- WRR64- WRR128- TWRR128- > > WRR256- > > Ctrl: Enable+ ID=0 ArbSelect=Fixed TC/VC=ff > > Status: NegoPending+ InProgress- > > Capabilities: [e00 v1] #12 > > Capabilities: [f24 v1] Access Control Services > > ACSCap: SrcValid+ TransBlk+ ReqRedir+ CmpltRedir+ > > UpstreamFwd+ EgressCtrl+ DirectTrans+ > > ACSCtl: SrcValid- TransBlk- ReqRedir- CmpltRedir- > > UpstreamFwd- EgressCtrl- DirectTrans- > > Capabilities: [b70 v1] Vendor Specific Information: ID=0001 > > Rev=0 Len=010 <?> > > Kernel driver in use: pcieport > > > > > > > If there is no attention button, then the behavior seen, matches with > > > what I expected of the code. > > > > > > Seemingly, the HW does not have a power controller (and attention > > > button), thus the power is automatically turned on (by HW) when card > > > is inserted and the link comes up. Thus prior to link state based > > > hot-plug, "echo 1 > power" used to do a little else than initiate a > > > rescan for this particular HW. > > > > > > > > > > > > > I think that for ExpressCard and similar devices, we want to turn on > > > > the device and attach a driver as soon as the card is inserted. But > > > > in your case, I assume we want a model like that in Table 2-7 of the > > > > "PCI Standard Hot-Plug Controller and Subsystem Specification," rev > > > > 1.0, i.e., "Hot Insertion Initiated via Software UI." So there must > > > > be some way to differentiate an ExpressCard slot from a slot like > > > > yours. > > > > > > Yes, that would help if there was some way. > > > > > > > Best regards, > > Taku Izumi
Thanks a bunch for Testing! > -----Original Message----- > From: Izumi, Taku [mailto:izumi.taku@jp.fujitsu.com] > Sent: Thursday, February 13, 2014 11:21 PM > To: Rajat Jain; rajatxjain@gmail.com; Bjorn Helgaas > Cc: linux-pci@vger.kernel.org; yinghai@kernel.org; Guenter Roeck; > Takashi Iwai > Subject: RE: [PATCH] PCI: pciehp: Fix the problem that the present bit > is not cleared though slot is empty > > Hi Rajat, > > > [+Takashi] > > > > Hello, > > > > > > > > Huh, that doesn't sound good. Does your slot have an attention > > > > > button? Can you collect the "lspci -vvv" output for the > > > > > Downstream Port leading to this slot? > > > > > > The slots have no Attention button and are not for ExpressCard. > > > lspci -vvv output is: > > > > > > Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- > > > ParErr+ Stepping- SERR+ FastB2B- DisINTx+ > > > Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast > > > >TAbort- > > > <TAbort- <MAbort- >SERR- <PERR- INTx- > > > Latency: 0 > > > Bus: primary=2c, secondary=2d, subordinate=2f, sec-latency=0 > > > I/O behind bridge: 0000f000-00000fff > > > Memory behind bridge: b1800000-b1ffffff > > > Prefetchable memory behind bridge: 00000000ae800000- > > > 00000000aeffffff > > > Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast > > > >TAbort- > > > <TAbort- <MAbort- <SERR- <PERR- > > > BridgeCtl: Parity- SERR+ NoISA- VGA- MAbort- >Reset- > FastB2B- > > > PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- > > > Capabilities: [40] Power Management version 3 > > > Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA > > > PME(D0+,D1- > > > ,D2-,D3hot+,D3cold+) > > > Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 > PME- > > > Capabilities: [48] MSI: Enable+ Count=1/8 Maskable+ 64bit+ > > > Address: 00000000fee00518 Data: 0000 > > > Masking: 000000fe Pending: 00000000 > > > Capabilities: [68] Express (v2) Downstream Port (Slot+), MSI > 00 > > > DevCap: MaxPayload 2048 bytes, PhantFunc 0, Latency > > > L0s <64ns, L1 <1us > > > ExtTag- RBE+ FLReset- > > > DevCtl: Report errors: Correctable+ Non-Fatal+ > > > Fatal+ > > > Unsupported- > > > RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+ > > > MaxPayload 128 bytes, MaxReadReq 128 bytes > > > DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq+ > > > AuxPwr- > > > TransPend- > > > LnkCap: Port #1, Speed 8GT/s, Width x8, ASPM L1, > > > Latency > > > L0 <4us, L1 <4us > > > ClockPM- Surprise+ LLActRep+ BwNot+ > > > LnkCtl: ASPM Disabled; Disabled- Retrain- CommClk- > > > ExtSynch- ClockPM- AutWidDis- BWInt- > AutBWInt- > > > LnkSta: Speed 2.5GT/s, Width x0, TrErr- Train- > > > SlotClk+ > > > DLActive- BWMgmt- ABWMgmt- > > > SltCap: AttnBtn- PwrCtrl+ MRL+ AttnInd+ PwrInd+ > > > HotPlug+ > > > Surprise- > > > Slot #67, PowerLimit 25.000W; Interlock- > > > NoCompl- > > > > Hmm... I see that the Slot has a power controller. Which means that > > the power to the slot shall not be turned on automatically (by HW) > > when the card is plugged in. Also meaning that the link will not come > up automatically - so this does not seem like the Link state based > hotplug kicking in. > > > > What I suspect is this one: > > > > f02d1843d83b "PCI: pciehp: Remove surprise bit checks" > > You are right. > In case of omitting comit-f02d1843, it worked as expected. > Slot power doesn't become ON automatically when PCIe card is inserted. > > Best regards, > Taku Izumi > > > > > Since this patch removes *all* the surprise bit checks causing all the > > presence detect changes (including 0->1) to be initiate hotplug. I > think it may be worthwhile to try it out while removing this particular > hunk: > > > > @@ -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; > > > > May be we should remove the surprise check for INT_PRESENCE_OFF only > (and let it stay for INT_PRESEWNCE_ON)? > > > > Thanks, > > > > Rajat > > > > > > > > > SltCtl: Enable: AttnBtn- PwrFlt- MRL+ PresDet+ > > > CmdCplt+ > > > HPIrq+ LinkChg+ > > > Control: AttnInd Off, PwrInd Off, Power+ > > > Interlock- > > > SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- > > > PresDet- Interlock- > > > Changed: MRL- PresDet- LinkState- > > > DevCap2: Completion Timeout: Not Supported, > > > TimeoutDis-, > > > LTR+, OBFF Via message ARIFwd+ > > > DevCtl2: Completion Timeout: 50us to 50ms, > > > TimeoutDis-, LTR-, OBFF Disabled ARIFwd- > > > LnkCtl2: Target Link Speed: 8GT/s, EnterCompliance- > > > SpeedDis-, Selectable De-emphasis: -6dB > > > Transmit Margin: Normal Operating Range, > > > EnterModifiedCompliance- ComplianceSOS- > > > Compliance De-emphasis: -6dB > > > LnkSta2: Current De-emphasis Level: -3.5dB, > > > EqualizationComplete-, EqualizationPhase1- > > > EqualizationPhase2-, EqualizationPhase3-, > > > LinkEqualizationRequest- > > > Capabilities: [a4] Subsystem: Fujitsu Limited. Device 1804 > > > Capabilities: [100 v1] Device Serial Number > > > aa-87-00-10-b5-df- > > > 0e-00 > > > Capabilities: [fb4 v1] Advanced Error Reporting > > > UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- > > > UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- > > > UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- > > > UnxCmplt- RxOF- MalfTLP- ECRC+ UnsupReq+ ACSViol- > > > UESvrt: DLP+ SDES+ TLP+ FCP+ CmpltTO- CmpltAbrt+ > > > UnxCmplt+ RxOF+ MalfTLP+ ECRC+ UnsupReq- ACSViol+ > > > CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- > > > NonFatalErr+ > > > CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- > > > NonFatalErr+ > > > AERCap: First Error Pointer: 1f, GenCap+ CGenEn- > > > ChkCap+ > > > ChkEn- > > > Capabilities: [138 v1] Power Budgeting <?> > > > Capabilities: [10c v1] #19 > > > Capabilities: [148 v1] Virtual Channel > > > Caps: LPEVC=0 RefClk=100ns PATEntryBits=1 > > > Arb: Fixed- WRR32- WRR64- WRR128- > > > Ctrl: ArbSelect=Fixed > > > Status: InProgress- > > > VC0: Caps: PATOffset=00 MaxTimeSlots=1 > > > RejSnoopTrans- > > > Arb: Fixed+ WRR32- WRR64- WRR128- > TWRR128- > > > WRR256- > > > Ctrl: Enable+ ID=0 ArbSelect=Fixed > TC/VC=ff > > > Status: NegoPending+ InProgress- > > > Capabilities: [e00 v1] #12 > > > Capabilities: [f24 v1] Access Control Services > > > ACSCap: SrcValid+ TransBlk+ ReqRedir+ CmpltRedir+ > > > UpstreamFwd+ EgressCtrl+ DirectTrans+ > > > ACSCtl: SrcValid- TransBlk- ReqRedir- CmpltRedir- > > > UpstreamFwd- EgressCtrl- DirectTrans- > > > Capabilities: [b70 v1] Vendor Specific Information: ID=0001 > > > Rev=0 Len=010 <?> > > > Kernel driver in use: pcieport > > > > > > > > > > If there is no attention button, then the behavior seen, matches > > > > with what I expected of the code. > > > > > > > > Seemingly, the HW does not have a power controller (and attention > > > > button), thus the power is automatically turned on (by HW) when > > > > card is inserted and the link comes up. Thus prior to link state > > > > based hot-plug, "echo 1 > power" used to do a little else than > > > > initiate a rescan for this particular HW. > > > > > > > > > > > > > > > > > I think that for ExpressCard and similar devices, we want to > > > > > turn on the device and attach a driver as soon as the card is > > > > > inserted. But in your case, I assume we want a model like that > > > > > in Table 2-7 of the "PCI Standard Hot-Plug Controller and > > > > > Subsystem Specification," rev 1.0, i.e., "Hot Insertion > > > > > Initiated via Software UI." So there must be some way to > > > > > differentiate an ExpressCard slot from a slot like yours. > > > > > > > > Yes, that would help if there was some way. > > > > > > > > > > Best regards, > > > Taku Izumi
On Fri, Feb 14, 2014 at 12:21 AM, Izumi, Taku <izumi.taku@jp.fujitsu.com> wrote: >> Hmm... I see that the Slot has a power controller. Which means that the power to the slot shall not be turned on automatically >> (by HW) when the card is plugged in. Also meaning that the link will not come up automatically - so this does not seem >> like the Link state based hotplug kicking in. >> >> What I suspect is this one: >> >> f02d1843d83b "PCI: pciehp: Remove surprise bit checks" > > You are right. > In case of omitting comit-f02d1843, it worked as expected. > Slot power doesn't become ON automatically when PCIe card is inserted. OK, I dropped f02d1843d83b ("PCI: pciehp: Remove surprise bit checks"). Thanks for testing, everybody! I think our handling of slot capabilities and control is a bit haphazard. It seems like we're mostly responding to things that break, and we don't really have a coherent explanation of how things *should* work in different configurations. I think it would be good if somebody wrote up something for Documentation/PCI, with references to the relevant specifications, that describes how we think things should work. For example, I think we have at least four models: 1) ExpressCard 2) Slots with attention button 3) Slots with no button where we expect a software UI, e.g., Taku's box 4) Devices with no actual slot, no button, etc., e.g., Rajat's system We handle these slightly differently, implicitly relying on various capability tests scattered throughout the code. I think we should be able to come up with a scheme that would make this more explicit and make pciehp simpler and more consistent. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bjorn, On Fri, Feb 14, 2014 at 9:31 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Fri, Feb 14, 2014 at 12:21 AM, Izumi, Taku <izumi.taku@jp.fujitsu.com> wrote: >>> Hmm... I see that the Slot has a power controller. Which means that the power to the slot shall not be turned on automatically >>> (by HW) when the card is plugged in. Also meaning that the link will not come up automatically - so this does not seem >>> like the Link state based hotplug kicking in. >>> >>> What I suspect is this one: >>> >>> f02d1843d83b "PCI: pciehp: Remove surprise bit checks" >> >> You are right. >> In case of omitting comit-f02d1843, it worked as expected. >> Slot power doesn't become ON automatically when PCIe card is inserted. > > OK, I dropped f02d1843d83b ("PCI: pciehp: Remove surprise bit > checks"). I think part of this commit was still good (The part that drops the surprise check when a card is yanked out). That is because when a card is yanked out, it shouldn't matter whether the surprise bit is set or not - its gotta go. Functionally, that scenario is already covered by my patches (because yanking out a card will make the link go down, hence kicking off link state based unplug) - thus no functional change. But just thought I'll mention since you were already at this cleanup phase. (If you agree, I can send a separate clean patch or you can use Takashi's one). > Thanks for testing, everybody! > > I think our handling of slot capabilities and control is a bit > haphazard. It seems like we're mostly responding to things that > break, and we don't really have a coherent explanation of how things > *should* work in different configurations. I think it would be good > if somebody wrote up something for Documentation/PCI, with references > to the relevant specifications, that describes how we think things > should work. Sure, I think I'll take a stab. Thanks, Rajat > For example, I think we have at least four models: > > 1) ExpressCard > 2) Slots with attention button > 3) Slots with no button where we expect a software UI, e.g., Taku's box > 4) Devices with no actual slot, no button, etc., e.g., Rajat's system > > We handle these slightly differently, implicitly relying on various > capability tests scattered throughout the code. I think we should be > able to come up with a scheme that would make this more explicit and > make pciehp simpler and more consistent. > > Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 14, 2014 at 12:39 PM, Rajat Jain <rajatxjain@gmail.com> wrote: > Hi Bjorn, > > On Fri, Feb 14, 2014 at 9:31 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> On Fri, Feb 14, 2014 at 12:21 AM, Izumi, Taku <izumi.taku@jp.fujitsu.com> wrote: >>>> Hmm... I see that the Slot has a power controller. Which means that the power to the slot shall not be turned on automatically >>>> (by HW) when the card is plugged in. Also meaning that the link will not come up automatically - so this does not seem >>>> like the Link state based hotplug kicking in. >>>> >>>> What I suspect is this one: >>>> >>>> f02d1843d83b "PCI: pciehp: Remove surprise bit checks" >>> >>> You are right. >>> In case of omitting comit-f02d1843, it worked as expected. >>> Slot power doesn't become ON automatically when PCIe card is inserted. >> >> OK, I dropped f02d1843d83b ("PCI: pciehp: Remove surprise bit >> checks"). > > I think part of this commit was still good (The part that drops the > surprise check when a card is yanked out). That is because when a card > is yanked out, it shouldn't matter whether the surprise bit is set or > not - its gotta go. > > Functionally, that scenario is already covered by my patches (because > yanking out a card will make the link go down, hence kicking off link > state based unplug) - thus no functional change. But just thought I'll > mention since you were already at this cleanup phase. (If you agree, I > can send a separate clean patch or you can use Takashi's one). That sounds reasonable. Send me a patch, if you don't mind, so it's clear what to do here. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 18, 2014 at 3:02 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Fri, Feb 14, 2014 at 12:39 PM, Rajat Jain <rajatxjain@gmail.com> wrote: >> Hi Bjorn, >> >> On Fri, Feb 14, 2014 at 9:31 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: >>> On Fri, Feb 14, 2014 at 12:21 AM, Izumi, Taku <izumi.taku@jp.fujitsu.com> wrote: >>>>> Hmm... I see that the Slot has a power controller. Which means that the power to the slot shall not be turned on automatically >>>>> (by HW) when the card is plugged in. Also meaning that the link will not come up automatically - so this does not seem >>>>> like the Link state based hotplug kicking in. >>>>> >>>>> What I suspect is this one: >>>>> >>>>> f02d1843d83b "PCI: pciehp: Remove surprise bit checks" >>>> >>>> You are right. >>>> In case of omitting comit-f02d1843, it worked as expected. >>>> Slot power doesn't become ON automatically when PCIe card is inserted. >>> >>> OK, I dropped f02d1843d83b ("PCI: pciehp: Remove surprise bit >>> checks"). >> >> I think part of this commit was still good (The part that drops the >> surprise check when a card is yanked out). That is because when a card >> is yanked out, it shouldn't matter whether the surprise bit is set or >> not - its gotta go. >> >> Functionally, that scenario is already covered by my patches (because >> yanking out a card will make the link go down, hence kicking off link >> state based unplug) - thus no functional change. But just thought I'll >> mention since you were already at this cleanup phase. (If you agree, I >> can send a separate clean patch or you can use Takashi's one). > > That sounds reasonable. Send me a patch, if you don't mind, so it's > clear what to do here. Just sent. Thanks, Rajat -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Bjorn, >> >> I think our handling of slot capabilities and control is a bit >> haphazard. It seems like we're mostly responding to things that >> break, and we don't really have a coherent explanation of how things >> *should* work in different configurations. I think it would be good >> if somebody wrote up something for Documentation/PCI, with references >> to the relevant specifications, that describes how we think things >> should work. > > Sure, I think I'll take a stab. > > Thanks, > > Rajat > >> For example, I think we have at least four models: >> >> 1) ExpressCard >> 2) Slots with attention button >> 3) Slots with no button where we expect a software UI, e.g., Taku's box >> 4) Devices with no actual slot, no button, etc., e.g., Rajat's system >> I'm trying to come up with a write up for this. And am trying to understand what differentiates (1) from others - from the view of use case model of course. I briefly looked at the ExpressCard specs at http://www.usb.org/developers/expresscard/EC_specifications/ExpressCard_2_0_FINAL.pdf I understand that ExpressCard shall be an actual slot and of course has a defined form factor etc, but, when I think of the _use case_ that the user is expected to follow, it shall be pretty much dependent on the HP elements. For e.g, if there is attention button, it would follow (2), but if link comes up automatically (no power controller etc), then it would follow (4) etc. Thus I think that the use cases (sequence to be followed for hotplug) shall be more appropriately categorized based on what hot-plug elements (button etc) are present in a system. Of course, we can give examples ("ExpressCards are one example in this category and is typically characterized by .. ") within those categories, while also making references to the appropriate standards. What do you think? Thanks, Rajat -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 14acfcc..163f0b4 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -508,7 +508,12 @@ void pciehp_power_off_slot(struct slot * slot) { struct controller *ctrl = slot->ctrl; - /* Disable the link at first */ + pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PWR_OFF, PCI_EXP_SLTCTL_PCC); + ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__, + pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, + PCI_EXP_SLTCTL_PWR_OFF); + + /* Disable the link */ pciehp_link_disable(ctrl); /* wait the link is down */ if (ctrl->link_active_reporting) @@ -516,10 +521,6 @@ void pciehp_power_off_slot(struct slot * slot) else msleep(1000); - pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PWR_OFF, PCI_EXP_SLTCTL_PCC); - ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__, - pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, - PCI_EXP_SLTCTL_PWR_OFF); } static irqreturn_t pcie_isr(int irq, void *dev_id)
I found the problem that the present bit does not be cleared though the slot is empty. Step to reproduce: # cd /sys/bus/pci/slots/66 # cat adapter 0 --- (insert PCIe card on slot#66) --- # cat adapter 1 # echo 1 > power # echo 0 > power --- (eject PCIe card on slot#66) --- # cat adapter 1 ==> should be 0. According to git-bisect this is caused by: 2debd9289997fc5d1c0043b41201a8b40d5e11d0 PCI: pciehp: Disable/enable link during slot power off/on is the first bad commit By reverting the above patch, this bug can be solved. And I also found this can be fixed by changing the timing of link-disable during power off the slot. So, this patch changes the timing of link-disable during power off. Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- drivers/pci/hotplug/pciehp_hpc.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)