[Patch v2] rpaphp: fix slot registration for multiple slots under a PHB

Subsystems: ibm power pci hotplug driver for rpa-compliant ppc64 platform, linux for powerpc (32-bit and 64-bit), pci subsystem, the rest

STALE3707d

3 messages, 3 authors, 2016-07-20 · open the first message on its own page

[Patch v2] rpaphp: fix slot registration for multiple slots under a PHB

From: Tyrel Datwyler <hidden>
Date: 2016-07-11 22:16:46

PowerVM seems to only ever provide a single hotplug slot per PHB.
The under lying slot hotplug registration code assumed multiple slots,
but the actual implementation is broken for multiple slots. This went
unnoticed for years due to the nature of PowerVM as mentioned
previously. Under qemu/kvm the hotplug slot model aligns more with
x86 where multiple slots are presented under a single PHB. As seen
in the following each additional slot after the first fails to
register due to each slot always being compared against the first
child node of the PHB in the device tree.

[    6.492291] rpaphp: RPA HOT Plug PCI Controller Driver version: 0.1
[    6.492569] rpaphp: Slot [Slot 0] registered
[    6.492577] rpaphp: pci_hp_register failed with error -16
[    6.493082] rpaphp: pci_hp_register failed with error -16
[    6.493138] rpaphp: pci_hp_register failed with error -16
[    6.493161] rpaphp: pci_hp_register failed with error -16

The registration logic is fixed so that each slot is compared
against the existing child devices of the PHB in the device tree to
determine present slots vs empty slots.

[   38.481750] rpaphp: RPA HOT Plug PCI Controller Driver version: 0.1
[   38.482004] rpaphp: Slot [C0] registered
[   38.482127] rpaphp: Slot [C1] registered
[   38.482241] rpaphp: Slot [C2] registered
[   38.482356] rpaphp: Slot [C3] registered
[   38.482495] rpaphp: Slot [C4] registered

Signed-off-by: Tyrel Datwyler <redacted>
---

Changes in v2: corrected ibm,my-drc-index property name

---
 drivers/pci/hotplug/rpaphp_slot.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/hotplug/rpaphp_slot.c b/drivers/pci/hotplug/rpaphp_slot.c
