RE: [PATCH 2/5] IB/core: Formalize the creation of immutable per port data within the ib_device object

["Weiny, Ira" <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]

> > In preparation for this create a structure for per port immutable data
> > and place the pkey and gid table lengths within this structure.
> >
> > This type of data requires a new call back "port_immutable" parameter
> > to ib_register_device to allow each driver to create this data as appropriate.
> > This callback is added to ib_register_device rather than as a new
> > device function because the callback should only be used when devices
> > are first registered.
> >

I tried to explain my decision here...

> > @@ -273,7 +273,9 @@ out:
> >   */
> >  int ib_register_device(struct ib_device *device,
> >  		       int (*port_callback)(struct ib_device *,
> > -					    u8, struct kobject *))
> > +					    u8, struct kobject *),
> > +		       int (*port_immutable)(struct ib_device *, u8,
> > +					     struct ib_port_immutable *))
> 
> I'm having a hard time getting past how ugly this is.  Passing callbacks as
> arguments to a registration function should be a last resort.  I would rather see
> this added to the device mandatory list (and it *is* mandatory, we use it
> without checking for NULL).  

Your correct I should have checked for NULL and returned an error if not supplied.

> In truth, I'd rather see both of those moved to the
> driver callback struct.  They can be placed at the end, after a comment that
> says something like "These are single use entry points for initialization, keep
> them away from the rest of the entry points to help prevent growing the entry
> point list beyond any more cachelines that needed for the more commonly
> used entry points".  I would find that much preferable to this.
> 

I'll change to this method in the next version.

Ira