[PATCH v2 0/9] PCI: Tidy up messages

[PATCH v2 0/9] PCI: Tidy up messages

[Bjorn Helgaas]

Remove a few messages that don't contain any useful information, e.g.,

  pcie_pme 0000:00:02.0:pcie01: service driver pcie_pme loaded
  pci_hotplug: PCI Hot Plug PCI Core version: 0.5
  pciehp 0000:80:02.0:pcie04: service driver pciehp loaded
  pciehp: PCI Express Hot Plug Controller Driver version: 0.4

NOTE: I added a patch to drop Root Complex Event Collector PME support.  I
suspect that we really want to keep that, but it looks like it's currently
broken.  I would welcome opinions and some help to test it if we fix the
current breakage.

Changes from v1->v2:
  - Drop Root Complex Event Collector PME support
  - Add PME IRQ to existing log message
  - Log PME message once per hierarchy, not once per device
  - Log AER probe errors with plain PCI device, not PCIe service device
  - Add new AER claim message, including IRQ
  - Remove unused AER version macros

---

Bjorn Helgaas (9):
      PCI/PME: Drop unused support for PMEs from Root Complex Event Collectors
      PCI/PME: Log PME IRQ when claiming Root Port
      PCI/AER: Remove unused version macros
      PCI/AER: Log errors with PCI device, not PCIe service device
      PCI/AER: Log AER IRQ when claiming Root Port
      PCI: Remove service driver load/unload messages
      PCI: hotplug: Remove hotplug core message
      PCI: pciehp: Remove loading message
      PCI: Expand "VPD access disabled" quirk message


 drivers/pci/hotplug/pci_hotplug_core.c |   10 +++-------
 drivers/pci/hotplug/pciehp_core.c      |    9 ++++-----
 drivers/pci/pcie/aer/aerdrv.c          |   18 ++++++------------
 drivers/pci/pcie/pme.c                 |   29 +++++++----------------------
 drivers/pci/pcie/portdrv_core.c        |    3 ---
 drivers/pci/quirks.c                   |    2 +-
 6 files changed, 21 insertions(+), 50 deletions(-)

[PATCH v2 1/9] PCI/PME: Drop unused support for PMEs from Root Complex Event Collectors

[Bjorn Helgaas]

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 <bhelgaas@google.com>
---
 drivers/pci/pcie/pme.c |   17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

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);
-	}
 }
 
 /**

[PATCH v2 2/9] PCI/PME: Log PME IRQ when claiming Root Port

[Bjorn Helgaas]

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 <bhelgaas@google.com>
---
 drivers/pci/pcie/pme.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

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)

[PATCH v2 3/9] PCI/AER: Remove unused version macros

[Bjorn Helgaas]

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 <bhelgaas@google.com>
---
 drivers/pci/pcie/aer/aerdrv.c |    7 -------
 1 file changed, 7 deletions(-)

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,

[PATCH v2 4/9] PCI/AER: Log errors with PCI device, not PCIe service device

[Bjorn Helgaas]

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 <bhelgaas@google.com>
---
 drivers/pci/pcie/aer/aerdrv.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

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;
 	}

[PATCH v2 5/9] PCI/AER: Log AER IRQ when claiming Root Port

[Bjorn Helgaas]

Add a log message when we enable AER on a Root Port and the hierarchy below
it.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pcie/aer/aerdrv.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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;
 }
 
 /**

[PATCH v2 6/9] PCI: Remove service driver load/unload messages

[Bjorn Helgaas]

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 <bhelgaas@google.com>
---
 drivers/pci/pcie/portdrv_core.c |    3 ---
 1 file changed, 3 deletions(-)

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);
 	}

[PATCH v2 7/9] PCI: hotplug: Remove hotplug core message

[Bjorn Helgaas]

Remove the "PCI Hot Plug PCI Core" version message.  I don't think it
contains any useful information.  Remove unused #defines and move the
author information to a comment.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/hotplug/pci_hotplug_core.c |   10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index fea0b8b..56013d0 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -23,6 +23,9 @@
  *
  * Send feedback to <kristen.c.accardi@intel.com>
  *
+ * Authors:
+ *   Greg Kroah-Hartman <greg@kroah.com>
+ *   Scott Murray <scottm@somanetworks.com>
  */
 
 #include <linux/module.h>	/* try_module_get & module_put */
@@ -50,15 +53,9 @@
 #define info(format, arg...) printk(KERN_INFO "%s: " format, MY_NAME, ## arg)
 #define warn(format, arg...) printk(KERN_WARNING "%s: " format, MY_NAME, ## arg)
 
-
 /* local variables */
 static bool debug;
 
-#define DRIVER_VERSION	"0.5"
-#define DRIVER_AUTHOR	"Greg Kroah-Hartman <greg@kroah.com>, Scott Murray <scottm@somanetworks.com>"
-#define DRIVER_DESC	"PCI Hot Plug PCI Core"
-
-
 static LIST_HEAD(pci_hotplug_slot_list);
 static DEFINE_MUTEX(pci_hp_mutex);
 
