[PATCH] sh_eth: call phy_scan_fixups() after PHY reset

[PATCH] sh_eth: call phy_scan_fixups() after PHY reset

[Sergei Shtylyov]

Sometimes the PHY reset that sh_eth_phy_start() does effects the PHY registers
registers values of which are vital for the correct functioning of the driver.
Unfortunately, the existing PHY platform fixup mechanism doesn't help  here as
it only hooks PHY resets done by ioctl() calls. Calling phy_scan_fixups() from
the driver helps here. With a proper platform fixup, this fixes NFS timeouts on
the SH-Mobile Lager board. 

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
This patch is against Dave Miller's 'net.git' tree.
If desired, I can merge it with the Lager platform patch I'll post next, altho
there's only runtime interdependency between them...

 drivers/net/ethernet/renesas/sh_eth.c |    5 +++++
 1 file changed, 5 insertions(+)

Index: net/drivers/net/ethernet/renesas/sh_eth.c
===================================================================
--- net.orig/drivers/net/ethernet/renesas/sh_eth.c
+++ net/drivers/net/ethernet/renesas/sh_eth.c
@@ -1701,6 +1701,11 @@ static int sh_eth_phy_start(struct net_d
 
 	/* reset phy - this also wakes it from PDOWN */
 	phy_write(mdp->phydev, MII_BMCR, BMCR_RESET);
+	/* some boards need their registers set to non-default state */
+	ret = phy_scan_fixups(mdp->phydev);
+	if (ret)
+		return ret;
+
 	phy_start(mdp->phydev);
 
 	return 0;

Re: [PATCH] sh_eth: call phy_scan_fixups() after PHY reset

[David Miller]

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Sat, 14 Sep 2013 04:10:37 +0400

> Sometimes the PHY reset that sh_eth_phy_start() does effects the PHY registers
> registers values of which are vital for the correct functioning of the driver.
> Unfortunately, the existing PHY platform fixup mechanism doesn't help  here as
> it only hooks PHY resets done by ioctl() calls. Calling phy_scan_fixups() from
> the driver helps here. With a proper platform fixup, this fixes NFS timeouts on
> the SH-Mobile Lager board. 
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

The PHY layer is designed to naturally already take care of this kind of
thing.  I think that part of the problem is that you're fighting the
natural control flow the PHY layer provides.

When the phy_connect() is performed, what we end up doing is calling
phy_attach_direct() which invokes the ->probe() method of the driver
and then afterwards we do phy_init_hw() which takes care of doing
the fixup calls.

So if you really need to do a BMCR reset then run the fixups I'd like
you to look into making that happen within the provided control
flow rather than with an exceptional explicit call to run the fixups.

I'm willing to be convinced that this is a better or necessary approach
but you'll need to explain it to me.

Thanks.

Re: [PATCH] sh_eth: call phy_scan_fixups() after PHY reset

[Sergei Shtylyov]

Hello.

On 09/17/2013 11:44 PM, David Miller wrote:

>> Sometimes the PHY reset that sh_eth_phy_start() does effects the PHY registers
>> registers values of which are vital for the correct functioning of the driver.
>> Unfortunately, the existing PHY platform fixup mechanism doesn't help  here as
>> it only hooks PHY resets done by ioctl() calls. Calling phy_scan_fixups() from
>> the driver helps here. With a proper platform fixup, this fixes NFS timeouts on
>> the SH-Mobile Lager board.

    "And sets the PHY LED pins to the desired mode", I should have added.

>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

> The PHY layer is designed to naturally already take care of this kind of
> thing.  I think that part of the problem is that you're fighting the
> natural control flow the PHY layer provides.

> When the phy_connect() is performed, what we end up doing is calling
> phy_attach_direct() which invokes the ->probe() method of the driver
> and then afterwards we do phy_init_hw() which takes care of doing
> the fixup calls.

    Yes, I have studied the code paths beforehand.

> So if you really need to do a BMCR reset then run the fixups I'd like
> you to look into making that happen within the provided control
> flow rather than with an exceptional explicit call to run the fixups.

    That could change the behavior of many Ethernet drivers in sometimes 
unpredictable ways I think (due to extended registers the PHYs sometimes have, 
like in this case) if you meant including the PHY reset into phylib control 
flows. Anyway, that would have required more complex patches only good for 
merging at the merge window time while I aimed at a quick fix for a problem at 
hand (which is NFS timeout/slowdown and LED mode mismatch to what was designed 
for the board).
    Some other drivers also do reset the PHYs but usually that's accompanied 
by a loop polling for reset completion, so a naive code like that one on the 
phylib's ioctl() path couldn't have helped if I wanted to hook reset writes in 
the same fashion in phy_write(). In my case reset seems just quick enough for 
the extended PHY register reads/writes to work correctly without polling the 
reset bit first...
    That's why I took an easy way and used already exported phy_scan_fixups() 
to undo what the PHY reset did to some of the PHY's registers. The question is 
why it was exported in the first place? It doesn't seem to be used by anything 
but phylib internally...

> I'm willing to be convinced that this is a better or necessary approach
> but you'll need to explain it to me.

    Well, I didn't write this driver, so I'm probably not the best person to 
be asked about its design (maybe Iwamatsu-san could add something here). I 
don't know about the purpose of the explicit PHY reset in the driver more than 
the accompanying comment says (and it doesn't say much other than that it 
takes the PHY out of power-down). Perhaps we could just painlessly remove it, 
who knows?

> Thanks.

WBR, Sergei

Re: [PATCH] sh_eth: call phy_scan_fixups() after PHY reset

[Sergei Shtylyov]

Hello.

On 09/21/2013 04:39 AM, Sergei Shtylyov wrote:

>>> Sometimes the PHY reset that sh_eth_phy_start() does effects the PHY registers
>>> registers values of which are vital for the correct functioning of the driver.
>>> Unfortunately, the existing PHY platform fixup mechanism doesn't help  here as
>>> it only hooks PHY resets done by ioctl() calls. Calling phy_scan_fixups() from
>>> the driver helps here. With a proper platform fixup, this fixes NFS
>>> timeouts on
>>> the SH-Mobile Lager board.

    Timeouts happen because of the sideband ETH_LINK signal connected to PHY's 
LED0 pin -- it bounces on/off after each packet in the default LED mode and 
that seems to hinder packet sending and/or reception...

>     "And sets the PHY LED pins to the desired mode", I should have added.

>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

>> The PHY layer is designed to naturally already take care of this kind of
>> thing.  I think that part of the problem is that you're fighting the
>> natural control flow the PHY layer provides.

>> When the phy_connect() is performed, what we end up doing is calling
>> phy_attach_direct() which invokes the ->probe() method of the driver
>> and then afterwards we do phy_init_hw() which takes care of doing
>> the fixup calls.

>     Yes, I have studied the code paths beforehand.

>> So if you really need to do a BMCR reset then run the fixups I'd like
>> you to look into making that happen within the provided control
>> flow rather than with an exceptional explicit call to run the fixups.

>     That could change the behavior of many Ethernet drivers in sometimes
> unpredictable ways I think (due to extended registers the PHYs sometimes have,
> like in this case) if you meant including the PHY reset into phylib control
> flows. Anyway, that would have required more complex patches only good for
> merging at the merge window time while I aimed at a quick fix for a problem at
> hand (which is NFS timeout/slowdown and LED mode mismatch to what was designed
> for the board).
>     Some other drivers also do reset the PHYs but usually that's accompanied
> by a loop polling for reset completion, so a naive code like that one on the
> phylib's ioctl() path couldn't have helped if I wanted to hook reset writes in
> the same fashion in phy_write(). In my case reset seems just quick enough for
> the extended PHY register reads/writes to work correctly without polling the
> reset bit first...
>     That's why I took an easy way and used already exported phy_scan_fixups()
> to undo what the PHY reset did to some of the PHY's registers. The question is
> why it was exported in the first place? It doesn't seem to be used by anything
> but phylib internally...

    Well, how about I create phy_reset() function (that will care about BMCR 
polling and calling PHY driver/fixups) that those drivers that currently do 
reset their PHYs can call (instead of open coding BMCR reset)? That way it 
seems to be less invasive than embedding PHY reset into phylib's control flow...

>> I'm willing to be convinced that this is a better or necessary approach
>> but you'll need to explain it to me.

>     Well, I didn't write this driver, so I'm probably not the best person to
> be asked about its design (maybe Iwamatsu-san could add something here). I
> don't know about the purpose of the explicit PHY reset in the driver more than
> the accompanying comment says (and it doesn't say much other than that it
> takes the PHY out of power-down). Perhaps we could just painlessly remove it,
> who knows?

    Unfortunately, Iwamatsu-san hasn't commented on its purpose... :-(

WBR, Sergei

Re: [PATCH] sh_eth: call phy_scan_fixups() after PHY reset

[David Miller]

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Tue, 05 Nov 2013 23:18:10 +0300

>    David, will you ever reply to this mail?

I really haven't had the time with my backlog, and it's been so long ago
that I've forgotten all of the context.

Sorry, someone will have to start the conversation up anew.

Re: [PATCH] sh_eth: call phy_scan_fixups() after PHY reset

[Sergei Shtylyov]

Hello.

On 10/30/2013 11:50 PM, Sergei Shtylyov wrote:

>>>> Sometimes the PHY reset that sh_eth_phy_start() does effects the PHY
>>>> registers
>>>> registers values of which are vital for the correct functioning of the
>>>> driver.
>>>> Unfortunately, the existing PHY platform fixup mechanism doesn't help
>>>> here as
>>>> it only hooks PHY resets done by ioctl() calls. Calling phy_scan_fixups()
>>>> from
>>>> the driver helps here. With a proper platform fixup, this fixes NFS
>>>> timeouts on
>>>> the SH-Mobile Lager board.

>     Timeouts happen because of the sideband ETH_LINK signal connected to PHY's
> LED0 pin -- it bounces on/off after each packet in the default LED mode and
> that seems to hinder packet sending and/or reception...

>>     "And sets the PHY LED pins to the desired mode", I should have added.

>>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

>>> The PHY layer is designed to naturally already take care of this kind of
>>> thing.  I think that part of the problem is that you're fighting the
>>> natural control flow the PHY layer provides.

>>> When the phy_connect() is performed, what we end up doing is calling
>>> phy_attach_direct() which invokes the ->probe() method of the driver
>>> and then afterwards we do phy_init_hw() which takes care of doing
>>> the fixup calls.

>>     Yes, I have studied the code paths beforehand.

>>> So if you really need to do a BMCR reset then run the fixups I'd like
>>> you to look into making that happen within the provided control
>>> flow rather than with an exceptional explicit call to run the fixups.

>>     That could change the behavior of many Ethernet drivers in sometimes
>> unpredictable ways I think (due to extended registers the PHYs sometimes have,
>> like in this case) if you meant including the PHY reset into phylib control
>> flows. Anyway, that would have required more complex patches only good for
>> merging at the merge window time while I aimed at a quick fix for a problem at
>> hand (which is NFS timeout/slowdown and LED mode mismatch to what was designed
>> for the board).
>>     Some other drivers also do reset the PHYs but usually that's accompanied
>> by a loop polling for reset completion, so a naive code like that one on the
>> phylib's ioctl() path couldn't have helped if I wanted to hook reset writes in
>> the same fashion in phy_write(). In my case reset seems just quick enough for
>> the extended PHY register reads/writes to work correctly without polling the
>> reset bit first...
>>     That's why I took an easy way and used already exported phy_scan_fixups()
>> to undo what the PHY reset did to some of the PHY's registers. The question is
>> why it was exported in the first place? It doesn't seem to be used by anything
>> but phylib internally...

>     Well, how about I create phy_reset() function (that will care about BMCR
> polling and calling PHY driver/fixups) that those drivers that currently do
> reset their PHYs can call (instead of open coding BMCR reset)? That way it
> seems to be less invasive than embedding PHY reset into phylib's control flow...

>>> I'm willing to be convinced that this is a better or necessary approach
>>> but you'll need to explain it to me.

    David, will you ever reply to this mail?

WBR, Sergei

Re: [PATCH] sh_eth: call phy_scan_fixups() after PHY reset

[Florian Fainelli]

2013/11/15 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>:
> Hello.
>
>
> On 11/05/2013 11:01 PM, David Miller wrote:
>
>>>     David, will you ever reply to this mail?
>
>
>> I really haven't had the time with my backlog, and it's been so long ago
>> that I've forgotten all of the context.
>
>
>    Hm, I thought I provided enough context and also that's what mail
> archives are for... :-)
>
>
>> Sorry, someone will have to start the conversation up anew.

