[PATCH 0/19]: RPAPHP pci hotplug cleanup patchbomb

[PATCH 0/19]: RPAPHP pci hotplug cleanup patchbomb

[Linas Vepstas]

Hi Greg, 

Please queue these cleanp patches for 2.6.22; cc'ing akpm.

This is a collection of very small patches that clean up various bits and 
pieces of the RPAPHP hotplug code. They eliminate about 100 lines of code, 
almost without changing any function; there are a few minor bugfixes to 
various error paths, and one memleak fix. Some documentation is added.
The result is, I beleive, slightly more readable, easier to understand
code. In particular, the enable/disable add/remove code paths are now
more obviously symmetrical in thier function.

--linas

[PATCH 1/19] PCI: rpaphp: Cleanup flow of control for rpaphp_add_slot

[Linas Vepstas]

Cleanup the flow of control for rpaphp_add_slot(), so as to
make it easier to read. The ext patch will fix a bug in this 
same code. 

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpaphp_core.c |   40 +++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_core.c	2007-03-22 16:35:39.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c	2007-03-22 16:46:59.000000000 -0500
@@ -299,32 +299,32 @@ int rpaphp_add_slot(struct device_node *
 	const int *indexes, *names, *types, *power_domains;
 	char *name, *type;
 
+	if (!dn || strcmp(dn->name, "pci"))
+		return 0;
+
+	if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
+		return 0;
+	
 	dbg("Entry %s: dn->full_name=%s\n", __FUNCTION__, dn->full_name);
 
 	/* register PCI devices */
-	if (dn->name != 0 && strcmp(dn->name, "pci") == 0) {
-		if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
-			goto exit;
-
-		name = (char *) &names[1];
-		type = (char *) &types[1];
-		for (i = 0; i < indexes[0]; i++,
-	     		name += (strlen(name) + 1), type += (strlen(type) + 1)) 		{
-
-			if (!(slot = alloc_slot_struct(dn, indexes[i + 1], name,
-				       power_domains[i + 1]))) {
-				retval = -ENOMEM;
-				goto exit;
-			}
-			slot->type = simple_strtoul(type, NULL, 10);
+	name = (char *) &names[1];
+	type = (char *) &types[1];
+	for (i = 0; i < indexes[0]; i++) {
+
+		slot = alloc_slot_struct(dn, indexes[i + 1], name, power_domains[i + 1]);
+		if (!slot)
+			return -ENOMEM;
+
+		slot->type = simple_strtoul(type, NULL, 10);
 				
-			dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
-					indexes[i + 1], name, type);
+		dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
+				indexes[i + 1], name, type);
 
-			retval = rpaphp_register_pci_slot(slot);
-		}
+		retval = rpaphp_register_pci_slot(slot);
+		name += strlen(name) + 1;
+		type += strlen(type) + 1;
 	}
-exit:
 	dbg("%s - Exit: num_slots=%d rc[%d]\n",
 	    __FUNCTION__, num_slots, retval);
 	return retval;

Re: [PATCH 1/19] PCI: rpaphp: Cleanup flow of control for rpaphp_add_slot

[Nathan Lynch]

Linas Vepstas wrote:
> 
> Cleanup the flow of control for rpaphp_add_slot(), so as to
> make it easier to read. The ext patch will fix a bug in this 
> same code. 

This:

> +	if (!dn || strcmp(dn->name, "pci"))
> +		return 0;

is not equivalent to the code it's replacing:

