[SRU] [Unstable/OEM-B] [PATCH 0/2] r8169 doesn't get woken up by ethernet cable plugging, no PME generated

[Kai-Heng Feng]

> On Feb 27, 2019, at 21:48, Seth Forshee <seth.forshee at canonical.com> wrote:
> 
> On Tue, Feb 26, 2019 at 04:56:55PM +0800, Kai-Heng Feng wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1817676
>> 
>> [Impact]
>> Once r8169 (Realtek ethernet) and its root port enter to D3, plugging
>> network cable doesn't wake r8169 up.
>> 
>> [Fix]
>> Revert "PCI/PME: Implement runtime PM callbacks".
>> Quote the original commit message:
>> "
>> The commit tried to prevent root port waking up from D3cold immediately but
>> looks like disabing root port PME interrupt is not the right way to fix
>> that issue so revert it now.  The patch following proposes an alternative
>> solution to that issue.
>> "
>> 
>> [Test]
>> After applying the fix, plugging the ethernet cable can generate PME IRQ
>> correctly, hence waking up r8169.
>> 
>> [Regression Potential]
>> Medium. The commits are relative new, rely on some PCI detail that I am
>> not familiar with (PCI spec isn't publicly available).
>> Fortunately we only need this fix for Unstable/OEM-B, so it won't hit
>> most users if any regression happens.
> 
> I think it would be safer to apply only the revert if the only problem
> we're trying to fix is the wakeup on cable plug, since that commit was
> new in 4.20 anyhow. If you think we also need the second commit, please
> explain why and let me know what kind of testing you've done with it.

Yes, the second commit is needed.

The reverted commit "PCI/PME: Implement runtime PM callbacks” is an
attempt to solve a TBT issue on X1 Carbon 6th Gen [1].

Patch [2/2] is a replacement for the reverted fix.
Without it, the native TBT device will be in a D0/D3cold loop [2]:

[  263.418400] pcieport 0000:00:1d.0: power state changed by ACPI to D3cold
[  264.919693] pcieport 0000:00:1d.0: power state changed by ACPI to D0
…
[  285.114443] pcieport 0000:00:1d.0: power state changed by ACPI to D3cold
[  286.614872] pcieport 0000:00:1d.0: power state changed by ACPI to D0
…
[  306.682371] pcieport 0000:00:1d.0: power state changed by ACPI to D3cold
[  308.183711] pcieport 0000:00:1d.0: power state changed by ACPI to D0

I think the dmesg itself is a good justification for picking patch [2/2].

Right now I can’t conduct any meaningful test because the one I have
(Dell TB16) has a device that prevents the full hierarchy to enter D3cold,
so the issue can't be reproduced here.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=202593
[2] https://bugzilla.kernel.org/attachment.cgi?id=281163

Kai-Heng

> 
> Thanks,
> Seth