diff mbox

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

Message ID 1347872076-5260-4-git-send-email-ying.huang@intel.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Huang, Ying Sept. 17, 2012, 8:54 a.m. UTC
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

Comments

Rafael Wysocki Sept. 20, 2012, 7:31 p.m. UTC | #1
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. UTC | #2
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. UTC | #3
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
diff mbox

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