Skip to contentSkip to conversation

pciehp: always implement resume

PATCH RESEND In review Rafael J. Wysocki 2 participants 4 messages Sep 7, 2012 – Sep 8, 2012 PCI

lore
Share & tools
lore
#0 lore raw

PATCH RESEND 1/1 pciehp: always implement resume

From: Oliver Neukum <[email protected]>
Subject: pciehp: always implement resume

The driver by default ignores resume. On some laptops that means that interrupts are not delivered after S3. So card removals and insertions will not be handled. Requiring a forced load isn't right.

---
drivers/pci/hotplug/pciehp_core.c | 28 ++++++++++++++--------------

 drivers/pci/hotplug/pciehp_core.c |   28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)
drivers/pci/hotplug/pciehp_core.c +14 -14
Index: linux/drivers/pci/hotplug/pciehp_core.c
===================================================================
@@ -300,24 +300,24 @@ static int pciehp_suspend (struct pcie_d
 
 static int pciehp_resume (struct pcie_device *dev)
 {
+	struct controller *ctrl;
+	struct slot *slot;
+	u8 status;
+
 	dev_info(&dev->device, "%s ENTRY\n", __func__);
-	if (pciehp_force) {
-		struct controller *ctrl = get_service_data(dev);
-		struct slot *slot;
-		u8 status;
+	ctrl = get_service_data(dev);
 
-		/* reinitialize the chipset's event detection logic */
-		pcie_enable_notification(ctrl);
+	/* reinitialize the chipset's event detection logic */
+	pcie_enable_notification(ctrl);
 
-		slot = ctrl->slot;
+	slot = ctrl->slot;
 
-		/* Check if slot is occupied */
-		pciehp_get_adapter_status(slot, &status);
-		if (status)
-			pciehp_enable_slot(slot);
-		else
-			pciehp_disable_slot(slot);
-	}
+	/* Check if slot is occupied */
+	pciehp_get_adapter_status(slot, &status);
+	if (status)
+		pciehp_enable_slot(slot);
+	else
+		pciehp_disable_slot(slot);
 	return 0;
 }
 #endif /* PM */
maintainer
#1 lore raw
Quoted 10 lines · Rafael J. Wysocki

From: Oliver Neukum <[email protected]>
Subject: pciehp: always implement resume

The driver by default ignores resume. On some laptops that means that interrupts are not delivered after S3. So card removals and insertions will not be handled. Requiring a forced load isn't right.

I applied this (with fixed From: email address and slightly modified
changelog) to http://git.kernel.org/?p=linux/kernel/git/helgaas/pci.git;a=shortlog;h=refs/heads/pci/oliver-pciehp-resume
and will merge it to "next" after Fengguang's build/smoke tests.

Thanks!

Bjorn

Quoted 48 lines

---
drivers/pci/hotplug/pciehp_core.c | 28 ++++++++++++++--------------

 drivers/pci/hotplug/pciehp_core.c |   28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

Index: linux/drivers/pci/hotplug/pciehp_core.c
===================================================================

drivers/pci/hotplug/pciehp_core.c +14 -14
@@ -300,24 +300,24 @@ static int pciehp_suspend (struct pcie_d
 
 static int pciehp_resume (struct pcie_device *dev)
 {
+       struct controller *ctrl;
+       struct slot *slot;
+       u8 status;
+
        dev_info(&dev->device, "%s ENTRY\n", __func__);
-       if (pciehp_force) {
-               struct controller *ctrl = get_service_data(dev);
-               struct slot *slot;
-               u8 status;
+       ctrl = get_service_data(dev);
 
-               /* reinitialize the chipset's event detection logic */
-               pcie_enable_notification(ctrl);
+       /* reinitialize the chipset's event detection logic */
+       pcie_enable_notification(ctrl);
 
-               slot = ctrl->slot;
+       slot = ctrl->slot;
 
-               /* Check if slot is occupied */
-               pciehp_get_adapter_status(slot, &status);
-               if (status)
-                       pciehp_enable_slot(slot);
-               else
-                       pciehp_disable_slot(slot);
-       }
+       /* Check if slot is occupied */
+       pciehp_get_adapter_status(slot, &status);
+       if (status)
+               pciehp_enable_slot(slot);
+       else
+               pciehp_disable_slot(slot);
        return 0;
 }
 #endif /* PM */
maintainer
#2 lore raw

+cc Mark & Kristen

Commit cd2fe83a815 by Mark added the code we're changing here, so this is your chance to speak up if we're not doing the right thing :)

Quoted 68 lines · Bjorn Helgaas
Quoted 10 lines · Rafael J. Wysocki

From: Oliver Neukum <[email protected]>
Subject: pciehp: always implement resume

The driver by default ignores resume. On some laptops that means that interrupts are not delivered after S3. So card removals and insertions will not be handled. Requiring a forced load isn't right.

I applied this (with fixed From: email address and slightly modified
changelog) to http://git.kernel.org/?p=linux/kernel/git/helgaas/pci.git;a=shortlog;h=refs/heads/pci/oliver-pciehp-resume
and will merge it to "next" after Fengguang's build/smoke tests.

Thanks!

Bjorn

Quoted 48 lines

---
drivers/pci/hotplug/pciehp_core.c | 28 ++++++++++++++--------------

 drivers/pci/hotplug/pciehp_core.c |   28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

Index: linux/drivers/pci/hotplug/pciehp_core.c
===================================================================

drivers/pci/hotplug/pciehp_core.c +14 -14
@@ -300,24 +300,24 @@ static int pciehp_suspend (struct pcie_d
 
 static int pciehp_resume (struct pcie_device *dev)
 {
+       struct controller *ctrl;
+       struct slot *slot;
+       u8 status;
+
        dev_info(&dev->device, "%s ENTRY\n", __func__);
-       if (pciehp_force) {
-               struct controller *ctrl = get_service_data(dev);
-               struct slot *slot;
-               u8 status;
+       ctrl = get_service_data(dev);
 
-               /* reinitialize the chipset's event detection logic */
-               pcie_enable_notification(ctrl);
+       /* reinitialize the chipset's event detection logic */
+       pcie_enable_notification(ctrl);
 
-               slot = ctrl->slot;
+       slot = ctrl->slot;
 
-               /* Check if slot is occupied */
-               pciehp_get_adapter_status(slot, &status);
-               if (status)
-                       pciehp_enable_slot(slot);
-               else
-                       pciehp_disable_slot(slot);
-       }
+       /* Check if slot is occupied */
+       pciehp_get_adapter_status(slot, &status);
+       if (status)
+               pciehp_enable_slot(slot);
+       else
+               pciehp_disable_slot(slot);
        return 0;
 }
 #endif /* PM */
maintainer
#3 lore raw

[fix Kristen's email addr]

Quoted 74 lines · Bjorn Helgaas

+cc Mark & Kristen

Commit cd2fe83a815 by Mark added the code we're changing here, so this is your chance to speak up if we're not doing the right thing :)

Quoted 68 lines · Bjorn Helgaas
Quoted 10 lines · Rafael J. Wysocki

From: Oliver Neukum <[email protected]>
Subject: pciehp: always implement resume

The driver by default ignores resume. On some laptops that means that interrupts are not delivered after S3. So card removals and insertions will not be handled. Requiring a forced load isn't right.

I applied this (with fixed From: email address and slightly modified
changelog) to http://git.kernel.org/?p=linux/kernel/git/helgaas/pci.git;a=shortlog;h=refs/heads/pci/oliver-pciehp-resume
and will merge it to "next" after Fengguang's build/smoke tests.

Thanks!

Bjorn

Quoted 48 lines

---
drivers/pci/hotplug/pciehp_core.c | 28 ++++++++++++++--------------

 drivers/pci/hotplug/pciehp_core.c |   28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

Index: linux/drivers/pci/hotplug/pciehp_core.c
===================================================================

drivers/pci/hotplug/pciehp_core.c +14 -14
@@ -300,24 +300,24 @@ static int pciehp_suspend (struct pcie_d
 
 static int pciehp_resume (struct pcie_device *dev)
 {
+       struct controller *ctrl;
+       struct slot *slot;
+       u8 status;
+
        dev_info(&dev->device, "%s ENTRY\n", __func__);
-       if (pciehp_force) {
-               struct controller *ctrl = get_service_data(dev);
-               struct slot *slot;
-               u8 status;
+       ctrl = get_service_data(dev);
 
-               /* reinitialize the chipset's event detection logic */
-               pcie_enable_notification(ctrl);
+       /* reinitialize the chipset's event detection logic */
+       pcie_enable_notification(ctrl);
 
-               slot = ctrl->slot;
+       slot = ctrl->slot;
 
-               /* Check if slot is occupied */
-               pciehp_get_adapter_status(slot, &status);
-               if (status)
-                       pciehp_enable_slot(slot);
-               else
-                       pciehp_disable_slot(slot);
-       }
+       /* Check if slot is occupied */
+       pciehp_get_adapter_status(slot, &status);
+       if (status)
+               pciehp_enable_slot(slot);
+       else
+               pciehp_disable_slot(slot);
        return 0;
 }
 #endif /* PM */