From fc4f57fadea906648a4ccae4c3ff4bfd7f277649 Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Sat, 29 Oct 2016 21:37:07 +0200 Subject: PCI/ASPM: Use permission-specific DEVICE_ATTR variants Use DEVICE_ATTR_RW for read-write attributes. This simplifies the source code, improves readability, and reduces the chance of inconsistencies. The semantic patch that makes this change is as follows: (http://coccinelle.lip6.fr/) // @rw@ declarer name DEVICE_ATTR; identifier x,x_show,x_store; @@ DEVICE_ATTR(x, \(0644\|S_IRUGO|S_IWUSR\), x_show, x_store); @script:ocaml@ x << rw.x; x_show << rw.x_show; x_store << rw.x_store; @@ if not (x^"_show" = x_show && x^"_store" = x_store) then Coccilib.include_match false @@ declarer name DEVICE_ATTR_RW; identifier rw.x,rw.x_show,rw.x_store; @@ - DEVICE_ATTR(x, \(0644\|S_IRUGO|S_IWUSR\), x_show, x_store); + DEVICE_ATTR_RW(x); // Signed-off-by: Julia Lawall Signed-off-by: Bjorn Helgaas --- drivers/pci/pcie/aspm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/pci/pcie') diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index 0ec649d..3b14d9e 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -886,8 +886,8 @@ static ssize_t clk_ctl_store(struct device *dev, return n; } -static DEVICE_ATTR(link_state, 0644, link_state_show, link_state_store); -static DEVICE_ATTR(clk_ctl, 0644, clk_ctl_show, clk_ctl_store); +static DEVICE_ATTR_RW(link_state); +static DEVICE_ATTR_RW(clk_ctl); static char power_group[] = "power"; void pcie_aspm_create_sysfs_dev_files(struct pci_dev *pdev) -- cgit v1.1 From c6a6330706148e7d5265c3dd658d25843c83390f Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Fri, 28 Oct 2016 10:52:06 +0200 Subject: PCI: Activate runtime PM on a PCIe port only if it can suspend Currently pcie_portdrv_probe() activates runtime PM on a PCIe port even if it will never actually suspend because the BIOS is too old or the "pcie_port_pm=off" option was specified on the kernel command line. A few CPU cycles can be saved by not activating runtime PM at all in these cases, because rpm_idle() and rpm_suspend() will bail out right at the beginning when calling rpm_check_suspend_allowed(), instead of carrying out various locking and assignments, invoking rpm_callback(), getting back -EBUSY and rolling everything back. The conditions checked in pci_bridge_d3_possible() are all static, they never change during uptime of the system, hence it's safe to call this to determine if runtime PM should be activated. No functional change intended. Tested-by: Mika Westerberg Signed-off-by: Lukas Wunner Signed-off-by: Bjorn Helgaas Reviewed-by: Rafael J. Wysocki --- drivers/pci/pcie/portdrv_pci.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/pci/pcie') diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c index 79327cc..1ae712c 100644 --- a/drivers/pci/pcie/portdrv_pci.c +++ b/drivers/pci/pcie/portdrv_pci.c @@ -19,6 +19,7 @@ #include #include +#include "../pci.h" #include "portdrv.h" #include "aer/aerdrv.h" @@ -157,7 +158,7 @@ static int pcie_portdrv_probe(struct pci_dev *dev, * subordinate devices). We can't be sure for native PCIe hotplug * either so prevent that as well. */ - if (!dev->is_hotplug_bridge) { + if (pci_bridge_d3_possible(dev) && !dev->is_hotplug_bridge) { /* * Keep the port resumed 100ms to make sure things like * config space accesses from userspace (lspci) will not @@ -175,7 +176,7 @@ static int pcie_portdrv_probe(struct pci_dev *dev, static void pcie_portdrv_remove(struct pci_dev *dev) { - if (!dev->is_hotplug_bridge) { + if (pci_bridge_d3_possible(dev) && !dev->is_hotplug_bridge) { pm_runtime_forbid(&dev->dev); pm_runtime_get_noresume(&dev->dev); pm_runtime_dont_use_autosuspend(&dev->dev); -- cgit v1.1 From 97a90aee5dab33aea0cd3f6802b3661990496262 Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Fri, 28 Oct 2016 10:52:06 +0200 Subject: PCI: Consolidate conditions to allow runtime PM on PCIe ports The conditions to allow runtime PM on PCIe ports are currently spread across two different files: The condition relating to hotplug ports is located in portdrv_pci.c whereas all other conditions are located in pci.c. Consolidate all conditions in a single place in pci.c, thus making it easier to follow the logic and amend conditions down the road. Note that the condition relating to hotplug ports is inserted *before* the condition relating to the "pcie_port_pm=force" command line option, so runtime PM is not afforded to hotplug ports even if this option is given. That's exactly how the code behaved up until now. If this is not desired, the ordering of the conditions can simply be reversed. No functional change intended. Tested-by: Mika Westerberg Signed-off-by: Lukas Wunner Signed-off-by: Bjorn Helgaas Reviewed-by: Rafael J. Wysocki --- drivers/pci/pcie/portdrv_pci.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) (limited to 'drivers/pci/pcie') diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c index 1ae712c..8aa3f14 100644 --- a/drivers/pci/pcie/portdrv_pci.c +++ b/drivers/pci/pcie/portdrv_pci.c @@ -150,15 +150,7 @@ static int pcie_portdrv_probe(struct pci_dev *dev, pci_save_state(dev); - /* - * Prevent runtime PM if the port is advertising support for PCIe - * hotplug. Otherwise the BIOS hotplug SMI code might not be able - * to enumerate devices behind this port properly (the port is - * powered down preventing all config space accesses to the - * subordinate devices). We can't be sure for native PCIe hotplug - * either so prevent that as well. - */ - if (pci_bridge_d3_possible(dev) && !dev->is_hotplug_bridge) { + if (pci_bridge_d3_possible(dev)) { /* * Keep the port resumed 100ms to make sure things like * config space accesses from userspace (lspci) will not @@ -176,7 +168,7 @@ static int pcie_portdrv_probe(struct pci_dev *dev, static void pcie_portdrv_remove(struct pci_dev *dev) { - if (pci_bridge_d3_possible(dev) && !dev->is_hotplug_bridge) { + if (pci_bridge_d3_possible(dev)) { pm_runtime_forbid(&dev->dev); pm_runtime_get_noresume(&dev->dev); pm_runtime_dont_use_autosuspend(&dev->dev); -- cgit v1.1 From e53f9a28bee35932a0ae4d2ec2784f55491ec6d3 Mon Sep 17 00:00:00 2001 From: David Daney Date: Thu, 17 Nov 2016 14:25:01 -0800 Subject: PCI/ASPM: Don't retrain link if ASPM not possible Some (defective) PCIe devices are not able to reliably do link retraining. Check to see if ASPM is possible between link partners before configuring common clocking, and doing the resulting link retraining. If ASPM is not possible, there is no reason to risk losing access to a device due to an unnecessary link retraining. Signed-off-by: David Daney Signed-off-by: Bjorn Helgaas --- drivers/pci/pcie/aspm.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) (limited to 'drivers/pci/pcie') diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index 3b14d9e..17ac1dc 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -351,12 +351,26 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist) return; } + /* Get upstream/downstream components' register state */ + pcie_get_aspm_reg(parent, &upreg); + child = list_entry(linkbus->devices.next, struct pci_dev, bus_list); + pcie_get_aspm_reg(child, &dwreg); + + /* + * If ASPM not supported, don't mess with the clocks and link, + * bail out now. + */ + if (!(upreg.support & dwreg.support)) + return; + /* Configure common clock before checking latencies */ pcie_aspm_configure_common_clock(link); - /* Get upstream/downstream components' register state */ + /* + * Re-read upstream/downstream components' register state + * after clock configuration + */ pcie_get_aspm_reg(parent, &upreg); - child = list_entry(linkbus->devices.next, struct pci_dev, bus_list); pcie_get_aspm_reg(child, &dwreg); /* -- cgit v1.1 From 0a1e1b26f560411cb58066201dddad5c224e8b9e Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Mon, 21 Nov 2016 11:30:45 -0600 Subject: PCI/PME: Drop unused support for PMEs from Root Complex Event Collectors Since we register pcie_pme_driver only for PCI_EXP_TYPE_ROOT_PORT, the PME driver never claims Root Complex Event Collectors. Remove unused code related to Root Complex Event Collectors. Signed-off-by: Bjorn Helgaas Acked-by: Rafael J. Wysocki --- drivers/pci/pcie/pme.c | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) (limited to 'drivers/pci/pcie') diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c index 884bad5..9e8aa9d 100644 --- a/drivers/pci/pcie/pme.c +++ b/drivers/pci/pcie/pme.c @@ -319,23 +319,8 @@ static int pcie_pme_set_native(struct pci_dev *dev, void *ign) static void pcie_pme_mark_devices(struct pci_dev *port) { pcie_pme_set_native(port, NULL); - if (port->subordinate) { + if (port->subordinate) pci_walk_bus(port->subordinate, pcie_pme_set_native, NULL); - } else { - struct pci_bus *bus = port->bus; - struct pci_dev *dev; - - /* Check if this is a root port event collector. */ - if (pci_pcie_type(port) != PCI_EXP_TYPE_RC_EC || !bus) - return; - - down_read(&pci_bus_sem); - list_for_each_entry(dev, &bus->devices, bus_list) - if (pci_is_pcie(dev) - && pci_pcie_type(dev) == PCI_EXP_TYPE_RC_END) - pcie_pme_set_native(dev, NULL); - up_read(&pci_bus_sem); - } } /** -- cgit v1.1 From a902d81ac802ca5eb06a140e024b903825117eaf Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Mon, 21 Nov 2016 15:07:53 -0600 Subject: PCI/PME: Log PME IRQ when claiming Root Port We already log a "Signaling PME" whenever the PME service driver claims a Root Port. In fact, we also log the same message for every device in the hierarchy below the Root Port. Log the "Signaling PME" once (only for the Root Port, since we can trivially find out which devices are below the Root Port), and include the IRQ number in the message to help connect the dots with /proc/interrupts. Signed-off-by: Bjorn Helgaas Acked-by: Rafael J. Wysocki --- drivers/pci/pcie/pme.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers/pci/pcie') diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c index 9e8aa9d..7175293 100644 --- a/drivers/pci/pcie/pme.c +++ b/drivers/pci/pcie/pme.c @@ -300,8 +300,6 @@ static irqreturn_t pcie_pme_irq(int irq, void *context) */ static int pcie_pme_set_native(struct pci_dev *dev, void *ign) { - dev_info(&dev->dev, "Signaling PME through PCIe PME interrupt\n"); - device_set_run_wake(&dev->dev, true); dev->pme_interrupt = true; return 0; @@ -349,12 +347,14 @@ static int pcie_pme_probe(struct pcie_device *srv) ret = request_irq(srv->irq, pcie_pme_irq, IRQF_SHARED, "PCIe PME", srv); if (ret) { kfree(data); - } else { - pcie_pme_mark_devices(port); - pcie_pme_interrupt_enable(port, true); + return ret; } - return ret; + dev_info(&port->dev, "Signaling PME with IRQ %d\n", srv->irq); + + pcie_pme_mark_devices(port); + pcie_pme_interrupt_enable(port, true); + return 0; } static bool pcie_pme_check_wakeup(struct pci_bus *bus) -- cgit v1.1 From 2298a7aaa837f460ac28458851d388b4abb38faa Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Mon, 21 Nov 2016 15:19:29 -0600 Subject: PCI/AER: Remove unused version macros Remove the unused DRIVER_VERSION, DRIVER_AUTHOR, and DRIVER_DESC macros. The author information is already included in a comment above. Signed-off-by: Bjorn Helgaas --- drivers/pci/pcie/aer/aerdrv.c | 7 ------- 1 file changed, 7 deletions(-) (limited to 'drivers/pci/pcie') diff --git a/drivers/pci/pcie/aer/aerdrv.c b/drivers/pci/pcie/aer/aerdrv.c index 139150b..1c189e6 100644 --- a/drivers/pci/pcie/aer/aerdrv.c +++ b/drivers/pci/pcie/aer/aerdrv.c @@ -30,13 +30,6 @@ #include "aerdrv.h" #include "../../pci.h" -/* - * Version Information - */ -#define DRIVER_VERSION "v1.0" -#define DRIVER_AUTHOR "tom.l.nguyen@intel.com" -#define DRIVER_DESC "Root Port Advanced Error Reporting Driver" - static int aer_probe(struct pcie_device *dev); static void aer_remove(struct pcie_device *dev); static pci_ers_result_t aer_error_detected(struct pci_dev *dev, -- cgit v1.1 From 576700b67a39ea422d28af085458748b50d591dc Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Mon, 21 Nov 2016 15:24:25 -0600 Subject: PCI/AER: Log errors with PCI device, not PCIe service device All other AER-related log messages use the PCI device, e.g., "pci 0000:00:1c.0", not the PCIe service device, e.g., "aer 0000:00:1c.0:pcie02". Change the probe error messages to match the rest and include a little context. Signed-off-by: Bjorn Helgaas --- drivers/pci/pcie/aer/aerdrv.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers/pci/pcie') diff --git a/drivers/pci/pcie/aer/aerdrv.c b/drivers/pci/pcie/aer/aerdrv.c index 1c189e6..60e63d6 100644 --- a/drivers/pci/pcie/aer/aerdrv.c +++ b/drivers/pci/pcie/aer/aerdrv.c @@ -290,12 +290,12 @@ static int aer_probe(struct pcie_device *dev) { int status; struct aer_rpc *rpc; - struct device *device = &dev->device; + struct device *device = &dev->port->dev; /* Alloc rpc data structure */ rpc = aer_alloc_rpc(dev); if (!rpc) { - dev_printk(KERN_DEBUG, device, "alloc rpc failed\n"); + dev_printk(KERN_DEBUG, device, "alloc AER rpc failed\n"); aer_remove(dev); return -ENOMEM; } @@ -303,7 +303,8 @@ static int aer_probe(struct pcie_device *dev) /* Request IRQ ISR */ status = request_irq(dev->irq, aer_irq, IRQF_SHARED, "aerdrv", dev); if (status) { - dev_printk(KERN_DEBUG, device, "request IRQ failed\n"); + dev_printk(KERN_DEBUG, device, "request AER IRQ %d failed\n", + dev->irq); aer_remove(dev); return status; } -- cgit v1.1 From 68a55ae5c01356c4c45a3a26d8abd8186638b2a7 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Mon, 21 Nov 2016 15:34:02 -0600 Subject: PCI/AER: Log AER IRQ when claiming Root Port Add a log message when we enable AER on a Root Port and the hierarchy below it. Signed-off-by: Bjorn Helgaas --- drivers/pci/pcie/aer/aerdrv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/pci/pcie') diff --git a/drivers/pci/pcie/aer/aerdrv.c b/drivers/pci/pcie/aer/aerdrv.c index 60e63d6..dea186a 100644 --- a/drivers/pci/pcie/aer/aerdrv.c +++ b/drivers/pci/pcie/aer/aerdrv.c @@ -312,8 +312,8 @@ static int aer_probe(struct pcie_device *dev) rpc->isr = 1; aer_enable_rootport(rpc); - - return status; + dev_info(device, "AER enabled with IRQ %d\n", dev->irq); + return 0; } /** -- cgit v1.1 From 98892fae40ed03ddf6f3bff847993e9d9dd8946a Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Tue, 15 Nov 2016 07:54:19 -0600 Subject: PCI: Remove service driver load/unload messages Remove the "service driver %s loaded" and unloaded messages. All service drivers already log something in their probe functions, where they can log more useful details. Signed-off-by: Bjorn Helgaas --- drivers/pci/pcie/portdrv_core.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/pci/pcie') diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index e9270b4..9698289 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -499,7 +499,6 @@ static int pcie_port_probe_service(struct device *dev) if (status) return status; - dev_printk(KERN_DEBUG, dev, "service driver %s loaded\n", driver->name); get_device(dev); return 0; } @@ -524,8 +523,6 @@ static int pcie_port_remove_service(struct device *dev) pciedev = to_pcie_device(dev); driver = to_service_driver(dev->driver); if (driver && driver->remove) { - dev_printk(KERN_DEBUG, dev, "unloading service driver %s\n", - driver->name); driver->remove(pciedev); put_device(dev); } -- cgit v1.1