Steven Stallion wrote:
> mac.h
> -----
> 
> I added an enum for link_speed_t; this was made since we already
> maintain the link_duplex_t and link_state_t. link_speed_t is very useful
> when setting a specific speed via MII.
> 
> The IFSPEED macro was added as an afterthought; I find it generally
> annoying that the ifspeed kstat typically is reported using a magic
> multiple (1000000). *val = IFSPEED(foop->speed) is much clearer than
> *val = foop->speed * 1000000.
> 
> mac_ether.h
> -----------
> 
> This contains the bulk of the changes. I've added what I thought the
> minimal MII/GMII interface should be. There are of course a number of
> features supported by MII which probably arent useful in the day-to-day
> (i.e. collision test, loopback, and so forth).
> 

Do the mii)read/mii_write entry point names make sense? There don't seem 
to be enough arguments. The interface across which you talk to a 
MII/GMII PHY is usually clause 22 MDIO (and a driver could make it look 
like this even if it isn't), so you need function prototypes taking a 
context, a register offset and then a data buffer to read into or write 
from. For clause 22 the register space is 8 bits wide and the data 16 
bits wide. Also, I'd call them mdio22_read/mdio22_write to make it clear 
what their purpose is.
Once you get to PHYs talking XGMII or XFI you're likely to need clause 
45 MDIO and that requires an extra 'MMD' argument, which is 8 bits wide, 
and the register space increases to 16 bits wide.

Also, since the MII/GMII register space is an IEEE standard I'd prefer 
to see enumerations for the register offsets. As for 10G PHYs, each MMD 
has an IEEE register space (< 0x8000) and a vendor specific register 
space (>= 0x8000) so you could also partially enumerate those.

Do you have implementations for the mii functions you have? Also be 
aware that not all link modes available through a given PHY will be 
supported by the MAC it is attached to (e.g. 1G half-duplex, which is 
not required by the IEEE IIRC) so you need to make sure the driver can 
veto things appropriately.

   Paul

_______________________________________________
networking-discuss mailing list
[email protected]

Reply via email to