> -	if (dn->name != 0 && strcmp(dn->name, "pci") == 0) {

With your version we'll oops if dn->name is NULL.

Re: [PATCH 1/19] PCI: rpaphp: Cleanup flow of control for rpaphp_add_slot

[Linas Vepstas]

On Tue, Apr 03, 2007 at 09:28:28AM -0500, Nathan Lynch wrote:
> Linas Vepstas wrote:
> > 
> > Cleanup the flow of control for rpaphp_add_slot(), so as to
> > make it easier to read. The ext patch will fix a bug in this 
> > same code. 
> 
> This:
> 
> > +	if (!dn || strcmp(dn->name, "pci"))
> > +		return 0;
> 
> is not equivalent to the code it's replacing:
> 
> > -	if (dn->name != 0 && strcmp(dn->name, "pci") == 0) {
> 
> With your version we'll oops if dn->name is NULL.

oops, thanks, I'll fix that.

--linas

[PATCH 2/19] PCI: rpaphp: Remove global num_slots variable

[Linas Vepstas]

Cleanup cruft: remove the global "num_slots" variable; 
although scattered across multiple files, it is used only 
once, in a debug statement.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpaphp.h      |    1 -
 drivers/pci/hotplug/rpaphp_core.c |    4 +---
 drivers/pci/hotplug/rpaphp_slot.c |    3 ---
 3 files changed, 1 insertion(+), 7 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_core.c	2007-03-28 11:10:47.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c	2007-03-28 17:38:24.000000000 -0500
@@ -41,7 +41,6 @@
 int debug;
 static struct semaphore rpaphp_sem;
 LIST_HEAD(rpaphp_slot_head);
-int num_slots;
 
 #define DRIVER_VERSION	"0.1"
 #define DRIVER_AUTHOR	"Linda Xie <lxie@us.ibm.com>"
@@ -325,8 +324,7 @@ int rpaphp_add_slot(struct device_node *
 		name += strlen(name) + 1;
 		type += strlen(type) + 1;
 	}
-	dbg("%s - Exit: num_slots=%d rc[%d]\n",
-	    __FUNCTION__, num_slots, retval);
+	dbg("%s - Exit: rc[%d]\n", __FUNCTION__, retval);
 	return retval;
 }
 
Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_slot.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_slot.c	2007-03-28 11:10:08.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_slot.c	2007-03-28 17:38:24.000000000 -0500
@@ -140,8 +140,6 @@ int rpaphp_deregister_slot(struct slot *
 	retval = pci_hp_deregister(php_slot);
 	if (retval)
 		err("Problem unregistering a slot %s\n", slot->name);
-	else
-		num_slots--;
 
 	dbg("%s - Exit: rc[%d]\n", __FUNCTION__, retval);
 	return retval;
@@ -181,7 +179,6 @@ int rpaphp_register_slot(struct slot *sl
 	list_add(&slot->rpaphp_slot_list, &rpaphp_slot_head);
 	info("Slot [%s](PCI location=%s) registered\n", slot->name,
 			slot->location);
-	num_slots++;
 	return 0;
 
 sysfs_fail:
Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp.h
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp.h	2007-03-28 17:38:24.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp.h	2007-03-28 17:38:49.000000000 -0500
@@ -83,7 +83,6 @@ struct slot {
 
 extern struct hotplug_slot_ops rpaphp_hotplug_slot_ops;
 extern struct list_head rpaphp_slot_head;
-extern int num_slots;
 
 /* function prototypes */
 

[PATCH 3/19] PCI: rpaphp: match up alloc and free in same routine

[Linas Vepstas]

The routine that called an alloc should be the same routine that 
calles the mathcing free, if anything in the middle failed.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpaphp_core.c |    5 +++++
 drivers/pci/hotplug/rpaphp_pci.c  |    1 -
 drivers/pci/hotplug/rpaphp_slot.c |    1 -
 3 files changed, 5 insertions(+), 2 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_core.c	2007-03-28 17:42:01.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c	2007-03-28 17:42:04.000000000 -0500
@@ -321,10 +321,15 @@ int rpaphp_add_slot(struct device_node *
 				indexes[i + 1], name, type);
 
 		retval = rpaphp_register_pci_slot(slot);
+		if (retval)
+			dealloc_slot_struct(slot);
+
 		name += strlen(name) + 1;
 		type += strlen(type) + 1;
 	}
 	dbg("%s - Exit: rc[%d]\n", __FUNCTION__, retval);
+
+	/* XXX FIXME: reports a failure only if last entry in loop failed */
 	return retval;
 }
 
Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_slot.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_slot.c	2007-03-28 17:42:01.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_slot.c	2007-03-28 17:42:04.000000000 -0500
@@ -184,7 +184,6 @@ int rpaphp_register_slot(struct slot *sl
 sysfs_fail:
 	pci_hp_deregister(php_slot);
 register_fail:
-	rpaphp_release_slot(php_slot);
 	return retval;
 }
 
Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_pci.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_pci.c	2007-03-28 17:42:01.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_pci.c	2007-03-28 17:42:04.000000000 -0500
@@ -195,7 +195,6 @@ static int setup_pci_slot(struct slot *s
 	}
 	return 0;
 exit_rc:
-	dealloc_slot_struct(slot);
 	return -EINVAL;
 }
 

[PATCH 4/19] PCI: rpaphp: Fix a memleak; slot->location string was never freed

[Linas Vepstas]

Fix a memleak; the slot->location string was never freed.
Fix some whitespace and overlong-line probelms while we're here.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpaphp_slot.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_slot.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_slot.c	2007-03-28 18:02:42.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_slot.c	2007-03-28 18:07:28.000000000 -0500
@@ -56,7 +56,6 @@ static struct hotplug_slot_attribute php
 static void rpaphp_release_slot(struct hotplug_slot *hotplug_slot)
 {
 	struct slot *slot = (struct slot *) hotplug_slot->private;
-
 	dealloc_slot_struct(slot);
 }
 
@@ -65,12 +64,12 @@ void dealloc_slot_struct(struct slot *sl
 	kfree(slot->hotplug_slot->info);
 	kfree(slot->hotplug_slot->name);
 	kfree(slot->hotplug_slot);
+	kfree(slot->location);
 	kfree(slot);
-	return;
 }
 
-struct slot *alloc_slot_struct(struct device_node *dn, int drc_index, char *drc_name,
-		  int power_domain)
+struct slot *alloc_slot_struct(struct device_node *dn,
+                       int drc_index, char *drc_name, int power_domain)
 {
 	struct slot *slot;
 	
@@ -115,7 +114,7 @@ error_nomem:
 
 static int is_registered(struct slot *slot)
 {
-	struct slot             *tmp_slot;
+	struct slot *tmp_slot;
 
 	list_for_each_entry(tmp_slot, &rpaphp_slot_head, rpaphp_slot_list) {
 		if (!strcmp(tmp_slot->name, slot->name))

[PATCH 5/19] PCI: rpaphp: Remove un-needed goto

[Linas Vepstas]

Remove un-needed goto.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpaphp_slot.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_slot.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_slot.c	2007-03-28 18:07:28.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_slot.c	2007-03-28 18:10:14.000000000 -0500
@@ -157,14 +157,13 @@ int rpaphp_register_slot(struct slot *sl
 	/* should not try to register the same slot twice */
 	if (is_registered(slot)) {
 		err("rpaphp_register_slot: slot[%s] is already registered\n", slot->name);
-		retval = -EAGAIN;
-		goto register_fail;
+		return -EAGAIN;
 	}	
 
 	retval = pci_hp_register(php_slot);
 	if (retval) {
 		err("pci_hp_register failed with error %d\n", retval);
-		goto register_fail;
+		return retval;
 	}
 
 	/* create "phy_location" file */
@@ -182,7 +181,6 @@ int rpaphp_register_slot(struct slot *sl
 
 sysfs_fail:
 	pci_hp_deregister(php_slot);
-register_fail:
 	return retval;
 }
 

Re: [PATCH 5/19] PCI: rpaphp: Remove un-needed goto

[Christoph Hellwig]

>  	/* should not try to register the same slot twice */
>  	if (is_registered(slot)) {
>  		err("rpaphp_register_slot: slot[%s] is already registered\n", slot->name);
> -		retval = -EAGAIN;
> -		goto register_fail;
> +		return -EAGAIN;
>  	}	
>  
>  	retval = pci_hp_register(php_slot);
>  	if (retval) {
>  		err("pci_hp_register failed with error %d\n", retval);
> -		goto register_fail;
> +		return retval;
>  	}
>  
>  	/* create "phy_location" file */
> @@ -182,7 +181,6 @@ int rpaphp_register_slot(struct slot *sl
>  
>  sysfs_fail:
>  	pci_hp_deregister(php_slot);
> -register_fail:
>  	return retval;
>  }

Using a goto for just returning an error is a common idiom if we
have other failure cases aswell.

Re: [PATCH 5/19] PCI: rpaphp: Remove un-needed goto

[Linas Vepstas]

On Tue, Apr 03, 2007 at 12:49:25PM +0200, Christoph Hellwig wrote:
> >  	/* should not try to register the same slot twice */
> >  	if (is_registered(slot)) {
> >  		err("rpaphp_register_slot: slot[%s] is already registered\n", slot->name);
> > -		retval = -EAGAIN;
> > -		goto register_fail;
> > +		return -EAGAIN;
> >  	}	
> >  
> >  	retval = pci_hp_register(php_slot);
> >  	if (retval) {
> >  		err("pci_hp_register failed with error %d\n", retval);
> > -		goto register_fail;
> > +		return retval;
> >  	}
> >  
> >  	/* create "phy_location" file */
> > @@ -182,7 +181,6 @@ int rpaphp_register_slot(struct slot *sl
> >  
> >  sysfs_fail:
> >  	pci_hp_deregister(php_slot);
> > -register_fail:
> >  	return retval;
> >  }
> 
> Using a goto for just returning an error is a common idiom if we
> have other failure cases aswell.

Yes, it is, and I rather like that way of doing things. But in this case 
it didn't seem warranted; I've been trying to take to heart that less
code == better code, without muntzing (ala bob pease) the code.  

--linas

[PATCH 6/19] PCI: rpaphp: remove a function that does nothing but wrap debug printk's

[Linas Vepstas]

Remove a stove-pipe-- a function that is called from only one place, 
does nothing but wraps another function with debug printk's.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpaphp_pci.c |   37 +++++++++++++------------------------
 1 file changed, 13 insertions(+), 24 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_pci.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_pci.c	2007-03-28 18:02:42.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_pci.c	2007-03-28 18:10:17.000000000 -0500
@@ -116,23 +116,6 @@ static void print_slot_pci_funcs(struct 
 	return;
 }
 
-static int setup_pci_hotplug_slot_info(struct slot *slot)
-{
-	struct hotplug_slot_info *hotplug_slot_info = slot->hotplug_slot->info;
-
-	dbg("%s Initilize the PCI slot's hotplug->info structure ...\n",
-	    __FUNCTION__);
-	rpaphp_get_power_status(slot, &hotplug_slot_info->power_status);
-	rpaphp_get_pci_adapter_status(slot, 1,
-				      &hotplug_slot_info->adapter_status);
-	if (hotplug_slot_info->adapter_status == NOT_VALID) {
-		err("%s: NOT_VALID: skip dn->full_name=%s\n",
-		    __FUNCTION__, slot->dn->full_name);
-		return -EINVAL;
-	}
-	return 0;
-}
-
 static void set_slot_name(struct slot *slot)
 {
 	struct pci_bus *bus = slot->bus;
@@ -200,14 +183,20 @@ exit_rc:
 
 int rpaphp_register_pci_slot(struct slot *slot)
 {
-	int rc = -EINVAL;
+	struct hotplug_slot_info *info = slot->hotplug_slot->info;
+
+	rpaphp_get_power_status(slot, &info->power_status);
+	rpaphp_get_pci_adapter_status(slot, 1, &info->adapter_status);
+
+	if (info->adapter_status == NOT_VALID) {
+		err("%s: NOT_VALID: skip dn->full_name=%s\n",
+		    __FUNCTION__, slot->dn->full_name);
+		return -EINVAL;
+	}
 
-	if (setup_pci_hotplug_slot_info(slot))
-		goto exit_rc;
 	if (setup_pci_slot(slot))
-		goto exit_rc;
-	rc = rpaphp_register_slot(slot);
-exit_rc:
-	return rc;
+		return -EINVAL;
+
+	return rpaphp_register_slot(slot);
 }
 

[PATCH 7/19] PCI: rpaphp: Remve another call that is a wrapper

[Linas Vepstas]

Remove another stovepipe: a call which wraps another call, and
just adds printks.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpaphp.h      |    1 -
 drivers/pci/hotplug/rpaphp_core.c |    6 ++++--
 drivers/pci/hotplug/rpaphp_pci.c  |    7 ++++++-
 drivers/pci/hotplug/rpaphp_slot.c |   18 ------------------
 4 files changed, 10 insertions(+), 22 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_pci.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_pci.c	2007-03-28 18:10:17.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_pci.c	2007-03-28 18:10:19.000000000 -0500
@@ -183,9 +183,14 @@ exit_rc:
 
 int rpaphp_register_pci_slot(struct slot *slot)
 {
+	int rc, level;
 	struct hotplug_slot_info *info = slot->hotplug_slot->info;
 
-	rpaphp_get_power_status(slot, &info->power_status);
+	rc = rtas_get_power_level(slot->power_domain, &level);
+	if (rc)
+		return rc;
+	info->power_status = level;
+
 	rpaphp_get_pci_adapter_status(slot, 1, &info->adapter_status);
 
 	if (info->adapter_status == NOT_VALID) {
Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp.h
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp.h	2007-03-28 18:02:42.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp.h	2007-03-28 18:10:19.000000000 -0500
@@ -103,7 +103,6 @@ extern void dealloc_slot_struct(struct s
 extern struct slot *alloc_slot_struct(struct device_node *dn, int drc_index, char *drc_name, int power_domain);
 extern int rpaphp_register_slot(struct slot *slot);
 extern int rpaphp_deregister_slot(struct slot *slot);
-extern int rpaphp_get_power_status(struct slot *slot, u8 * value);
 extern int rpaphp_set_attention_status(struct slot *slot, u8 status);
 	
 #endif				/* _PPC64PHP_H */
Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_core.c	2007-03-28 18:02:42.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c	2007-03-28 18:10:19.000000000 -0500
@@ -100,11 +100,13 @@ static int set_attention_status(struct h
  */
 static int get_power_status(struct hotplug_slot *hotplug_slot, u8 * value)
 {
-	int retval;
+	int retval, level;
 	struct slot *slot = (struct slot *)hotplug_slot->private;
 
 	down(&rpaphp_sem);
-	retval = rpaphp_get_power_status(slot, value);
+	retval = rtas_get_power_level (slot->power_domain, &level);
+	if (!retval)
+		*value = level;
 	up(&rpaphp_sem);
 	return retval;
 }
Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_slot.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_slot.c	2007-03-28 18:10:14.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_slot.c	2007-03-28 18:10:19.000000000 -0500
@@ -184,24 +184,6 @@ sysfs_fail:
 	return retval;
 }
 
-int rpaphp_get_power_status(struct slot *slot, u8 * value)
-{
-	int rc = 0, level;
-	
-	rc = rtas_get_power_level(slot->power_domain, &level);
-	if (rc < 0) {
-		err("failed to get power-level for slot(%s), rc=0x%x\n",
-			slot->location, rc);
-		return rc;
-	}
-
-	dbg("%s the power level of slot %s(pwd-domain:0x%x) is %d\n",
-		__FUNCTION__, slot->name, slot->power_domain, level);
-	*value = level;
-
-	return rc;
-}
-
 int rpaphp_set_attention_status(struct slot *slot, u8 status)
 {
 	int rc;

[PATCH 8/19] PCI: rpaphp: Remove another wrappered function

[Linas Vepstas]

Remove another stove-pipe; this funcion was called from
two different places, with a compile-time const that is
then run-time checked to perform two different things.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpaphp.h      |    1 
 drivers/pci/hotplug/rpaphp_core.c |   16 ++++++++--
 drivers/pci/hotplug/rpaphp_pci.c  |   59 ++++++++++----------------------------
 3 files changed, 29 insertions(+), 47 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_core.c	2007-03-28 18:10:19.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c	2007-03-28 18:10:22.000000000 -0500
@@ -130,12 +130,22 @@ static int get_attention_status(struct h
 static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 * value)
 {
 	struct slot *slot = (struct slot *)hotplug_slot->private;
-	int retval = 0;
+	int rc, state;
 
 	down(&rpaphp_sem);
-	retval = rpaphp_get_pci_adapter_status(slot, 0, value);
+	rc = rpaphp_get_sensor_state(slot, &state);
 	up(&rpaphp_sem);
-	return retval;
+
+	*value = NOT_VALID;
+	if (rc)
+		return rc;
+
+	if (state == EMPTY)
+		*value = EMPTY;
+	else if (state == PRESENT)
+		*value = slot->state;
+
+	return 0;
 }
 
 static int get_max_bus_speed(struct hotplug_slot *hotplug_slot, enum pci_bus_speed *value)
Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_pci.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_pci.c	2007-03-28 18:10:19.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_pci.c	2007-03-28 18:10:22.000000000 -0500
@@ -64,43 +64,6 @@ int rpaphp_get_sensor_state(struct slot 
 	return rc;
 }
 
-/**
- * get_pci_adapter_status - get the status of a slot
- * 
- * 0-- slot is empty
- * 1-- adapter is configured
- * 2-- adapter is not configured
- * 3-- not valid
- */
-int rpaphp_get_pci_adapter_status(struct slot *slot, int is_init, u8 * value)
-{
-	struct pci_bus *bus;
-	int state, rc;
-
-	*value = NOT_VALID;
-	rc = rpaphp_get_sensor_state(slot, &state);
-	if (rc)
-		goto exit;
-
- 	if (state == EMPTY)
- 		*value = EMPTY;
- 	else if (state == PRESENT) {
-		if (!is_init) {
-			/* at run-time slot->state can be changed by */
-			/* config/unconfig adapter */
-			*value = slot->state;
-		} else {
-			bus = pcibios_find_pci_bus(slot->dn);
-			if (bus && !list_empty(&bus->devices))
-				*value = CONFIGURED;
-			else
-				*value = NOT_CONFIGURED;
-		}
-	}
-exit:
-	return rc;
-}
-
 static void print_slot_pci_funcs(struct pci_bus *bus)
 {
 	struct device_node *dn;
@@ -183,20 +146,30 @@ exit_rc:
 
 int rpaphp_register_pci_slot(struct slot *slot)
 {
-	int rc, level;
+	int rc, level, state;
+	struct pci_bus *bus;
 	struct hotplug_slot_info *info = slot->hotplug_slot->info;
 
+	/* Find out if the power is turned on for the slot */
 	rc = rtas_get_power_level(slot->power_domain, &level);
 	if (rc)
 		return rc;
 	info->power_status = level;
 
-	rpaphp_get_pci_adapter_status(slot, 1, &info->adapter_status);
+	/* Figure out if there is an adapter in the slot */
+	info->adapter_status = NOT_VALID;
+	rc = rpaphp_get_sensor_state(slot, &state);
+	if (rc)
+		return rc;
 
-	if (info->adapter_status == NOT_VALID) {
-		err("%s: NOT_VALID: skip dn->full_name=%s\n",
-		    __FUNCTION__, slot->dn->full_name);
-		return -EINVAL;
+	if (state == EMPTY)
+		info->adapter_status = EMPTY;
+	else if (state == PRESENT) {
+		bus = pcibios_find_pci_bus(slot->dn);
+		if (bus && !list_empty(&bus->devices))
+			info->adapter_status = CONFIGURED;
+		else
+			info->adapter_status = NOT_CONFIGURED;
 	}
 
 	if (setup_pci_slot(slot))
Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp.h
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp.h	2007-03-28 18:10:19.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp.h	2007-03-28 18:10:22.000000000 -0500
@@ -89,7 +89,6 @@ extern struct list_head rpaphp_slot_head
 /* rpaphp_pci.c */
 extern int rpaphp_enable_pci_slot(struct slot *slot);
 extern int rpaphp_register_pci_slot(struct slot *slot);
-extern int rpaphp_get_pci_adapter_status(struct slot *slot, int is_init, u8 * value);
 extern int rpaphp_get_sensor_state(struct slot *slot, int *state);
 
 /* rpaphp_core.c */

[PATCH 9/19] PCI: rpaphp: remove a call that does nothing but a pointer lookup

[Linas Vepstas]

Delete another stovepipe: a call to a routine which does nothing.
Remove un-needed semaphore as well.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----

 drivers/pci/hotplug/rpaphp_core.c |   17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_core.c	2007-03-28 18:10:22.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c	2007-03-28 18:10:24.000000000 -0500
@@ -54,11 +54,6 @@ MODULE_LICENSE("GPL");
 
 module_param(debug, bool, 0644);
 
-static int rpaphp_get_attention_status(struct slot *slot)
-{
-	return slot->hotplug_slot->info->attention_status;
-}
-
 /**
  * set_attention_status - set attention LED
  * echo 0 > attention -- set LED OFF
@@ -95,8 +90,6 @@ static int set_attention_status(struct h
  * get_power_status - get power status of a slot
  * @hotplug_slot: slot to get status
  * @value: pointer to store status
- *
- *
  */
 static int get_power_status(struct hotplug_slot *hotplug_slot, u8 * value)
 {
@@ -113,18 +106,12 @@ static int get_power_status(struct hotpl
 
 /**
  * get_attention_status - get attention LED status
- *
- *
  */
 static int get_attention_status(struct hotplug_slot *hotplug_slot, u8 * value)
 {
-	int retval = 0;
 	struct slot *slot = (struct slot *)hotplug_slot->private;
-
-	down(&rpaphp_sem);
-	*value = rpaphp_get_attention_status(slot);
-	up(&rpaphp_sem);
-	return retval;
+	*value = slot->hotplug_slot->info->attention_status;
+	return 0;
 }
 
 static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 * value)

[PATCH 10/19] PCI: rpaphp: Remove setup_pci_slot()

[Linas Vepstas]

The setup_pci_slot() routine appears to be nothing else than
a big, complicated wrapper around pcibios_add_pci_devices().
Remove the wrapping, and call pcibios_add_pci_devices() directly.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpaphp_pci.c |   96 +++++++++++++--------------------------
 1 file changed, 33 insertions(+), 63 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_pci.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_pci.c	2007-03-28 18:10:22.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_pci.c	2007-03-28 18:11:03.000000000 -0500
@@ -92,64 +92,15 @@ static void set_slot_name(struct slot *s
 			bus->number);
 }
 
-static int setup_pci_slot(struct slot *slot)
-{
-	struct device_node *dn = slot->dn;
-	struct pci_bus *bus;
-
-	BUG_ON(!dn);
-	bus = pcibios_find_pci_bus(dn);
-	if (!bus) {
-		err("%s: no pci_bus for dn %s\n", __FUNCTION__, dn->full_name);
-		goto exit_rc;
-	}
-
-	slot->bus = bus;
-	slot->pci_devs = &bus->devices;
-	set_slot_name(slot);
-
-	/* find slot's pci_dev if it's not empty */
-	if (slot->hotplug_slot->info->adapter_status == EMPTY) {
-		slot->state = EMPTY;	/* slot is empty */
-	} else {
-		/* slot is occupied */
-		if (!dn->child) {
-			/* non-empty slot has to have child */
-			err("%s: slot[%s]'s device_node doesn't have child for adapter\n", 
-				__FUNCTION__, slot->name);
-			goto exit_rc;
-		}
-
-		if (slot->hotplug_slot->info->adapter_status == NOT_CONFIGURED) {
-			dbg("%s CONFIGURING pci adapter in slot[%s]\n",  
-				__FUNCTION__, slot->name);
-			pcibios_add_pci_devices(slot->bus);
-
-		} else if (slot->hotplug_slot->info->adapter_status != CONFIGURED) {
-			err("%s: slot[%s]'s adapter_status is NOT_VALID.\n",
-				__FUNCTION__, slot->name);
-			goto exit_rc;
-		}
-		print_slot_pci_funcs(slot->bus);
-		if (!list_empty(slot->pci_devs)) {
-			slot->state = CONFIGURED;
-		} else {
-			/* DLPAR add as opposed to 
-		 	 * boot time */
-			slot->state = NOT_CONFIGURED;
-		}
-	}
-	return 0;
-exit_rc:
-	return -EINVAL;
-}
-
 int rpaphp_register_pci_slot(struct slot *slot)
 {
 	int rc, level, state;
 	struct pci_bus *bus;
 	struct hotplug_slot_info *info = slot->hotplug_slot->info;
 
+	info->adapter_status = NOT_VALID;
+	slot->state = EMPTY;
+
 	/* Find out if the power is turned on for the slot */
 	rc = rtas_get_power_level(slot->power_domain, &level);
 	if (rc)
@@ -157,23 +108,42 @@ int rpaphp_register_pci_slot(struct slot
 	info->power_status = level;
 
 	/* Figure out if there is an adapter in the slot */
-	info->adapter_status = NOT_VALID;
 	rc = rpaphp_get_sensor_state(slot, &state);
 	if (rc)
 		return rc;
 
-	if (state == EMPTY)
-		info->adapter_status = EMPTY;
-	else if (state == PRESENT) {
-		bus = pcibios_find_pci_bus(slot->dn);
-		if (bus && !list_empty(&bus->devices))
-			info->adapter_status = CONFIGURED;
-		else
-			info->adapter_status = NOT_CONFIGURED;
+	bus = pcibios_find_pci_bus(slot->dn);
+	if (!bus) {
+		err("%s: no pci_bus for dn %s\n", __FUNCTION__, slot->dn->full_name);
+		return -EINVAL;
 	}
 
-	if (setup_pci_slot(slot))
-		return -EINVAL;
+	info->adapter_status = EMPTY;
+	slot->bus = bus;
+	slot->pci_devs = &bus->devices;
+	set_slot_name(slot);
+
+	/* if there's an adapter in the slot, go add the pci devices */
+	if (state == PRESENT) {
+		info->adapter_status = NOT_CONFIGURED;
+		slot->state = NOT_CONFIGURED;
+
+		/* non-empty slot has to have child */
+		if (!slot->dn->child) {
+			err("%s: slot[%s]'s device_node doesn't have child for adapter\n",
+			    __FUNCTION__, slot->name);
+			return -EINVAL;
+		}
+
+		if (list_empty(&bus->devices))
+			pcibios_add_pci_devices(bus);
+
+		print_slot_pci_funcs(bus);
+		if (!list_empty(&bus->devices)) {
+			info->adapter_status = CONFIGURED;
+			slot->state = CONFIGURED;
+		}
+	}
 
 	return rpaphp_register_slot(slot);
 }

[PATCH 11/19] PCI: rpaphp: remove print_slot_pci_funcs()

[Linas Vepstas]

The debug function print_slot_pci_funcs() is a large wrapper
around two debug print statements.  Just invoke these directly.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----

 drivers/pci/hotplug/rpaphp_pci.c |   23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_pci.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_pci.c	2007-03-28 18:11:03.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_pci.c	2007-03-28 18:11:36.000000000 -0500
@@ -64,21 +64,6 @@ int rpaphp_get_sensor_state(struct slot 
 	return rc;
 }
 
-static void print_slot_pci_funcs(struct pci_bus *bus)
-{
-	struct device_node *dn;
-	struct pci_dev *dev;
-
-	dn = pci_bus_to_OF_node(bus);
-	if (!dn)
-		return;
-
-	dbg("%s: pci_devs of slot[%s]\n", __FUNCTION__, dn->full_name);
-	list_for_each_entry (dev, &bus->devices, bus_list)
-		dbg("\t%s\n", pci_name(dev));
-	return;
-}
-
 static void set_slot_name(struct slot *slot)
 {
 	struct pci_bus *bus = slot->bus;
@@ -138,11 +123,17 @@ int rpaphp_register_pci_slot(struct slot
 		if (list_empty(&bus->devices))
 			pcibios_add_pci_devices(bus);
 
-		print_slot_pci_funcs(bus);
 		if (!list_empty(&bus->devices)) {
 			info->adapter_status = CONFIGURED;
 			slot->state = CONFIGURED;
 		}
+
+		if (debug) {
+			struct pci_dev *dev;
+			dbg("%s: pci_devs of slot[%s]\n", __FUNCTION__, slot->dn->full_name);
+			list_for_each_entry (dev, &bus->devices, bus_list)
+				dbg("\t%s\n", pci_name(dev));
+		}
 	}
 
 	return rpaphp_register_slot(slot);

[PATCH 12/19] PCI: rpaphp: remove rpaphp_set_attention_status()

[Linas Vepstas]

The rpaphp_set_attention_status() routine seems to be a wrapper
around a single rtas call. Abolish it.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpaphp.h      |    1 -
 drivers/pci/hotplug/rpaphp_core.c |   21 ++++++++++-----------
 drivers/pci/hotplug/rpaphp_slot.c |   12 ------------
 3 files changed, 10 insertions(+), 24 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp.h
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp.h	2007-03-28 18:10:22.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp.h	2007-04-02 14:34:46.000000000 -0500
@@ -102,6 +102,5 @@ extern void dealloc_slot_struct(struct s
 extern struct slot *alloc_slot_struct(struct device_node *dn, int drc_index, char *drc_name, int power_domain);
 extern int rpaphp_register_slot(struct slot *slot);
 extern int rpaphp_deregister_slot(struct slot *slot);
-extern int rpaphp_set_attention_status(struct slot *slot, u8 status);
 	
 #endif				/* _PPC64PHP_H */
Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_core.c	2007-03-28 18:10:24.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c	2007-04-02 14:38:25.000000000 -0500
@@ -63,27 +63,26 @@ module_param(debug, bool, 0644);
  */
 static int set_attention_status(struct hotplug_slot *hotplug_slot, u8 value)
 {
-	int retval = 0;
+	int rc;
 	struct slot *slot = (struct slot *)hotplug_slot->private;
 
 	down(&rpaphp_sem);
 	switch (value) {
 	case 0:
-		retval = rpaphp_set_attention_status(slot, LED_OFF);
-		hotplug_slot->info->attention_status = 0;
-		break;
 	case 1:
-	default:
-		retval = rpaphp_set_attention_status(slot, LED_ON);
-		hotplug_slot->info->attention_status = 1;
-		break;
 	case 2:
-		retval = rpaphp_set_attention_status(slot, LED_ID);
-		hotplug_slot->info->attention_status = 2;
+		break;
+	default:
+		value = 1;
 		break;
 	}
 	up(&rpaphp_sem);
-	return retval;
+
+	rc = rtas_set_indicator(DR_INDICATOR, slot->index, value);
+	if (!rc)
+		hotplug_slot->info->attention_status = value;
+
+	return rc;
 }
 
 /**
Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_slot.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_slot.c	2007-03-28 18:10:19.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_slot.c	2007-04-02 14:31:00.000000000 -0500
@@ -184,15 +184,3 @@ sysfs_fail:
 	return retval;
 }
 
-int rpaphp_set_attention_status(struct slot *slot, u8 status)
-{
-	int rc;
-
-	/* status: LED_OFF or LED_ON */
-	rc = rtas_set_indicator(DR_INDICATOR, slot->index, status);
-	if (rc < 0)
-		err("slot(name=%s location=%s index=0x%x) set attention-status(%d) failed! rc=0x%x\n",
-		    slot->name, slot->location, slot->index, status, rc);
-
-	return rc;
-}

Re: [PATCH 12/19] PCI: rpaphp: remove rpaphp_set_attention_status()

[Nathan Lynch]

Linas Vepstas wrote:
> 
> The rpaphp_set_attention_status() routine seems to be a wrapper
> around a single rtas call. Abolish it.

...

>  static int set_attention_status(struct hotplug_slot *hotplug_slot, u8 value)
>  {
> -	int retval = 0;
> +	int rc;
>  	struct slot *slot = (struct slot *)hotplug_slot->private;
>  
>  	down(&rpaphp_sem);
>  	switch (value) {
>  	case 0:
> -		retval = rpaphp_set_attention_status(slot, LED_OFF);
> -		hotplug_slot->info->attention_status = 0;
> -		break;
>  	case 1:
> -	default:
> -		retval = rpaphp_set_attention_status(slot, LED_ON);
> -		hotplug_slot->info->attention_status = 1;
> -		break;
>  	case 2:
> -		retval = rpaphp_set_attention_status(slot, LED_ID);
> -		hotplug_slot->info->attention_status = 2;
> +		break;
> +	default:
> +		value = 1;
>  		break;
>  	}
>  	up(&rpaphp_sem);
> -	return retval;
> +
> +	rc = rtas_set_indicator(DR_INDICATOR, slot->index, value);
> +	if (!rc)
> +		hotplug_slot->info->attention_status = value;
> +
> +	return rc;

You're changing the locking behavior here -- you've moved the
rtas_set_indicator and the modification of
hotplug_slot->info->attention_status outside of the code which holds
rpaphp_sem.

P.S.  Should rpaphp_sem be changed to a mutex?

Re: [PATCH 12/19] PCI: rpaphp: remove rpaphp_set_attention_status()

[Linas Vepstas]

On Tue, Apr 03, 2007 at 11:19:38AM -0500, Nathan Lynch wrote:
> Linas Vepstas wrote:
> 
> You're changing the locking behavior here -- you've moved the
> rtas_set_indicator and the modification of
> hotplug_slot->info->attention_status outside of the code which holds
> rpaphp_sem.
> 
> P.S.  Should rpaphp_sem be changed to a mutex?

In patch 19, I remove the lock entirely. It doesn't seem to serve any
function that I could make out; it does not protect anything in the rpa
code, nor does it appear to be mandated by the hotplug code. Certainly,
none of the other hotplug drivers do this.  

--linas

[PATCH 13/19] PCI: rpaphp: refactor tail call to rpaphp_register_slot()

[Linas Vepstas]

Eliminate the tail call to rpaphp_register_slot() 
by placing it in the caller. This will help later
dis-entanglement.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----

 drivers/pci/hotplug/rpaphp_core.c |    3 +++
 drivers/pci/hotplug/rpaphp_pci.c  |    2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_core.c	2007-04-02 14:38:25.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c	2007-04-02 14:40:00.000000000 -0500
@@ -319,6 +319,9 @@ int rpaphp_add_slot(struct device_node *
 				indexes[i + 1], name, type);
 
 		retval = rpaphp_register_pci_slot(slot);
+		if (!retval)
+			retval = rpaphp_register_slot(slot);
+
 		if (retval)
 			dealloc_slot_struct(slot);
 
Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_pci.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_pci.c	2007-04-02 14:34:46.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_pci.c	2007-04-02 14:40:00.000000000 -0500
@@ -136,6 +136,6 @@ int rpaphp_register_pci_slot(struct slot
 		}
 	}
 
-	return rpaphp_register_slot(slot);
+	return 0;
 }
 

[PATCH 14/19] PCI: rpaphp: Rename rpaphp_register_pci_slot() to rpaphp_enable_slot()

[Linas Vepstas]

Rename rpaphp_register_pci_slot() because its easy to confuse
with  rpaphp_register_slot() even though it does something 
completely different. Rename it to rpaphp_enable_slot() because
its almost identical to enbale_slot().

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpaphp.h      |    4 +---
 drivers/pci/hotplug/rpaphp_core.c |    2 +-
 drivers/pci/hotplug/rpaphp_pci.c  |   10 +++++++++-
 3 files changed, 11 insertions(+), 5 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp.h
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp.h	2007-04-02 14:34:46.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp.h	2007-04-02 14:40:07.000000000 -0500
@@ -87,13 +87,11 @@ extern struct list_head rpaphp_slot_head
 /* function prototypes */
 
 /* rpaphp_pci.c */
-extern int rpaphp_enable_pci_slot(struct slot *slot);
-extern int rpaphp_register_pci_slot(struct slot *slot);
+extern int rpaphp_enable_slot(struct slot *slot);
 extern int rpaphp_get_sensor_state(struct slot *slot, int *state);
 
 /* rpaphp_core.c */
 extern int rpaphp_add_slot(struct device_node *dn);
-extern int rpaphp_remove_slot(struct slot *slot);
 extern int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
 		char **drc_name, char **drc_type, int *drc_power_domain);
 
Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_core.c	2007-04-02 14:40:00.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c	2007-04-02 14:40:07.000000000 -0500
@@ -318,7 +318,7 @@ int rpaphp_add_slot(struct device_node *
 		dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
 				indexes[i + 1], name, type);
 
-		retval = rpaphp_register_pci_slot(slot);
+		retval = rpaphp_enable_slot(slot);
 		if (!retval)
 			retval = rpaphp_register_slot(slot);
 
Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_pci.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_pci.c	2007-04-02 14:40:00.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_pci.c	2007-04-02 14:40:07.000000000 -0500
@@ -77,7 +77,15 @@ static void set_slot_name(struct slot *s
 			bus->number);
 }
 
-int rpaphp_register_pci_slot(struct slot *slot)
+/**
+ * rpaphp_enable_slot - record slot state, config pci device
+ *
+ * Initialize values in the slot, and the hotplug_slot info
+ * structures to indicate if there is a pci card plugged into
+ * the slot. If the slot is not empty, run the pcibios routine
+ * to get pcibios stuff correctly set up.
+ */
+int rpaphp_enable_slot(struct slot *slot)
 {
 	int rc, level, state;
 	struct pci_bus *bus;

[PATCH 15/19] PCI: rpaphp: Document find_php_slot()

[Linas Vepstas]

Document some of the interaction between dlpar and hotplug.
viz, the a dlpar remove of a htoplug slot uses hotplug to remove it.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpadlpar_core.c |   18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpadlpar_core.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpadlpar_core.c	2007-04-02 14:34:46.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpadlpar_core.c	2007-04-02 14:40:09.000000000 -0500
@@ -98,7 +98,15 @@ static struct device_node *find_dlpar_no
 	return NULL;
 }
 
-static struct slot *find_slot(struct device_node *dn)
+/**
+ * find_php_slot - return hotplug slot structure for device node
+ *
+ * This routine will return the hotplug slot structure
+ * for a given device node. Note that built-in PCI slots
+ * may be dlpar-able, but not hot-pluggable, so this routine
+ * will return NULL for built-in PCI slots.
+ */
+static struct slot *find_php_slot(struct device_node *dn)
 {
 	struct list_head *tmp, *n;
 	struct slot *slot;
@@ -224,9 +232,9 @@ static int dlpar_remove_phb(char *drc_na
 	if (!pcibios_find_pci_bus(dn))
 		return -EINVAL;
 
-	slot = find_slot(dn);
+	/* If pci slot is hotplugable, use hotplug to remove it */
+	slot = find_php_slot(dn);
 	if (slot) {
-		/* Remove hotplug slot */
 		if (rpaphp_deregister_slot(slot)) {
 			printk(KERN_ERR
 				"%s: unable to remove hotplug slot %s\n",
@@ -370,9 +378,9 @@ int dlpar_remove_pci_slot(char *drc_name
 	if (!bus)
 		return -EINVAL;
 
-	slot = find_slot(dn);
+	/* If pci slot is hotplugable, use hotplug to remove it */
+	slot = find_php_slot(dn);
 	if (slot) {
-		/* Remove hotplug slot */
 		if (rpaphp_deregister_slot(slot)) {
 			printk(KERN_ERR
 				"%s: unable to remove hotplug slot %s\n",

[PATCH 16/19] PCI: rpaphp: Document is_php_dn()

[Linas Vepstas]

Fix up the documentation: the rpaphp_add_slot() does not actually
handle embedded slots: in fact, it ignores them. Fix the flow of
control in the routine that checks for embedded slots.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpaphp_core.c |   42 ++++++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 13 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_core.c	2007-04-02 14:40:07.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c	2007-04-02 14:40:10.000000000 -0500
@@ -262,6 +262,14 @@ static int is_php_type(char *drc_type)
 	return 1;
 }
 
+/**
+ * is_php_dn() - return 1 if this is a hotpluggable pci slot, else 0
+ *
+ * This routine will return true only if the device node is
+ * a hotpluggable slot. This routine will return false
+ * for built-in pci slots (even when the built-in slots are
+ * dlparable.)
+ */
 static int is_php_dn(struct device_node *dn, const int **indexes,
 		const int **names, const int **types, const int **power_domains)
 {
@@ -269,24 +277,31 @@ static int is_php_dn(struct device_node 
 	int rc;
 
 	rc = get_children_props(dn, indexes, names, &drc_types, power_domains);
-	if (rc >= 0) {
-		if (is_php_type((char *) &drc_types[1])) {
-			*types = drc_types;
-			return 1;
-		}
-	}
+	if (rc < 0)
+		return 0;
 
-	return 0;
+	if (!is_php_type((char *) &drc_types[1]))
+		return 0;
+
+	*types = drc_types;
+	return 1;
 }
 
 /**
- * rpaphp_add_slot -- add hotplug or dlpar slot
+ * rpaphp_add_slot -- declare a hotplug slot to the hotplug subsystem.
+ * @dn device node of slot
+ *
+ * This subroutine will register a hotplugable slot with the
+ * PCI hotplug infrastructure. This routine is typicaly called
+ * during boot time, if the hotplug slots are present at boot time,
+ * or is called later, by the dlpar add code, if the slot is
+ * being dynamically added during runtime.
+ *
+ * If the device node points at an embedded (built-in) slot, this
+ * routine will just return without doing anything, since embedded
+ * slots cannot be hotplugged.
  *
- *	rpaphp not only registers PCI hotplug slots(HOTPLUG), 
- *	but also logical DR slots(EMBEDDED).
- *	HOTPLUG slot: An adapter can be physically added/removed. 
- *	EMBEDDED slot: An adapter can be logically removed/added
- *		  from/to a partition with the slot.
+ * To remove a slot, it suffices to call rpaphp_deregister_slot()
  */
 int rpaphp_add_slot(struct device_node *dn)
 {
@@ -299,6 +314,7 @@ int rpaphp_add_slot(struct device_node *
 	if (!dn || strcmp(dn->name, "pci"))
 		return 0;
 
+	/* If this is not a hotplug slot, return without doing anything. */
 	if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
 		return 0;
 	

[PATCH 17/19] PCI: rpaphp: Use pcibios_remove_pci_devices() symmetrically

[Linas Vepstas]

At first blush, the disable_slot() routine does not look
at all like its symmetric with the enable_slot() routine;
as it seems to call a very different set of routines.
However, this is easily fixed: pcibios_remove_pci_devices()
does the right thing.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----

 drivers/pci/hotplug/rpaphp_core.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_core.c	2007-04-02 14:40:10.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c	2007-04-02 14:40:12.000000000 -0500
@@ -424,18 +424,12 @@ static int enable_slot(struct hotplug_sl
 	return retval;
 }
 
-static int __disable_slot(struct slot *slot)
+static inline int __disable_slot(struct slot *slot)
 {
-	struct pci_dev *dev, *tmp;
-
 	if (slot->state == NOT_CONFIGURED)
 		return -EINVAL;
 
-	list_for_each_entry_safe(dev, tmp, &slot->bus->devices, bus_list) {
-		eeh_remove_bus_device(dev);
-		pci_remove_bus_device(dev);
-	}
-
+	pcibios_remove_pci_devices(slot->bus);
 	slot->state = NOT_CONFIGURED;
 	return 0;
 }

[PATCH 18/19] PCI: rpaphp: Ensure more pcibios_add/pcibios_remove symmetry

[Linas Vepstas]

Calls to pcibios_add should be symmetric with calls to pcibios_remove.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpadlpar_core.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpadlpar_core.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpadlpar_core.c	2007-04-02 14:40:09.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpadlpar_core.c	2007-04-02 14:40:14.000000000 -0500
@@ -387,13 +387,8 @@ int dlpar_remove_pci_slot(char *drc_name
 				__FUNCTION__, drc_name);
 			return -EIO;
 		}
-	} else {
-		struct pci_dev *dev, *tmp;
-		list_for_each_entry_safe(dev, tmp, &bus->devices, bus_list) {
-			eeh_remove_bus_device(dev);
-			pci_remove_bus_device(dev);
-		}
-	}
+	} else
+		pcibios_remove_pci_devices(bus);
 
 	if (unmap_bus_range(bus)) {
 		printk(KERN_ERR "%s: failed to unmap bus range\n",

[PATCH 19/19] PCI: rpaphp: Remove semaphores

[Linas Vepstas]

Remove the semaphores from the get routine. These do not 
appear to be protecting anything at all, and are not needed.


Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpaphp_core.c |   38 +++-----------------------------------
 1 file changed, 3 insertions(+), 35 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_core.c	2007-04-02 14:40:12.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c	2007-04-02 14:40:43.000000000 -0500
@@ -39,7 +39,6 @@
 #include "rpaphp.h"
 
 int debug;
-static struct semaphore rpaphp_sem;
 LIST_HEAD(rpaphp_slot_head);
 
 #define DRIVER_VERSION	"0.1"
@@ -66,7 +65,6 @@ static int set_attention_status(struct h
 	int rc;
 	struct slot *slot = (struct slot *)hotplug_slot->private;
 
-	down(&rpaphp_sem);
 	switch (value) {
 	case 0:
 	case 1:
@@ -76,7 +74,6 @@ static int set_attention_status(struct h
 		value = 1;
 		break;
 	}
-	up(&rpaphp_sem);
 
 	rc = rtas_set_indicator(DR_INDICATOR, slot->index, value);
 	if (!rc)
@@ -95,11 +92,9 @@ static int get_power_status(struct hotpl
 	int retval, level;
 	struct slot *slot = (struct slot *)hotplug_slot->private;
 
-	down(&rpaphp_sem);
 	retval = rtas_get_power_level (slot->power_domain, &level);
 	if (!retval)
 		*value = level;
-	up(&rpaphp_sem);
 	return retval;
 }
 
@@ -118,9 +113,7 @@ static int get_adapter_status(struct hot
 	struct slot *slot = (struct slot *)hotplug_slot->private;
 	int rc, state;
 
-	down(&rpaphp_sem);
 	rc = rpaphp_get_sensor_state(slot, &state);
-	up(&rpaphp_sem);
 
 	*value = NOT_VALID;
 	if (rc)
@@ -138,7 +131,6 @@ static int get_max_bus_speed(struct hotp
 {
 	struct slot *slot = (struct slot *)hotplug_slot->private;
 
-	down(&rpaphp_sem);
 	switch (slot->type) {
 	case 1:
 	case 2:
@@ -169,7 +161,6 @@ static int get_max_bus_speed(struct hotp
 		break;
 
 	}
-	up(&rpaphp_sem);
 	return 0;
 }
 
@@ -374,7 +365,6 @@ static int __init rpaphp_init(void)
 	struct device_node *dn = NULL;
 
 	info(DRIVER_DESC " version: " DRIVER_VERSION "\n");
-	init_MUTEX(&rpaphp_sem);
 
 	while ((dn = of_find_node_by_name(dn, "pci")))
 		rpaphp_add_slot(dn);
@@ -387,8 +377,9 @@ static void __exit rpaphp_exit(void)
 	cleanup_slots();
 }
 
-static int __enable_slot(struct slot *slot)
+static int enable_slot(struct hotplug_slot *hotplug_slot)
 {
+	struct slot *slot = (struct slot *)hotplug_slot->private;
 	int state;
 	int retval;
 
@@ -412,20 +403,9 @@ static int __enable_slot(struct slot *sl
 	return 0;
 }
 
-static int enable_slot(struct hotplug_slot *hotplug_slot)
+static int disable_slot(struct hotplug_slot *hotplug_slot)
 {
-	int retval;
 	struct slot *slot = (struct slot *)hotplug_slot->private;
-
-	down(&rpaphp_sem);
-	retval = __enable_slot(slot);
-	up(&rpaphp_sem);
-
-	return retval;
-}
-
-static inline int __disable_slot(struct slot *slot)
-{
 	if (slot->state == NOT_CONFIGURED)
 		return -EINVAL;
 
@@ -434,18 +414,6 @@ static inline int __disable_slot(struct 
 	return 0;
 }
 
-static int disable_slot(struct hotplug_slot *hotplug_slot)
-{
-	struct slot *slot = (struct slot *)hotplug_slot->private;
-	int retval;
-
-	down(&rpaphp_sem);
-	retval = __disable_slot (slot);
-	up(&rpaphp_sem);
-
-	return retval;
-}
-
 struct hotplug_slot_ops rpaphp_hotplug_slot_ops = {
 	.owner = THIS_MODULE,
 	.enable_slot = enable_slot,

Re: [PATCH 0/19]: RPAPHP pci hotplug cleanup patchbomb

[Greg KH]

On Mon, Apr 02, 2007 at 07:26:29PM -0500, Linas Vepstas wrote:
> 
> Hi Greg, 
> 
> Please queue these cleanp patches for 2.6.22; cc'ing akpm.

Kristen is the PCI Hotplug maintainer now (been that way for a number of
months).  Please cc: her on them so that she can review them, and if
acceptable, she will forward them up the foodchain properly.

Also, you might want to CC: the linux pci hotplug list at the same time
so others can review them.

thanks,

greg k-h

Re: [PATCH 0/19]: RPAPHP pci hotplug cleanup patchbomb

[Linas Vepstas]

On Mon, Apr 02, 2007 at 09:15:58PM -0700, Greg KH wrote:
> On Mon, Apr 02, 2007 at 07:26:29PM -0500, Linas Vepstas wrote:
> > 
> > Hi Greg, 
> > 
> > Please queue these cleanp patches for 2.6.22; cc'ing akpm.
> 
> Kristen is the PCI Hotplug maintainer now (been that way for a number of
> months).  Please cc: her on them so that she can review them, and if
> acceptable, she will forward them up the foodchain properly.
> 
> Also, you might want to CC: the linux pci hotplug list at the same time
> so others can review them.

Dohh. OK. However, I'll trim linux-pci@atrey.karlin.mff.cuni.cz from
the resend. Too much cross posting for silly patches such as these 
makes me uncomfortable.

--linas

[PATCH 0/19]: RPAPHP pci hotplug cleanup patchbomb

[Linas Vepstas]

Hi Kristen,

Please queue these cleanup patches for 2.6.22.

This is a collection of very small, mostly trite, patches that clean up 
various bits and pieces of the RPAPHP hotplug code. They eliminate
almost 10% of the code, while making almost no funcional change. 
There are a few bugfixes to various error paths, and one memleak fix. 
Some documentation is added.  The result is, I beleive, slightly more 
readable, easier to understand code. In particular, the enable/disable 
add/remove code paths are now more obviously symmetrical in thier function.

--linas

p.s. some more simplifcation is possible: one could probably merge
__enable_slot() and rpaphp_enable_slot() with a bit of elbow grease,
and the asymmetric pairing of rpaphp_deregister_slot()  with
rpaphp_add_slot() as "opposites" of each other still bugs me.

I'm also irked that dlpar_pci_add_bus() is quite similar to 
pcibios_add_pci_devices() which is quite similar to init_phb_dynamic()
and think that these should be refactored so that they are more clearly
orthogonal to one another. Just right now, I'm not planning on doing 
anything about this, at least, not without prodding.

--linas 

[PATCH 0/19]: RPAPHP pci hotplug cleanup patchbomb

[Linas Vepstas]

(Resend, I flubbed the cc list.)

Hi Kristen,

Please queue these cleanup patches for 2.6.22.

This is a collection of very small, mostly trite, patches that clean up 
various bits and pieces of the RPAPHP hotplug code. They eliminate
almost 10% of the code, while making almost no funcional change. 
There are a few bugfixes to various error paths, and one memleak fix. 
Some documentation is added.  The result is, I beleive, slightly more 
readable, easier to understand code. In particular, the enable/disable 
add/remove code paths are now more obviously symmetrical in thier function.

--linas

p.s. some more simplifcation is possible: one could probably merge
__enable_slot() and rpaphp_enable_slot() with a bit of elbow grease,
and the asymmetric pairing of rpaphp_deregister_slot()  with
rpaphp_add_slot() as "opposites" of each other still bugs me.

I'm also irked that dlpar_pci_add_bus() is quite similar to 
pcibios_add_pci_devices() which is quite similar to init_phb_dynamic()
and think that these should be refactored so that they are more clearly
orthogonal to one another. Just right now, I'm not planning on doing 
anything about this, at least, not without prodding.

Re: [PATCH 0/19]: RPAPHP pci hotplug cleanup patchbomb

[Kristen Carlson Accardi]

On Tue, 3 Apr 2007 11:34:14 -0500
linas@austin.ibm.com (Linas Vepstas) wrote:

> 
> Hi Kristen,
> 
> Please queue these cleanup patches for 2.6.22.

Ok - will do.

> 
> This is a collection of very small, mostly trite, patches that clean up 
> various bits and pieces of the RPAPHP hotplug code. They eliminate
> almost 10% of the code, while making almost no funcional change. 
> There are a few bugfixes to various error paths, and one memleak fix. 
> Some documentation is added.  The result is, I beleive, slightly more 
> readable, easier to understand code. In particular, the enable/disable 
> add/remove code paths are now more obviously symmetrical in thier function.
> 
> --linas
> 
> p.s. some more simplifcation is possible: one could probably merge
> __enable_slot() and rpaphp_enable_slot() with a bit of elbow grease,
> and the asymmetric pairing of rpaphp_deregister_slot()  with
> rpaphp_add_slot() as "opposites" of each other still bugs me.
> 
> I'm also irked that dlpar_pci_add_bus() is quite similar to 
> pcibios_add_pci_devices() which is quite similar to init_phb_dynamic()
> and think that these should be refactored so that they are more clearly
> orthogonal to one another. Just right now, I'm not planning on doing 
> anything about this, at least, not without prodding.
> 
> --linas 
> 

Thanks for the cleanups - most of the drivers in the hotplug tree badly
need cleaning up, but... 

Kristen

Re: [PATCH 0/19]: RPAPHP pci hotplug cleanup patchbomb

[Linas Vepstas]

On Thu, Apr 05, 2007 at 08:33:48AM -0700, Kristen Carlson Accardi wrote:
> On Tue, 3 Apr 2007 11:34:14 -0500
> linas@austin.ibm.com (Linas Vepstas) wrote:
> 
> > Please queue these cleanup patches for 2.6.22.
> 
> Ok - will do.

Thanks,

> Thanks for the cleanups - 

I needed to do that to isolate a bug in some older distro kernels.

> most of the drivers in the hotplug tree badly
> need cleaning up, but... 

Hmm. Trying to get me to volunteer? I have no hardware to test 
any of the others.

--linas