[Skiboot] [PATCH v2 4/9] core/pci: Update PCI topology after power change

[Gavin Shan]

On Fri, Oct 14, 2016 at 01:24:40PM +1100, Andrew Donnellan wrote:
>On 13/10/16 12:16, Gavin Shan wrote:
>>When OPAL_SUCCESS is returned from slot->ops.set_power_state(),
>>we need update the PCI toplogy accordingly. This scenario can
>>happen when builtin power control functionality is ignored to
>>accomodate PCI surprise hotplug or not supported at all by the
>>hardware.
>
>I think I get what this is doing, but correct me if I'm wrong, it took me a
>bit of work... if set_power_state() returns OPAL_ASYNC_COMPLETION, then that
>means we're using normal power control functionality and the topology will be
>updated in set_power_timer(). If set_power_state() returns OPAL_SUCCESS on
>the other hand, then that means the slot doesn't have power control
>capability and as such we should update the topology immediately.
>

Or the power control functioanlity is supported, but we don't want
to toggle that in hardware in order to support surprise hotplug.

Yes, your understanding is correct except: the power control functioanlity
is supported in hardware, but we don't want to toggle that in order to
support surprise hotplug.


>Perhaps a small comment in the code would be helpful, but that's not too big
>a deal.
>

Yeah, Will do when I have chance to give it a respin.

Thanks,
Gavin

>Reviewed-by: Andrew Donnellan <andrew.donnellan at au1.ibm.com>
>
>>---
>> core/pci-opal.c | 20 +++++++++++++++-----
>> 1 file changed, 15 insertions(+), 5 deletions(-)
>>
>>diff --git a/core/pci-opal.c b/core/pci-opal.c
>>index ba7a261..7ba64f5 100644
>>--- a/core/pci-opal.c
>>+++ b/core/pci-opal.c
>>@@ -805,8 +805,8 @@ static int64_t opal_pci_set_power_state(uint64_t async_token,
>> 			return OPAL_PARAMETER;
>>
>> 		pci_remove_bus(phb, &pd->children);
>>-		rc = OPAL_SUCCESS;
>>-		break;
>>+		phb_unlock(phb);
>>+		return OPAL_SUCCESS;
>> 	case OPAL_PCI_SLOT_ONLINE:
>> 		if (!pd)
>> 			return OPAL_PARAMETER;
>>@@ -814,19 +814,29 @@ static int64_t opal_pci_set_power_state(uint64_t async_token,
>> 			     &pd->children, pd, true);
>> 		pci_add_device_nodes(phb, &pd->children, pd->dn,
>> 				     &phb->lstate, 0);
>>-		rc = OPAL_SUCCESS;
>>-		break;
>>+		phb_unlock(phb);
>>+		return OPAL_SUCCESS;
>> 	default:
>> 		rc = OPAL_PARAMETER;
>> 	}
>>
>>-	phb_unlock(phb);
>> 	if (rc == OPAL_ASYNC_COMPLETION) {
>> 		slot->retries = 500;
>> 		init_timer(&slot->timer, set_power_timer, slot);
>> 		schedule_timer(&slot->timer, msecs_to_tb(10));
>>+	} else if (rc == OPAL_SUCCESS) {
>>+		if (*state == OPAL_PCI_SLOT_POWER_OFF) {
>>+			pci_remove_bus(phb, &pd->children);
>>+		} else {
>>+			slot->ops.prepare_link_change(slot, true);
>>+			pci_scan_bus(phb, pd->secondary_bus,
>>+				pd->subordinate_bus, &pd->children, pd, true);
>>+			pci_add_device_nodes(phb, &pd->children, pd->dn,
>>+				&phb->lstate, 0);
>>+		}
>> 	}
>>
>>+	phb_unlock(phb);
>> 	return rc;
>> }
>> opal_call(OPAL_PCI_SET_POWER_STATE, opal_pci_set_power_state, 3);
>>
>
>-- 
>Andrew Donnellan              OzLabs, ADL Canberra
>andrew.donnellan at au1.ibm.com  IBM Australia Limited