index 6937c72..388c4d8 100644
--- a/drivers/pci/hotplug/rpaphp_slot.c
+++ b/drivers/pci/hotplug/rpaphp_slot.c
@@ -117,8 +117,10 @@ EXPORT_SYMBOL_GPL(rpaphp_deregister_slot);
 int rpaphp_register_slot(struct slot *slot)
 {
 	struct hotplug_slot *php_slot = slot->hotplug_slot;
+	struct device_node *child;
+	u32 my_index;
 	int retval;
-	int slotno;
+	int slotno = -1;
 
 	dbg("%s registering slot:path[%s] index[%x], name[%s] pdomain[%x] type[%d]\n",
 		__func__, slot->dn->full_name, slot->index, slot->name,
@@ -130,10 +132,15 @@ int rpaphp_register_slot(struct slot *slot)
 		return -EAGAIN;
 	}
 
-	if (slot->dn->child)
-		slotno = PCI_SLOT(PCI_DN(slot->dn->child)->devfn);
-	else
-		slotno = -1;
+	for_each_child_of_node(slot->dn, child) {
+		retval = of_property_read_u32(child, "ibm,my-drc-index", &my_index);
+		if (my_index == slot->index) {
+			slotno = PCI_SLOT(PCI_DN(child)->devfn);
+			of_node_put(child);
+			break;
+		}
+	}
+
 	retval = pci_hp_register(php_slot, slot->bus, slotno, slot->name);
 	if (retval) {
 		err("pci_hp_register failed with error %d\n", retval);
-- 
2.7.4

Re: [Patch v2] rpaphp: fix slot registration for multiple slots under a PHB

From: Nathan Fontenot <hidden>
Date: 2016-07-19 13:41:50

On 07/11/2016 05:16 PM, Tyrel Datwyler wrote:
PowerVM seems to only ever provide a single hotplug slot per PHB.
The under lying slot hotplug registration code assumed multiple slots,
but the actual implementation is broken for multiple slots. This went
unnoticed for years due to the nature of PowerVM as mentioned
previously. Under qemu/kvm the hotplug slot model aligns more with
x86 where multiple slots are presented under a single PHB. As seen
in the following each additional slot after the first fails to
register due to each slot always being compared against the first
child node of the PHB in the device tree.

[    6.492291] rpaphp: RPA HOT Plug PCI Controller Driver version: 0.1
[    6.492569] rpaphp: Slot [Slot 0] registered
[    6.492577] rpaphp: pci_hp_register failed with error -16
[    6.493082] rpaphp: pci_hp_register failed with error -16
[    6.493138] rpaphp: pci_hp_register failed with error -16
[    6.493161] rpaphp: pci_hp_register failed with error -16

The registration logic is fixed so that each slot is compared
against the existing child devices of the PHB in the device tree to
determine present slots vs empty slots.

[   38.481750] rpaphp: RPA HOT Plug PCI Controller Driver version: 0.1
[   38.482004] rpaphp: Slot [C0] registered
[   38.482127] rpaphp: Slot [C1] registered
[   38.482241] rpaphp: Slot [C2] registered
[   38.482356] rpaphp: Slot [C3] registered
[   38.482495] rpaphp: Slot [C4] registered

Signed-off-by: Tyrel Datwyler <redacted>
Reviewed-by: Nathan Fontenot <redacted>
quoted hunk
---

Changes in v2: corrected ibm,my-drc-index property name

---
 drivers/pci/hotplug/rpaphp_slot.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/hotplug/rpaphp_slot.c b/drivers/pci/hotplug/rpaphp_slot.c
index 6937c72..388c4d8 100644
--- a/drivers/pci/hotplug/rpaphp_slot.c
+++ b/drivers/pci/hotplug/rpaphp_slot.c
@@ -117,8 +117,10 @@ EXPORT_SYMBOL_GPL(rpaphp_deregister_slot);
 int rpaphp_register_slot(struct slot *slot)
 {
 	struct hotplug_slot *php_slot = slot->hotplug_slot;
+	struct device_node *child;
+	u32 my_index;
 	int retval;
-	int slotno;
+	int slotno = -1;

 	dbg("%s registering slot:path[%s] index[%x], name[%s] pdomain[%x] type[%d]\n",
 		__func__, slot->dn->full_name, slot->index, slot->name,
@@ -130,10 +132,15 @@ int rpaphp_register_slot(struct slot *slot)
 		return -EAGAIN;
 	}

-	if (slot->dn->child)
-		slotno = PCI_SLOT(PCI_DN(slot->dn->child)->devfn);
-	else
-		slotno = -1;
+	for_each_child_of_node(slot->dn, child) {
+		retval = of_property_read_u32(child, "ibm,my-drc-index", &my_index);
+		if (my_index == slot->index) {
+			slotno = PCI_SLOT(PCI_DN(child)->devfn);
+			of_node_put(child);
+			break;
+		}
+	}
+
 	retval = pci_hp_register(php_slot, slot->bus, slotno, slot->name);
 	if (retval) {
 		err("pci_hp_register failed with error %d\n", retval);

Re: [v2] rpaphp: fix slot registration for multiple slots under a PHB

From: Michael Ellerman <hidden>
Date: 2016-07-20 09:10:30

On Mon, 2016-11-07 at 22:16:27 UTC, Tyrel Datwyler wrote:
PowerVM seems to only ever provide a single hotplug slot per PHB.
The under lying slot hotplug registration code assumed multiple slots,
but the actual implementation is broken for multiple slots. This went
unnoticed for years due to the nature of PowerVM as mentioned
previously. Under qemu/kvm the hotplug slot model aligns more with
x86 where multiple slots are presented under a single PHB. As seen
in the following each additional slot after the first fails to
register due to each slot always being compared against the first
child node of the PHB in the device tree.
...
Signed-off-by: Tyrel Datwyler <redacted>
Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/e2413a7dae52fab290b7a8d11e

cheers
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help