Olof Johansson [off-list ref] :

On Tue, Jan 3, 2012 at 3:30 PM, Francois Romieu [off-list ref] wrote:

quoted

Francois Romieu [off-list ref] :
[...]

quoted

The description of the patch implies that the initial power management
state is not right. I would be more inclined to set it correctly when
the device goes up instead of checking repeatedly for a loss of sync
through rtl8169_runtime_idle. Todd, any comment ?

... which could be as simple as (completely untested) :

That change is just to the code path of open(), which means that for
the power savings to kick in, you have to bring the interface up. The
point of the original patch is to check even if the interface has
never been touched.

Fair enough.

If so the original patch should not be needed as the driver core is
supposed to invoke the idle callback after probe (aka rtl8169_init_one).
After probe the device is in TxDescArray == 0 state and rtl8169_runtime_idle
returns 0. pm_runtime_suspend should thus be called [*]... I must have
missed something. :o/

[*] Not sure if it always imply a transition to D3 though.

Perhaps call check_link_status at the end if init_one instead to
activate pm then ?

It should not be needed, see above.

Moreover one should not care for the link status until the device is
brought up. It may seem a bit counterintuitive but the device and its
PHY are tightly coupled here. You do not manage them separately.

You should forget it anyway: PHY init happens late in the r8169 driver
and it can not be done sooner as it sometimes depends on firmware loading.

-- 
Ueimor