diff mbox series

[5/5] PCI: PM: Fold __pci_complete_power_transition() into its caller

Message ID 1769241.yKxyosiRnQ@kreacher (mailing list archive)
State Accepted, archived
Headers show
Series PCI: PM: Cleanups related to power state changes | expand

Commit Message

Rafael J. Wysocki Nov. 5, 2019, 10:32 a.m. UTC 
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Because pci_set_power_state() has become the only caller of
__pci_complete_power_transition(), there is no need for the latter to
be a separate function any more, so fold it into the former, drop a
redundant check and reduce the number of lines of code somewhat.

Code rearrangement, no intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pci.c |   30 ++++++------------------------
 1 file changed, 6 insertions(+), 24 deletions(-)

Comments

Christoph Hellwig Nov. 5, 2019, 4:09 p.m. UTC | #1 
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;
Rafael J. Wysocki Nov. 5, 2019, 4:15 p.m. UTC | #2 
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.
diff mbox series

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);