p.s.
On 9/29/20 10:08 PM, Hans de Goede wrote:
<snip>
So I believe that the proper fix for this is to revert
commit 9f0b54cd167219
("r8169: move switching optional clock on/off to pll power functions")
Heiner, assuming you agree that reverting this commit is
the best way to fix this, can you please submit a revert
for this upstream ?
With a:
Fixes: 9f0b54cd167219 ("r8169: move switching optional clock on/off to pll power
functions")
Tag in the commit-message so that this gets cherry-picked into
the stable series where necessary.
Regards,
Hans
As that caused the whole chip's clock to be left off after
a suspend/resume while the interface is down.
Also some remarks about this while I'm being a bit grumpy about
all this anyways (sorry):
1. 9f0b54cd167219 ("r8169: move switching optional clock on/off
to pll power functions") commit's message does not seem to really
explain why this change was made...
2. If a git blame would have been done to find the commit adding
the clk support: commit c2f6f3ee7f22 ("r8169: Get and enable optional ether_clk
clock")
then you could have known that the clk in question is an external
clock for the entire chip, the commit message pretty clearly states
this (although "the entire" part is implied only) :
"On some boards a platform clock is used as clock for the r8169 chip,
this commit adds support for getting and enabling this clock (assuming
it has an "ether_clk" alias set on it).
This is related to commit d31fd43c0f9a ("clk: x86: Do not gate clocks
enabled by the firmware") which is a previous attempt to fix this for some
x86 boards, but this causes all Cherry Trail SoC using boards to not reach
there lowest power states when suspending.
This commit (together with an atom-pmc-clk driver commit adding the alias)
fixes things properly by making the r8169 get the clock and enable it when
it needs it."
Regards,
Hans
