Small change to ukphy

[Marius Strobl]

On Wed, Apr 01, 2009 at 07:09:39PM +0900, Pyun YongHyeon wrote:
> On Wed, Apr 01, 2009 at 01:32:46AM -0600, M. Warner Losh wrote:
> > I've encountered a number of PHY chips that need auto negotiation
> > kicked off to come out of ISO state.  This makes sense, because the
> > ukphy driver never seems to take the PHY out of isolation state
> > otherwise.
> > 
> > Index: ukphy.c
> > ===================================================================
> > --- ukphy.c	(revision 190463)
> > +++ ukphy.c	(working copy)
> > @@ -146,6 +146,7 @@
> >  	sc->mii_phy = ma->mii_phyno;
> >  	sc->mii_service = ukphy_service;
> >  	sc->mii_pdata = mii;
> > +	sc->mii_flags |= MIIF_FORCEANEG;
> >  
> >  	mii->mii_instance++;
> >  
> > 
> > This forces auto negotiation.  The reason for this is that it takes it
> > out of ISO state (Isolate).  Once out of that state, things work
> 
> If the purpose is to take PHY out of isolated state couldn't this
> be handled in ifm_change_cb_t handler of parent interface? I guess
> the callback can reset the PHY and subsequent mii_mediachg() call
> may start auto-negotiation.
> 
> > well.  The question I have is will we properly go back into ISO state
> > for PHYs that should be isolated.
> > 
> 
> If the PHY requires special handing for ISO state in reset it may
> need separated PHY driver as ukphy(4) does not set MIIF_NOISOLATE. 
> As you said it would be really great if we have a generic way to
> pass various MII flags or driver specific information to mii(4).
> 
> > NetBSD has many of its NIC drivers setting this flag.  Their APIs
> > allow them to set this directly at mii attach time.  Ours don't, so
> > none of our drivers set this flag.
> > 
> > The other fix for this might be:
> > Index: mii_physubr.c
> > ===================================================================
> > --- mii_physubr.c	(revision 190463)
> > +++ mii_physubr.c	(working copy)
> > @@ -113,7 +113,9 @@
> >  	int bmcr, anar, gtcr;
> >  
> >  	if (IFM_SUBTYPE(ife->ifm_media) == IFM_AUTO) {
> > -		if ((PHY_READ(sc, MII_BMCR) & BMCR_AUTOEN) == 0 ||
> > +		bmcr = PHY_READ(sc, MII_BMCR);
> > +		if ((bmcr & BMCR_AUTOEN) == 0 ||
> > +		    (bmcr & BMCR_ISO) ||
> >  		    (sc->mii_flags & MIIF_FORCEANEG))
> >  			(void) mii_phy_auto(sc);
> >  		return;
> > 
> > Which says that if auto negotiation is enabled, and ISO is set to go
> > ahead and kick off an auto negotiation.  I'm less sure of this path,
> > but it is an alternative.  Otherwise, we never write to the BMCR to
> > take the device out of isolation.  If there's a better place to do
> > this, then I'm all ears.
> > 
> > Either one of these hacks make several PC Cards that I have start to
> > work...  In fact, I'm starting to approach 100% (up from 50%) of my
> > ed-based PC Cards working with this simple change (and others to the
> > ed driver).  I know that these cards are a little behind the leading
> > edge, but I'd like to get them working since I've put a few hours into
> > investigating things here.
> > 
> > Comments?
> > 

FYI, the idea I had for passing MIIF_DOPAUSE from the NIC
drivers to the PHY drivers as required by the flow-control
support without breaking the ABI was to use device flags.
A proof-of-concept patch with an example application of
that approach is:
http://people.freebsd.org/~marius/mii_flags.diff
One could even or the flags together in miibus_attach(),
allowing MIIF_FORCEANEG etc to be additionally set via
hints.

Marius