Garrett D'Amore wrote: > 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.
Touche :) (It sounds like the ISMII capability you mentioned would really help with this issue; see below) >> >>> 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. That works for me too. I cannot really think of a reason to use mc_notify (i.e. link state change) if the MII/GMII bits do the work for the driver. I'll take this out. >> >>> 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. :-) Thats an excellent point. I hadn't thought of using the MII Brussels properties. That would definitely be a more transparent method of configuring MII. > >> >>> 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. It's more invasive (I hadn't counted on mucking about in Nemo proper), buts its *definitely* a better approach. I really didn't like the idea of delegating kstat and property calls from each driver. >> >> >> 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. dnet should be pretty straightforward and I also have an hme available. This originally came about by working on cge, so I may use that as my gigabit guinea pig; otherwise bge or nge would have to suffice. Regards, Steve -- Yet magic and hierarchy arise from the same source, and this source has a null pointer. Reference the NULL within NULL, it is the gateway to all wizardry. _______________________________________________ networking-discuss mailing list [email protected]
