From patchwork Mon Sep 12 21:09:12 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 669005 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3sY0pP0KFnz9sdn for ; Tue, 13 Sep 2016 07:10:53 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932582AbcILVJU (ORCPT ); Mon, 12 Sep 2016 17:09:20 -0400 Received: from mail.kernel.org ([198.145.29.136]:54292 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932160AbcILVJQ (ORCPT ); Mon, 12 Sep 2016 17:09:16 -0400 Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 1834B203FB; Mon, 12 Sep 2016 21:09:15 +0000 (UTC) Received: from localhost (unknown [69.71.1.1]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B21662035E; Mon, 12 Sep 2016 21:09:13 +0000 (UTC) Subject: [PATCH v2 3/8] PCI: pciehp: Process all hotplug events before looking for new ones To: Mayurkumar Patel From: Bjorn Helgaas Cc: Rajat Jain , MikaWesterberg@linux.intel.com, linux-pci@vger.kernel.org, Rafael J Wysocki , linux-kernel@vger.kernel.org, Keith Busch , Luis Antonio Tarazona-Duarte , Andy Shevchenko , mika.westerberg@linux.intel.com Date: Mon, 12 Sep 2016 16:09:12 -0500 Message-ID: <20160912210911.22258.70054.stgit@bhelgaas-glaptop2.roam.corp.google.com> In-Reply-To: <20160912210507.22258.63097.stgit@bhelgaas-glaptop2.roam.corp.google.com> References: <20160912210507.22258.63097.stgit@bhelgaas-glaptop2.roam.corp.google.com> User-Agent: StGit/0.16 MIME-Version: 1.0 X-Spam-Status: No, score=-1.0 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, NML_ADSP_CUSTOM_MED,UNPARSEABLE_RELAY autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org From: Mayurkumar Patel Previously we accumulated hotplug events, then processed them, essentially like this: events = 0 do { status = read(Slot Status) status &= EVENT_MASK # only look at events events |= status # accumulate events write(Slot Status, events) # clear events } while (status) process events The problem is that as soon as we clear events in Slot Status, the hardware may send notifications for new events, and we lose information about the first events. For example, we might see two Presence Detect Changed events, but lose the fact that the slot was temporarily empty: read PCI_EXP_SLTSTA_PDC set, PCI_EXP_SLTSTA_PDS clear # slot empty write PCI_EXP_SLTSTA_PDC # clear PDC event read PCI_EXP_SLTSTA_PDC set, PCI_EXP_SLTSTA_PDS set # slot occupied The current code does not process a removal; it only processes the insertion, which fails because we didn't remove the original device. To avoid this problem, read Slot Status once and process all the events before reading it again, like this: do { read events clear events process events } while (events) [bhelgaas: changelog, add external loop around pciehp_isr()] Signed-off-by: Mayurkumar Patel Signed-off-by: Bjorn Helgaas --- drivers/pci/hotplug/pciehp_hpc.c | 58 +++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 26 deletions(-) -- 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 b8efe1b..625fa6a 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -535,7 +535,7 @@ void pciehp_power_off_slot(struct slot *slot) PCI_EXP_SLTCTL_PWR_OFF); } -static irqreturn_t pcie_isr(int irq, void *dev_id) +static irqreturn_t pciehp_isr(int irq, void *dev_id) { struct controller *ctrl = (struct controller *)dev_id; struct pci_dev *pdev = ctrl_dev(ctrl); @@ -550,36 +550,23 @@ static irqreturn_t pcie_isr(int irq, void *dev_id) if (pdev->current_state == PCI_D3cold) return IRQ_NONE; + pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &status); + if (status == (u16) ~0) { + ctrl_info(ctrl, "%s: no response from device\n", __func__); + return IRQ_NONE; + } + /* - * In order to guarantee that all interrupt events are - * serviced, we need to re-inspect Slot Status register after - * clearing what is presumed to be the last pending interrupt. + * Slot Status contains plain status bits as well as event + * notification bits; right now we only want the event bits. */ - events = 0; - do { - pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &status); - if (status == (u16) ~0) { - ctrl_info(ctrl, "%s: no response from device\n", - __func__); - return IRQ_NONE; - } - - /* - * Slot Status contains plain status bits as well as event - * notification bits; right now we only want the event bits. - */ - status &= (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD | + events = status & (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD | PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_CC | PCI_EXP_SLTSTA_DLLSC); - status &= ~events; - events |= status; - if (!events) - return IRQ_NONE; - if (status) - pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, - events); - } while (status); + if (!events) + return IRQ_NONE; + pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, events); ctrl_dbg(ctrl, "pending interrupts %#06x from Slot Status\n", events); /* Check Command Complete Interrupt Pending */ @@ -636,6 +623,25 @@ static irqreturn_t pcie_isr(int irq, void *dev_id) return IRQ_HANDLED; } +static irqreturn_t pcie_isr(int irq, void *dev_id) +{ + irqreturn_t rc, handled = IRQ_NONE; + + /* + * To guarantee that all interrupt events are serviced, we need to + * re-inspect Slot Status register after clearing what is presumed + * to be the last pending interrupt. + */ + do { + rc = pciehp_isr(irq, dev_id); + if (rc == IRQ_HANDLED) + handled = IRQ_HANDLED; + } while (rc == IRQ_HANDLED); + + /* Return IRQ_HANDLED if we handled one or more events */ + return handled; +} + void pcie_enable_notification(struct controller *ctrl) { u16 cmd, mask;