Patchwork [RFC,3/3] PCI/PM: Disable PME poll for PCIe devices

login
register
mail settings
Submitter Huang Ying
Date Sept. 17, 2012, 8:54 a.m.
Message ID <1347872076-5260-4-git-send-email-ying.huang@intel.com>
Download mbox | patch
Permalink /patch/184322/
State Not Applicable
Headers show

Comments

Huang Ying - Sept. 17, 2012, 8:54 a.m.
PME poll is not necessary for PCIe devices, because PCIe devices use
in-band PME message and IRQ on PCIe port to report PME.

PME poll is useful for PCI devices.  Because for PCI devices, PME is
reported via a side-band PME# line and some platform logic, and the
platform logic is often missing on many systems.

Signed-off-by: Huang Ying <ying.huang@intel.com>
---
 drivers/pci/pcie/pme.c |    1 +
 1 file changed, 1 insertion(+)

--
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
Rafael J. Wysocki - Sept. 20, 2012, 7:31 p.m.
On Monday, September 17, 2012, Huang Ying wrote:
> PME poll is not necessary for PCIe devices, because PCIe devices use
> in-band PME message and IRQ on PCIe port to report PME.

Alas, not all of them as it turns out and even if they do, it doesn't
work for some of them. That's why we've added the PCIe devices polling
(quite recently, for that matter).

If you'd spent some time on some proper research regarding that (like browsing
the changelogs of git commits modifying the relevant part of drivers/pci/pci.c),
you'd have known that already.

And that actually is quite important, because I don't have to remember every
single PM-related change we're making in the PCI layer.  I _incidentally_ do
remember this one, but that may not happen next time.  Please do the research
_before_ proposing changes of this kind.

Thanks,
Rafael


> PME poll is useful for PCI devices.  Because for PCI devices, PME is
> reported via a side-band PME# line and some platform logic, and the
> platform logic is often missing on many systems.
> 
> Signed-off-by: Huang Ying <ying.huang@intel.com>
> ---
>  drivers/pci/pcie/pme.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- a/drivers/pci/pcie/pme.c
> +++ b/drivers/pci/pcie/pme.c
> @@ -313,6 +313,7 @@ static int pcie_pme_set_native(struct pc
>  
>  	device_set_run_wake(&dev->dev, true);
>  	dev->pme_interrupt = true;
> +	dev->pme_poll = false;
>  	return 0;
>  }
>  
> 
> 

--
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
Matthew Garrett - Sept. 20, 2012, 7:39 p.m.
On Thu, Sep 20, 2012 at 09:31:13PM +0200, Rafael J. Wysocki wrote:
> On Monday, September 17, 2012, Huang Ying wrote:
> > PME poll is not necessary for PCIe devices, because PCIe devices use
> > in-band PME message and IRQ on PCIe port to report PME.
> 
> Alas, not all of them as it turns out and even if they do, it doesn't
> work for some of them. That's why we've added the PCIe devices polling
> (quite recently, for that matter).

Right. I'd believe that there's some underlying bug that's causing the 
missing PMEs and maybe it does work in theory, but the reality is that 
right now some PCIe devices are not getting their PMEs delivered to us. 
We don't know if it's our bug or a hardware bug, and until we do we need 
to leave the polling code.
Huang Ying - Sept. 21, 2012, 1:50 a.m.
On Thu, 2012-09-20 at 21:31 +0200, Rafael J. Wysocki wrote:
> On Monday, September 17, 2012, Huang Ying wrote:
> > PME poll is not necessary for PCIe devices, because PCIe devices use
> > in-band PME message and IRQ on PCIe port to report PME.
> 
> Alas, not all of them as it turns out and even if they do, it doesn't
> work for some of them. That's why we've added the PCIe devices polling
> (quite recently, for that matter).
> 
> If you'd spent some time on some proper research regarding that (like browsing
> the changelogs of git commits modifying the relevant part of drivers/pci/pci.c),
> you'd have known that already.
> 
> And that actually is quite important, because I don't have to remember every
> single PM-related change we're making in the PCI layer.  I _incidentally_ do
> remember this one, but that may not happen next time.  Please do the research
> _before_ proposing changes of this kind.

Sorry.  I should have done more research before sending the patch out.
Will do more research next time.

Best Regards,
Huang Ying


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

--- a/drivers/pci/pcie/pme.c
+++ b/drivers/pci/pcie/pme.c
@@ -313,6 +313,7 @@  static int pcie_pme_set_native(struct pc
 
 	device_set_run_wake(&dev->dev, true);
 	dev->pme_interrupt = true;
+	dev->pme_poll = false;
 	return 0;
 }