Re: Fwd: Micrel KSZ9031RN PHY problem

From: Trent Piepho <tpiepho@kymetacorp.com>
To: Guillermo Rodriguez Garcia <guille.rodriguez@gmail.com>
Cc: "barebox@lists.infradead.org" <barebox@lists.infradead.org>,
	Philipp Zabel <pza@pengutronix.de>
Subject: Re: Fwd: Micrel KSZ9031RN PHY problem
Date: Fri, 29 Apr 2016 18:18:44 +0000	[thread overview]
Message-ID: <1461953980.9103.189.camel@rtred1test09.kymeta.local> (raw)
In-Reply-To: <CABDcavYOujwe8iQt4m8VQQcajkBiEWpw+Vj=6ghqtp=q09oXGw@mail.gmail.com>

On Fri, 2016-04-29 at 13:00 +0200, Guillermo Rodriguez Garcia wrote:
> 2016-04-28 23:09 GMT+02:00 Trent Piepho <tpiepho@kymetacorp.com>:
> >
> > 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.

Mystery solved!

> 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.

The kernel will take the phy in/out of powerdown mode as part of the PM
suspend/resume calls, which is supported on all micrel phys since 2013.
I don't see a delay in the kernel code and wonder why this hasn't been a
problem?  Might be worth asking on net-dev if this is a known issue with
some phys and how it is solved?  Maybe it's an undiscovered cause of
network flakiness after a resume.

> 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.

Could add a custom init function that un-powerdowns the phy and does the
wait.

Or have restart_aneg check if the powerdown bit was set before it clears
it, and only delay in that case.

Having the un-powerdown in the restart_aneg isn't really the right place
for it.  If there is no reason the restart aneg, then the phy will not
be powered up.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox