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

[James Carlson]

Garrett D'Amore - sun microsystems writes:
> We're seeking Minor binding at the moment, as we have concrete plans
> to backport this.  We do believe that at some point in the future it

As Darren noted, there's a 'not' missing somewhere.

> struct mii_ops {
>        int mii_version;
>        uint16_t mii_read();
>        void mii_write();
>        void mii_notify();
>        void mii_reset
> };

Is mii_reset a function?  It's not described.

> uint16_t mii_read(void *state, uint8_t phy_addr, uint8_t reg_num);
> 
>     Reads a register, and stores the 16-bit register value in valuep.
>     Returns the value on success, or 0xffff on failure.  Note that
>     the register and phy addresses are constrained to values 0-31 per
>     IEEE 802.3 clause 22.  The state value is an opaque pointer to
>     the mac driver's soft state.

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.

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

Missing from here are context details.  Can any of these functions be
called due to an interrupt (such as a timer) thread?  Are any locks
held?  Are there restrictions on what functions the driver's callback
can call?  Could the driver's callback block?

> mii_handle_t *mii_alloc(void *private, dev_info_t*dip, mii_ops_t *ops);
> 
>     Allocate an MII handle.  Called during driver's attach(9e)
>     handling, this routine is valid in kernel context only.  The
>     "private" argument points to a driver private state structure,
>     which will be passed back to the driver as the first argument to
>     each of its entry points supplied in the ops entry point.  The dip
>     is the dev_info node for the MAC instance structure.  On success a
>     valid MII handle is returned, otherwise NULL may be returned.

You don't have to do it for a private interface, but having allocator
and free functions for mii_ops_t will simplify updates in the future.

Does the MII module make a copy of the 'ops' vector, or does it need
to remain allocated for the time that the mii_handle_t is allocated?
(Are you assuming that mii_ops_t is in static duration storage?)

> void mii_set_instance(mii_handle_t *mii, uint_t instance);
> 
>     Sets the instance (PPA) of the MAC associated with the MII.
>     Optional, but if called, must be done after mii_alloc(), and
>     before the first time mii_start() is called.  If not called, a
>     default instance using ddi_get_instance() from the dev_info node
>     was supplied to mii_alloc will be used.  (This function is only
>     needed for drivers that override the value of m_instance in the
>     mac_register structure.)

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.

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

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