Re: [PATCH] PCIe/PM: Do not suspend port if any subordinate device need PME polling
On Sun, Dec 16, 2012 at 6:11 AM, Richard Yang
wrote:
> On Sat, Dec 15, 2012 at 12:03:33AM +0100, Rafael J. Wysocki wrote:
>>On Friday, December 14, 2012 01:11:31 PM Richard Yang wrote:
>>> On Fri, Dec 14, 2012 at 10:52:11AM +0800, Huang Ying wrote:
>>> >In
>>> >
>>> > http://www.mail-archive.com/linux-usb@vger.kernel.org/msg07976.html
>>> >
>>> >Ulrich reported that his USB3 cardreader does not work reliably when
>>> >connected to the USB3 port. It turns out that USB3 controller failed
>>> >to be waken up when plugging in the USB3 cardreader. Further
>>> >experiment found that the USB3 host controller can only be waken up
>>> >via polling, while not via PME interrupt. But if the PCIe port that
>>> >the USB3 host controller is connected is suspended, we can not poll
>>> >the USB3 host controller because its config space is not accessible if
>>> >the PCIe port is put into low power state.
>>> >
>>> >To solve the issue, the PCIe port will not be suspended if any
>>> >subordinate device need PME polling.
>>> >
>>> >Reported-by: Ulrich Eckhardt
>>> >Signed-off-by: Huang Ying
>>> >Tested-by: Sarah Sharp
>>> >Cc: sta...@vger.kernel.org # 3.6+
>>> >---
>>> > drivers/pci/pcie/portdrv_pci.c | 18 +-
>>> > 1 file changed, 17 insertions(+), 1 deletion(-)
>>> >
>>> >--- a/drivers/pci/pcie/portdrv_pci.c
>>> >+++ b/drivers/pci/pcie/portdrv_pci.c
>>> >@@ -134,10 +134,26 @@ static int pcie_port_runtime_resume(stru
>>> >return 0;
>>> > }
>>> >
>>> >+static int pci_dev_pme_poll(struct pci_dev *pdev, void *data)
>>> >+{
>>> >+ int *pme_poll = data;
>>> >+ *pme_poll = *pme_poll || pdev->pme_poll;
>>> >+ return 0;
>>> >+}
>>> >+
>>> > static int pcie_port_runtime_idle(struct device *dev)
>>> > {
>>> >+ struct pci_dev *pdev = to_pci_dev(dev);
>>> >+ int pme_poll = false;
>>>
>>> You want to use int or bool?
>>>
>>> I think bool is better?
>>
>>Well, bool would be nicer, but it's not a big deal.
>>
>
> Yep, not a big deal.
I fixed up the int/bool confusion and added this to my pci/for-3.8
branch. I'll push it soon after v3.8-rc1. Thanks!
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PCIe/PM: Do not suspend port if any subordinate device need PME polling
On Sun, Dec 16, 2012 at 6:11 AM, Richard Yang
weiy...@linux.vnet.ibm.com wrote:
On Sat, Dec 15, 2012 at 12:03:33AM +0100, Rafael J. Wysocki wrote:
On Friday, December 14, 2012 01:11:31 PM Richard Yang wrote:
On Fri, Dec 14, 2012 at 10:52:11AM +0800, Huang Ying wrote:
In
http://www.mail-archive.com/linux-usb@vger.kernel.org/msg07976.html
Ulrich reported that his USB3 cardreader does not work reliably when
connected to the USB3 port. It turns out that USB3 controller failed
to be waken up when plugging in the USB3 cardreader. Further
experiment found that the USB3 host controller can only be waken up
via polling, while not via PME interrupt. But if the PCIe port that
the USB3 host controller is connected is suspended, we can not poll
the USB3 host controller because its config space is not accessible if
the PCIe port is put into low power state.
To solve the issue, the PCIe port will not be suspended if any
subordinate device need PME polling.
Reported-by: Ulrich Eckhardt u...@uli-eckhardt.de
Signed-off-by: Huang Ying ying.hu...@intel.com
Tested-by: Sarah Sharp sarah.a.sh...@linux.intel.com
Cc: sta...@vger.kernel.org # 3.6+
---
drivers/pci/pcie/portdrv_pci.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -134,10 +134,26 @@ static int pcie_port_runtime_resume(stru
return 0;
}
+static int pci_dev_pme_poll(struct pci_dev *pdev, void *data)
+{
+ int *pme_poll = data;
+ *pme_poll = *pme_poll || pdev-pme_poll;
+ return 0;
+}
+
static int pcie_port_runtime_idle(struct device *dev)
{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ int pme_poll = false;
You want to use int or bool?
I think bool is better?
Well, bool would be nicer, but it's not a big deal.
Yep, not a big deal.
I fixed up the int/bool confusion and added this to my pci/for-3.8
branch. I'll push it soon after v3.8-rc1. Thanks!
Bjorn
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PCIe/PM: Do not suspend port if any subordinate device need PME polling
On Friday, December 14, 2012 10:52:11 AM Huang Ying wrote:
> In
>
> http://www.mail-archive.com/linux-usb@vger.kernel.org/msg07976.html
>
> Ulrich reported that his USB3 cardreader does not work reliably when
> connected to the USB3 port. It turns out that USB3 controller failed
> to be waken up when plugging in the USB3 cardreader. Further
> experiment found that the USB3 host controller can only be waken up
> via polling, while not via PME interrupt. But if the PCIe port that
> the USB3 host controller is connected is suspended, we can not poll
> the USB3 host controller because its config space is not accessible if
> the PCIe port is put into low power state.
>
> To solve the issue, the PCIe port will not be suspended if any
> subordinate device need PME polling.
>
> Reported-by: Ulrich Eckhardt
> Signed-off-by: Huang Ying
> Tested-by: Sarah Sharp
> Cc: sta...@vger.kernel.org# 3.6+
> ---
> drivers/pci/pcie/portdrv_pci.c | 18 +-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> --- a/drivers/pci/pcie/portdrv_pci.c
> +++ b/drivers/pci/pcie/portdrv_pci.c
> @@ -134,10 +134,26 @@ static int pcie_port_runtime_resume(stru
> return 0;
> }
>
> +static int pci_dev_pme_poll(struct pci_dev *pdev, void *data)
> +{
> + int *pme_poll = data;
> + *pme_poll = *pme_poll || pdev->pme_poll;
I would write that as
*pme_poll ||= pdev->pme_poll;
It is not a big deal, though.
> + return 0;
> +}
> +
> static int pcie_port_runtime_idle(struct device *dev)
> {
> + struct pci_dev *pdev = to_pci_dev(dev);
> + int pme_poll = false;
> +
> + /*
> + * If any subordinate device needs pme poll, we should keep
> + * the port in D0, because we need port in D0 to poll it.
> + */
> + pci_walk_bus(pdev->subordinate, pci_dev_pme_poll, _poll);
> /* Delay for a short while to prevent too frequent suspend/resume */
> - pm_schedule_suspend(dev, 10);
> + if (!pme_poll)
> + pm_schedule_suspend(dev, 10);
> return -EBUSY;
> }
> #else
Acked-by: Rafael J. Wysocki
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PCIe/PM: Do not suspend port if any subordinate device need PME polling
On Friday, December 14, 2012 01:11:31 PM Richard Yang wrote:
> On Fri, Dec 14, 2012 at 10:52:11AM +0800, Huang Ying wrote:
> >In
> >
> > http://www.mail-archive.com/linux-usb@vger.kernel.org/msg07976.html
> >
> >Ulrich reported that his USB3 cardreader does not work reliably when
> >connected to the USB3 port. It turns out that USB3 controller failed
> >to be waken up when plugging in the USB3 cardreader. Further
> >experiment found that the USB3 host controller can only be waken up
> >via polling, while not via PME interrupt. But if the PCIe port that
> >the USB3 host controller is connected is suspended, we can not poll
> >the USB3 host controller because its config space is not accessible if
> >the PCIe port is put into low power state.
> >
> >To solve the issue, the PCIe port will not be suspended if any
> >subordinate device need PME polling.
> >
> >Reported-by: Ulrich Eckhardt
> >Signed-off-by: Huang Ying
> >Tested-by: Sarah Sharp
> >Cc: sta...@vger.kernel.org # 3.6+
> >---
> > drivers/pci/pcie/portdrv_pci.c | 18 +-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> >
> >--- a/drivers/pci/pcie/portdrv_pci.c
> >+++ b/drivers/pci/pcie/portdrv_pci.c
> >@@ -134,10 +134,26 @@ static int pcie_port_runtime_resume(stru
> > return 0;
> > }
> >
> >+static int pci_dev_pme_poll(struct pci_dev *pdev, void *data)
> >+{
> >+int *pme_poll = data;
> >+*pme_poll = *pme_poll || pdev->pme_poll;
> >+return 0;
> >+}
> >+
> > static int pcie_port_runtime_idle(struct device *dev)
> > {
> >+struct pci_dev *pdev = to_pci_dev(dev);
> >+int pme_poll = false;
>
> You want to use int or bool?
>
> I think bool is better?
Well, bool would be nicer, but it's not a big deal.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PCIe/PM: Do not suspend port if any subordinate device need PME polling
On Friday, December 14, 2012 01:11:31 PM Richard Yang wrote:
On Fri, Dec 14, 2012 at 10:52:11AM +0800, Huang Ying wrote:
In
http://www.mail-archive.com/linux-usb@vger.kernel.org/msg07976.html
Ulrich reported that his USB3 cardreader does not work reliably when
connected to the USB3 port. It turns out that USB3 controller failed
to be waken up when plugging in the USB3 cardreader. Further
experiment found that the USB3 host controller can only be waken up
via polling, while not via PME interrupt. But if the PCIe port that
the USB3 host controller is connected is suspended, we can not poll
the USB3 host controller because its config space is not accessible if
the PCIe port is put into low power state.
To solve the issue, the PCIe port will not be suspended if any
subordinate device need PME polling.
Reported-by: Ulrich Eckhardt u...@uli-eckhardt.de
Signed-off-by: Huang Ying ying.hu...@intel.com
Tested-by: Sarah Sharp sarah.a.sh...@linux.intel.com
Cc: sta...@vger.kernel.org # 3.6+
---
drivers/pci/pcie/portdrv_pci.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -134,10 +134,26 @@ static int pcie_port_runtime_resume(stru
return 0;
}
+static int pci_dev_pme_poll(struct pci_dev *pdev, void *data)
+{
+int *pme_poll = data;
+*pme_poll = *pme_poll || pdev-pme_poll;
+return 0;
+}
+
static int pcie_port_runtime_idle(struct device *dev)
{
+struct pci_dev *pdev = to_pci_dev(dev);
+int pme_poll = false;
You want to use int or bool?
I think bool is better?
Well, bool would be nicer, but it's not a big deal.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PCIe/PM: Do not suspend port if any subordinate device need PME polling
On Friday, December 14, 2012 10:52:11 AM Huang Ying wrote:
In
http://www.mail-archive.com/linux-usb@vger.kernel.org/msg07976.html
Ulrich reported that his USB3 cardreader does not work reliably when
connected to the USB3 port. It turns out that USB3 controller failed
to be waken up when plugging in the USB3 cardreader. Further
experiment found that the USB3 host controller can only be waken up
via polling, while not via PME interrupt. But if the PCIe port that
the USB3 host controller is connected is suspended, we can not poll
the USB3 host controller because its config space is not accessible if
the PCIe port is put into low power state.
To solve the issue, the PCIe port will not be suspended if any
subordinate device need PME polling.
Reported-by: Ulrich Eckhardt u...@uli-eckhardt.de
Signed-off-by: Huang Ying ying.hu...@intel.com
Tested-by: Sarah Sharp sarah.a.sh...@linux.intel.com
Cc: sta...@vger.kernel.org# 3.6+
---
drivers/pci/pcie/portdrv_pci.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -134,10 +134,26 @@ static int pcie_port_runtime_resume(stru
return 0;
}
+static int pci_dev_pme_poll(struct pci_dev *pdev, void *data)
+{
+ int *pme_poll = data;
+ *pme_poll = *pme_poll || pdev-pme_poll;
I would write that as
*pme_poll ||= pdev-pme_poll;
It is not a big deal, though.
+ return 0;
+}
+
static int pcie_port_runtime_idle(struct device *dev)
{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ int pme_poll = false;
+
+ /*
+ * If any subordinate device needs pme poll, we should keep
+ * the port in D0, because we need port in D0 to poll it.
+ */
+ pci_walk_bus(pdev-subordinate, pci_dev_pme_poll, pme_poll);
/* Delay for a short while to prevent too frequent suspend/resume */
- pm_schedule_suspend(dev, 10);
+ if (!pme_poll)
+ pm_schedule_suspend(dev, 10);
return -EBUSY;
}
#else
Acked-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
