Some platforms cannot detect ethernet hotplug once its upstream port is
runtime suspended because PME isn't granted by BIOS _OSC. The issue can
be workarounded by "pcie_ports=native".
The vendor confirmed that the PME in _OSC is disabled intentionally for
system stability issues on the other OS, so we should also honor the PME
setting here.
So before marking PME support status for the device, check
PCI_EXP_RTCTL_PMEIE bit to ensure PME interrupt is either enabled by
firmware or OS.
Cc: Lukas Wunner <[email protected]>
Cc: Mika Westerberg <[email protected]>
Cc: Rafael J. Wysocki <[email protected]>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=213873
Signed-off-by: Kai-Heng Feng <[email protected]>
---
v2:
- Instead of prevent root port from runtime suspending, skip
initializing PME status for the downstream device.
drivers/pci/pci.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index aacf575c15cf..4344dc302edd 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2294,6 +2294,32 @@ void pci_pme_wakeup_bus(struct pci_bus *bus)
pci_walk_bus(bus, pci_pme_wakeup, (void *)true);
}
+#ifdef CONFIG_PCIE_PME
+static bool pci_pcie_port_pme_enabled(struct pci_dev *dev)
+{
+ struct pci_dev *bridge = pci_upstream_bridge(dev);
+ u16 val;
+ int ret;
+
+ if (!bridge)
+ return true;
+
+ if (pci_pcie_type(bridge) != PCI_EXP_TYPE_ROOT_PORT &&
+ pci_pcie_type(bridge) != PCI_EXP_TYPE_RC_EC)
+ return true;
+
+ ret = pcie_capability_read_word(bridge, PCI_EXP_RTCTL, &val);
+ if (ret)
+ return false;
+
+ return val & PCI_EXP_RTCTL_PMEIE;
+}
+#else
+static bool pci_pcie_port_pme_enabled(struct pci_dev *dev)
+{
+ return true;
+}
+#endif
/**
* pci_pme_capable - check the capability of PCI device to generate PME#
@@ -3095,7 +3121,7 @@ void pci_pm_init(struct pci_dev *dev)
}
pmc &= PCI_PM_CAP_PME_MASK;
- if (pmc) {
+ if (pmc && pci_pcie_port_pme_enabled(dev)) {
pci_info(dev, "PME# supported from%s%s%s%s%s\n",
(pmc & PCI_PM_CAP_PME_D0) ? " D0" : "",
(pmc & PCI_PM_CAP_PME_D1) ? " D1" : "",
--
2.32.0
On Thu, Aug 12, 2021 at 11:39 PM Kai-Heng Feng
<[email protected]> wrote:
>
> Some platforms cannot detect ethernet hotplug once its upstream port is
> runtime suspended because PME isn't granted by BIOS _OSC. The issue can
> be workarounded by "pcie_ports=native".
>
> The vendor confirmed that the PME in _OSC is disabled intentionally for
> system stability issues on the other OS, so we should also honor the PME
> setting here.
>
> So before marking PME support status for the device, check
> PCI_EXP_RTCTL_PMEIE bit to ensure PME interrupt is either enabled by
> firmware or OS.
>
> Cc: Lukas Wunner <[email protected]>
> Cc: Mika Westerberg <[email protected]>
> Cc: Rafael J. Wysocki <[email protected]>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=213873
> Signed-off-by: Kai-Heng Feng <[email protected]>
A gentle ping...
> ---
> v2:
> - Instead of prevent root port from runtime suspending, skip
> initializing PME status for the downstream device.
>
> drivers/pci/pci.c | 28 +++++++++++++++++++++++++++-
> 1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index aacf575c15cf..4344dc302edd 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2294,6 +2294,32 @@ void pci_pme_wakeup_bus(struct pci_bus *bus)
> pci_walk_bus(bus, pci_pme_wakeup, (void *)true);
> }
>
> +#ifdef CONFIG_PCIE_PME
> +static bool pci_pcie_port_pme_enabled(struct pci_dev *dev)
> +{
> + struct pci_dev *bridge = pci_upstream_bridge(dev);
> + u16 val;
> + int ret;
> +
> + if (!bridge)
> + return true;
> +
> + if (pci_pcie_type(bridge) != PCI_EXP_TYPE_ROOT_PORT &&
> + pci_pcie_type(bridge) != PCI_EXP_TYPE_RC_EC)
> + return true;
> +
> + ret = pcie_capability_read_word(bridge, PCI_EXP_RTCTL, &val);
> + if (ret)
> + return false;
> +
> + return val & PCI_EXP_RTCTL_PMEIE;
> +}
> +#else
> +static bool pci_pcie_port_pme_enabled(struct pci_dev *dev)
> +{
> + return true;
> +}
> +#endif
>
> /**
> * pci_pme_capable - check the capability of PCI device to generate PME#
> @@ -3095,7 +3121,7 @@ void pci_pm_init(struct pci_dev *dev)
> }
>
> pmc &= PCI_PM_CAP_PME_MASK;
> - if (pmc) {
> + if (pmc && pci_pcie_port_pme_enabled(dev)) {
> pci_info(dev, "PME# supported from%s%s%s%s%s\n",
> (pmc & PCI_PM_CAP_PME_D0) ? " D0" : "",
> (pmc & PCI_PM_CAP_PME_D1) ? " D1" : "",
> --
> 2.32.0
>
On 10/21/2021 8:56 AM, Kai-Heng Feng wrote:
> On Thu, Aug 12, 2021 at 11:39 PM Kai-Heng Feng
> <[email protected]> wrote:
>> Some platforms cannot detect ethernet hotplug once its upstream port is
>> runtime suspended because PME isn't granted by BIOS _OSC. The issue can
>> be workarounded by "pcie_ports=native".
>>
>> The vendor confirmed that the PME in _OSC is disabled intentionally for
>> system stability issues on the other OS, so we should also honor the PME
>> setting here.
>>
>> So before marking PME support status for the device, check
>> PCI_EXP_RTCTL_PMEIE bit to ensure PME interrupt is either enabled by
>> firmware or OS.
>>
>> Cc: Lukas Wunner <[email protected]>
>> Cc: Mika Westerberg <[email protected]>
>> Cc: Rafael J. Wysocki <[email protected]>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=213873
>> Signed-off-by: Kai-Heng Feng <[email protected]>
> A gentle ping...
Any chance to CC this to linux-pm too?
So you basically want to check whether or not the PME interrupts are
configured on the port?
>> ---
>> v2:
>> - Instead of prevent root port from runtime suspending, skip
>> initializing PME status for the downstream device.
>>
>> drivers/pci/pci.c | 28 +++++++++++++++++++++++++++-
>> 1 file changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index aacf575c15cf..4344dc302edd 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -2294,6 +2294,32 @@ void pci_pme_wakeup_bus(struct pci_bus *bus)
>> pci_walk_bus(bus, pci_pme_wakeup, (void *)true);
>> }
>>
>> +#ifdef CONFIG_PCIE_PME
>> +static bool pci_pcie_port_pme_enabled(struct pci_dev *dev)
>> +{
>> + struct pci_dev *bridge = pci_upstream_bridge(dev);
>> + u16 val;
>> + int ret;
>> +
>> + if (!bridge)
>> + return true;
>> +
>> + if (pci_pcie_type(bridge) != PCI_EXP_TYPE_ROOT_PORT &&
>> + pci_pcie_type(bridge) != PCI_EXP_TYPE_RC_EC)
>> + return true;
>> +
>> + ret = pcie_capability_read_word(bridge, PCI_EXP_RTCTL, &val);
>> + if (ret)
>> + return false;
>> +
>> + return val & PCI_EXP_RTCTL_PMEIE;
>> +}
>> +#else
>> +static bool pci_pcie_port_pme_enabled(struct pci_dev *dev)
>> +{
>> + return true;
>> +}
>> +#endif
>>
>> /**
>> * pci_pme_capable - check the capability of PCI device to generate PME#
>> @@ -3095,7 +3121,7 @@ void pci_pm_init(struct pci_dev *dev)
>> }
>>
>> pmc &= PCI_PM_CAP_PME_MASK;
>> - if (pmc) {
>> + if (pmc && pci_pcie_port_pme_enabled(dev)) {
>> pci_info(dev, "PME# supported from%s%s%s%s%s\n",
>> (pmc & PCI_PM_CAP_PME_D0) ? " D0" : "",
>> (pmc & PCI_PM_CAP_PME_D1) ? " D1" : "",
>> --
>> 2.32.0
>>
On Fri, Oct 22, 2021 at 3:13 AM Rafael J. Wysocki
<[email protected]> wrote:
>
> On 10/21/2021 8:56 AM, Kai-Heng Feng wrote:
> > On Thu, Aug 12, 2021 at 11:39 PM Kai-Heng Feng
> > <[email protected]> wrote:
> >> Some platforms cannot detect ethernet hotplug once its upstream port is
> >> runtime suspended because PME isn't granted by BIOS _OSC. The issue can
> >> be workarounded by "pcie_ports=native".
> >>
> >> The vendor confirmed that the PME in _OSC is disabled intentionally for
> >> system stability issues on the other OS, so we should also honor the PME
> >> setting here.
> >>
> >> So before marking PME support status for the device, check
> >> PCI_EXP_RTCTL_PMEIE bit to ensure PME interrupt is either enabled by
> >> firmware or OS.
> >>
> >> Cc: Lukas Wunner <[email protected]>
> >> Cc: Mika Westerberg <[email protected]>
> >> Cc: Rafael J. Wysocki <[email protected]>
> >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=213873
> >> Signed-off-by: Kai-Heng Feng <[email protected]>
> > A gentle ping...
>
> Any chance to CC this to linux-pm too?
Will remember this next time.
>
> So you basically want to check whether or not the PME interrupts are
> configured on the port?
Yes, that's the idea here.
Kai-Heng
>
>
> >> ---
> >> v2:
> >> - Instead of prevent root port from runtime suspending, skip
> >> initializing PME status for the downstream device.
> >>
> >> drivers/pci/pci.c | 28 +++++++++++++++++++++++++++-
> >> 1 file changed, 27 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> >> index aacf575c15cf..4344dc302edd 100644
> >> --- a/drivers/pci/pci.c
> >> +++ b/drivers/pci/pci.c
> >> @@ -2294,6 +2294,32 @@ void pci_pme_wakeup_bus(struct pci_bus *bus)
> >> pci_walk_bus(bus, pci_pme_wakeup, (void *)true);
> >> }
> >>
> >> +#ifdef CONFIG_PCIE_PME
> >> +static bool pci_pcie_port_pme_enabled(struct pci_dev *dev)
> >> +{
> >> + struct pci_dev *bridge = pci_upstream_bridge(dev);
> >> + u16 val;
> >> + int ret;
> >> +
> >> + if (!bridge)
> >> + return true;
> >> +
> >> + if (pci_pcie_type(bridge) != PCI_EXP_TYPE_ROOT_PORT &&
> >> + pci_pcie_type(bridge) != PCI_EXP_TYPE_RC_EC)
> >> + return true;
> >> +
> >> + ret = pcie_capability_read_word(bridge, PCI_EXP_RTCTL, &val);
> >> + if (ret)
> >> + return false;
> >> +
> >> + return val & PCI_EXP_RTCTL_PMEIE;
> >> +}
> >> +#else
> >> +static bool pci_pcie_port_pme_enabled(struct pci_dev *dev)
> >> +{
> >> + return true;
> >> +}
> >> +#endif
> >>
> >> /**
> >> * pci_pme_capable - check the capability of PCI device to generate PME#
> >> @@ -3095,7 +3121,7 @@ void pci_pm_init(struct pci_dev *dev)
> >> }
> >>
> >> pmc &= PCI_PM_CAP_PME_MASK;
> >> - if (pmc) {
> >> + if (pmc && pci_pcie_port_pme_enabled(dev)) {
> >> pci_info(dev, "PME# supported from%s%s%s%s%s\n",
> >> (pmc & PCI_PM_CAP_PME_D0) ? " D0" : "",
> >> (pmc & PCI_PM_CAP_PME_D1) ? " D1" : "",
> >> --
> >> 2.32.0
> >>
>
On Thu, Oct 21, 2021 at 09:13:29PM +0200, Rafael J. Wysocki wrote:
> On 10/21/2021 8:56 AM, Kai-Heng Feng wrote:
> > On Thu, Aug 12, 2021 at 11:39 PM Kai-Heng Feng <[email protected]> wrote:
> > > Some platforms cannot detect ethernet hotplug once its upstream port is
> > > runtime suspended because PME isn't granted by BIOS _OSC. The issue can
> > > be workarounded by "pcie_ports=native".
> > >
> > > The vendor confirmed that the PME in _OSC is disabled intentionally for
> > > system stability issues on the other OS, so we should also honor the PME
> > > setting here.
> > >
> > > So before marking PME support status for the device, check
> > > PCI_EXP_RTCTL_PMEIE bit to ensure PME interrupt is either enabled by
> > > firmware or OS.
>
> So you basically want to check whether or not the PME interrupts are
> configured on the port?
This platform doesn't grant PME handling to OSPM, but the platform
doesn't handle PME itself either (recognizable by the fact that it
didn't set the PME Interrupt Enable bit in the Root Control Register).
The rationale of the patch is to recognize this situation and rely
on PME polling instead.
That is achieved by assuming no PME support for the device, despite
the device claiming that PME is supported.
(This information should probably be included in the commit message.)
> > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > index aacf575c15cf..4344dc302edd 100644
> > > --- a/drivers/pci/pci.c
> > > +++ b/drivers/pci/pci.c
> > > @@ -2294,6 +2294,32 @@ void pci_pme_wakeup_bus(struct pci_bus *bus)
> > > pci_walk_bus(bus, pci_pme_wakeup, (void *)true);
> > > }
> > >
> > > +#ifdef CONFIG_PCIE_PME
> > > +static bool pci_pcie_port_pme_enabled(struct pci_dev *dev)
> > > +{
> > > + struct pci_dev *bridge = pci_upstream_bridge(dev);
> > > + u16 val;
> > > + int ret;
> > > +
> > > + if (!bridge)
> > > + return true;
> > > +
> > > + if (pci_pcie_type(bridge) != PCI_EXP_TYPE_ROOT_PORT &&
> > > + pci_pcie_type(bridge) != PCI_EXP_TYPE_RC_EC)
> > > + return true;
> > > +
> > > + ret = pcie_capability_read_word(bridge, PCI_EXP_RTCTL, &val);
> > > + if (ret)
> > > + return false;
> > > +
> > > + return val & PCI_EXP_RTCTL_PMEIE;
> > > +}
> > > +#else
> > > +static bool pci_pcie_port_pme_enabled(struct pci_dev *dev)
> > > +{
> > > + return true;
> > > +}
> > > +#endif
> > >
> > > /**
> > > * pci_pme_capable - check the capability of PCI device to generate PME#
> > > @@ -3095,7 +3121,7 @@ void pci_pm_init(struct pci_dev *dev)
> > > }
> > >
> > > pmc &= PCI_PM_CAP_PME_MASK;
> > > - if (pmc) {
> > > + if (pmc && pci_pcie_port_pme_enabled(dev)) {
> > > pci_info(dev, "PME# supported from%s%s%s%s%s\n",
> > > (pmc & PCI_PM_CAP_PME_D0) ? " D0" : "",
> > > (pmc & PCI_PM_CAP_PME_D1) ? " D1" : "",
> > > --
> > > 2.32.0