Re: [PATCH] Revert "net: phy: turn carrier off on phy attach"

[Florian Fainelli]

On February 14, 2016 10:25:30 AM PST, Clemens Gruber <clemens.gruber@xxxxxxxxxxxx> wrote:
>On Fri, Feb 12, 2016 at 10:56:04AM -0800, Florian Fainelli wrote:
>> On 12/02/16 10:01, Clemens Gruber wrote:
>> > Commit 113c74d83eef ("net: phy: turn carrier off on phy attach")
>breaks
>> > the eth0 link coming up on all my i.MX6Q boards with a Marvell
>88E1510.
>> > If I then do a ifconfig eth0 down/up cycle I first get a MDIO read
>> > timeout but then the link becomes ready and everything is back to
>> > normal.
>> > Without this step however, the link stays down forever, an
>unusually
>> > high amount of phy interrupts occur (about 10000/second) and
>kworker/0:2
>> > is constantly using over 60% of the CPU.
>> > 
>> > Reverting it fixes the problems with the link not coming up at boot
>as
>> > well as the high amount of phy interrupts and kworker load in that
>> > state.
>> 
>> You are seeing this with the FEC driver right? We probably want to
>> carefully audit the driver and understand what could be going wrong,
>the
>> initial change is correct, so there must be something else going on
>here.
>
>I think I found the underlying problem!
>It was not the fec driver but the marvell phy driver, more specifically
>the marvell_of_reg_init call being made too late, which lead to the
>observed problem at half the boot ups where the link never came up.
>
>With the marvell,reg-init device tree parameter, a flag needs to be set
>to tell the Marvell 88E1510 that it should enable the interrupt output.
>(At a specific pin, in my case LED[2])
>If this is not set (or set too late), the phydev->state is set to UP in
>phy_start (called from fec_enet_open) but then, the auto-negotiation
>never starts.
>In comparison, now, after I called marvell_of_reg_init not in
>m88e1510_config_aneg but in marvell_probe, everything works again :)
>About a second after the fec_enet_open/phy_start calls, the
>auto-negotiation starts (m88e1510_config_aneg) and the phy state
>changes
>from UP to AN, to CHANGELINK and finally to RUNNING.
>
>I will send a patch shortly, calling marvell_of_reg_init from a new
>m88e1510_probe function instead of the m88e1510_config_aneg function.

config_init is more appropriate here since this call back will be called even if there is a software reset (e.g: from phy_init_hw). config_aneg is definitely too late, thanks for finding this!

-- 
Florian