* [PATCH] PCI: pciehp: Remove surprise bit checks @ 2014-02-11 11:38 Takashi Iwai 2014-02-11 18:52 ` Rajat Jain 0 siblings, 1 reply; 4+ messages in thread From: Takashi Iwai @ 2014-02-11 11:38 UTC (permalink / raw) To: Bjorn Helgaas; +Cc: Alex Williamson, linux-pci, linux-kernel Currently pciehp driver checks the surprise removal bit and handles the hotplug event only when the bit is set. But there are devices that don't set that bit but yet expect the hotplug working, e.g. the PCI card reader on HP ProBook 445 and 455 laptops appears only when you insert a card, and it needs the hotplug event handling. For fixing this, basically we may ignore the surprise bit and always handle the event. The only big concern in the past was the KVM device assignment, but this has been fixed in KVM side (ignoring hotplug events during secondary bus resets), there should be no obstacle ahead. The earlier discussion thread is found at: https://lkml.org/lkml/2013/3/20/274 Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=70261 Signed-off-by: Takashi Iwai <tiwai@suse.de> --- drivers/pci/hotplug/pciehp_ctrl.c | 11 ----------- drivers/pci/hotplug/pciehp_hpc.c | 19 +++++++------------ 2 files changed, 7 insertions(+), 23 deletions(-) diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c index 50628487597d..8f6c626b4e21 100644 --- a/drivers/pci/hotplug/pciehp_ctrl.c +++ b/drivers/pci/hotplug/pciehp_ctrl.c @@ -434,8 +434,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; @@ -494,15 +492,6 @@ int pciehp_disable_slot(struct slot *p_slot) if (!p_slot->ctrl) return 1; - if (!HP_SUPR_RM(p_slot->ctrl)) { - pciehp_get_adapter_status(p_slot, &getstatus); - if (!getstatus) { - ctrl_info(ctrl, "No adapter on slot(%s)\n", - slot_name(p_slot)); - return -ENODEV; - } - } - if (MRL_SENS(p_slot->ctrl)) { pciehp_get_latch_status(p_slot, &getstatus); if (getstatus) { diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 14acfccb7670..5f9196a85a53 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -636,21 +636,16 @@ int pciehp_reset_slot(struct slot *slot, int probe) if (probe) return 0; - if (HP_SUPR_RM(ctrl)) { - pcie_write_cmd(ctrl, 0, PCI_EXP_SLTCTL_PDCE); - if (pciehp_poll_mode) - del_timer_sync(&ctrl->poll_timer); - } + pcie_write_cmd(ctrl, 0, PCI_EXP_SLTCTL_PDCE); + if (pciehp_poll_mode) + del_timer_sync(&ctrl->poll_timer); pci_reset_bridge_secondary_bus(ctrl->pcie->port); - if (HP_SUPR_RM(ctrl)) { - pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, - PCI_EXP_SLTSTA_PDC); - pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PDCE, PCI_EXP_SLTCTL_PDCE); - if (pciehp_poll_mode) - int_poll_timeout(ctrl->poll_timer.data); - } + pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, PCI_EXP_SLTSTA_PDC); + pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PDCE, PCI_EXP_SLTCTL_PDCE); + if (pciehp_poll_mode) + int_poll_timeout(ctrl->poll_timer.data); return 0; } -- 1.8.5.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: [PATCH] PCI: pciehp: Remove surprise bit checks 2014-02-11 11:38 [PATCH] PCI: pciehp: Remove surprise bit checks Takashi Iwai @ 2014-02-11 18:52 ` Rajat Jain 2014-02-11 23:18 ` Bjorn Helgaas 0 siblings, 1 reply; 4+ messages in thread From: Rajat Jain @ 2014-02-11 18:52 UTC (permalink / raw) To: Takashi Iwai, Bjorn Helgaas Cc: Alex Williamson, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org SGVsbG8gVGFrYXNoaSwNCg0KPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBs aW51eC1wY2ktb3duZXJAdmdlci5rZXJuZWwub3JnIFttYWlsdG86bGludXgtcGNpLQ0KPiBvd25l ckB2Z2VyLmtlcm5lbC5vcmddIE9uIEJlaGFsZiBPZiBUYWthc2hpIEl3YWkNCj4gU2VudDogVHVl c2RheSwgRmVicnVhcnkgMTEsIDIwMTQgMzozOSBBTQ0KPiBUbzogQmpvcm4gSGVsZ2Fhcw0KPiBD YzogQWxleCBXaWxsaWFtc29uOyBsaW51eC1wY2lAdmdlci5rZXJuZWwub3JnOyBsaW51eC0NCj4g a2VybmVsQHZnZXIua2VybmVsLm9yZw0KPiBTdWJqZWN0OiBbUEFUQ0hdIFBDSTogcGNpZWhwOiBS ZW1vdmUgc3VycHJpc2UgYml0IGNoZWNrcw0KPiANCj4gQ3VycmVudGx5IHBjaWVocCBkcml2ZXIg Y2hlY2tzIHRoZSBzdXJwcmlzZSByZW1vdmFsIGJpdCBhbmQgaGFuZGxlcw0KPiB0aGUgaG90cGx1 ZyBldmVudCBvbmx5IHdoZW4gdGhlIGJpdCBpcyBzZXQuICBCdXQgdGhlcmUgYXJlIGRldmljZXMN Cj4gdGhhdCBkb24ndCBzZXQgdGhhdCBiaXQgYnV0IHlldCBleHBlY3QgdGhlIGhvdHBsdWcgd29y a2luZywgZS5nLiB0aGUNCj4gUENJIGNhcmQgcmVhZGVyIG9uIEhQIFByb0Jvb2sgNDQ1IGFuZCA0 NTUgbGFwdG9wcyBhcHBlYXJzIG9ubHkgd2hlbg0KPiB5b3UgaW5zZXJ0IGEgY2FyZCwgYW5kIGl0 IG5lZWRzIHRoZSBob3RwbHVnIGV2ZW50IGhhbmRsaW5nLg0KPiANCj4gRm9yIGZpeGluZyB0aGlz LCBiYXNpY2FsbHkgd2UgbWF5IGlnbm9yZSB0aGUgc3VycHJpc2UgYml0IGFuZCBhbHdheXMNCj4g aGFuZGxlIHRoZSBldmVudC4NCg0KU29tZSBvZiB0aGlzIGhhcyBiZWVuIHRha2VuIGNhcmUgb2Yg aW4gdGhlIHJlY2VudCBjaGFuZ2VzIHRvIHBjaWVoY3AgKGxhc3QgbmlnaHQpLg0KDQpQbGVhc2Ug c2VlDQoNCmh0dHBzOi8vZ2l0Lmtlcm5lbC5vcmcvY2dpdC9saW51eC9rZXJuZWwvZ2l0L2hlbGdh YXMvcGNpLmdpdC9jb21taXQvP2g9cGNpL3BjaWVocCZpZD1hM2ViYTM4ODc5NjBkMDBhYmI5MTJm ZTM3MDNjZDNhMDU4YzcyMWI4DQoNCkFsc28sIGxpbmsgc3RhdGUgaG90cGx1ZyBoYXMgYmVlbiBh ZGRlZCBhcyBwYXJ0IG9mIHRoaXMgcGF0Y2ggc2V0LiBDYW4geW91IHBsZWFzZSB0cnkgd2l0aCBC am9ybidzIGxhdGVzdCBwY2kgdHJlZT8NCg0KV2l0aCB0aGF0LCBJIHRoaW5rIHlvdSBzaG91bGQg bm90IHNlZSB0aGUgaXNzdWUgeW91IGFyZSBkZXNjcmliaW5nLCBiZWNhdXNlIGlmIHRoZSBQQ0ll IExpbmsgY29tZXMgdXAgd2hlbiB5b3UgaW5zZXJ0IHRoZSBjYXJkLCBpdCBzaGFsbCBiZSBhZGRl ZCAobm93KSBpcnJlc3BlY3RpdmUgb2Ygd2hldGhlciBvciBub3QgeW91IGhhdmUgdGhlIHN1cnBy aXNlIGJpdCBzZXQuDQoNClRoYW5rcywNCg0KUmFqYXQNCg0KDQo+IFRoZSBvbmx5IGJpZyBjb25j ZXJuIGluIHRoZSBwYXN0IHdhcyB0aGUgS1ZNIGRldmljZQ0KPiBhc3NpZ25tZW50LCBidXQgdGhp cyBoYXMgYmVlbiBmaXhlZCBpbiBLVk0gc2lkZSAoaWdub3JpbmcgaG90cGx1Zw0KPiBldmVudHMg ZHVyaW5nIHNlY29uZGFyeSBidXMgcmVzZXRzKSwgdGhlcmUgc2hvdWxkIGJlIG5vIG9ic3RhY2xl DQo+IGFoZWFkLg0KPiANCj4gVGhlIGVhcmxpZXIgZGlzY3Vzc2lvbiB0aHJlYWQgaXMgZm91bmQg YXQ6DQo+ICAgICBodHRwczovL2xrbWwub3JnL2xrbWwvMjAxMy8zLzIwLzI3NA0KPiANCj4gQnVn emlsbGE6IGh0dHBzOi8vYnVnemlsbGEua2VybmVsLm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NzAyNjEN Cj4gU2lnbmVkLW9mZi1ieTogVGFrYXNoaSBJd2FpIDx0aXdhaUBzdXNlLmRlPg0KPiAtLS0NCj4g IGRyaXZlcnMvcGNpL2hvdHBsdWcvcGNpZWhwX2N0cmwuYyB8IDExIC0tLS0tLS0tLS0tDQo+ICBk cml2ZXJzL3BjaS9ob3RwbHVnL3BjaWVocF9ocGMuYyAgfCAxOSArKysrKysrLS0tLS0tLS0tLS0t DQo+ICAyIGZpbGVzIGNoYW5nZWQsIDcgaW5zZXJ0aW9ucygrKSwgMjMgZGVsZXRpb25zKC0pDQo+ IA0KPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9wY2kvaG90cGx1Zy9wY2llaHBfY3RybC5jDQo+IGIv ZHJpdmVycy9wY2kvaG90cGx1Zy9wY2llaHBfY3RybC5jDQo+IGluZGV4IDUwNjI4NDg3NTk3ZC4u OGY2YzYyNmI0ZTIxIDEwMDY0NA0KPiAtLS0gYS9kcml2ZXJzL3BjaS9ob3RwbHVnL3BjaWVocF9j dHJsLmMNCj4gKysrIGIvZHJpdmVycy9wY2kvaG90cGx1Zy9wY2llaHBfY3RybC5jDQo+IEBAIC00 MzQsOCArNDM0LDYgQEAgc3RhdGljIHZvaWQgaW50ZXJydXB0X2V2ZW50X2hhbmRsZXIoc3RydWN0 DQo+IHdvcmtfc3RydWN0ICp3b3JrKQ0KPiAgCQlicmVhazsNCj4gIAljYXNlIElOVF9QUkVTRU5D RV9PTjoNCj4gIAljYXNlIElOVF9QUkVTRU5DRV9PRkY6DQo+IC0JCWlmICghSFBfU1VQUl9STShj dHJsKSkNCj4gLQkJCWJyZWFrOw0KPiAgCQljdHJsX2RiZyhjdHJsLCAiU3VycHJpc2UgUmVtb3Zh bFxuIik7DQo+ICAJCWhhbmRsZV9zdXJwcmlzZV9ldmVudChwX3Nsb3QpOw0KPiAgCQlicmVhazsN Cj4gQEAgLTQ5NCwxNSArNDkyLDYgQEAgaW50IHBjaWVocF9kaXNhYmxlX3Nsb3Qoc3RydWN0IHNs b3QgKnBfc2xvdCkNCj4gIAlpZiAoIXBfc2xvdC0+Y3RybCkNCj4gIAkJcmV0dXJuIDE7DQo+IA0K PiAtCWlmICghSFBfU1VQUl9STShwX3Nsb3QtPmN0cmwpKSB7DQo+IC0JCXBjaWVocF9nZXRfYWRh cHRlcl9zdGF0dXMocF9zbG90LCAmZ2V0c3RhdHVzKTsNCj4gLQkJaWYgKCFnZXRzdGF0dXMpIHsN Cj4gLQkJCWN0cmxfaW5mbyhjdHJsLCAiTm8gYWRhcHRlciBvbiBzbG90KCVzKVxuIiwNCj4gLQkJ CQkgIHNsb3RfbmFtZShwX3Nsb3QpKTsNCj4gLQkJCXJldHVybiAtRU5PREVWOw0KPiAtCQl9DQo+ IC0JfQ0KPiAtDQo+ICAJaWYgKE1STF9TRU5TKHBfc2xvdC0+Y3RybCkpIHsNCj4gIAkJcGNpZWhw X2dldF9sYXRjaF9zdGF0dXMocF9zbG90LCAmZ2V0c3RhdHVzKTsNCj4gIAkJaWYgKGdldHN0YXR1 cykgew0KPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9wY2kvaG90cGx1Zy9wY2llaHBfaHBjLmMNCj4g Yi9kcml2ZXJzL3BjaS9ob3RwbHVnL3BjaWVocF9ocGMuYw0KPiBpbmRleCAxNGFjZmNjYjc2NzAu LjVmOTE5NmE4NWE1MyAxMDA2NDQNCj4gLS0tIGEvZHJpdmVycy9wY2kvaG90cGx1Zy9wY2llaHBf aHBjLmMNCj4gKysrIGIvZHJpdmVycy9wY2kvaG90cGx1Zy9wY2llaHBfaHBjLmMNCj4gQEAgLTYz NiwyMSArNjM2LDE2IEBAIGludCBwY2llaHBfcmVzZXRfc2xvdChzdHJ1Y3Qgc2xvdCAqc2xvdCwg aW50DQo+IHByb2JlKQ0KPiAgCWlmIChwcm9iZSkNCj4gIAkJcmV0dXJuIDA7DQo+IA0KPiAtCWlm IChIUF9TVVBSX1JNKGN0cmwpKSB7DQo+IC0JCXBjaWVfd3JpdGVfY21kKGN0cmwsIDAsIFBDSV9F WFBfU0xUQ1RMX1BEQ0UpOw0KPiAtCQlpZiAocGNpZWhwX3BvbGxfbW9kZSkNCj4gLQkJCWRlbF90 aW1lcl9zeW5jKCZjdHJsLT5wb2xsX3RpbWVyKTsNCj4gLQl9DQo+ICsJcGNpZV93cml0ZV9jbWQo Y3RybCwgMCwgUENJX0VYUF9TTFRDVExfUERDRSk7DQo+ICsJaWYgKHBjaWVocF9wb2xsX21vZGUp DQo+ICsJCWRlbF90aW1lcl9zeW5jKCZjdHJsLT5wb2xsX3RpbWVyKTsNCj4gDQo+ICAJcGNpX3Jl c2V0X2JyaWRnZV9zZWNvbmRhcnlfYnVzKGN0cmwtPnBjaWUtPnBvcnQpOw0KPiANCj4gLQlpZiAo SFBfU1VQUl9STShjdHJsKSkgew0KPiAtCQlwY2llX2NhcGFiaWxpdHlfd3JpdGVfd29yZChwZGV2 LCBQQ0lfRVhQX1NMVFNUQSwNCj4gLQkJCQkJICAgUENJX0VYUF9TTFRTVEFfUERDKTsNCj4gLQkJ cGNpZV93cml0ZV9jbWQoY3RybCwgUENJX0VYUF9TTFRDVExfUERDRSwNCj4gUENJX0VYUF9TTFRD VExfUERDRSk7DQo+IC0JCWlmIChwY2llaHBfcG9sbF9tb2RlKQ0KPiAtCQkJaW50X3BvbGxfdGlt ZW91dChjdHJsLT5wb2xsX3RpbWVyLmRhdGEpOw0KPiAtCX0NCj4gKwlwY2llX2NhcGFiaWxpdHlf d3JpdGVfd29yZChwZGV2LCBQQ0lfRVhQX1NMVFNUQSwNCj4gUENJX0VYUF9TTFRTVEFfUERDKTsN Cj4gKwlwY2llX3dyaXRlX2NtZChjdHJsLCBQQ0lfRVhQX1NMVENUTF9QRENFLCBQQ0lfRVhQX1NM VENUTF9QRENFKTsNCj4gKwlpZiAocGNpZWhwX3BvbGxfbW9kZSkNCj4gKwkJaW50X3BvbGxfdGlt ZW91dChjdHJsLT5wb2xsX3RpbWVyLmRhdGEpOw0KPiANCj4gIAlyZXR1cm4gMDsNCj4gIH0NCj4g LS0NCj4gMS44LjUuMg0KPiANCj4gLS0NCj4gVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6 IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGxpbnV4LXBjaSIgaW4NCj4gdGhlIGJvZHkgb2Yg YSBtZXNzYWdlIHRvIG1ham9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcNCj4gTW9yZSBtYWpvcmRvbW8g aW5mbyBhdCAgaHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sDQo+IA0K DQo= ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: pciehp: Remove surprise bit checks 2014-02-11 18:52 ` Rajat Jain @ 2014-02-11 23:18 ` Bjorn Helgaas 2014-02-12 12:54 ` Takashi Iwai 0 siblings, 1 reply; 4+ messages in thread From: Bjorn Helgaas @ 2014-02-11 23:18 UTC (permalink / raw) To: Rajat Jain Cc: Takashi Iwai, Alex Williamson, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org On Tue, Feb 11, 2014 at 06:52:30PM +0000, Rajat Jain wrote: > Hello Takashi, > > > -----Original Message----- > > From: linux-pci-owner@vger.kernel.org [mailto:linux-pci- > > owner@vger.kernel.org] On Behalf Of Takashi Iwai > > Sent: Tuesday, February 11, 2014 3:39 AM > > To: Bjorn Helgaas > > Cc: Alex Williamson; linux-pci@vger.kernel.org; linux- > > kernel@vger.kernel.org > > Subject: [PATCH] PCI: pciehp: Remove surprise bit checks > > > > Currently pciehp driver checks the surprise removal bit and handles > > the hotplug event only when the bit is set. But there are devices > > that don't set that bit but yet expect the hotplug working, e.g. the > > PCI card reader on HP ProBook 445 and 455 laptops appears only when > > you insert a card, and it needs the hotplug event handling. > > > > For fixing this, basically we may ignore the surprise bit and always > > handle the event. > > Some of this has been taken care of in the recent changes to pciehcp (last night). > > Please see > > https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/pciehp&id=a3eba3887960d00abb912fe3703cd3a058c721b8 > > Also, link state hotplug has been added as part of this patch set. Can you please try with Bjorn's latest pci tree? > > With that, I think you should not see the issue you are describing, because if the PCIe Link comes up when you insert the card, it shall be added (now) irrespective of whether or not you have the surprise bit set. Takashi's patch removed more HP_SUPR_RM() usage than yours, and I think those additional changes are probably necessary. I rebased Takashi's patch on top of my pci/pciehp branch, resulting in the patch below. I pushed this, so you can try the whole thing out at: http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/pciehp&id=237e0218bcad52d3df6746cd8d15a0d353bae84f Bjorn PCI: pciehp: Remove surprise bit checks From: Takashi Iwai <tiwai@suse.de> Currently pciehp driver checks the surprise removal bit and handles the hotplug event only when the bit is set. But there are devices that don't set that bit, yet expect hotplug to work, e.g., the SD/MMC card reader on HP ProBook 445 and 455 laptops appears only when you insert a card, and it needs the hotplug event handling. For fixing this, basically we may ignore the surprise bit and always handle the event. The only big concern in the past was the KVM device assignment, but this has been fixed in KVM side (ignoring hotplug events during secondary bus resets), there should be no obstacle ahead. The earlier discussion thread is found at: https://lkml.kernel.org/r/s5h38vqjhk6.wl%25tiwai@suse.de [bhelgaas: rebase on Rajat's changes, remove HP_SUPR_RM() completely] Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=70261 Signed-off-by: Takashi Iwai <tiwai@suse.de> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> --- drivers/pci/hotplug/pciehp.h | 1 - drivers/pci/hotplug/pciehp_ctrl.c | 2 -- drivers/pci/hotplug/pciehp_hpc.c | 11 +++++------ 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h index 8a66866b8cf1..d2a67aa0e49d 100644 --- a/drivers/pci/hotplug/pciehp.h +++ b/drivers/pci/hotplug/pciehp.h @@ -124,7 +124,6 @@ struct controller { #define MRL_SENS(ctrl) ((ctrl)->slot_cap & PCI_EXP_SLTCAP_MRLSP) #define ATTN_LED(ctrl) ((ctrl)->slot_cap & PCI_EXP_SLTCAP_AIP) #define PWR_LED(ctrl) ((ctrl)->slot_cap & PCI_EXP_SLTCAP_PIP) -#define HP_SUPR_RM(ctrl) ((ctrl)->slot_cap & PCI_EXP_SLTCAP_HPS) #define EMI(ctrl) ((ctrl)->slot_cap & PCI_EXP_SLTCAP_EIP) #define NO_CMD_CMPL(ctrl) ((ctrl)->slot_cap & PCI_EXP_SLTCAP_NCCS) #define PSN(ctrl) ((ctrl)->slot_cap >> 19) diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c index fec99a164d93..a4d29d24057d 100644 --- a/drivers/pci/hotplug/pciehp_ctrl.c +++ b/drivers/pci/hotplug/pciehp_ctrl.c @@ -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; diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index da4b0204b4f7..1076a51623b5 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -628,17 +628,18 @@ int pciehp_reset_slot(struct slot *slot, int probe) { struct controller *ctrl = slot->ctrl; struct pci_dev *pdev = ctrl_dev(ctrl); - u16 stat_mask = 0, ctrl_mask = 0; + u16 stat_mask, ctrl_mask; if (probe) return 0; - if (HP_SUPR_RM(ctrl) && !ATTN_BUTTN(ctrl)) { + ctrl_mask = PCI_EXP_SLTCTL_DLLSCE; + stat_mask = PCI_EXP_SLTSTA_DLLSC; + + if (!ATTN_BUTTN(ctrl)) { /* as in pcie_enable_notification() */ ctrl_mask |= PCI_EXP_SLTCTL_PDCE; stat_mask |= PCI_EXP_SLTSTA_PDC; } - ctrl_mask |= PCI_EXP_SLTCTL_DLLSCE; - stat_mask |= PCI_EXP_SLTSTA_DLLSC; pcie_write_cmd(ctrl, 0, ctrl_mask); if (pciehp_poll_mode) @@ -741,8 +742,6 @@ static inline void dbg_ctrl(struct controller *ctrl) ATTN_LED(ctrl) ? "yes" : "no"); ctrl_info(ctrl, " Power Indicator : %3s\n", PWR_LED(ctrl) ? "yes" : "no"); - ctrl_info(ctrl, " Hot-Plug Surprise : %3s\n", - HP_SUPR_RM(ctrl) ? "yes" : "no"); ctrl_info(ctrl, " EMI Present : %3s\n", EMI(ctrl) ? "yes" : "no"); ctrl_info(ctrl, " Command Completed : %3s\n", ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: pciehp: Remove surprise bit checks 2014-02-11 23:18 ` Bjorn Helgaas @ 2014-02-12 12:54 ` Takashi Iwai 0 siblings, 0 replies; 4+ messages in thread From: Takashi Iwai @ 2014-02-12 12:54 UTC (permalink / raw) To: Bjorn Helgaas Cc: Rajat Jain, Takashi Iwai, Alex Williamson, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org At Tue, 11 Feb 2014 16:18:35 -0700, Bjorn Helgaas wrote: > > On Tue, Feb 11, 2014 at 06:52:30PM +0000, Rajat Jain wrote: > > Hello Takashi, > > > > > -----Original Message----- > > > From: linux-pci-owner@vger.kernel.org [mailto:linux-pci- > > > owner@vger.kernel.org] On Behalf Of Takashi Iwai > > > Sent: Tuesday, February 11, 2014 3:39 AM > > > To: Bjorn Helgaas > > > Cc: Alex Williamson; linux-pci@vger.kernel.org; linux- > > > kernel@vger.kernel.org > > > Subject: [PATCH] PCI: pciehp: Remove surprise bit checks > > > > > > Currently pciehp driver checks the surprise removal bit and handles > > > the hotplug event only when the bit is set. But there are devices > > > that don't set that bit but yet expect the hotplug working, e.g. the > > > PCI card reader on HP ProBook 445 and 455 laptops appears only when > > > you insert a card, and it needs the hotplug event handling. > > > > > > For fixing this, basically we may ignore the surprise bit and always > > > handle the event. > > > > Some of this has been taken care of in the recent changes to pciehcp (last night). > > > > Please see > > > > https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/pciehp&id=a3eba3887960d00abb912fe3703cd3a058c721b8 > > > > Also, link state hotplug has been added as part of this patch set. Can you please try with Bjorn's latest pci tree? > > > > With that, I think you should not see the issue you are describing, because if the PCIe Link comes up when you insert the card, it shall be added (now) irrespective of whether or not you have the surprise bit set. > > Takashi's patch removed more HP_SUPR_RM() usage than yours, and I think > those additional changes are probably necessary. I rebased Takashi's patch > on top of my pci/pciehp branch, resulting in the patch below. > > I pushed this, so you can try the whole thing out at: > > http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/pciehp&id=237e0218bcad52d3df6746cd8d15a0d353bae84f Thanks, I confirmed that the tree works. Takashi > > Bjorn > > > > PCI: pciehp: Remove surprise bit checks > > From: Takashi Iwai <tiwai@suse.de> > > Currently pciehp driver checks the surprise removal bit and handles the > hotplug event only when the bit is set. But there are devices that don't > set that bit, yet expect hotplug to work, e.g., the SD/MMC card reader on > HP ProBook 445 and 455 laptops appears only when you insert a card, and it > needs the hotplug event handling. > > For fixing this, basically we may ignore the surprise bit and always handle > the event. The only big concern in the past was the KVM device assignment, > but this has been fixed in KVM side (ignoring hotplug events during > secondary bus resets), there should be no obstacle ahead. > > The earlier discussion thread is found at: > https://lkml.kernel.org/r/s5h38vqjhk6.wl%25tiwai@suse.de > > [bhelgaas: rebase on Rajat's changes, remove HP_SUPR_RM() completely] > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=70261 > Signed-off-by: Takashi Iwai <tiwai@suse.de> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > --- > drivers/pci/hotplug/pciehp.h | 1 - > drivers/pci/hotplug/pciehp_ctrl.c | 2 -- > drivers/pci/hotplug/pciehp_hpc.c | 11 +++++------ > 3 files changed, 5 insertions(+), 9 deletions(-) > > diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h > index 8a66866b8cf1..d2a67aa0e49d 100644 > --- a/drivers/pci/hotplug/pciehp.h > +++ b/drivers/pci/hotplug/pciehp.h > @@ -124,7 +124,6 @@ struct controller { > #define MRL_SENS(ctrl) ((ctrl)->slot_cap & PCI_EXP_SLTCAP_MRLSP) > #define ATTN_LED(ctrl) ((ctrl)->slot_cap & PCI_EXP_SLTCAP_AIP) > #define PWR_LED(ctrl) ((ctrl)->slot_cap & PCI_EXP_SLTCAP_PIP) > -#define HP_SUPR_RM(ctrl) ((ctrl)->slot_cap & PCI_EXP_SLTCAP_HPS) > #define EMI(ctrl) ((ctrl)->slot_cap & PCI_EXP_SLTCAP_EIP) > #define NO_CMD_CMPL(ctrl) ((ctrl)->slot_cap & PCI_EXP_SLTCAP_NCCS) > #define PSN(ctrl) ((ctrl)->slot_cap >> 19) > diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c > index fec99a164d93..a4d29d24057d 100644 > --- a/drivers/pci/hotplug/pciehp_ctrl.c > +++ b/drivers/pci/hotplug/pciehp_ctrl.c > @@ -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; > diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c > index da4b0204b4f7..1076a51623b5 100644 > --- a/drivers/pci/hotplug/pciehp_hpc.c > +++ b/drivers/pci/hotplug/pciehp_hpc.c > @@ -628,17 +628,18 @@ int pciehp_reset_slot(struct slot *slot, int probe) > { > struct controller *ctrl = slot->ctrl; > struct pci_dev *pdev = ctrl_dev(ctrl); > - u16 stat_mask = 0, ctrl_mask = 0; > + u16 stat_mask, ctrl_mask; > > if (probe) > return 0; > > - if (HP_SUPR_RM(ctrl) && !ATTN_BUTTN(ctrl)) { > + ctrl_mask = PCI_EXP_SLTCTL_DLLSCE; > + stat_mask = PCI_EXP_SLTSTA_DLLSC; > + > + if (!ATTN_BUTTN(ctrl)) { /* as in pcie_enable_notification() */ > ctrl_mask |= PCI_EXP_SLTCTL_PDCE; > stat_mask |= PCI_EXP_SLTSTA_PDC; > } > - ctrl_mask |= PCI_EXP_SLTCTL_DLLSCE; > - stat_mask |= PCI_EXP_SLTSTA_DLLSC; > > pcie_write_cmd(ctrl, 0, ctrl_mask); > if (pciehp_poll_mode) > @@ -741,8 +742,6 @@ static inline void dbg_ctrl(struct controller *ctrl) > ATTN_LED(ctrl) ? "yes" : "no"); > ctrl_info(ctrl, " Power Indicator : %3s\n", > PWR_LED(ctrl) ? "yes" : "no"); > - ctrl_info(ctrl, " Hot-Plug Surprise : %3s\n", > - HP_SUPR_RM(ctrl) ? "yes" : "no"); > ctrl_info(ctrl, " EMI Present : %3s\n", > EMI(ctrl) ? "yes" : "no"); > ctrl_info(ctrl, " Command Completed : %3s\n", > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-02-12 12:54 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-02-11 11:38 [PATCH] PCI: pciehp: Remove surprise bit checks Takashi Iwai 2014-02-11 18:52 ` Rajat Jain 2014-02-11 23:18 ` Bjorn Helgaas 2014-02-12 12:54 ` Takashi Iwai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).