| Message ID | 1476321413-5245-2-git-send-email-gwshan@linux.vnet.ibm.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On 13/10/16 12:16, Gavin Shan wrote: > This returns error (OPAL_PARAMETER) when having invalid power state > transition requests. The invalid requests include: ON to ON, OFF to > OFF. > > Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> What's the advantage of doing this rather than immediately returning OPAL_SUCCESS? > --- > core/pcie-slot.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/core/pcie-slot.c b/core/pcie-slot.c > index 6854ef1..7fe3785 100644 > --- a/core/pcie-slot.c > +++ b/core/pcie-slot.c > @@ -199,13 +199,13 @@ static int64_t pcie_slot_set_power_state(struct pci_slot *slot, uint8_t val) > uint32_t ecap; > uint16_t state; > > + if (slot->power_state == val) > + return OPAL_PARAMETER; > + > /* Drop the request if functionality doesn't exist */ > if (!(slot->slot_cap & PCICAP_EXP_SLOTCAP_PWCTRL)) > return OPAL_SUCCESS; > > - if (slot->power_state == val) > - return OPAL_SUCCESS; > - > pci_slot_set_state(slot, PCI_SLOT_STATE_SPOWER_START); > slot->power_state = val; > ecap = pci_cap(pd, PCI_CFG_CAP_ID_EXP, false); >
On Thu, Oct 13, 2016 at 03:18:10PM +1100, Andrew Donnellan wrote: >On 13/10/16 12:16, Gavin Shan wrote: >>This returns error (OPAL_PARAMETER) when having invalid power state >>transition requests. The invalid requests include: ON to ON, OFF to >>OFF. >> >>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> > >What's the advantage of doing this rather than immediately returning >OPAL_SUCCESS? > There are two purposes: (1) To notify the caller the invalid power state switching; (2) After this patchset is applied, OPAL_SUCCESS will causes an PCI topology update, which is totally unecessary if we don't have power (hotplug) state change. This is required by surprise hotplug that relies on interrupts caused by link up/down events, meaning we cannot put the slot into power-off state under the situation. Thanks, Gavin >>--- >> core/pcie-slot.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >>diff --git a/core/pcie-slot.c b/core/pcie-slot.c >>index 6854ef1..7fe3785 100644 >>--- a/core/pcie-slot.c >>+++ b/core/pcie-slot.c >>@@ -199,13 +199,13 @@ static int64_t pcie_slot_set_power_state(struct pci_slot *slot, uint8_t val) >> uint32_t ecap; >> uint16_t state; >> >>+ if (slot->power_state == val) >>+ return OPAL_PARAMETER; >>+ >> /* Drop the request if functionality doesn't exist */ >> if (!(slot->slot_cap & PCICAP_EXP_SLOTCAP_PWCTRL)) >> return OPAL_SUCCESS; >> >>- if (slot->power_state == val) >>- return OPAL_SUCCESS; >>- >> pci_slot_set_state(slot, PCI_SLOT_STATE_SPOWER_START); >> slot->power_state = val; >> ecap = pci_cap(pd, PCI_CFG_CAP_ID_EXP, false); >> > >-- >Andrew Donnellan OzLabs, ADL Canberra >andrew.donnellan@au1.ibm.com IBM Australia Limited
On 13/10/16 15:29, Gavin Shan wrote: > On Thu, Oct 13, 2016 at 03:18:10PM +1100, Andrew Donnellan wrote: >> On 13/10/16 12:16, Gavin Shan wrote: >>> This returns error (OPAL_PARAMETER) when having invalid power state >>> transition requests. The invalid requests include: ON to ON, OFF to >>> OFF. >>> >>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >> >> What's the advantage of doing this rather than immediately returning >> OPAL_SUCCESS? >> > > There are two purposes: (1) To notify the caller the invalid power state > switching; (2) After this patchset is applied, OPAL_SUCCESS will causes > an PCI topology update, which is totally unecessary if we don't have > power (hotplug) state change. This is required by surprise hotplug that > relies on interrupts caused by link up/down events, meaning we cannot > put the slot into power-off state under the situation. OK. Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
diff --git a/core/pcie-slot.c b/core/pcie-slot.c index 6854ef1..7fe3785 100644 --- a/core/pcie-slot.c +++ b/core/pcie-slot.c @@ -199,13 +199,13 @@ static int64_t pcie_slot_set_power_state(struct pci_slot *slot, uint8_t val) uint32_t ecap; uint16_t state; + if (slot->power_state == val) + return OPAL_PARAMETER; + /* Drop the request if functionality doesn't exist */ if (!(slot->slot_cap & PCICAP_EXP_SLOTCAP_PWCTRL)) return OPAL_SUCCESS; - if (slot->power_state == val) - return OPAL_SUCCESS; - pci_slot_set_state(slot, PCI_SLOT_STATE_SPOWER_START); slot->power_state = val; ecap = pci_cap(pd, PCI_CFG_CAP_ID_EXP, false);
This returns error (OPAL_PARAMETER) when having invalid power state transition requests. The invalid requests include: ON to ON, OFF to OFF. Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> --- core/pcie-slot.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)