Hi,
On 29-08-18 18:31, Andy Shevchenko wrote:
On Mon, Aug 27, 2018 at 04:31:56PM +0200, Hans de Goede wrote:
Hi All,
This series has as goal to revert commit d31fd43c0f9a ("clk: x86: Do not gate
clocks enabled by the firmware"), because that commit causes almost all
Cherry Trail devices to not use the S0i3 powerstate when suspending, leading
to them quickly draining their battery when suspended.
This commit was added to fix issues with the r8169 NIC on some Bay Trail
devices, where the pmc_plt_clk_4 was used as clk for the r8169 chip.
This series fixes this properly, so that we can undo the trouble some
commit.
The first patch adds an "ether_clk" alias to the clk-pmc-atom driver so
that the r8169 can pass "ether_clk" as generic id to clk_get instead of
having to add x86 specific code to the r8169 driver.
The second patch makes the r8169 driver do a clk_get for "ether_clk"
(ignoring -ENOENT errors so this is optional) and if that succeeds then
it enables the clock.
The third patch effectively revert the troublesome commit.
This series has been tested on a HP Stream x360 - 11-p000nd, which uses
a Bay Trail SoC with a r8169 ethernet chip and I can confirm that the
pmc_plt_clk_4 gets properly enabled, so this series should not cause any
regressions wrt devices needing pmc_plt_clk_4 enabled (which is not the
case on the Stream x360).
To avoid regressions we need to have patches 1 and 2 merged before 3,
so it is probably best if this entire series gets merged through a single
tree. Given that clk-pmc-atom.c has not seen any changes for over a
year I suggest that all 3 patches are merged through the netdev tree,
with an ack from the clk maintainers. Assuming that is ok with the clk
maintainers of course.
Note there is a fourth patch in this series, this patch is necessary to
reach S0i3 state on Bay Trail devices wich have a r8169 ethernet chip,
since I do not have access to hardware where the r8169 actually needs
the pmc_plt_clk_4 I have not been able to test this, hence it is marked
as RFC for now.
What a nice stuff, thanks!
Reviewed-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
Thank you (and also thank you for the other reviews)
Btw, you probably better to refer to
https://bugzilla.kernel.org/show_bug.cgi?id=196861 which is dedicated for S0ix
issue.
Ok, I've added a link to that. I've also added:
Reported-by: Johannes Stezenbach <j...@sig21.net>
To honor Johannes for figuring out that leaving the clocks enabled
was a problem in the first place.
This will all be included in v2 of the series when I send it out.
Another thing, clk_prepare_enable() can fail, I dunno what you can do at
->resume() stage with it failed.
Right, I know that it can fail, but in practice it never fails unless
things are seriously foo-barred already and there is not much we can
do when it fails, so I decided to just ignore the return code.
Regards,
Hans
