Steven Stallion wrote:
> Garrett D'Amore wrote:
>> The IFSPEED() macro should probably be confined to the bowels of the 
>> mac_ether.c implementation.
>
> I suppose I had hoped this macro could be used in a wider context 
> (dnet could certainly use a macro like this as well as others). It 
> seems that the bulk of drivers just multiply the speed which can be 
> error prone (easy to catch though). Maybe a premature feature or 
> wishful thinking?

No, the point is, drivers don't need to concern themselves with that 
detail -- the kstats where the multiplication is most useful are in 
mac_ether and Brussels implementation details.  Or they should be.

>
>> I'm a little confused though about mc_notify.  Who calls this, and 
>> when?   I *suspect* that the role is reversed here, and rather than 
>> mc_notify() being an entry point in the driver, its the other way 
>> around, and the driver should call mii_notify() to make the framework 
>> understand that a change has happened.
>
> This was something I struggled with too. The existing mii code 
> (/usr/src/uts/intel/io) uses a structure similar to this. Essentially, 
> a portmon is spawned which uses a timeout to poll for state. I like it 
> from the perspective that the driver developer does not have to care 
> about state changes (which may or may not be interrupt driven anyway) 
> when using an MII xcvr.
>
> I don't like polls, but a poll issued every second or more shouldnt 
> impact throughput noticably, especially since the resultant call 
> should make a single PIO read. Of course, my experience so far has 
> been with fast ethernet and gigabit ethernet - I honestly have no idea 
> if this could impact wirespeed on faster chipsets.

The point is, the implementation of that polling function is probably 
identical from one chip to another.  You shouldn't need a driver entry 
point for it.

I wouldn't necessarily follow the existing mii module too closely.   It 
isn't widely used, has lots of limitations, and is probably a poor basis 
to model upon.

>
>> The other tidbit is that I'm not entirely sure that the drivers need 
>> to have access to the mii_xxx functions directly.  While they could 
>> be useful, I think I see them being *more* useful to the framework 
>> itself.
>
> This was an artifact of the original mii code as well. I suppose the 
> gist of this was to support manual configuration rather than forcing a 
> driver to configure a manual link by bypassing mii.

Ah, but Brussels should deal with that now, rather than making the 
driver even worry about it. :-)

>
>> To that end, it does also seem that the framework could do with 
>> knowing that the part is an MII based device, and hence provide 
>> mii-equivalents for some of the functionality that currently exists 
>> in device drivers directly.  (I'm thinking for things like mac_stat() 
>> and the Brussels property interfaces.)
>
> Definitely. I'm not completely familiar with Brussels support (yet), 
> but I did add a call (mii_getstat) to support MII kstats. This may not 
> be the best approach (I was thinking it would be a combination of 
> ETHER_STAT_ISMII and mii_getstat). I am definitely curious if there 
> are any other established patterns for this kind of behavior.

How about a mac capability for ISMII, which then causes the framework to 
use MII routines internally for MII kstats, so that the driver doesn't 
even need to provide implementation for those?

Likewise for Brussels property routines.
>
>
> Were there any crucial operations or behavior that I missed? This has 
> been deceptively easy (so far) and I even have a fair amount of impl 
> code complete.
>
> Also, I suspect that I will need to pick a driver to use as a guinea 
> pig. Any suggestions? (afe seems a likely candidate since the code is 
> well written and already has its own MII impl which should reveal any 
> missing functionality).

afe is indeed a good choice.  Plus it will be easier for me to review it.

If you do dnet or mxfe, you'll also get some of the more obnoxious MII 
stuff.  You should also do a gigabit part, and maybe hme.  Then you'll 
have a pretty complete coverage.

    -- Garrett
>
>
> Regards,
>
> Steve

_______________________________________________
networking-discuss mailing list
[email protected]

Reply via email to