[PATCH] PCIE: create sysfs directory on first use

[PATCH] PCIE: create sysfs directory on first use

[Randy.Dunlap]

From: Randy Dunlap <rdunlap@xenotime.net>

Dave Jones had a question about why /sys/bus/pci_express/devices
shows up even when there are no PCI-Express devices in a system
(if the config option is enabled).
Greg KH said that it could be added on its first use.

This patch creates that directory only when a device is discovered
and added to the directory.  On my ancient P-III test system,
the directory is never added, but on my Dell D610 notebook and
other dual-core system, it is added during boot discovery.

Does the "drivers" directory need some special handling also?

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
---
 drivers/pci/pcie/portdrv_core.c |   11 ++++++++++-
 drivers/pci/pcie/portdrv_pci.c  |    1 -
 2 files changed, 10 insertions(+), 2 deletions(-)

--- linux-2618-rc1.orig/drivers/pci/pcie/portdrv_core.c
+++ linux-2618-rc1/drivers/pci/pcie/portdrv_core.c
@@ -274,6 +274,8 @@ int pcie_port_device_probe(struct pci_de
 	return -ENODEV;
 }
 
+static int pcie_dev_registered;	/* register on first use */
+
 int pcie_port_device_register(struct pci_dev *dev)
 {
 	struct pcie_port_device_ext *p_ext;
@@ -303,6 +305,9 @@ int pcie_port_device_register(struct pci
 		struct pcie_device *child;
 
 		if (capabilities & (1 << i)) {
+			if (!pcie_dev_registered)
+				pcie_port_bus_register();
+
 			child = alloc_pcie_device(
 				dev, 		/* parent */
 				type,		/* port type */
@@ -405,11 +410,15 @@ void pcie_port_device_remove(struct pci_
 void pcie_port_bus_register(void)
 {
 	bus_register(&pcie_port_bus_type);
+	pcie_dev_registered = 1;
 }
 
 void pcie_port_bus_unregister(void)
 {
-	bus_unregister(&pcie_port_bus_type);
+	if (pcie_dev_registered) {
+		bus_unregister(&pcie_port_bus_type);
+		pcie_dev_registered = 0;
+	}
 }
 
 int pcie_port_service_register(struct pcie_port_service_driver *new)
--- linux-2618-rc1.orig/drivers/pci/pcie/portdrv_pci.c
+++ linux-2618-rc1/drivers/pci/pcie/portdrv_pci.c
@@ -129,7 +129,6 @@ static int __init pcie_portdrv_init(void
 {
 	int retval = 0;
 
-	pcie_port_bus_register();
 	retval = pci_register_driver(&pcie_portdrv);
 	if (retval)
 		pcie_port_bus_unregister();


---

Re: [PATCH] PCIE: create sysfs directory on first use

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 668 bytes --]

On Fri, 07 Jul 2006 16:52:38 PDT, "Randy.Dunlap" said:

> @@ -405,11 +410,15 @@ void pcie_port_device_remove(struct pci_
>  void pcie_port_bus_register(void)
>  {
>  	bus_register(&pcie_port_bus_type);
> +	pcie_dev_registered = 1;

Shouldn't this be 'pcie_dev_registered++;'
>  }
>  
>  void pcie_port_bus_unregister(void)
>  {
> -	bus_unregister(&pcie_port_bus_type);
> +	if (pcie_dev_registered) {
> +		bus_unregister(&pcie_port_bus_type);
> +		pcie_dev_registered = 0;

and 'pciedev_registered--;'

> +	}
>  }

to keep it from blowing up if 2 bus get registered, then one de-registered,
and then re-registered again? I could see this happening in a hotplug design?

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [PATCH] PCIE: create sysfs directory on first use

[Andrew Morton]

"Randy.Dunlap" <rdunlap@xenotime.net> wrote:
>
> +			if (!pcie_dev_registered)
> +				pcie_port_bus_register();
> +

Wonderful.  You're forced to drop all error checking on the floor because
pcie_port_bus_register() assumes that nobody could possibly ever be
interested in actually checking for errors.

What happens if the bus_register() fails and the driver cheerily blunders
along assuming that pcie_port_bus_type is registered?  Incomprehensible lkml
oops reports, I'm suspecting..

Let's start stamping this out.  Could I please ask that you first prepare a
patch which fixes pcie_port_bus_register() (and mark it __must_check) and
then let's actually, like, check for errors?

Re: [PATCH] PCIE: create sysfs directory on first use

[Randy.Dunlap]

On Fri, 7 Jul 2006 17:10:15 -0700 Andrew Morton wrote:

> "Randy.Dunlap" <rdunlap@xenotime.net> wrote:
> >
> > +			if (!pcie_dev_registered)
> > +				pcie_port_bus_register();
> > +
> 
> Wonderful.  You're forced to drop all error checking on the floor because
> pcie_port_bus_register() assumes that nobody could possibly ever be
> interested in actually checking for errors.
> 
> What happens if the bus_register() fails and the driver cheerily blunders
> along assuming that pcie_port_bus_type is registered?  Incomprehensible lkml
> oops reports, I'm suspecting..
> 
> Let's start stamping this out.  Could I please ask that you first prepare a
> patch which fixes pcie_port_bus_register() (and mark it __must_check) and
> then let's actually, like, check for errors?

Sure, will do.

And will make changes that Valdis mentioned.

---
~Randy

[PATCH] PCIE: check and return bus_register errors

[Randy.Dunlap]

On Fri, 7 Jul 2006 17:10:15 -0700 Andrew Morton wrote:

> "Randy.Dunlap" <rdunlap@xenotime.net> wrote:
> >
> > +			if (!pcie_dev_registered)
> > +				pcie_port_bus_register();
> > +
> 
> Wonderful.  You're forced to drop all error checking on the floor because
> pcie_port_bus_register() assumes that nobody could possibly ever be
> interested in actually checking for errors.
> 
> What happens if the bus_register() fails and the driver cheerily blunders
> along assuming that pcie_port_bus_type is registered?  Incomprehensible lkml
> oops reports, I'm suspecting..
> 
> Let's start stamping this out.  Could I please ask that you first prepare a
> patch which fixes pcie_port_bus_register() (and mark it __must_check) and
> then let's actually, like, check for errors?



From: Randy Dunlap <rdunlap@xenotime.net>

Have pcie_port_bus_register() notice and return errors.
Mark it __must_check so that its caller(s) must check its return value.

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
---
 drivers/pci/pcie/portdrv.h      |    2 +-
 drivers/pci/pcie/portdrv_core.c |    5 +++--
 drivers/pci/pcie/portdrv_pci.c  |    9 +++++++--
 3 files changed, 11 insertions(+), 5 deletions(-)

--- linux-2618-rc1.orig/drivers/pci/pcie/portdrv_core.c
+++ linux-2618-rc1/drivers/pci/pcie/portdrv_core.c
@@ -6,6 +6,7 @@
  * Copyright (C) Tom Long Nguyen (tom.l.nguyen@intel.com)
  */
 
+#include <linux/compiler.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/kernel.h>
@@ -402,9 +403,9 @@ void pcie_port_device_remove(struct pci_
 		pci_disable_msi(dev);
 }
 
-void pcie_port_bus_register(void)
+int __must_check pcie_port_bus_register(void)
 {
-	bus_register(&pcie_port_bus_type);
+	return bus_register(&pcie_port_bus_type);
 }
 
 void pcie_port_bus_unregister(void)
--- linux-2618-rc1.orig/drivers/pci/pcie/portdrv_pci.c
+++ linux-2618-rc1/drivers/pci/pcie/portdrv_pci.c
@@ -127,12 +127,17 @@ static struct pci_driver pcie_portdrv = 
 
 static int __init pcie_portdrv_init(void)
 {
-	int retval = 0;
+	int retval;
 
-	pcie_port_bus_register();
+	retval = pcie_port_bus_register();
+	if (retval) {
+		printk(KERN_WARNING "PCIE: bus_register error: %d\n", retval);
+		goto out;
+	}
 	retval = pci_register_driver(&pcie_portdrv);
 	if (retval)
 		pcie_port_bus_unregister();
+ out:
 	return retval;
 }
 
--- linux-2618-rc1.orig/drivers/pci/pcie/portdrv.h
+++ linux-2618-rc1/drivers/pci/pcie/portdrv.h
@@ -39,7 +39,7 @@ extern int pcie_port_device_suspend(stru
 extern int pcie_port_device_resume(struct pci_dev *dev);
 #endif
 extern void pcie_port_device_remove(struct pci_dev *dev);
-extern void pcie_port_bus_register(void);
+extern int pcie_port_bus_register(void);
 extern void pcie_port_bus_unregister(void);
 
 #endif /* _PORTDRV_H_ */



---