Patchwork [RFC,v5,8/8] PCI/PCIe: only claim PME from firmware when CONFIG_PCIE_PME is enabled

login
register
mail settings
Submitter Jiang Liu
Date Jan. 18, 2013, 4:07 p.m.
Message ID <1358525267-14268-9-git-send-email-jiang.liu@huawei.com>
Download mbox | patch
Permalink /patch/213660/
State Changes Requested
Headers show

Comments

Jiang Liu - Jan. 18, 2013, 4:07 p.m.
If CONFIG_PCIE_PME is not defined, system should avoid claiming PME from
firmware so firmware could still manage PME events for those devices.
Also don't create PCIe port device for PME service if CONFIG_PCIE_PME
is not defined.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 drivers/acpi/pci_root.c         |    5 +++--
 drivers/pci/pcie/portdrv_core.c |    4 +++-
 2 files changed, 6 insertions(+), 3 deletions(-)
Rafael J. Wysocki - Jan. 20, 2013, 11:43 p.m.
On Saturday, January 19, 2013 12:07:46 AM Jiang Liu wrote:
> If CONFIG_PCIE_PME is not defined, system should avoid claiming PME from
> firmware so firmware could still manage PME events for those devices.
> Also don't create PCIe port device for PME service if CONFIG_PCIE_PME
> is not defined.

No, this isn't correct.

We know from experience that it is in fact necessary to request control of
either all PCIe native features or none of them.  Anything else leads to
very "interesting" failure modes on some systems.

I think we should just remove CONFIG_PCIE_PME instead.

Thanks,
Rafael


> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> ---
>  drivers/acpi/pci_root.c         |    5 +++--
>  drivers/pci/pcie/portdrv_core.c |    4 +++-
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index c37eedb..7f7e464 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -550,8 +550,9 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
>  
>  	if (!pcie_ports_disabled
>  	    && (flags & ACPI_PCIE_REQ_SUPPORT) == ACPI_PCIE_REQ_SUPPORT) {
> -		flags = OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL
> -			| OSC_PCI_EXPRESS_PME_CONTROL;
> +		flags = OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL;
> +		if (IS_ENABLED(CONFIG_PCIE_PME))
> +			flags |= OSC_PCI_EXPRESS_PME_CONTROL;
>  		if (!pcie_native_hotplug_disabled)
>  			flags |= OSC_PCI_EXPRESS_NATIVE_HP_CONTROL;
>  
> diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
> index e7e1679..7e6546f 100644
> --- a/drivers/pci/pcie/portdrv_core.c
> +++ b/drivers/pci/pcie/portdrv_core.c
> @@ -263,7 +263,9 @@ static int get_port_device_capability(struct pci_dev *dev)
>  
>  	err = pcie_port_platform_notify(dev, &cap_mask);
>  	if (!pcie_ports_auto) {
> -		cap_mask = PCIE_PORT_SERVICE_PME | PCIE_PORT_SERVICE_VC;
> +		cap_mask = PCIE_PORT_SERVICE_VC;
> +		if (IS_ENABLED(CONFIG_PCIE_PME))
> +			cap_mask |= PCIE_PORT_SERVICE_PME;
>  		if (!pcie_native_hotplug_disabled)
>  			cap_mask |= PCIE_PORT_SERVICE_HP;
>  		if (pci_aer_available())
>
Jiang Liu - Jan. 21, 2013, 5:06 p.m.
On 01/21/2013 07:43 AM, Rafael J. Wysocki wrote:
> On Saturday, January 19, 2013 12:07:46 AM Jiang Liu wrote:
>> If CONFIG_PCIE_PME is not defined, system should avoid claiming PME from
>> firmware so firmware could still manage PME events for those devices.
>> Also don't create PCIe port device for PME service if CONFIG_PCIE_PME
>> is not defined.
> 
> No, this isn't correct.
> 
> We know from experience that it is in fact necessary to request control of
> either all PCIe native features or none of them.  Anything else leads to
> very "interesting" failure modes on some systems.
> 
> I think we should just remove CONFIG_PCIE_PME instead.
Hi Rafael,
	Thanks for reminder, don't know these tricky things about BIOS:)
Seems we can't easily remove CONFIG_PCIE_PME currently, so I will just
drop this patch.
Regards!
Gerry

> 
> Thanks,
> Rafael
> 
> 
>> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
>> ---
>>  drivers/acpi/pci_root.c         |    5 +++--
>>  drivers/pci/pcie/portdrv_core.c |    4 +++-
>>  2 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>> index c37eedb..7f7e464 100644
>> --- a/drivers/acpi/pci_root.c
>> +++ b/drivers/acpi/pci_root.c
>> @@ -550,8 +550,9 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
>>  
>>  	if (!pcie_ports_disabled
>>  	    && (flags & ACPI_PCIE_REQ_SUPPORT) == ACPI_PCIE_REQ_SUPPORT) {
>> -		flags = OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL
>> -			| OSC_PCI_EXPRESS_PME_CONTROL;
>> +		flags = OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL;
>> +		if (IS_ENABLED(CONFIG_PCIE_PME))
>> +			flags |= OSC_PCI_EXPRESS_PME_CONTROL;
>>  		if (!pcie_native_hotplug_disabled)
>>  			flags |= OSC_PCI_EXPRESS_NATIVE_HP_CONTROL;
>>  
>> diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
>> index e7e1679..7e6546f 100644
>> --- a/drivers/pci/pcie/portdrv_core.c
>> +++ b/drivers/pci/pcie/portdrv_core.c
>> @@ -263,7 +263,9 @@ static int get_port_device_capability(struct pci_dev *dev)
>>  
>>  	err = pcie_port_platform_notify(dev, &cap_mask);
>>  	if (!pcie_ports_auto) {
>> -		cap_mask = PCIE_PORT_SERVICE_PME | PCIE_PORT_SERVICE_VC;
>> +		cap_mask = PCIE_PORT_SERVICE_VC;
>> +		if (IS_ENABLED(CONFIG_PCIE_PME))
>> +			cap_mask |= PCIE_PORT_SERVICE_PME;
>>  		if (!pcie_native_hotplug_disabled)
>>  			cap_mask |= PCIE_PORT_SERVICE_HP;
>>  		if (pci_aer_available())
>>

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

Patch

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index c37eedb..7f7e464 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -550,8 +550,9 @@  static int __devinit acpi_pci_root_add(struct acpi_device *device)
 
 	if (!pcie_ports_disabled
 	    && (flags & ACPI_PCIE_REQ_SUPPORT) == ACPI_PCIE_REQ_SUPPORT) {
-		flags = OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL
-			| OSC_PCI_EXPRESS_PME_CONTROL;
+		flags = OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL;
+		if (IS_ENABLED(CONFIG_PCIE_PME))
+			flags |= OSC_PCI_EXPRESS_PME_CONTROL;
 		if (!pcie_native_hotplug_disabled)
 			flags |= OSC_PCI_EXPRESS_NATIVE_HP_CONTROL;
 
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index e7e1679..7e6546f 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -263,7 +263,9 @@  static int get_port_device_capability(struct pci_dev *dev)
 
 	err = pcie_port_platform_notify(dev, &cap_mask);
 	if (!pcie_ports_auto) {
-		cap_mask = PCIE_PORT_SERVICE_PME | PCIE_PORT_SERVICE_VC;
+		cap_mask = PCIE_PORT_SERVICE_VC;
+		if (IS_ENABLED(CONFIG_PCIE_PME))
+			cap_mask |= PCIE_PORT_SERVICE_PME;
 		if (!pcie_native_hotplug_disabled)
 			cap_mask |= PCIE_PORT_SERVICE_HP;
 		if (pci_aer_available())