Bug 15147

Summary: phylib: no re-negotiation after link partner abilities changed
Product: Drivers Reporter: Danny Baumann (dannybaumann)
Component: NetworkAssignee: drivers_network (drivers_network)
Status: RESOLVED OBSOLETE    
Severity: normal CC: alan, chkr
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: Subsystem:
Regression: No Bisected commit-id:

Description Danny Baumann 2010-01-26 16:24:33 UTC
E.g. with the macb driver, which uses phylib, there is no link anymore when the link partner changes its abilities. This is because no autonegotiation is done again on link changes and hence the LPA register is not updated.

Steps for reproduction:
- Connect a 10/100 device to a 10/100 switch, configure switch port to 100Full
- Enable interface, link will be present
- Configure device to 10Full, link will be lost
- Configure switch to 10Full

Result:
Link should be present at that point, but isn't ... debug messages in genphy_read_status show that lpa is still at 0x4101 (100Full).

Note that all that is on an AT91SAM9G20 board (hence macb driver) with a Dawicom 9161 PHY connected to its Ethernet port, but AFAICT all phylib enabled drivers should suffer from this.
Comment 1 Andrew Morton 2010-01-28 00:48:38 UTC
(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Tue, 26 Jan 2010 16:24:35 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=15147
> 
>            Summary: phylib: no re-negotiation after link partner abilities
>                     changed
>            Product: Drivers
>            Version: 2.5
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Network
>         AssignedTo: drivers_network@kernel-bugs.osdl.org
>         ReportedBy: dannybaumann@web.de
>         Regression: No
> 
> 
> E.g. with the macb driver, which uses phylib, there is no link anymore when
> the
> link partner changes its abilities. This is because no autonegotiation is
> done
> again on link changes and hence the LPA register is not updated.
> 
> Steps for reproduction:
> - Connect a 10/100 device to a 10/100 switch, configure switch port to
> 100Full
> - Enable interface, link will be present
> - Configure device to 10Full, link will be lost
> - Configure switch to 10Full
> 
> Result:
> Link should be present at that point, but isn't ... debug messages in
> genphy_read_status show that lpa is still at 0x4101 (100Full).
> 
> Note that all that is on an AT91SAM9G20 board (hence macb driver) with a
> Dawicom 9161 PHY connected to its Ethernet port, but AFAICT all phylib
> enabled
> drivers should suffer from this.
> 

From the above it appears that this is a regression.

Can you tell us which kernel version(s) were OK and which are not?

Thanks.
Comment 2 Danny Baumann 2010-01-28 08:27:41 UTC
Hi,

> >            Summary: phylib: no re-negotiation after link partner abilities
> >                     changed

[...]

> >          Component: Network
> >         AssignedTo: drivers_network@kernel-bugs.osdl.org
> >         ReportedBy: dannybaumann@web.de
> >         Regression: No
            ^^^^^^^^^^^^^^

;-)

> > E.g. with the macb driver, which uses phylib, there is no link anymore when
> the
> > link partner changes its abilities. This is because no autonegotiation is
> done
> > again on link changes and hence the LPA register is not updated.
> > 
> > Steps for reproduction:
> > - Connect a 10/100 device to a 10/100 switch, configure switch port to
> 100Full
> > - Enable interface, link will be present
> > - Configure device to 10Full, link will be lost
> > - Configure switch to 10Full
> > 
> > Result:
> > Link should be present at that point, but isn't ... debug messages in
> > genphy_read_status show that lpa is still at 0x4101 (100Full).
> > 
> > Note that all that is on an AT91SAM9G20 board (hence macb driver) with a
> > Dawicom 9161 PHY connected to its Ethernet port, but AFAICT all phylib
> enabled
> > drivers should suffer from this.
> > 
> 
> From the above it appears that this is a regression.
> 
> Can you tell us which kernel version(s) were OK and which are not?

As far as I am aware of, this is not a regression, at least not in
phylib (if any, it's a regression for drivers that were converted to
phylib). A code part that does re-negotiation when link is down after
link was up for some time has never been present in the state machine.
In the PHY_NOLINK state, all that has ever been done is doing
phy_read_status and checking the new link state calculated from there.

Unfortunately, I don't have a good idea what would be the best thing to
do there. As far as I can tell from the basic MII PHY registers, there
is no way to distinct the "no link and no carrier = no cable connected"
and the "no link, but carrier = negotiation failed" states. If there was
such a way, the fix probably would involve re-negotiating when going
from state 1 to state 2 (or regularly using a timer when staying in
state 2). But maybe I am missing something.

Regards,

Danny