MII & GMII Common Layer [PSARC/2009/319 FastTrack timeout 06/01/2009]

[James Carlson]

Garrett D'Amore writes:
> > It might be a good idea to return a value outside the 0-0xffff range
> > for error.  I'd use 'int' as the return type and -1 for error.
> >   
> 
> I could do that.  The problem is that usually an error is reported by 
> the hardware as 0xffff.  (E.g. if you try to read a register that is not 
> present or a PHY that is not present.)  So adding a different error 
> return (-1) would add confusion.

Ah, ok.  As long as none of the registers can ever legitimately have
0xffff in them, I guess it's ok.

> >> void mii_write(void *state, uint8_t phy_addr, uint8_t reg_num, uint16_t value);
> >>
> >>     Writes the 16-bit value to the named phy and register.  Returns 0
> >>     on success, or an errno on failure.
> >>     
> >
> > Was this supposed to be 'int' instead of 'void'?  Or can't it fail?
> >   
> 
> For most hardware types, it can't fail.  (There are some devices that 
> have "shared" access to the MII bus where the MAC driver could time out 
> waiting to grant direct access to the MII bus.  Most drivers don't have 
> or need that, though.)  I believe the better solution to handling a 
> failure here is through the use of FMA.  That said, if a write failed, 
> its almost certain that the MII reads would also be failing, with the 
> situation that the MII would be left in a state of link failure -- so 
> this would be reported via the normal link failure mechanism.   So no 
> return is given back to the caller -- the void is intentional.

The text says that it returns 0 or errno.  :->

> > When would this be used?  m_instance isn't often overridden, at least
> > in the current code, and I don't see this function in the example code
> > you provided.
> >   
> 
> I've changed the code a bit since writing this.  I've removed the 
> function altogether, and added a new argument (instance) to 
> mii_alloc().  This was done because I needed the instance number to 
> formulate a unique name for the taskq that I create at allocation.  The 
> new signature of mii_alloc() is:
> 
> mii_handle_t *mii_alloc(void *private, dev_info_t*dip, int instance, mii_ops_t *ops);

OK.

> Drivers that have PPA == ddi_get_instance() may just supply 0 for the 
> instance.

So you can't override and specify (otherwise legal) instance 0?

> >>     active, the value -1 will be returned.  Note that it is perfectly
> >>     reasonable for a PHY to have no ID registers, in which case a
> >>     value of either 0 or (uint32_t)-1 may be returned.
> >>     
> >
> > Either is possible in that one case?
> >   
> 
> Yes.  The hardware may return either value.  (I've seen both cases.)

"ick."

-- 
James Carlson, Solaris Networking              <james.d.carlson at sun.com>
Sun Microsystems / 35 Network Drive        71.232W   Vox +1 781 442 2084
MS UBUR02-212 / Burlington MA 01803-2757   42.496N   Fax +1 781 442 1677