| Message ID | 1476321413-5245-5-git-send-email-gwshan@linux.vnet.ibm.com |
|---|---|
| State | Changes Requested |
| Headers | show |
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. Perhaps a small comment in the code would be helpful, but that's not too big a deal. Reviewed-by: Andrew Donnellan <andrew.donnellan@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); >
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@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@au1.ibm.com IBM Australia Limited
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);
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. Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> --- core/pci-opal.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)