diff mbox

[6/9] PCI: Unfold conditions to block runtime PM on PCIe ports

Message ID ba2f9f322b4aeea1aeb01f98a2eda8efe055494f.1476875113.git.lukas@wunner.de
State Superseded
Headers show

Commit Message

Lukas Wunner Oct. 19, 2016, 2:07 p.m. UTC
The conditions to block D3 on parent ports are currently condensed into
a single expression in pci_dev_check_d3cold().  Upcoming commits will
add further conditions for hotplug ports, making this expression fairly
large and impenetrable.  Unfold the conditions to maintain readability
when they are amended.

No functional change intended.

Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/pci.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

Comments

Mika Westerberg Oct. 20, 2016, 2:18 p.m. UTC | #1
On Wed, Oct 19, 2016 at 04:07:13PM +0200, Lukas Wunner wrote:
> The conditions to block D3 on parent ports are currently condensed into
> a single expression in pci_dev_check_d3cold().  Upcoming commits will
> add further conditions for hotplug ports, making this expression fairly
> large and impenetrable.  Unfold the conditions to maintain readability
> when they are amended.
> 
> No functional change intended.

This actually results a functional change because now all the conditions
are evaluated whereas previosly it bailed out immediately when the
condition was true ;-)

Not that it matters here.

> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  drivers/pci/pci.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index a19056e..f1ddb6b 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2270,19 +2270,20 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
>  static int pci_dev_check_d3cold(struct pci_dev *dev, void *data)
>  {
>  	bool *d3cold_ok = data;
> -	bool no_d3cold;
>  
> -	/*
> -	 * The device needs to be allowed to go D3cold and if it is wake
> -	 * capable to do so from D3cold.
> -	 */
> -	no_d3cold = dev->no_d3cold || !dev->d3cold_allowed ||
> -		(device_may_wakeup(&dev->dev) && !pci_pme_capable(dev, PCI_D3cold)) ||
> -		!pci_power_manageable(dev);
> +	/* The device needs to be allowed to go D3cold. */
> +	if (dev->no_d3cold || !dev->d3cold_allowed)
> +		*d3cold_ok = false;
> +
> +	/* If it is wakeup capable it must be able to do so from D3cold. */
> +	if (device_may_wakeup(&dev->dev) && !pci_pme_capable(dev, PCI_D3cold))
> +		*d3cold_ok = false;
>  
> -	*d3cold_ok = !no_d3cold;
> +	/* If it is a bridge it must be allowed to go to D3. */
> +	if (!pci_power_manageable(dev))
> +		*d3cold_ok = false;
>  
> -	return no_d3cold;
> +	return !*d3cold_ok;
>  }
>  
>  /*
> -- 
> 2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lukas Wunner Oct. 25, 2016, 5:28 a.m. UTC | #2
On Thu, Oct 20, 2016 at 05:18:10PM +0300, Mika Westerberg wrote:
> On Wed, Oct 19, 2016 at 04:07:13PM +0200, Lukas Wunner wrote:
> > The conditions to block D3 on parent ports are currently condensed into
> > a single expression in pci_dev_check_d3cold().  Upcoming commits will
> > add further conditions for hotplug ports, making this expression fairly
> > large and impenetrable.  Unfold the conditions to maintain readability
> > when they are amended.
> > 
> > No functional change intended.
> 
> This actually results a functional change because now all the conditions
> are evaluated whereas previosly it bailed out immediately when the
> condition was true ;-)

You're right. That's a step backward, good catch.

I've reworked this commit to restore the old behaviour and will respin
after some more testing and comparing disassembler output.

Thanks!

Lukas

> 
> Not that it matters here.
> 
> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > ---
> >  drivers/pci/pci.c | 21 +++++++++++----------
> >  1 file changed, 11 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index a19056e..f1ddb6b 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -2270,19 +2270,20 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
> >  static int pci_dev_check_d3cold(struct pci_dev *dev, void *data)
> >  {
> >  	bool *d3cold_ok = data;
> > -	bool no_d3cold;
> >  
> > -	/*
> > -	 * The device needs to be allowed to go D3cold and if it is wake
> > -	 * capable to do so from D3cold.
> > -	 */
> > -	no_d3cold = dev->no_d3cold || !dev->d3cold_allowed ||
> > -		(device_may_wakeup(&dev->dev) && !pci_pme_capable(dev, PCI_D3cold)) ||
> > -		!pci_power_manageable(dev);
> > +	/* The device needs to be allowed to go D3cold. */
> > +	if (dev->no_d3cold || !dev->d3cold_allowed)
> > +		*d3cold_ok = false;
> > +
> > +	/* If it is wakeup capable it must be able to do so from D3cold. */
> > +	if (device_may_wakeup(&dev->dev) && !pci_pme_capable(dev, PCI_D3cold))
> > +		*d3cold_ok = false;
> >  
> > -	*d3cold_ok = !no_d3cold;
> > +	/* If it is a bridge it must be allowed to go to D3. */
> > +	if (!pci_power_manageable(dev))
> > +		*d3cold_ok = false;
> >  
> > -	return no_d3cold;
> > +	return !*d3cold_ok;
> >  }
> >  
> >  /*
> > -- 
> > 2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index a19056e..f1ddb6b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2270,19 +2270,20 @@  bool pci_bridge_d3_possible(struct pci_dev *bridge)
 static int pci_dev_check_d3cold(struct pci_dev *dev, void *data)
 {
 	bool *d3cold_ok = data;
-	bool no_d3cold;
 
-	/*
-	 * The device needs to be allowed to go D3cold and if it is wake
-	 * capable to do so from D3cold.
-	 */
-	no_d3cold = dev->no_d3cold || !dev->d3cold_allowed ||
-		(device_may_wakeup(&dev->dev) && !pci_pme_capable(dev, PCI_D3cold)) ||
-		!pci_power_manageable(dev);
+	/* The device needs to be allowed to go D3cold. */
+	if (dev->no_d3cold || !dev->d3cold_allowed)
+		*d3cold_ok = false;
+
+	/* If it is wakeup capable it must be able to do so from D3cold. */
+	if (device_may_wakeup(&dev->dev) && !pci_pme_capable(dev, PCI_D3cold))
+		*d3cold_ok = false;
 
-	*d3cold_ok = !no_d3cold;
+	/* If it is a bridge it must be allowed to go to D3. */
+	if (!pci_power_manageable(dev))
+		*d3cold_ok = false;
 
-	return no_d3cold;
+	return !*d3cold_ok;
 }
 
 /*