Thanks a bunch for Testing! > -----Original Message----- > From: Izumi, Taku [mailto:izumi.taku@xxxxxxxxxxxxxx] > Sent: Thursday, February 13, 2014 11:21 PM > To: Rajat Jain; rajatxjain@xxxxxxxxx; Bjorn Helgaas > Cc: linux-pci@xxxxxxxxxxxxxxx; yinghai@xxxxxxxxxx; 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 ��.n��������+%������w��{.n�����{���"�)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
