Re: [PATCH] PCI Hotplug: remove incorrect rpaphp firmware dependency

Re: [PATCH] PCI Hotplug: remove incorrect rpaphp firmware dependency

[David Woodhouse]

On Thu, 2005-02-03 at 08:41 +0000, Linux Kernel Mailing List wrote:
>         [PATCH] PCI Hotplug: remove incorrect rpaphp firmware dependency
>         
>         The RPA PCI Hotplug module incorrectly uses a certain firmware property when
>         determining the hotplug capabilities of a slot.  Recent firmware changes have
>         demonstrated that this property should not be referenced or depended upon by
>         the OS.  This patch removes the dependency, and implements a correct set of
>         logic for determining hotplug capabilities.
>         
>         Signed-off-by: John Rose <johnrose@austin.ibm.com>
>         Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
>         
> +       rc = get_children_props(dn, indexes, names, &drc_types, power_domains);
> +       if (rc) {
> +               if (is_php_type((char *) &drc_types[1])) {
> +                       *types = drc_types;
> +                       return 1;
> +               }
> +       }

Er, use the result of the get_children_props() call only if it _failed_?
I suspect that wasn't your intention. This makes my G5 boot again:

--- linux-2.6.10/drivers/pci/hotplug/rpaphp_core.c.orig	2005-02-07 16:41:45.830990208 +0000
+++ linux-2.6.10/drivers/pci/hotplug/rpaphp_core.c	2005-02-07 16:46:15.495868912 +0000
@@ -307,7 +307,7 @@ static int is_php_dn(struct device_node 
 	int rc;
 
 	rc = get_children_props(dn, indexes, names, &drc_types, power_domains);
-	if (rc) {
+	if (!rc) {
 		if (is_php_type((char *) &drc_types[1])) {
 			*types = drc_types;
 			return 1;




-- 
dwmw2

Re: [PATCH] PCI Hotplug: remove incorrect rpaphp firmware dependency

[John Rose]

> Er, use the result of the get_children_props() call only if it _failed_?
> I suspect that wasn't your intention. This makes my G5 boot again:

Doh, good catch!  This was an oversight while patching multiple trees
for this bug.  Previous versions of that function use 1 for success. 
Sigh.  BTW, you're running an RPA module on your G5?

Thanks-
John

Re: [PATCH] PCI Hotplug: remove incorrect rpaphp firmware dependency

[John Rose]

Could we please get David's fix in for 2.6.11, since it's apparently
affecting boot in some situations?

Thanks-
John

On Mon, 2005-02-07 at 11:41, John Rose wrote:
> > Er, use the result of the get_children_props() call only if it _failed_?
> > I suspect that wasn't your intention. This makes my G5 boot again:
> 
> Doh, good catch!  This was an oversight while patching multiple trees
> for this bug.  Previous versions of that function use 1 for success. 
> Sigh.  BTW, you're running an RPA module on your G5?
> 
> Thanks-
> John

Re: [PATCH] PCI Hotplug: remove incorrect rpaphp firmware dependency

[Andrew Morton]

John Rose <johnrose@austin.ibm.com> wrote:
>
> Could we please get David's fix in for 2.6.11, since it's apparently
>  affecting boot in some situations?

Yup, I can take care of that.

Re: [PATCH] PCI Hotplug: remove incorrect rpaphp firmware dependency

[David Woodhouse]

On Mon, 2005-02-07 at 11:41 -0600, John Rose wrote:
> BTW, you're running an RPA module on your G5?

Fedora uses the same ppc64 kernel for both pSeries and G5.

-- 
dwmw2

Re: [PATCH] PCI Hotplug: remove incorrect rpaphp firmware dependency

[John Rose]

> Er, use the result of the get_children_props() call only if it _failed_?
> I suspect that wasn't your intention. This makes my G5 boot again:

Here's an alternate fix for the ppc64 crash during boot.  This corrects
the offending function to use more conventional error codes.  I'll
follow up with return code cleanups for the entire module, and for RTAS
code, since these are probably too big for 2.6.11.

Please apply, if appropriate.

Thanks-
John

Signed-off-by: John Rose <johnrose@austin.ibm.com>

diff -puN drivers/pci/hotplug/rpaphp_core.c~01_rpaphp_is_php_fix drivers/pci/hotplug/rpaphp_core.c
--- 2_6_linus/drivers/pci/hotplug/rpaphp_core.c~01_rpaphp_is_php_fix	2005-02-07 18:06:29.000000000 -0600
+++ 2_6_linus-johnrose/drivers/pci/hotplug/rpaphp_core.c	2005-02-07 18:10:15.000000000 -0600
@@ -224,7 +224,7 @@ static int get_children_props(struct dev
 
 	if (!indexes || !names || !types || !domains) {
 		/* Slot does not have dynamically-removable children */
-		return 1;
+		return -EINVAL;
 	}
 	if (drc_indexes)
 		*drc_indexes = indexes;
@@ -260,7 +260,7 @@ int rpaphp_get_drc_props(struct device_n
 	}
 
 	rc = get_children_props(dn->parent, &indexes, &names, &types, &domains);
-	if (rc) {
+	if (rc < 0) {
 		return 1;
 	}
 
@@ -307,7 +307,7 @@ static int is_php_dn(struct device_node 
 	int rc;
 
 	rc = get_children_props(dn, indexes, names, &drc_types, power_domains);
-	if (rc) {
+	if (rc >= 0) {
 		if (is_php_type((char *) &drc_types[1])) {
 			*types = drc_types;
 			return 1;
@@ -331,7 +331,7 @@ static int is_dr_dn(struct device_node *
 
 	rc = get_children_props(dn->parent, indexes, names, types,
 				power_domains);
-	return (rc == 0);
+	return (rc >= 0);
 }
 
 static inline int is_vdevice_root(struct device_node *dn)

_

[PATCH] PCI Hotplug: remove incorrect rpaphp firmware dependency

[Greg KH]

ChangeSet 1.2044, 2005/02/03 00:41:04-08:00, johnrose@austin.ibm.com

[PATCH] PCI Hotplug: remove incorrect rpaphp firmware dependency

The RPA PCI Hotplug module incorrectly uses a certain firmware property when
determining the hotplug capabilities of a slot.  Recent firmware changes have
demonstrated that this property should not be referenced or depended upon by
the OS.  This patch removes the dependency, and implements a correct set of
logic for determining hotplug capabilities.

Signed-off-by: John Rose <johnrose@austin.ibm.com>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>

diff -puN drivers/pci/hotplug/rpadlpar_core.c~02_rpadebug drivers/pci/hotplug/rpadlpar_core.c


 drivers/pci/hotplug/rpaphp.h      |    7 ------
 drivers/pci/hotplug/rpaphp_core.c |   39 +++++++++++++++++++++++++++-----------
 2 files changed, 28 insertions(+), 18 deletions(-)


diff -Nru a/drivers/pci/hotplug/rpaphp.h b/drivers/pci/hotplug/rpaphp.h
--- a/drivers/pci/hotplug/rpaphp.h	2005-02-03 09:28:39 -08:00
+++ b/drivers/pci/hotplug/rpaphp.h	2005-02-03 09:28:39 -08:00
@@ -109,13 +109,6 @@
 extern struct list_head rpaphp_slot_head;
 extern int num_slots;
 
-static inline int is_hotplug_capable(struct device_node *dn)
-{
-	unsigned char *ptr = get_property(dn, "ibm,fw-pci-hot-plug-ctrl", NULL);
-
-	return (int) (ptr != NULL);
-}
-
 /* function prototypes */
 
 /* rpaphp_pci.c */
diff -Nru a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
--- a/drivers/pci/hotplug/rpaphp_core.c	2005-02-03 09:28:39 -08:00
+++ b/drivers/pci/hotplug/rpaphp_core.c	2005-02-03 09:28:39 -08:00
@@ -287,26 +287,43 @@
 	return 1;
 }
 
-static int is_php_dn(struct device_node *dn, int **indexes, int **names, int **types,
-	  int **power_domains)
+static int is_php_type(char *drc_type)
 {
+	unsigned long value;
+	char *endptr;
+
+	/* PCI Hotplug nodes have an integer for drc_type */
+	value = simple_strtoul(drc_type, &endptr, 10);
+	if (endptr == drc_type)
+		return 0;
+
+	return 1;
+}
+
+static int is_php_dn(struct device_node *dn, int **indexes, int **names,
+		int **types, int **power_domains)
+{
+	int *drc_types;
 	int rc;
 
-	if (!is_hotplug_capable(dn))
-		return (0);
-	rc = get_children_props(dn, indexes, names, types, power_domains);
-	if (rc)
-		return (0);
-	return (1);
+	rc = get_children_props(dn, indexes, names, &drc_types, power_domains);
+	if (rc) {
+		if (is_php_type((char *) &drc_types[1])) {
+			*types = drc_types;
+			return 1;
+		}
+	}
+
+	return 0;
 }
 
-static int is_dr_dn(struct device_node *dn, int **indexes, int **names, int **types,
-	  int **power_domains, int **my_drc_index)
+static int is_dr_dn(struct device_node *dn, int **indexes, int **names,
+		int **types, int **power_domains, int **my_drc_index)
 {
 	int rc;
 
 	*my_drc_index = (int *) get_property(dn, "ibm,my-drc-index", NULL);
-	if(!*my_drc_index) 		
+	if(!*my_drc_index)
 		return (0);
 
 	if (!dn->parent)