linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] PCI: PM: Cleanups related to power state changes
@ 2019-11-05 10:11 Rafael J. Wysocki
  2019-11-05 10:13 ` [PATCH 1/5] PCI: PM: Move power state update away from pci_power_up() Rafael J. Wysocki
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Rafael J. Wysocki @ 2019-11-05 10:11 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: LKML, Linux PCI, Linux PM, Mika Westerberg

Hi,

This series rearranges some PCI power management code to make it somewhat
easier to follow and explicitly consolidate the power-up (transitions to
D0) code path.

It is not intended to change the functionality of the code.

Thanks,
Rafael




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/5] PCI: PM: Move power state update away from pci_power_up()
  2019-11-05 10:11 [PATCH 0/5] PCI: PM: Cleanups related to power state changes Rafael J. Wysocki
@ 2019-11-05 10:13 ` Rafael J. Wysocki
  2019-11-05 10:27 ` [PATCH 2/5] PCI: PM: Use pci_power_up() in pci_set_power_state() Rafael J. Wysocki
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Rafael J. Wysocki @ 2019-11-05 10:13 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: LKML, Linux PCI, Linux PM, Mika Westerberg

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Move the invocation of pci_update_current_state() from pci_power_up()
to pci_pm_default_resume_early(), which is the only caller of that
function.

Preparatory change, no functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pci-driver.c |    1 +
 drivers/pci/pci.c        |    1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

Index: linux-pm/drivers/pci/pci-driver.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-driver.c
+++ linux-pm/drivers/pci/pci-driver.c
@@ -524,6 +524,7 @@ static int pci_restore_standard_config(s
 static void pci_pm_default_resume_early(struct pci_dev *pci_dev)
 {
 	pci_power_up(pci_dev);
+	pci_update_current_state(pci_dev, PCI_D0);
 	pci_restore_state(pci_dev);
 	pci_pme_restore(pci_dev);
 }
Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -1148,7 +1148,6 @@ void pci_power_up(struct pci_dev *dev)
 {
 	__pci_start_power_transition(dev, PCI_D0);
 	pci_raw_set_power_state(dev, PCI_D0);
-	pci_update_current_state(dev, PCI_D0);
 }
 
 /**




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 2/5] PCI: PM: Use pci_power_up() in pci_set_power_state()
  2019-11-05 10:11 [PATCH 0/5] PCI: PM: Cleanups related to power state changes Rafael J. Wysocki
  2019-11-05 10:13 ` [PATCH 1/5] PCI: PM: Move power state update away from pci_power_up() Rafael J. Wysocki
@ 2019-11-05 10:27 ` Rafael J. Wysocki
  2019-11-05 10:29 ` [PATCH 3/5] PCI: PM: Fold __pci_start_power_transition() into its caller Rafael J. Wysocki
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Rafael J. Wysocki @ 2019-11-05 10:27 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: LKML, Linux PCI, Linux PM, Mika Westerberg

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Make it explicitly clear that the code to put devices into D0 in
pci_set_power_state() and in pci_pm_default_resume_early() is the
same by making the latter use pci_power_up() for transitions into
D0.

Code rearrangement, no intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This is on top of https://patchwork.kernel.org/patch/11223673/