[snip]

>
>    Now, about the ways to deal with this issue that I see.
>    First I thought about using PHY platform fixup mechanism which is already
> hooked up to a PHY reset in phy_mii_ioctl() (the code is somewhat naive
> though as it doesn't wait for the reset completion before calling fixups and
> the driver's config_init() method).

That's something that needs fixing and also a dedicated helper or such
should be used because we are potentially messing up with the
configuration the PHY library thinks it has applied to the PHY (e.g:
disabling auto-negotiation etc...).

> All I needed is calling fixups after
> direct PHY reset done via phy_write() but I didn't want to add a hook there
> due to that reset completion wait loop that was obviously necessary (this
> function is simple *inline*), so went for a simplistic local
> phy_scan_fixups() call (that function was helpfully already exported
> although not called by anyone except phylib itself) after PHY reset in the
> 'sh_eth' driver which you saw in my patch and which you didn't like; I've
> also coded the PHY platform fixup which got successfully merged via
> arch/arm/mach-shmobile/ subtree at that time. Your argument was that the
> driver is going against the control flow provided by phylib (it's not alone
> doing PHY reset, I should note) and so you wanted me to embed everything
> into phylib control flow. I replied that I fear the unexpected consequencies
> of doing compulsory PHY reset for every driver using phylib (that's how I
> understood your idea at least)... then, after a long pause, I suggested to
> code phy_reset() function within phylib which the drivers that currently do
> reset their PHYs could use instead of open coding the reset bit polling
> loop, etc. and which would include a hook for the platform fixups and PHY
> driver config_init() call. I'd still like to hear your opinion on this
> approach -- which is less invasive.
>    What I can also do in this case is again ignore ETH_LINK signal in the
> 'sh_eth' driver and stop caring about the LED functions matching to what's
> designed for the boards. This doesn't need PHY platform fixup or any change
> in phylib and Ethernet driver.

This signal is probably present for a good reason: avoiding expensive
MDIO register access, and if that works for most platforms why not
keeping it?

>    What I also can do is stop resetting PHY in the 'sh_eth' driver (only
> getting it out of sleep for which the reset doesn't seem necessary),
> although we'd still need the PHY platform fixup for the PHY resets done via
> phy_mii_ioctl()... I'm not so much familiar with the driver to be sure it
> won't hurt (the driver is used on e.g. some SH platforms I don't get any
> access to) and I couldn't get any useful input from the driver's primary
> author -- but perhaps it's really not necessary. What do you think?

Quite a lot of PHYs actually require a reset through BMCR_RESET when
resuming from S2/S3 sleep states and even if they do not, it does not
hurt doing so, I would welcome a generic solution to do that which
would not circumvent the libphy state machine and APIs. So the general
idea would be to:

- provide a helper, e.g: phy_reset(phydev) which:
- toggles BMCR_RESET, waits for it to be clear
- call fixups on the phy_device
- call config_init on the phy_device
- restores any kind of duplex/autoneg settings we enabled/disabled
- resume the PHY state machine at the appropriate state
--
Florian

Re: [PATCH] sh_eth: call phy_scan_fixups() after PHY reset

[Sergei Shtylyov]

Hello.

On 11/05/2013 11:01 PM, David Miller wrote:

>>     David, will you ever reply to this mail?

> I really haven't had the time with my backlog, and it's been so long ago
> that I've forgotten all of the context.

    Hm, I thought I provided enough context and also that's what mail archives 
are for... :-)

