Message ID | 1769241.yKxyosiRnQ@kreacher |
---|---|
State | Accepted |
Delegated to: | Bjorn Helgaas |
Headers | show |
Series | PCI: PM: Cleanups related to power state changes | expand |
On Tue, Nov 05, 2019 at 11:32:02AM +0100, Rafael J. Wysocki wrote: > if (state > PCI_D3cold) > @@ -1132,10 +1112,12 @@ int pci_set_power_state(struct pci_dev * > error = pci_raw_set_power_state(dev, state > PCI_D3hot ? > PCI_D3hot : state); > > - if (!__pci_complete_power_transition(dev, state)) > - error = 0; > + ret = pci_platform_power_transition(dev, state); > + /* Powering off a bridge may power off the whole hierarchy */ > + if (!ret && state == PCI_D3cold) > + pci_bus_set_current_state(dev->subordinate, PCI_D3cold); > > - return error; > + return ret ? error : 0; Total nitpick, but why not: if (pci_platform_power_transition(dev, state)) return error; /* Powering off a bridge may power off the whole hierarchy */ if (state == PCI_D3cold) pci_bus_set_current_state(dev->subordinate, PCI_D3cold); return 0;
On Tue, Nov 5, 2019 at 5:09 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Tue, Nov 05, 2019 at 11:32:02AM +0100, Rafael J. Wysocki wrote: > > if (state > PCI_D3cold) > > @@ -1132,10 +1112,12 @@ int pci_set_power_state(struct pci_dev * > > error = pci_raw_set_power_state(dev, state > PCI_D3hot ? > > PCI_D3hot : state); > > > > - if (!__pci_complete_power_transition(dev, state)) > > - error = 0; > > + ret = pci_platform_power_transition(dev, state); > > + /* Powering off a bridge may power off the whole hierarchy */ > > + if (!ret && state == PCI_D3cold) > > + pci_bus_set_current_state(dev->subordinate, PCI_D3cold); > > > > - return error; > > + return ret ? error : 0; > > Total nitpick, but why not: > > if (pci_platform_power_transition(dev, state)) > return error; > > /* Powering off a bridge may power off the whole hierarchy */ > if (state == PCI_D3cold) > pci_bus_set_current_state(dev->subordinate, PCI_D3cold); > return 0; Yes, that looks better, thanks! I'll send an update of this patch.
Index: linux-pm/drivers/pci/pci.c =================================================================== --- linux-pm.orig/drivers/pci/pci.c +++ linux-pm/drivers/pci/pci.c @@ -1056,26 +1056,6 @@ void pci_bus_set_current_state(struct pc } /** - * __pci_complete_power_transition - Complete power transition of a PCI device - * @dev: PCI device to handle. - * @state: State to put the device into. - * - * This function should not be called directly by device drivers. - */ -static int __pci_complete_power_transition(struct pci_dev *dev, pci_power_t state) -{ - int ret; - - if (state <= PCI_D0) - return -EINVAL; - ret = pci_platform_power_transition(dev, state); - /* Power off the bridge may power off the whole hierarchy */ - if (!ret && state == PCI_D3cold) - pci_bus_set_current_state(dev->subordinate, PCI_D3cold); - return ret; -} - -/** * pci_set_power_state - Set the power state of a PCI device * @dev: PCI device to handle. * @state: PCI power state (D0, D1, D2, D3hot) to put the device into. @@ -1094,7 +1074,7 @@ static int __pci_complete_power_transiti */ int pci_set_power_state(struct pci_dev *dev, pci_power_t state) { - int error; + int error, ret; /* Bound the state we're entering */ if (state > PCI_D3cold) @@ -1132,10 +1112,12 @@ int pci_set_power_state(struct pci_dev * error = pci_raw_set_power_state(dev, state > PCI_D3hot ? PCI_D3hot : state); - if (!__pci_complete_power_transition(dev, state)) - error = 0; + ret = pci_platform_power_transition(dev, state); + /* Powering off a bridge may power off the whole hierarchy */ + if (!ret && state == PCI_D3cold) + pci_bus_set_current_state(dev->subordinate, PCI_D3cold); - return error; + return ret ? error : 0; } EXPORT_SYMBOL(pci_set_power_state);