Fwd: Micrel KSZ9031RN PHY problem

[Guillermo Rodriguez Garcia]

Hello,

2016-04-28 23:09 GMT+02:00 Trent Piepho <tpiepho at kymetacorp.com>:
> On Thu, 2016-04-28 at 11:51 +0200, Guillermo Rodriguez Garcia wrote:
>> >
>> > The code you are referring to seems to be the first phy access after the
>> > network device opens. Maybe the hardware is not yet ready for some
>> > reason. Could you add the same delay in macb_open() right before the
>> > call to phy_device_connect()?
>>
>> Tried this, but doesn't help. Here's the call sequence, for example
>> when you run 'dhcp' from the command line prompt:
>>
>> macb_open
>> phy_device_connect
>> phy_device_attach
>> phy_init_hw
>> ksz9031_config_init
>> ksz9031_center_flp_timing
>> genphy_restart_aneg
>> genphy_config_advert -> fails (adv = 0xffff)
>>
>> I can verify that:
>> - Any delay happening before the call to genphy_restart_aneg (which
>> itself is called at the end of ksz9031_center_flp_timing) does not fix
>> the problem.
>> - Any delay happening after the phy_read(phydev, MII_ADVERTISE) insde
>> genphy_config_advert does not fix the problem.
>> - A delay of 1ms at any point after genphy_restart_aneg and before the
>> phy_read call in genphy_config_advert fixes the problem
>
> So the same problem appears if you take out the FLP call out entirely,
> if there is an aneg restart done immediately before trying to read from
> MII_ADVERTISE?

Yes, this is correct. Even if I comment out the FLP timing writes, the
fact that ksz9031_center_flp_timing calls genphy_restart_aneg is
enough to trigger the problem.

static int ksz9031_center_flp_timing(struct phy_device *phydev)
{
 /* Center KSZ9031RNX FLP timing at 16ms. */
// phy_write_mmd_indirect(phydev, MII_KSZ9031RN_FLP_BURST_TX_HI, 0, 0x0006);
// phy_write_mmd_indirect(phydev, MII_KSZ9031RN_FLP_BURST_TX_LO, 0, 0x1a80);

 return genphy_restart_aneg(phydev);
}

>
> Does only MII_ADVERTISE fail to read, or does reading any PHY register
> fail?  Specifically, does reading MII_BMSR fail?

Looks like it will happen with any register. If I try to read MII_BMSR
right before MII_ADVERTISE, then MII_BMSR reads as 0xffff.

(In this case, the negotiation will actually complete, since this
additional MII_BMSR read seems to be enough to cause the next
MII_ADVERTISE read to succeed).


>
> Reading PHY register after restarting aneg isn't unusual.  Especially
> polling MII_BMSR to check for BMSR_ANEGCOMPLETE.  It would be odd if one
> couldn't do that.
>
> The first anreg start call will also un-powerdown the PHY if BMCR_PDOWN
> was set.  I wonder if that is happening?

That was a very good hint and it looks like this is exactly what is happening.

genphy_restart_aneg() clears the BMCR_PDOWN bit and would get the phy
out of powerdown mode. I have added a trace right at the beginning of
genphy_restart_aneg and verified that BMCR_PDOWN bit was set before
genphy_restart_aneg clears it.

Then, the datasheet for the ksz9031 [1], page 44, says:

After this bit is changed from '1' to '0', an internal global reset is
automatically generated. Wait a minimum of 1ms before read/write
access to the PHY registers.

 [1]: http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9031RNX.pdf

So this seems to be what is causing the problem. At least on the
ksz9031 (don't know about others), a delay of 1ms is required when
coming out of powerdown mode.

What is the best way to fix this? We can add a 1ms delay in
genphy_restart_aneg (this is probably the easiest, and the delay is
small enough that it shouldn't make a difference for other phys that
might not need it). Or if this is not acceptable, perhaps add a custom
restart_aneg function for the ksz9031.

Best,

Guillermo Rodriguez Garcia
guille.rodriguez at gmail.com