> Sorry, someone will have to start the conversation up anew.

    Anyway, here's the story told anew, with all the gory details. :-)
    Many Renesas R-Car development boards use Micrel KSZ8041 as a PHY for the 
SoC Ethernet device. This PHY has bits 14-15 of the PHY control register 1 (at 
address 0x1E) selecting LED mode for 2 LED outputs: value 00 means LED0 acting 
as LINK/ACTIVITY and LED1 as SPEED, value 01 means LED0 acting as LINK and 
LED1 as ACTIVITY; these bits are subject to the PHY reset via BMCR and get 
reset to 00 -- this is IMO bad design since it makes more sense for the LED 
mode to be pin-strapped from the external switches and so not changed on every 
PHY reset.
    Now LED0 is also connected to the sideband ETH_LINK signal of the SoC 
Ethernet device which indicates the link state to the Ethernet core (so called 
feLic); strictly speaking, it's not needed by the driver as it can receive the 
link state from the PHY via phylib. On the BOCK-W board we had the Ethenet 
LEDs labelled matching the 00 value of the bits 14-15, we had no issues with 
the LED mode changing after reset but we had an issue with ETH_LINK bouncing 
off/on after each packet with which we dealt by completely ignoring this 
signal in the 'sh_eth' driver (this is possible using 'no_ether_link' driver's 
platform data field). Now, on the newer Lager/ Koelsch boards we have the 
Ethenet LEDs labelled matching the 01 value of the bits 14-15, so we should 
get the valid ETH_LINK signal (except it's inverted but we deal with this 
using 'ether_link_active_low' platform data field)... if the 'sh_eth' driver 
didn't reset the PHY each time the device is opened, which gets us back to the 
value of 00 of the PHY LED control bits and so to the issue of bouncing 
ETH_LINK signal (that unfortunately leads to an NFS timeout with NFS root) and 
the LED behavior doesn't match to the LED labels anymore.

    Now, about the ways to deal with this issue that I see.
    First I thought about using PHY platform fixup mechanism which is already 
