[9fans] kirkwood phy problems

[9fans] kirkwood phy problems

[erik quanstrom]

[-- Attachment #1: Type: text/plain, Size: 2932 bytes --]

recently i got my machine into a state were it would never
link.  after quite a while with the documentation, and puzzling
over how the existing the phy code in ether1116 matches,
i think i've gotten to the bottom of the problems.

i don't currently have a working guru or sheva.  i'd be interested if
anyone tries this on one of those machines.

here's the breakdown of the changes, and some commentary
on what and why it was changed:

- first and foremost, reg->phy was being set to ctlrno in reset().  this
is not correct for any phy we know about.  one usually gets 8, 9 or
8, 24.  since this register is already correct, just use it.  since this
register controls mac/phy interaction during autonegotiation,
incorrect setting explains failure to link.
	/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1718,1724 - ether1116.c:1630,1635

- since we already have a correct phy, and the documentation
says that the smi register is only connected for mac0, we don't
need mymii().  also, setting ctlrs[1]->mii == ctlrs[0]->mii, means
that mii->curphy is wrong for one (ctlr[1]).  use different mii structures
that differ only in ->currphy instead to correct this.
	/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1213,1230 - ether1116.c:1120,1139
	/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1096,1203 - ether1116.c:1101,1110

- additionally, since the phy is now correct, we can reset the phy
in kirkwoodmii().  the original code didn't work for me.
	/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1213,1230 - ether1116.c:1120,1139

- miiwr() and miird() didn't match the documentation.  this has been
corrected.  in addition, it seems to me that we need to lock smi access
as it's shared between macs, and there are two-phase operations.
	/n/dump/2011/1023/sys/src/9/kw/ether1116.c:993,1022 - ether1116.c:996,1023
	/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1026,1051 - ether1116.c:1027,1054
	/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1052,1058 - ether1116.c:1055,1060
	/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1061,1072 - ether1116.c:1063,1077

- the Bmsr register has a wierd protocol, and requires a back-to-back
read to get the current value rather than the last edge.  so a few eads
in kirkwoodmii() were doubled.  when we are resetting the link, this is
key to seeing a link.
	/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1233,1238 - ether1116.c:1142,1148

- in ctlrinit, miiphyinit, turn on phy interrupts so we can get link events.
	/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1496,1504 - ether1116.c:1408,1416
	/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1307,1312 - ether1116.c:1217,1224

- in interrupt() set the link both on link down and on link up.
	/n/dump/2011/1023/sys/src/9/kw/ether1116.c:883,890 - ether1116.c:883,892
 	/n/dump/2011/1023/sys/src/9/kw/ether1116.c:893,899 - ether1116.c:895,901
 	/n/dump/2011/1023/sys/src/9/kw/ether1116.c:916,921 - ether1116.c:918,924

- erik

[-- Attachment #2: ether1116.diff --]
[-- Type: text/plain, Size: 9379 bytes --]

/n/dump/2011/1023/sys/src/9/kw/ether1116.c:883,890 - ether1116.c:883,892
  		 * thus we note the link change here, and check for
  		 * that and autonegotiation done below.
  		 */
- 		if(irqe & IEphystschg) {
- //			ether->link = (reg->ps0 & PS0linkup) != 0;
+ 		if(irqe & (IElinkchg | IEphystschg)){
+ 			iprint("#l%d: ether1116: phy status changed %lux\n",
+ 				ctlr->port, irqe & (IElinkchg | IEphystschg));
+ 			ether->link = reg->ps1 & PS1an_done && (reg->ps0 & PS0linkup) != 0;
  			ether->linkchg = 1;
  		}
  		if(irqe & IEtxerrq(Qno))
/n/dump/2011/1023/sys/src/9/kw/ether1116.c:893,899 - ether1116.c:895,901
  			ether->overflows++;
  		if(irqe & IEtxunderrun)
  			ctlr->txunderrun++;
- 		if(irqe & (IEphystschg | IEtxerrq(Qno) | IErxoverrun |
+ 		if(irqe & (IElinkchg | IEphystschg | IEtxerrq(Qno) | IErxoverrun |
  		    IEtxunderrun))
  			handled++;
  	}
/n/dump/2011/1023/sys/src/9/kw/ether1116.c:916,921 - ether1116.c:918,924
  		handled++;
  		ether->link = (reg->ps0 & PS0linkup) != 0;
  		ether->linkchg = 0;
+ 		iprint("#l%d: ether1116: link changed\n", ctlr->port);
  	}
  	ctlr->newintrs++;

/n/dump/2011/1023/sys/src/9/kw/ether1116.c:993,1022 - ether1116.c:996,1023
  /*
   * phy/mii goo
   */
+ static Lock smilock;

  static int
- smibusywait(Gbereg *reg, ulong waitbit)
+ smiwaitclear(Gbereg *reg, ulong waitbit)
  {
- 	ulong timeout, smi_reg;
+ 	ulong timeout, smireg;

- 	timeout = PhysmiTimeout;
- 	/* wait till the SMI is not busy */
- 	do {
- 		/* read smi register */
- 		smi_reg = reg->smi;
- 		if (timeout-- == 0) {
+ 	for(timeout = PhysmiTimeout;; timeout--){
+ 		smireg = reg->smi;
+ 		if((smireg & waitbit) == 0)
+ 			return 0;
+ 		if (timeout == 0) {
  			MIIDBG("SMI busy timeout\n");
  			return -1;
  		}
- //		delay(1);
- 	} while (smi_reg & waitbit);
- 	return 0;
+ 	}
  }

  static int
  miird(Mii *mii, int pa, int ra)
  {
- 	ulong smi_reg, timeout;
+ 	ulong smireg, timeout;
  	Gbereg *reg;

  	reg = ((Ctlr*)mii->ctlr)->reg;
/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1026,1051 - ether1116.c:1027,1054
  	    (ra<<SmiRegaddroff) & ~SmiRegaddrmask)
  		return -1;

- 	smibusywait(reg, PhysmiBusy);
+ 	lock(&smilock);
+ 	if(smiwaitclear(reg, PhysmiBusy) == -1){
+ 		unlock(&smilock);
+ 		return -1;
+ 	}

  	/* fill the phy address and register offset and read opcode */
  	reg->smi = pa << Physmiaddroff | ra << SmiRegaddroff | PhysmiopRd;
- 	coherence();

  	/* wait til read value is ready */
- 	timeout = PhysmiTimeout;
- 	do {
- 		smi_reg = reg->smi;
- 		if (timeout-- == 0) {
+ 	for(timeout = PhysmiTimeout;; timeout--){
+ 		if (timeout == 0) {
+ 			unlock(&smilock);
  			MIIDBG("SMI read-valid timeout\n");
  			return -1;
  		}
- 	} while (!(smi_reg & PhysmiReadok));
-
- 	/* Wait for the data to update in the SMI register */
- 	for (timeout = 0; timeout < PhysmiTimeout; timeout++)
- 		;
- 	return reg->smi & Physmidatamask;
+ 		smireg = reg->smi;
+ 		if(smireg & PhysmiReadok){
+ 			unlock(&smilock);
+ 			return smireg & 0xffff;
+ 		}
+ 	}
  }

  static int
/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1052,1058 - ether1116.c:1055,1060
  miiwr(Mii *mii, int pa, int ra, int v)
  {
  	Gbereg *reg;
- 	ulong smi_reg;

  	reg = ((Ctlr*)mii->ctlr)->reg;

/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1061,1072 - ether1116.c:1063,1077
  	    ((ra<<SmiRegaddroff) & ~SmiRegaddrmask))
  		return -1;

- 	smibusywait(reg, PhysmiBusy);
+ 	lock(&smilock);
+ 	if(smiwaitclear(reg, PhysmiBusy) == -1){
+ 		unlock(&smilock);
+ 		return -1;
+ 	}

  	/* fill the phy address and register offset and read opcode */
- 	smi_reg = v << Physmidataoff | pa << Physmiaddroff | ra << SmiRegaddroff;
- 	reg->smi = smi_reg & ~PhysmiopRd;
- 	coherence();
+ 	reg->smi = v << Physmidataoff | pa << Physmiaddroff | ra << SmiRegaddroff;
+ 	unlock(&smilock);
  	return 0;
  }

/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1096,1203 - ether1116.c:1101,1110
  	Phy3016		= 0x26,		/* 88E3016 10/100 */
  };

- static int hackflavour;
-
  /*
   * on openrd, ether0's phy has address 8, ether1's is ether0's 24.
   * on guruplug, ether0's is phy 0 and ether1's is ether0's phy 1.
   */
- int
- mymii(Mii* mii, int mask)
- {
- 	Ctlr *ctlr;
- 	MiiPhy *miiphy;
- 	int bit, ctlrno, oui, model, phyno, r, rmask;
- 	static int dualport, phyidx;
- 	static int phynos[NMiiPhy];
-
- 	ctlr = mii->ctlr;
- 	ctlrno = ctlr->ether->ctlrno;
-
- 	/* first pass: figure out what kind of phy(s) we have. */
- 	dualport = 0;
- 	if (ctlrno == 0) {
- 		for(phyno = 0; phyno < NMiiPhy; phyno++){
- 			bit = 1<<phyno;
- 			if(!(mask & bit) || mii->mask & bit)
- 				continue;
- 			if(mii->mir(mii, phyno, Bmsr) == -1)
- 				continue;
- 			r = mii->mir(mii, phyno, Phyidr1);
- 			oui = (r & 0x3FFF)<<6;
- 			r = mii->mir(mii, phyno, Phyidr2);
- 			oui |= r>>10;
- 			model = MIIMODEL(r);
- 			if (oui == 0xfffff && model == 0x3f)
- 				continue;
- 			MIIDBG("ctlrno %d phy %d oui %#ux model %#ux\n",
- 				ctlrno, phyno, oui, model);
- 			if (oui == Ouimarvell &&
- 			    (model == Phy1121r || model == Phy1116r))
- 				++dualport;
- 			phynos[phyidx++] = phyno;
- 		}
- 		hackflavour = dualport == 2 && phyidx == 2? Hackdual: Hacknone;
- 		MIIDBG("ether1116: %s-port phy\n",
- 			hackflavour == Hackdual? "dual": "single");
- 	}
-
- 	/*
- 	 * Probe through mii for PHYs in mask;
- 	 * return the mask of those found in the current probe.
- 	 * If the PHY has not already been probed, update
- 	 * the Mii information.
- 	 */
- 	rmask = 0;
- 	if (hackflavour == Hackdual && ctlrno < phyidx) {
- 		/*
- 		 * openrd, guruplug or the like: use ether0's phys.
- 		 * this is a nasty hack, but so is the hardware.
- 		 */
- 		MIIDBG("ctlrno %d using ctlrno 0's phyno %d\n",
- 			ctlrno, phynos[ctlrno]);
- 		ctlr->mii = mii = ctlrs[0]->mii;
- 		mask = 1 << phynos[ctlrno];
- 		mii->mask = ~mask;
- 	}
- 	for(phyno = 0; phyno < NMiiPhy; phyno++){
- 		bit = 1<<phyno;
- 		if(!(mask & bit))
- 			continue;
- 		if(mii->mask & bit){
- 			rmask |= bit;
- 			continue;
- 		}
- 		if(mii->mir(mii, phyno, Bmsr) == -1)
- 			continue;
- 		r = mii->mir(mii, phyno, Phyidr1);
- 		oui = (r & 0x3FFF)<<6;
- 		r = mii->mir(mii, phyno, Phyidr2);
- 		oui |= r>>10;
- 		if(oui == 0xFFFFF || oui == 0)
- 			continue;
-
- 		if((miiphy = malloc(sizeof(MiiPhy))) == nil)
- 			continue;
- 		miiphy->mii = mii;
- 		miiphy->oui = oui;
- 		miiphy->phyno = phyno;
-
- 		miiphy->anar = ~0;
- 		miiphy->fc = ~0;
- 		miiphy->mscr = ~0;
-
- 		mii->phy[phyno] = miiphy;
- 		if(ctlrno == 0 || hackflavour != Hackdual && mii->curphy == nil)
- 			mii->curphy = miiphy;
- 		mii->mask |= bit;
- 		mii->nphy++;
-
- 		rmask |= bit;
- 	}
- 	return rmask;
- }
-
  static int
  kirkwoodmii(Ether *ether)
  {
/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1213,1230 - ether1116.c:1120,1139
  	ctlr->mii->mir = miird;
  	ctlr->mii->miw = miiwr;

- 	if(mymii(ctlr->mii, ~0) == 0 || (phy = ctlr->mii->curphy) == nil){
- 		print("#l%d: ether1116: init mii failure\n", ether->ctlrno);
+ 	if(ctlr->port>0){
+ 		if(ctlrs[0]->mii == nil || ctlrs[0]->mii->phy[ctlr->reg->phy] == nil)
+ 			return -1;
+ 		memmove(ctlr->mii, ctlrs[0]->mii, sizeof *ctlr->mii);
+ 		phy = ctlr->mii->curphy = ctlr->mii->phy[ctlr->reg->phy];
+ 	}else if(mii(ctlr->mii, ~0) == 0 || (phy = ctlr->mii->curphy) == nil){
+ 		print("#l%d: ether1116: init mii failure phy %#ld\n", ether->ctlrno, ctlr->reg->phy);
  		free(ctlr->mii);
  		ctlr->mii = nil;
  		return -1;
  	}

- 	/* oui 005043 is marvell */
- 	MIIDBG("oui %#ux phyno %d\n", phy->oui, phy->phyno);
- 	// TODO: does this make sense? shouldn't each phy be initialised?
- 	if((ctlr->ether->ctlrno == 0 || hackflavour != Hackdual) &&
- 	    miistatus(ctlr->mii) < 0){
+ 	MIIDBG("oui %#.6ux phyno %d\n", phy->oui, phy->phyno);
+ 	if(miistatus(ctlr->mii) < 0){
  		miireset(ctlr->mii);
  		MIIDBG("miireset\n");
  		if(miiane(ctlr->mii, ~0, 0, ~0) < 0){
/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1233,1238 - ether1116.c:1142,1148
  		}
  		MIIDBG("miistatus\n");
  		miistatus(ctlr->mii);
+ 		miird(ctlr->mii, phy->phyno, Bmsr);		/* need back-to-back read */
  		if(miird(ctlr->mii, phy->phyno, Bmsr) & BmsrLs){
  			for(i = 0; ; i++){
  				if(i > 600){
/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1307,1312 - ether1116.c:1217,1224
  	miiwr(mii, dev, Scr,
  		(miird(mii, dev, Scr) & ~(Pwrdown|Endetect)) | Mdix);

+ 	miimiw(mii, 18, ~0);	/* phy interrupts */
+
  	return 0;
  }

/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1496,1504 - ether1116.c:1408,1416

  	/* allow just these interrupts */
  	/* guruplug generates Irxerr interrupts continually */
- 	reg->irqmask = Isum | Irx | Irxbufferq(Qno) | Irxerr | Itxendq(Qno);
+ 	reg->irqmask = Isum | Irx | Irxbufferq(Qno) | Irxerr | Itxendq(Qno) | Iextend;
  	reg->irqemask = IEsum | IEtxerrq(Qno) | IEphystschg | IErxoverrun |
- 		IEtxunderrun;
+ 		IEtxunderrun | IElinkchg;

  	reg->irqe = 0;
  	reg->euirqmask = 0;
/n/dump/2011/1023/sys/src/9/kw/ether1116.c:1718,1724 - ether1116.c:1630,1635

  	/* Set phy address of the port */
  	ctlr->port = ether->ctlrno;
- 	ctlr->reg->phy = ether->ctlrno;
  	coherence();
  	ether->port = (uintptr)ctlr->reg;