---
 drivers/pci/pci.c |   25 +++++++++++++------------
 drivers/pci/pci.h |    2 +-
 2 files changed, 14 insertions(+), 13 deletions(-)

Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -1033,6 +1033,16 @@ static void __pci_start_power_transition
 }
 
 /**
+ * pci_power_up - Put the given device into D0
+ * @dev: PCI device to power up
+ */
+int pci_power_up(struct pci_dev *dev)
+{
+	__pci_start_power_transition(dev, PCI_D0);
+	return pci_raw_set_power_state(dev, PCI_D0);
+}
+
+/**
  * __pci_dev_set_current_state - Set current state of a PCI device
  * @dev: Device to handle
  * @data: pointer to state to be set
@@ -1117,6 +1127,9 @@ int pci_set_power_state(struct pci_dev *
 	if (dev->current_state == state)
 		return 0;
 
+	if (state == PCI_D0)
+		return pci_power_up(dev);
+
 	/*
 	 * This device is quirked not to be put into D3, so don't put it in
 	 * D3
@@ -1124,8 +1137,6 @@ int pci_set_power_state(struct pci_dev *
 	if (state >= PCI_D3hot && (dev->dev_flags & PCI_DEV_FLAGS_NO_D3))
 		return 0;
 
-	__pci_start_power_transition(dev, state);
-
 	/*
 	 * To put device in D3cold, we put device into D3hot in native
 	 * way, then put device into D3cold with platform ops
@@ -1141,16 +1152,6 @@ int pci_set_power_state(struct pci_dev *
 EXPORT_SYMBOL(pci_set_power_state);
 
 /**
- * pci_power_up - Put the given device into D0 forcibly
- * @dev: PCI device to power up
- */
-void pci_power_up(struct pci_dev *dev)
-{
-	__pci_start_power_transition(dev, PCI_D0);
-	pci_raw_set_power_state(dev, PCI_D0);
-}
-
-/**
  * pci_choose_state - Choose the power state of a PCI device
  * @dev: PCI device to be suspended
  * @state: target sleep state for the whole system. This is the value
Index: linux-pm/drivers/pci/pci.h
===================================================================
--- linux-pm.orig/drivers/pci/pci.h
+++ linux-pm/drivers/pci/pci.h
@@ -85,7 +85,7 @@ struct pci_platform_pm_ops {
 int pci_set_platform_pm(const struct pci_platform_pm_ops *ops);
 void pci_update_current_state(struct pci_dev *dev, pci_power_t state);
 void pci_refresh_power_state(struct pci_dev *dev);
-void pci_power_up(struct pci_dev *dev);
+int pci_power_up(struct pci_dev *dev);
 void pci_disable_enabled_device(struct pci_dev *dev);
 int pci_finish_runtime_suspend(struct pci_dev *dev);
 void pcie_clear_root_pme_status(struct pci_dev *dev);




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 3/5] PCI: PM: Fold __pci_start_power_transition() into its caller
  2019-11-05 10:11 [PATCH 0/5] PCI: PM: Cleanups related to power state changes Rafael J. Wysocki
  2019-11-05 10:13 ` [PATCH 1/5] PCI: PM: Move power state update away from pci_power_up() Rafael J. Wysocki
  2019-11-05 10:27 ` [PATCH 2/5] PCI: PM: Use pci_power_up() in pci_set_power_state() Rafael J. Wysocki
@ 2019-11-05 10:29 ` Rafael J. Wysocki
  2019-11-05 10:30 ` [PATCH 4/5] PCI: PM: Avoid exporting __pci_complete_power_transition() Rafael J. Wysocki
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Rafael J. Wysocki @ 2019-11-05 10:29 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: LKML, Linux PCI, Linux PM, Mika Westerberg

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Because pci_power_up() has become the only caller of
__pci_start_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 |   48 ++++++++++++++++++------------------------------
 1 file changed, 18 insertions(+), 30 deletions(-)

Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -1003,42 +1003,30 @@ void pci_wakeup_bus(struct pci_bus *bus)
 }
 
 /**
- * __pci_start_power_transition - Start power transition of a PCI device
- * @dev: PCI device to handle.
- * @state: State to put the device into.
+ * pci_power_up - Put the given device into D0
+ * @dev: PCI device to power up
  */
