PERFORCE change 179713 for review

[Alexander Fiveg]

On Thursday 08 July 2010 15:01:29 John Baldwin wrote:
> On Thursday, July 08, 2010 7:39:17 am Alexander Fiveg wrote:
> > On Wednesday 23 June 2010 19:19:49 John Baldwin wrote:
> > > On Thursday 17 June 2010 10:46:27 am Alexandre Fiveg wrote:
> > > > http://p4web.freebsd.org/@@179713?ac=10
> > > >
> > > > Change 179713 by afiveg at cottonmouth on 2010/06/17 14:46:03
> > > >
> > > > 	Begin with new design for ringmap:
> > > > 		1. The new structure with pointers to hardware dependent
>
> functions:
> > > > 			"struct ringmap_functions" (/net/ringmap.h)
> > > > 		2. Pointer to this structure placed in ringmap structure.
> > > > 		3. In the ringmap_attach function look for pci Id of network
> > > > controller, and then, depending on controllers type, initialize the
> > > > functions pointers: (ringmap.c: set_ringmap_funcs())
> >
> > Hello John,
> >
> > > I think 3) is the wrong way to go about it.  Can't you have the NIC
> > > driver attach a ringmap and supply the function pointers to the
> > > NIC-specific functionality instead?
> >
> > I think I didn't exactly understand what do you mean :(
> >
> > Actually the NIC driver in its attach() function calls ringmap_attach(),
> > and all settings which appears in the ringmap_attach() are related only
> > to one specific NIC.
>
> Instead of having ringmap_attach() figure out the per-NIC function pointers
> based on device IDs, can't you have ringmap_attach() accept the function
> pointers for the device-specific routines directly?  That is, instead of
> having a set_ringmap_funcs() function that queries device IDs, instead have
> the list of functions (or perhaps a full blown function switch ala cdevsw)
> passed in as a parameter to ringmap_attach().
>
> > > You really don't want to have two separate lists of
> > > device IDs.  The ringmap list will invariably become stale.
> >
> > The second device's list contains only ID's of devices supported by
> > ringmap. The "em" driver in -CURRENT supports both 8254x and 8257x
> > controllers. But ringmap supports currently only 8254x. In the future
> > ringmap should support all devices supported by the driver which ringmap
> > is based on. This means
>
> the
>
> > second ringmap-device-list will be unnecessary.
>
> I think the driver should only call ringmap_attach() for supported devices
> and that the logic decision for which devices are supported should be in
> the driver, not in the ringmap code.  The current organization will make it
> harder to add support for ringmap in newer drivers far more complicated and
> will likely not work with modules, etc.

Ok, I see. 
what you suggest is a better architectural  solution. I think I know how to 
implement it without to do a lot of changes in the native NIC-driver :)

thanx a lot,

Alex