Hello, > -----Original Message----- > From: Takashi Iwai [mailto:tiwai@xxxxxxx] > Sent: Thursday, February 13, 2014 8:37 AM > To: Rajat Jain > Cc: Izumi, Taku; rajatxjain@xxxxxxxxx; Bjorn Helgaas; linux- > pci@xxxxxxxxxxxxxxx; yinghai@xxxxxxxxxx; Guenter Roeck > Subject: Re: [PATCH] PCI: pciehp: Fix the problem that the present bit > is not cleared though slot is empty > > 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. Thanks a lot for testing! > > That said, my patch can be reverted completely. Can anyone check > whether it cures? I still think that part of your patch is needed. At least the one that removes the surprise bit check if the card is suddenly yanked out. Thanks, Rajat > > > thanks, > > Takashi > ��.n��������+%������w��{.n�����{���"�)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