-static void __pci_start_power_transition(struct pci_dev *dev, pci_power_t state)
+int pci_power_up(struct pci_dev *dev)
 {
-	if (state == PCI_D0) {
-		pci_platform_power_transition(dev, PCI_D0);
+	pci_platform_power_transition(dev, PCI_D0);
+
+	/*
+	 * Mandatory power management transition delays, see PCI Express Base
+	 * Specification Revision 2.0 Section 6.6.1: Conventional Reset.  Do not
+	 * delay for devices powered on/off by corresponding bridge, because
+	 * have already delayed for the bridge.
+	 */
+	if (dev->runtime_d3cold) {
+		if (dev->d3cold_delay && !dev->imm_ready)
+			msleep(dev->d3cold_delay);
 		/*
-		 * Mandatory power management transition delays, see
-		 * PCI Express Base Specification Revision 2.0 Section
-		 * 6.6.1: Conventional Reset.  Do not delay for
-		 * devices powered on/off by corresponding bridge,
-		 * because have already delayed for the bridge.
+		 * When powering on a bridge from D3cold, the whole hierarchy
+		 * may be powered on into D0uninitialized state, resume them to
+		 * give them a chance to suspend again
 		 */
-		if (dev->runtime_d3cold) {
-			if (dev->d3cold_delay && !dev->imm_ready)
-				msleep(dev->d3cold_delay);
-			/*
-			 * When powering on a bridge from D3cold, the
-			 * whole hierarchy may be powered on into
-			 * D0uninitialized state, resume them to give
-			 * them a chance to suspend again
-			 */
-			pci_wakeup_bus(dev->subordinate);
-		}
+		pci_wakeup_bus(dev->subordinate);
 	}
-}
 
-/**
- * pci_power_up - Put the given device into D0
- * @dev: PCI device to power up
- */
-int pci_power_up(struct pci_dev *dev)
-{
-	__pci_start_power_transition(dev, PCI_D0);
 	return pci_raw_set_power_state(dev, PCI_D0);
 }
 




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 4/5] PCI: PM: Avoid exporting __pci_complete_power_transition()
  2019-11-05 10:11 [PATCH 0/5] PCI: PM: Cleanups related to power state changes Rafael J. Wysocki
                   ` (2 preceding siblings ...)
  2019-11-05 10:29 ` [PATCH 3/5] PCI: PM: Fold __pci_start_power_transition() into its caller Rafael J. Wysocki
@ 2019-11-05 10:30 ` Rafael J. Wysocki
  2019-11-05 10:32 ` [PATCH 5/5] PCI: PM: Fold __pci_complete_power_transition() into its caller Rafael J. Wysocki
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Rafael J. Wysocki @ 2019-11-05 10:30 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: LKML, Linux PCI, Linux PM, Mika Westerberg, Alex Deucher, amd-gfx

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Notice that radeon_set_suspend(), which is the only caller of
__pci_complete_power_transition() outside of pci.c, really only
cares about the pci_platform_power_transition() invoked by it,
so export the latter instead of it, update the radeon driver to
call pci_platform_power_transition() directly and make
__pci_complete_power_transition() static.

