[PATCH] PCI: Document connection between pci_power_t and hardware PM capability

[PATCH] PCI: Document connection between pci_power_t and hardware PM capability

[Bjorn Helgaas]

The dev.pme_support field, pci_pm_init(), and pci_pme_capable() depend on
the fact that the pci_power_t values (PCI_D0, PCI_D1, etc.) match the
definition of the PME_Support field of the Power Management Capabilities
register in the Power Management capability (see PCI Bus Power Management
spec r1.2, sec 3.2.3).

Add a note to this effect at the pci_power_t typedef.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 include/linux/pci.h |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 8597b42..85cede3 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -101,6 +101,10 @@ enum {
 	DEVICE_COUNT_RESOURCE = PCI_NUM_RESOURCES,
 };
 
+/*
+ * pci_power_t values must match the bits in the PME_Support field of the
+ * Power Management Capabilities register in the Power Management capability.
+ */
 typedef int __bitwise pci_power_t;
 
 #define PCI_D0		((pci_power_t __force) 0)

Re: [PATCH] PCI: Document connection between pci_power_t and hardware PM capability

[Lukas Wunner]

On Sat, Jun 18, 2016 at 01:00:56PM -0500, Bjorn Helgaas wrote:
> The dev.pme_support field, pci_pm_init(), and pci_pme_capable() depend on
> the fact that the pci_power_t values (PCI_D0, PCI_D1, etc.) match the
> definition of the PME_Support field of the Power Management Capabilities
> register in the Power Management capability (see PCI Bus Power Management
> spec r1.2, sec 3.2.3).

In addition, pci_power_t is assumed to match the power state in the
PM Control and Status Register, see pci_raw_set_power_state().
The PMCSR reserves only 2 bits for the power state, so it can only
represent D0 to D3hot. If the device is in D3cold, reads from its
config space usually result in an "all ones" fabricated response.
Thus D3hot and D3cold cannot be differentiated when reading the
power state in the PMCSR, they're represented by the same value.

Best regards,

Lukas

> 
> Add a note to this effect at the pci_power_t typedef.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  include/linux/pci.h |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 8597b42..85cede3 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -101,6 +101,10 @@ enum {
>  	DEVICE_COUNT_RESOURCE = PCI_NUM_RESOURCES,
>  };
>  
> +/*
> + * pci_power_t values must match the bits in the PME_Support field of the
> + * Power Management Capabilities register in the Power Management capability.
> + */
>  typedef int __bitwise pci_power_t;
>  
>  #define PCI_D0		((pci_power_t __force) 0)
> 

Re: [PATCH] PCI: Document connection between pci_power_t and hardware PM capability

[Andy Shevchenko]

On Sat, 2016-06-18 at 13:00 -0500, Bjorn Helgaas wrote:
> The dev.pme_support field, pci_pm_init(), and pci_pme_capable() depend
> on
> the fact that the pci_power_t values (PCI_D0, PCI_D1, etc.) match the
> definition of the PME_Support field of the Power Management
> Capabilities
> register in the Power Management capability (see PCI Bus Power
> Management
> spec r1.2, sec 3.2.3).

Nice to know! Thanks.

If it matters:
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> 
> Add a note to this effect at the pci_power_t typedef.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  include/linux/pci.h |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 8597b42..85cede3 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -101,6 +101,10 @@ enum {
>  	DEVICE_COUNT_RESOURCE = PCI_NUM_RESOURCES,
>  };
>  
> +/*
> + * pci_power_t values must match the bits in the PME_Support field of
> the
> + * Power Management Capabilities register in the Power Management
> capability.
> + */
>  typedef int __bitwise pci_power_t;
>  
>  #define PCI_D0		((pci_power_t __force) 0)
> 

-- 

Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH] PCI: Document connection between pci_power_t and hardware PM capability

[Mika Westerberg]

On Sat, Jun 18, 2016 at 01:00:56PM -0500, Bjorn Helgaas wrote:
> The dev.pme_support field, pci_pm_init(), and pci_pme_capable() depend on
> the fact that the pci_power_t values (PCI_D0, PCI_D1, etc.) match the
> definition of the PME_Support field of the Power Management Capabilities
> register in the Power Management capability (see PCI Bus Power Management
> spec r1.2, sec 3.2.3).
> 
> Add a note to this effect at the pci_power_t typedef.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Good idea,

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

Re: [PATCH] PCI: Document connection between pci_power_t and hardware PM capability

[Bjorn Helgaas]

On Sun, Jun 19, 2016 at 02:49:46PM +0300, Andy Shevchenko wrote:
> On Sat, 2016-06-18 at 13:00 -0500, Bjorn Helgaas wrote:
> > The dev.pme_support field, pci_pm_init(), and pci_pme_capable() depend
> > on
> > the fact that the pci_power_t values (PCI_D0, PCI_D1, etc.) match the
> > definition of the PME_Support field of the Power Management
> > Capabilities
> > register in the Power Management capability (see PCI Bus Power
> > Management
> > spec r1.2, sec 3.2.3).
> 
> Nice to know! Thanks.
> 
> If it matters:
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Definitely!  I updated based on Lukas' comment and added your and Mika's
Reviewed-by:

commit 224abb67e6eb5ac062de9239163136d5ec3155c8
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Fri Jun 17 15:23:52 2016 -0500

    PCI: Document connection between pci_power_t and hardware PM capability
    
    The dev.pme_support field, pci_pm_init(), pci_pme_capable(), and
    pci_raw_set_power_state() depend on the fact that the pci_power_t values
    (PCI_D0, PCI_D1, etc.) match the definition of the Capabilities PME_Support
    and the Control/Status PowerState fields in the Power Management capability
    (see PCI Bus Power Management spec r1.2, sec 3.2.3).
    
    Add a note to this effect at the pci_power_t typedef.
    
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
    Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 8597b42..0a1a9e3 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -101,6 +101,10 @@ enum {
 	DEVICE_COUNT_RESOURCE = PCI_NUM_RESOURCES,
 };
 
+/*
+ * pci_power_t values must match the bits in the Capabilities PME_Support
+ * and Control/Status PowerState fields in the Power Management capability.
+ */
 typedef int __bitwise pci_power_t;
 
 #define PCI_D0		((pci_power_t __force) 0)