Re: [PATCH] rpaphp: fix slot registration for multiple slots under a PHB
On 07/08/2016 04:57 PM, Nathan Fontenot wrote:
> On 07/08/2016 06:19 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
>> ---
>> 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..c90fa8d 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, "my,ibm-drc-index",
>> &my_index);
>
> Shouldn't this be reading ibm,my-drc-index? instead of my,ibm-drc-index.
Doh, indeed it should.
-Tyrel
>
> -Nathan
>
>> +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);
>>
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] rpaphp: fix slot registration for multiple slots under a PHB
On 07/08/2016 06:19 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
> ---
> 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..c90fa8d 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, "my,ibm-drc-index",
> &my_index);
Shouldn't this be reading ibm,my-drc-index? instead of my,ibm-drc-index.
-Nathan
> + 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);
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