Code rearrangement, no intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pci.c                   |    6 +++---
 drivers/video/fbdev/aty/radeon_pm.c |    2 +-
 include/linux/pci.h                 |    2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -963,7 +963,7 @@ void pci_refresh_power_state(struct pci_
  * @dev: PCI device to handle.
  * @state: State to put the device into.
  */
-static int pci_platform_power_transition(struct pci_dev *dev, pci_power_t state)
+int pci_platform_power_transition(struct pci_dev *dev, pci_power_t state)
 {
 	int error;
 
@@ -979,6 +979,7 @@ static int pci_platform_power_transition
 
 	return error;
 }
+EXPORT_SYMBOL_GPL(pci_platform_power_transition);
 
 /**
  * pci_wakeup - Wake up a PCI device
@@ -1061,7 +1062,7 @@ void pci_bus_set_current_state(struct pc
  *
  * This function should not be called directly by device drivers.
  */
-int __pci_complete_power_transition(struct pci_dev *dev, pci_power_t state)
+static int __pci_complete_power_transition(struct pci_dev *dev, pci_power_t state)
 {
 	int ret;
 
@@ -1073,7 +1074,6 @@ int __pci_complete_power_transition(stru
 		pci_bus_set_current_state(dev->subordinate, PCI_D3cold);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(__pci_complete_power_transition);
 
 /**
  * pci_set_power_state - Set the power state of a PCI device
Index: linux-pm/drivers/video/fbdev/aty/radeon_pm.c
===================================================================
--- linux-pm.orig/drivers/video/fbdev/aty/radeon_pm.c
+++ linux-pm/drivers/video/fbdev/aty/radeon_pm.c
@@ -2593,7 +2593,7 @@ static void radeon_set_suspend(struct ra
 		 * calling pci_set_power_state()
 		 */
 		radeonfb_whack_power_state(rinfo, PCI_D2);
-		__pci_complete_power_transition(rinfo->pdev, PCI_D2);
+		pci_platform_power_transition(rinfo->pdev, PCI_D2);
 	} else {
 		printk(KERN_DEBUG "radeonfb (%s): switching to D0 state...\n",
 		       pci_name(rinfo->pdev));
Index: linux-pm/include/linux/pci.h
===================================================================
--- linux-pm.orig/include/linux/pci.h
+++ linux-pm/include/linux/pci.h
@@ -1232,7 +1232,7 @@ struct pci_cap_saved_state *pci_find_sav
 int pci_add_cap_save_buffer(struct pci_dev *dev, char cap, unsigned int size);
 int pci_add_ext_cap_save_buffer(struct pci_dev *dev,
 				u16 cap, unsigned int size);
-int __pci_complete_power_transition(struct pci_dev *dev, pci_power_t state);
+int pci_platform_power_transition(struct pci_dev *dev, pci_power_t state);
 int pci_set_power_state(struct pci_dev *dev, pci_power_t state);
 pci_power_t pci_choose_state(struct pci_dev *dev, pm_message_t state);
 bool pci_pme_capable(struct pci_dev *dev, pci_power_t state);




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 5/5] PCI: PM: Fold __pci_complete_power_transition() into its caller
  2019-11-05 10:11 [PATCH 0/5] PCI: PM: Cleanups related to power state changes Rafael J. Wysocki
                   ` (3 preceding siblings ...)
  2019-11-05 10:30 ` [PATCH 4/5] PCI: PM: Avoid exporting __pci_complete_power_transition() Rafael J. Wysocki
@ 2019-11-05 10:32 ` Rafael J. Wysocki
  2019-11-05 16:09   ` Christoph Hellwig
  2019-11-05 11:28 ` [PATCH 0/5] PCI: PM: Cleanups related to power state changes Rafael J. Wysocki
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Rafael J. Wysocki @ 2019-11-05 10:32 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: LKML, Linux PCI, Linux PM, Mika Westerberg

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

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




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] PCI: PM: Cleanups related to power state changes
  2019-11-05 10:11 [PATCH 0/5] PCI: PM: Cleanups related to power state changes Rafael J. Wysocki
                   ` (4 preceding siblings ...)
  2019-11-05 10:32 ` [PATCH 5/5] PCI: PM: Fold __pci_complete_power_transition() into its caller Rafael J. Wysocki
@ 2019-11-05 11:28 ` Rafael J. Wysocki
  2019-11-05 22:35   ` Bjorn Helgaas
  2019-11-05 13:02 ` Mika Westerberg
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Rafael J. Wysocki @ 2019-11-05 11:28 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: LKML, Linux PCI, Linux PM, Mika Westerberg

On Tuesday, November 5, 2019 11:11:57 AM CET Rafael J. Wysocki wrote:
> Hi,
> 
> This series rearranges some PCI power management code to make it somewhat
> easier to follow and explicitly consolidate the power-up (transitions to
> D0) code path.
> 
> It is not intended to change the functionality of the code.

This series applies on top of 5.4-rc6 with your pci/pm-2 branch from today
merged on top of it.

I guess I can make it apply on top of pci/pm-2, but there were some PCI PM
changes in 5.4-rc later than -rc1 in that area and they need to be taken
into account anyway.

Cheers!




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] PCI: PM: Cleanups related to power state changes
  2019-11-05 10:11 [PATCH 0/5] PCI: PM: Cleanups related to power state changes Rafael J. Wysocki
                   ` (5 preceding siblings ...)
  2019-11-05 11:28 ` [PATCH 0/5] PCI: PM: Cleanups related to power state changes Rafael J. Wysocki
@ 2019-11-05 13:02 ` Mika Westerberg
  2019-11-05 16:32 ` [PATCH update 5/5] PCI: PM: Fold __pci_complete_power_transition() into its caller Rafael J. Wysocki
  2019-11-06 19:02 ` [PATCH 0/5] PCI: PM: Cleanups related to power state changes Bjorn Helgaas
  8 siblings, 0 replies; 17+ messages in thread
From: Mika Westerberg @ 2019-11-05 13:02 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Bjorn Helgaas, LKML, Linux PCI, Linux PM

On Tue, Nov 05, 2019 at 11:11:57AM +0100, Rafael J. Wysocki wrote:
> Hi,
> 
> This series rearranges some PCI power management code to make it somewhat
> easier to follow and explicitly consolidate the power-up (transitions to
> D0) code path.
> 
> It is not intended to change the functionality of the code.

The whole series looks good to me,

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/5] PCI: PM: Fold __pci_complete_power_transition() into its caller
  2019-11-05 10:32 ` [PATCH 5/5] PCI: PM: Fold __pci_complete_power_transition() into its caller Rafael J. Wysocki
@ 2019-11-05 16:09   ` Christoph Hellwig
  2019-11-05 16:15     ` Rafael J. Wysocki
  0 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2019-11-05 16:09 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Bjorn Helgaas, LKML, Linux PCI, Linux PM, Mika Westerberg

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;


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/5] PCI: PM: Fold __pci_complete_power_transition() into its caller
  2019-11-05 16:09   ` Christoph Hellwig
@ 2019-11-05 16:15     ` Rafael J. Wysocki
  0 siblings, 0 replies; 17+ messages in thread
