Re: [OpenRD Ultimate] e1000phy(88E1149/88E1121) has a initialize issue

On Sun, Jun 20, 2010 at 09:15:00PM +0900, Norikatsu Shigemura wrote:
> Hi yongari.
> 
> On Tue, 15 Jun 2010 11:09:23 -0700
> Pyun YongHyeon <pyunyh_at_gmail.com> wrote:
> > > - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
> > > Jun 13 05:02:14 sidearms kernel: mge1: watchdog timeout
> > > Jun 13 05:02:14 sidearms kernel: mge1: Timeout on link-up
> >   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > This part looks like a bug in mge(4). Driver does not know how long
> > it would take to get a valid link. Waiting for valid link in driver
> > initialization does not work(e.g Starting controller without UTP
> > cable may always fail on mge(4)). I think mge(4) should implement
> > correct link state change handling.
> 
> 	Thanks for your pointed out.  I didn't notice.
> 	I'll fix this issue like following on mge_start_locked().
> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
> -       if ((ifp->if_drv_flags & (IFF_DRV_RUNNING | IFF_DRV_OACTIVE)) !=
> -           IFF_DRV_RUNNING)
> +       if (IFM_SUBTYPE(sc->mii->mii_media_active) == IFM_NONE ||
> +           (ifp->if_drv_flags & (IFF_DRV_RUNNING | IFF_DRV_OACTIVE)) != IFF_DRV_RUNNING)

No, that change may not enough to fix a missing link state handling.
mge(4) needs a miibus_statchg KOBJ handler and it requires a lot of
code changes in mge(4).

> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
> 
> > > 	I found a initialize issue in e1000phy.c.
> > > - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
> > > --- sys/dev/mii/e1000phy.c.orig	2010-05-01 10:17:15.282196000 +0900
> > > +++ sys/dev/mii/e1000phy.c	2010-06-13 16:19:46.616650536 +0900
> > > _at__at_ -278,6 +278,7 _at__at_
> > >  	case MII_MODEL_MARVELL_E1118:
> > >  		break;
> > >  	case MII_MODEL_MARVELL_E1116:
> > > +	case MII_MODEL_MARVELL_E1149:
> > >  		page = PHY_READ(sc, E1000_EADR);
> > >  		/* Select page 3, LED control register. */
> > >  		PHY_WRITE(sc, E1000_EADR, 3);
> > > - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
> > > 	I confirmed OK on my environment, OpenRD Ultimate has a 88E1121(I
> > > 	saw it, physically):
> > Once it was there but I removed it due to some other issues seen on
> > Yukon Ultra. That part will program LED access so I guess it
> > wouldn't affect normal network activity.
> 
> 	Hum..., like following?  At least, by current way, second NIC
> 	doesn't link-up if link connected.
> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
>  	case MII_MODEL_MARVELL_E1118:
> +	case MII_MODEL_MARVELL_E1149:
>  		break;

This change was also backed out due to 88E8072 issue on
establishing 100Mbps link. e1000phy(4)'s 88E1149 PHY handling seems
to require more work as some Yukon Ultra/Ultra II still have
problems. ATM I have no idea whether this issue comes from MAC
controller or not.

>  	case MII_MODEL_MARVELL_E1116:
>  		page = PHY_READ(sc, E1000_EADR);
>  		/* Select page 3, LED control register. */
>  		PHY_WRITE(sc, E1000_EADR, 3);
> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
> 
> 
> > 88E1116R might be RGMII variant of 88E1116. Because I don't have
> > controller that has 88E1116R I didn't treat it as 88E1116. With
> > that change could you use straight cable without help of MDI/MDI-X
> > converter?
> 
> 	Sorry, I don't have 88E1116R machine, so I don't know.

Ok.