@@ -534,7 +531,6 @@ static int __init pci_hotplug_init(void)
 		return result;
 	}
 
-	info(DRIVER_DESC " version: " DRIVER_VERSION "\n");
 	return result;
 }
 device_initcall(pci_hotplug_init);

[PATCH v2 8/9] PCI: pciehp: Remove loading message

[Bjorn Helgaas]

Remove the "PCI Express Hot Plug Controller Driver" version message.  I
don't think it contains any useful information.  Remove unused #defines
and move the author information to a comment.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/hotplug/pciehp_core.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
index 7d32fa33..35d8484 100644
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -25,6 +25,10 @@
  *
  * Send feedback to <greg@kroah.com>, <kristen.c.accardi@intel.com>
  *
+ * Authors:
+ *   Dan Zink <dan.zink@compaq.com>
+ *   Greg Kroah-Hartman <greg@kroah.com>
+ *   Dely Sy <dely.l.sy@intel.com>"
  */
 
 #include <linux/moduleparam.h>
@@ -42,10 +46,6 @@ bool pciehp_poll_mode;
 int pciehp_poll_time;
 static bool pciehp_force;
 
-#define DRIVER_VERSION	"0.4"
-#define DRIVER_AUTHOR	"Dan Zink <dan.zink@compaq.com>, Greg Kroah-Hartman <greg@kroah.com>, Dely Sy <dely.l.sy@intel.com>"
-#define DRIVER_DESC	"PCI Express Hot Plug Controller Driver"
-
 /*
  * not really modular, but the easiest way to keep compat with existing
  * bootargs behaviour is to continue using module_param here.
@@ -333,7 +333,6 @@ static int __init pcied_init(void)
 
 	retval = pcie_port_service_register(&hpdriver_portdrv);
 	dbg("pcie_port_service_register = %d\n", retval);
-	info(DRIVER_DESC " version: " DRIVER_VERSION "\n");
 	if (retval)
 		dbg("Failure to register service\n");
 

[PATCH v2 9/9] PCI: Expand "VPD access disabled" quirk message

[Bjorn Helgaas]

It's not very enlightening to see

  pci 0000:07:00.0: [Firmware Bug]: VPD access disabled

in the dmesg log because there's no clue about what the firmware bug is.
Expand the message to explain why we're disabling VPD.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/quirks.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index c232729..7329796 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2156,7 +2156,7 @@ static void quirk_blacklist_vpd(struct pci_dev *dev)
 {
 	if (dev->vpd) {
 		dev->vpd->len = 0;
-		dev_warn(&dev->dev, FW_BUG "VPD access disabled\n");
+		dev_warn(&dev->dev, FW_BUG "disabling VPD access (can't determine size of non-standard VPD format)\n");
 	}
 }
 

Re: [PATCH v2 1/9] PCI/PME: Drop unused support for PMEs from Root Complex Event Collectors

[Rafael J. Wysocki]

On Monday, November 21, 2016 03:45:19 PM Bjorn Helgaas wrote:
> 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 <bhelgaas@google.com>
> ---
>  drivers/pci/pcie/pme.c |   17 +----------------
>  1 file changed, 1 insertion(+), 16 deletions(-)
> 
> 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);
> -	}
>  }
>  
>  /**

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Re: [PATCH v2 2/9] PCI/PME: Log PME IRQ when claiming Root Port

[Rafael J. Wysocki]

On Monday, November 21, 2016 03:45:26 PM Bjorn Helgaas wrote:
> 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 <bhelgaas@google.com>
> ---
>  drivers/pci/pcie/pme.c |   12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> 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)
> 
> --

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Re: [PATCH v2 1/9] PCI/PME: Drop unused support for PMEs from Root Complex Event Collectors

[Bjorn Helgaas]

On Mon, Nov 21, 2016 at 11:28:56PM +0100, Rafael J. Wysocki wrote:
> On Monday, November 21, 2016 03:45:19 PM Bjorn Helgaas wrote:
> > 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 <bhelgaas@google.com>
> > ---
> >  drivers/pci/pcie/pme.c |   17 +----------------
> >  1 file changed, 1 insertion(+), 16 deletions(-)
> > 
> > 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);
> > -	}
> >  }
> >  
> >  /**
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

I'm shocked :)  You mean we really don't care about PME from RC event
collectors?  Without the RCEC support, we won't see any PMEs from RC
integrated devices.  I mean, I don't think we see those PMEs today, but I
thought maybe we would *want* to.  No?

Bjorn

Re: [PATCH v2 1/9] PCI/PME: Drop unused support for PMEs from Root Complex Event Collectors

[Rafael J. Wysocki]

On Monday, November 21, 2016 04:42:10 PM Bjorn Helgaas wrote:
> On Mon, Nov 21, 2016 at 11:28:56PM +0100, Rafael J. Wysocki wrote:
> > On Monday, November 21, 2016 03:45:19 PM Bjorn Helgaas wrote:
> > > 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 <bhelgaas@google.com>
> > > ---
> > >  drivers/pci/pcie/pme.c |   17 +----------------
> > >  1 file changed, 1 insertion(+), 16 deletions(-)
> > > 
> > > 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);
> > > -	}
> > >  }
> > >  
> > >  /**
> > 
> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> I'm shocked :)

Well, the code is not used as a matter of fact and it can be added back
when needed, can't it?

> You mean we really don't care about PME from RC event
> collectors?  Without the RCEC support, we won't see any PMEs from RC
> integrated devices.  I mean, I don't think we see those PMEs today, but I
> thought maybe we would *want* to.  No?

In the majority of cases those are signaled via ACPI GPEs anyway.  I'm not
sure why, but event collectors have not been extremely popular for the last
several years.  I haven't seen a single one with native PME handling so far
and I kind of don't expect to see any.

Thanks,
Rafael

Re: [PATCH v2 1/9] PCI/PME: Drop unused support for PMEs from Root Complex Event Collectors

[Bjorn Helgaas]

On Mon, Nov 21, 2016 at 11:58:09PM +0100, Rafael J. Wysocki wrote:
> On Monday, November 21, 2016 04:42:10 PM Bjorn Helgaas wrote:
> > On Mon, Nov 21, 2016 at 11:28:56PM +0100, Rafael J. Wysocki wrote:
> > > On Monday, November 21, 2016 03:45:19 PM Bjorn Helgaas wrote:
> > > > 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 <bhelgaas@google.com>
> > > > ---
> > > >  drivers/pci/pcie/pme.c |   17 +----------------
> > > >  1 file changed, 1 insertion(+), 16 deletions(-)
> > > > 
> > > > 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);
> > > > -	}
> > > >  }
> > > >  
> > > >  /**
> > > 
> > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > I'm shocked :)
> 
> Well, the code is not used as a matter of fact and it can be added back
> when needed, can't it?

Indeed.

> > You mean we really don't care about PME from RC event
> > collectors?  Without the RCEC support, we won't see any PMEs from RC
> > integrated devices.  I mean, I don't think we see those PMEs today, but I
> > thought maybe we would *want* to.  No?
> 
> In the majority of cases those are signaled via ACPI GPEs anyway.  I'm not
> sure why, but event collectors have not been extremely popular for the last
> several years.  I haven't seen a single one with native PME handling so far
> and I kind of don't expect to see any.

Ok, great.  We'll just remove it until we need it then.

Thanks!

Bjorn

Re: [PATCH v2 0/9] PCI: Tidy up messages

[Bjorn Helgaas]

On Mon, Nov 21, 2016 at 03:45:11PM -0600, Bjorn Helgaas wrote:
> Remove a few messages that don't contain any useful information, e.g.,
> 
>   pcie_pme 0000:00:02.0:pcie01: service driver pcie_pme loaded
>   pci_hotplug: PCI Hot Plug PCI Core version: 0.5
>   pciehp 0000:80:02.0:pcie04: service driver pciehp loaded
>   pciehp: PCI Express Hot Plug Controller Driver version: 0.4
> 
> NOTE: I added a patch to drop Root Complex Event Collector PME support.  I
> suspect that we really want to keep that, but it looks like it's currently
> broken.  I would welcome opinions and some help to test it if we fix the
> current breakage.
> 
> Changes from v1->v2:
>   - Drop Root Complex Event Collector PME support
>   - Add PME IRQ to existing log message
>   - Log PME message once per hierarchy, not once per device
>   - Log AER probe errors with plain PCI device, not PCIe service device
>   - Add new AER claim message, including IRQ
>   - Remove unused AER version macros
> 
> ---
> 
> Bjorn Helgaas (9):
>       PCI/PME: Drop unused support for PMEs from Root Complex Event Collectors
>       PCI/PME: Log PME IRQ when claiming Root Port
>       PCI/AER: Remove unused version macros
>       PCI/AER: Log errors with PCI device, not PCIe service device
>       PCI/AER: Log AER IRQ when claiming Root Port
>       PCI: Remove service driver load/unload messages
>       PCI: hotplug: Remove hotplug core message
>       PCI: pciehp: Remove loading message
>       PCI: Expand "VPD access disabled" quirk message
> 
> 
>  drivers/pci/hotplug/pci_hotplug_core.c |   10 +++-------
>  drivers/pci/hotplug/pciehp_core.c      |    9 ++++-----
>  drivers/pci/pcie/aer/aerdrv.c          |   18 ++++++------------
>  drivers/pci/pcie/pme.c                 |   29 +++++++----------------------
>  drivers/pci/pcie/portdrv_core.c        |    3 ---
>  drivers/pci/quirks.c                   |    2 +-
>  6 files changed, 21 insertions(+), 50 deletions(-)

I applied these (with Rafael's ack on the first two) to pci/misc for v4.10.