From: Rafael J. Wysocki @ 2019-11-05 16:15 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Rafael J. Wysocki, Bjorn Helgaas, LKML, Linux PCI, Linux PM,
	Mika Westerberg

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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH update 5/5] PCI: PM: Fold __pci_complete_power_transition() into its caller
  2019-11-05 10:11 [PATCH 0/5] PCI: PM: Cleanups related to power state changes Rafael J. Wysocki
                   ` (6 preceding siblings ...)
  2019-11-05 13:02 ` Mika Westerberg
@ 2019-11-05 16:32 ` Rafael J. Wysocki
  2019-11-06 19:02 ` [PATCH 0/5] PCI: PM: Cleanups related to power state changes Bjorn Helgaas
  8 siblings, 0 replies; 17+ messages in thread
From: Rafael J. Wysocki @ 2019-11-05 16:32 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: LKML, Linux PCI, Linux PM, Mika Westerberg, Christoph Hellwig

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

Changes from the original:
 - Avoid using the extra local var in pci_set_power_state() (Christoph).

---
 drivers/pci/pci.c |   30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)

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.
@@ -1132,10 +1112,14 @@ 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;
+	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 error;
+	return 0;
 }
 EXPORT_SYMBOL(pci_set_power_state);
 




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] PCI: PM: Cleanups related to power state changes
  2019-11-05 11:28 ` [PATCH 0/5] PCI: PM: Cleanups related to power state changes Rafael J. Wysocki
@ 2019-11-05 22:35   ` Bjorn Helgaas
  2019-11-05 22:48     ` Rafael J. Wysocki
  0 siblings, 1 reply; 17+ messages in thread
From: Bjorn Helgaas @ 2019-11-05 22:35 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: LKML, Linux PCI, Linux PM, Mika Westerberg

On Tue, Nov 05, 2019 at 12:28:47PM +0100, Rafael J. Wysocki wrote:
> On Tuesday, November 5, 2019 11:11:57 AM CET Rafael J. Wysocki wrote:
> > Hi,
> > 
> > This series rearranges some PCI power management code to make it somewhat
> > easier to follow and explicitly consolidate the power-up (transitions to
> > D0) code path.
> > 
> > It is not intended to change the functionality of the code.
> 
> This series applies on top of 5.4-rc6 with your pci/pm-2 branch from today
> merged on top of it.
> 
> I guess I can make it apply on top of pci/pm-2, but there were some PCI PM
> changes in 5.4-rc later than -rc1 in that area and they need to be taken
> into account anyway.

I applied the commits from pci/pm-2 to pci/pm (pci/pm-2 was really
just to get the 0-day robot to build test it).

pci/pm is based on v5.4-rc1, which doesn't have 45144d42f299 ("PCI:
PM: Fix pci_power_up()"), which appeared in -rc4.

All my branches are based on -rc1.  I *could* rebase them all to -rc4,
but that's quite a bit of work and affects Lorenzo as well, so I'd
rather not.  What's the git expert's way of doing this?

I guess worst case I could rebase this series to apply on pci/pm
(-rc1-based), accept that Linus will see a conflict, and resolve it
during his merge.

Bjorn

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] PCI: PM: Cleanups related to power state changes
  2019-11-05 22:35   ` Bjorn Helgaas