hooked up to a PHY reset in phy_mii_ioctl() (the code is somewhat naive though 
as it doesn't wait for the reset completion before calling fixups and the 
driver's config_init() method). All I needed is calling fixups after direct 
PHY reset done via phy_write() but I didn't want to add a hook there due to 
that reset completion wait loop that was obviously necessary (this function is 
simple *inline*), so went for a simplistic local phy_scan_fixups() call (that 
function was helpfully already exported although not called by anyone except 
phylib itself) after PHY reset in the 'sh_eth' driver which you saw in my 
patch and which you didn't like; I've also coded the PHY platform fixup which 
got successfully merged via arch/arm/mach-shmobile/ subtree at that time. Your 
argument was that the driver is going against the control flow provided by 
phylib (it's not alone doing PHY reset, I should note) and so you wanted me to 
embed everything into phylib control flow. I replied that I fear the 
unexpected consequencies of doing compulsory PHY reset for every driver using 
phylib (that's how I understood your idea at least)... then, after a long 
pause, I suggested to code phy_reset() function within phylib which the 
drivers that currently do reset their PHYs could use instead of open coding 
the reset bit polling loop, etc. and which would include a hook for the 
platform fixups and PHY driver config_init() call. I'd still like to hear your 
opinion on this approach -- which is less invasive.
    What I can also do in this case is again ignore ETH_LINK signal in the 
'sh_eth' driver and stop caring about the LED functions matching to what's 
designed for the boards. This doesn't need PHY platform fixup or any change in 
phylib and Ethernet driver.
    What I also can do is stop resetting PHY in the 'sh_eth' driver (only 
getting it out of sleep for which the reset doesn't seem necessary), although 
we'd still need the PHY platform fixup for the PHY resets done via 
phy_mii_ioctl()... I'm not so much familiar with the driver to be sure it 
won't hurt (the driver is used on e.g. some SH platforms I don't get any 
access to) and I couldn't get any useful input from the driver's primary 
author -- but perhaps it's really not necessary. What do you think?

WBR, Sergei

Re: [PATCH] sh_eth: call phy_scan_fixups() after PHY reset

[David Miller]

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Fri, 15 Nov 2013 16:04:52 -0800

> Quite a lot of PHYs actually require a reset through BMCR_RESET when
> resuming from S2/S3 sleep states and even if they do not, it does not
> hurt doing so, I would welcome a generic solution to do that which
> would not circumvent the libphy state machine and APIs.
> idea would be to:

There was recently a patch I had to reject which had the same problem,
it wanted to BMCR_RESET the PHY when the link goes down to avoid a
problem with the hardware that causes a hang.

I brought up the same objection you are here, namely that resetting
the chip puts the hardware and phylib out of sync wrt. what is
configured into the chip.

So the generic solution you suggest here could help that driver
achieve what it wants too.

Re: [PATCH] sh_eth: call phy_scan_fixups() after PHY reset

[Sergei Shtylyov]

Hello.

On 16-11-2013 4:04, Florian Fainelli wrote:

>>>>      David, will you ever reply to this mail?

>>> I really haven't had the time with my backlog, and it's been so long ago
>>> that I've forgotten all of the context.

>>     Hm, I thought I provided enough context and also that's what mail
>> archives are for... :-)

>>> Sorry, someone will have to start the conversation up anew.

> [snip]

>>     Now, about the ways to deal with this issue that I see.
>>     First I thought about using PHY platform fixup mechanism which is already
>> hooked up to a PHY reset in phy_mii_ioctl() (the code is somewhat naive
>> though as it doesn't wait for the reset completion before calling fixups and
>> the driver's config_init() method).

> That's something that needs fixing and also a dedicated helper or such
> should be used because we are potentially messing up with the
> configuration the PHY library thinks it has applied to the PHY (e.g:
> disabling auto-negotiation etc...).

    Agreed.

>> All I needed is calling fixups after
>> direct PHY reset done via phy_write() but I didn't want to add a hook there
>> due to that reset completion wait loop that was obviously necessary (this
>> function is simple *inline*), so went for a simplistic local
>> phy_scan_fixups() call (that function was helpfully already exported
>> although not called by anyone except phylib itself) after PHY reset in the
>> 'sh_eth' driver which you saw in my patch and which you didn't like; I've
>> also coded the PHY platform fixup which got successfully merged via
>> arch/arm/mach-shmobile/ subtree at that time. Your argument was that the
>> driver is going against the control flow provided by phylib (it's not alone
>> doing PHY reset, I should note) and so you wanted me to embed everything
>> into phylib control flow. I replied that I fear the unexpected consequencies
>> of doing compulsory PHY reset for every driver using phylib (that's how I
>> understood your idea at least)... then, after a long pause, I suggested to
>> code phy_reset() function within phylib which the drivers that currently do
>> reset their PHYs could use instead of open coding the reset bit polling
>> loop, etc. and which would include a hook for the platform fixups and PHY
>> driver config_init() call. I'd still like to hear your opinion on this
>> approach -- which is less invasive.
>>     What I can also do in this case is again ignore ETH_LINK signal in the
>> 'sh_eth' driver and stop caring about the LED functions matching to what's
>> designed for the boards. This doesn't need PHY platform fixup or any change
>> in phylib and Ethernet driver.

> This signal is probably present for a good reason: avoiding expensive
> MDIO register access,

    As the driver is using phylib (and that in the polled mode) anyway, this 
advantage is nullified.

> and if that works for most platforms why not
> keeping it?

    Because it starts to bounce on/off after each packet as I've already told 
(which most probably affects the packet reception as seen from NFS timeout) in 
the default mode of KSZ8041. We had to completely ignore this signal on BOCK-W 
where LED mode 00 is used (and yet I had to fix the 'sh_eth' driver's logic 
that should have relied on phylib's callback to become aware of the link 
state). On the newer boards, we have this issue again after we reset the PHY 
when opening 'eth0' device. I'm not sure why I have to repeat the text that 
you've skipped.

>>     What I also can do is stop resetting PHY in the 'sh_eth' driver (only
>> getting it out of sleep for which the reset doesn't seem necessary),
>> although we'd still need the PHY platform fixup for the PHY resets done via
>> phy_mii_ioctl()... I'm not so much familiar with the driver to be sure it
>> won't hurt (the driver is used on e.g. some SH platforms I don't get any
>> access to) and I couldn't get any useful input from the driver's primary
>> author -- but perhaps it's really not necessary. What do you think?

> Quite a lot of PHYs actually require a reset through BMCR_RESET when
> resuming from S2/S3 sleep states and even if they do not, it does not

    I'm not very familiar with PHY power management ATM, and only know about 
the BMCR powerdown bit...

> hurt doing so, I would welcome a generic solution to do that which
> would not circumvent the libphy state machine and APIs. So the general
> idea would be to:

> - provide a helper, e.g: phy_reset(phydev) which:
> - toggles BMCR_RESET, waits for it to be clear
> - call fixups on the phy_device
> - call config_init on the phy_device
> - restores any kind of duplex/autoneg settings we enabled/disabled

    I'm not sure I follow here. If the driver calls phy_reset(), it should 
know already that these settings will be affected, no? This sentence only 
makes sense to me if we do phy_reset() somewhere in the PHY PM code as you 
have hinted...

> - resume the PHY state machine at the appropriate state

    Too bad I'm only slightly familiar with phylib ATM.
    Being in hospital also doesn't help to advance on that matter. :-(

> --
> Florian

WBR, Sergei