@ 2019-11-05 22:48     ` Rafael J. Wysocki
  0 siblings, 0 replies; 17+ messages in thread
From: Rafael J. Wysocki @ 2019-11-05 22:48 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Rafael J. Wysocki, LKML, Linux PCI, Linux PM, Mika Westerberg

On Tue, Nov 5, 2019 at 11:35 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Tue, Nov 05, 2019 at 12:28:47PM +0100, Rafael J. Wysocki wrote:
> > On Tuesday, November 5, 2019 11:11:57 AM CET Rafael J. Wysocki wrote:
> > > Hi,
> > >
> > > This series rearranges some PCI power management code to make it somewhat
> > > easier to follow and explicitly consolidate the power-up (transitions to
> > > D0) code path.
> > >
> > > It is not intended to change the functionality of the code.
> >
> > This series applies on top of 5.4-rc6 with your pci/pm-2 branch from today
> > merged on top of it.
> >
> > I guess I can make it apply on top of pci/pm-2, but there were some PCI PM
> > changes in 5.4-rc later than -rc1 in that area and they need to be taken
> > into account anyway.
>
> I applied the commits from pci/pm-2 to pci/pm (pci/pm-2 was really
> just to get the 0-day robot to build test it).
>
> pci/pm is based on v5.4-rc1, which doesn't have 45144d42f299 ("PCI:
> PM: Fix pci_power_up()"), which appeared in -rc4.
>
> All my branches are based on -rc1.  I *could* rebase them all to -rc4,
> but that's quite a bit of work and affects Lorenzo as well, so I'd
> rather not.  What's the git expert's way of doing this?

It is not necessary to rebase.

I would just merge the current pci/pm on top of v5.4-rc4 (or any later
-rc), commit my patches on top of that and push the result as the new
pci/pm.  That should be a fast-forward update.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] PCI: PM: Cleanups related to power state changes
  2019-11-05 10:11 [PATCH 0/5] PCI: PM: Cleanups related to power state changes Rafael J. Wysocki
                   ` (7 preceding siblings ...)
  2019-11-05 16:32 ` [PATCH update 5/5] PCI: PM: Fold __pci_complete_power_transition() into its caller Rafael J. Wysocki
@ 2019-11-06 19:02 ` Bjorn Helgaas
  2019-11-07 13:33   ` Rafael J. Wysocki
  2019-11-08 11:49   ` Rafael J. Wysocki
  8 siblings, 2 replies; 17+ messages in thread
From: Bjorn Helgaas @ 2019-11-06 19:02 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: LKML, Linux PCI, Linux PM, Mika Westerberg

On Tue, Nov 05, 2019 at 11:11:57AM +0100, Rafael J. Wysocki wrote:
> Hi,
> 
> This series rearranges some PCI power management code to make it somewhat
> easier to follow and explicitly consolidate the power-up (transitions to
> D0) code path.
> 
> It is not intended to change the functionality of the code.

Applied with Mika's reviewed-by to pci/pm for v5.5, thanks!

Thanks for the git tips, too!

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] PCI: PM: Cleanups related to power state changes
  2019-11-06 19:02 ` [PATCH 0/5] PCI: PM: Cleanups related to power state changes Bjorn Helgaas
@ 2019-11-07 13:33   ` Rafael J. Wysocki
  2019-11-08 11:49   ` Rafael J. Wysocki
  1 sibling, 0 replies; 17+ messages in thread
From: Rafael J. Wysocki @ 2019-11-07 13:33 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: LKML, Linux PCI, Linux PM, Mika Westerberg

On Wednesday, November 6, 2019 8:02:50 PM CET Bjorn Helgaas wrote:
> On Tue, Nov 05, 2019 at 11:11:57AM +0100, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > This series rearranges some PCI power management code to make it somewhat
> > easier to follow and explicitly consolidate the power-up (transitions to
> > D0) code path.
> > 
> > It is not intended to change the functionality of the code.
> 
> Applied with Mika's reviewed-by to pci/pm for v5.5, thanks!

Thank you!

> Thanks for the git tips, too!

You're vwelcome. :-)




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] PCI: PM: Cleanups related to power state changes
  2019-11-06 19:02 ` [PATCH 0/5] PCI: PM: Cleanups related to power state changes Bjorn Helgaas
  2019-11-07 13:33   ` Rafael J. Wysocki
@ 2019-11-08 11:49   ` Rafael J. Wysocki
  2019-11-08 20:13     ` Bjorn Helgaas
  1 sibling, 1 reply; 17+ messages in thread
From: Rafael J. Wysocki @ 2019-11-08 11:49 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: LKML, Linux PCI, Linux PM, Mika Westerberg

On Wednesday, November 6, 2019 8:02:50 PM CET Bjorn Helgaas wrote:
> On Tue, Nov 05, 2019 at 11:11:57AM +0100, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > This series rearranges some PCI power management code to make it somewhat
> > easier to follow and explicitly consolidate the power-up (transitions to
> > D0) code path.
> > 
> > It is not intended to change the functionality of the code.
> 
> Applied with Mika's reviewed-by to pci/pm for v5.5, thanks!

Any chance to push this out, though?  That would help with integration/testing
a bit ...




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] PCI: PM: Cleanups related to power state changes
  2019-11-08 11:49   ` Rafael J. Wysocki
@ 2019-11-08 20:13     ` Bjorn Helgaas
  0 siblings, 0 replies; 17+ messages in thread
From: Bjorn Helgaas @ 2019-11-08 20:13 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: LKML, Linux PCI, Linux PM, Mika Westerberg

On Fri, Nov 08, 2019 at 12:49:59PM +0100, Rafael J. Wysocki wrote:
> On Wednesday, November 6, 2019 8:02:50 PM CET Bjorn Helgaas wrote:
> > On Tue, Nov 05, 2019 at 11:11:57AM +0100, Rafael J. Wysocki wrote:
> > > Hi,
> > > 
> > > This series rearranges some PCI power management code to make it somewhat
> > > easier to follow and explicitly consolidate the power-up (transitions to
> > > D0) code path.
> > > 
> > > It is not intended to change the functionality of the code.
> > 
> > Applied with Mika's reviewed-by to pci/pm for v5.5, thanks!
> 
> Any chance to push this out, though?  That would help with integration/testing
> a bit ...

Done, thanks for the reminder.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2019-11-08 20:13 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-05 10:11 [PATCH 0/5] PCI: PM: Cleanups related to power state changes Rafael J. Wysocki
2019-11-05 10:13 ` [PATCH 1/5] PCI: PM: Move power state update away from pci_power_up() Rafael J. Wysocki
2019-11-05 10:27 ` [PATCH 2/5] PCI: PM: Use pci_power_up() in pci_set_power_state() Rafael J. Wysocki
2019-11-05 10:29 ` [PATCH 3/5] PCI: PM: Fold __pci_start_power_transition() into its caller Rafael J. Wysocki
2019-11-05 10:30 ` [PATCH 4/5] PCI: PM: Avoid exporting __pci_complete_power_transition() Rafael J. Wysocki
2019-11-05 10:32 ` [PATCH 5/5] PCI: PM: Fold __pci_complete_power_transition() into its caller Rafael J. Wysocki
2019-11-05 16:09   ` Christoph Hellwig
2019-11-05 16:15     ` Rafael J. Wysocki
2019-11-05 11:28 ` [PATCH 0/5] PCI: PM: Cleanups related to power state changes Rafael J. Wysocki
2019-11-05 22:35   ` Bjorn Helgaas
2019-11-05 22:48     ` Rafael J. Wysocki
2019-11-05 13:02 ` Mika Westerberg
2019-11-05 16:32 ` [PATCH update 5/5] PCI: PM: Fold __pci_complete_power_transition() into its caller Rafael J. Wysocki
2019-11-06 19:02 ` [PATCH 0/5] PCI: PM: Cleanups related to power state changes Bjorn Helgaas
2019-11-07 13:33   ` Rafael J. Wysocki
2019-11-08 11:49   ` Rafael J. Wysocki
2019-11-08 20:13     ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).