Minor improvement of orinoco driver

[Jean Tourrilhes]

On Tue, Dec 18, 2001 at 11:44:14AM +1100, David Gibson wrote:
>
> > 	What I like about the current setup is the constant cost. If I
> > double the number of ioctl, the code stays exactly the same. It also
> > mean that people can just yank a new wireless.h in their kernel and
> > expect it to work.
> 
> Except that the source of every driver needs to change to expand the
> standard handler table.  With structures using the gcc structure
> initialization extension (as used all over the kernel) only a
> recompile is needed.

	Not true. The old driver would work fine even not
recompiled. I was very careful about that.

> It's a nop in the .o, but both breaking the union and pulling the
> various items out (rather than having them as separate parameters)
> makes the source uglier.

	Just do :
----------------------------------------------------------------
static int orinoco_ioctl_getwap(struct net_device *dev,
				struct iw_request_info *info,
				struct sockaddr *arq,
				char *extra)
{
	struct orinoco_private *priv = dev->priv;

	arq->sa_family = ARPHRD_ETHER;
	return orinoco_hw_get_bssid(priv, arq->sa_data);
}
[...]
static const iw_handler		orinoco_handler[] =
{
[...]
	(iw_handler) orinoco_ioctl_getwap,		/* SIOCGIWAP */
----------------------------------------------------------------

	Satisfied ?


> > 	Also : not all driver check/set all the fields of the
> > structure. For example, the old Wavelan doesn't use key index and key
> > flags, whereas the Orinoco does. By expanding things too much in the
> > kernel you would make those drivers that use only a few things pay a
> > price.
> 
> I don't see that it's a price of any significance.  ioctl() is not
> critical path.

	Bloat. Especially static bloat that is there even for people
who don't use it.

> > 	The only issue is if people put the iw_handler in the wrong
> > slot. Your proposal would detect it. But people would realise that
> > pretty quicly (I try to set the encryption, and the frequency changes
> > - how bizzare !).
> 
> Or it SEGVs or oopses because it tries to copy_from_user() when
> actually passed an iw_param.

	Nope. The copy_to/from_user is only between user space and the
kernel wrapper. The driver doesn't see the pointer anymore, so doesn't
get a chance to mess with it.

> > 	And best, you can mix/matches the two approaches, which gives
> > you maximum flexibility. If you look, this is what I have done for the
> > force_reset and card_reset which share the same iw_handler (of course,
> > you won't like it). I think it's a cool feature of the API, and I want
> > to keep it.
> 
> Nifty, but not a decisive advantage IMO>

	Well, it sure depend on your hardware.
	For example, I added the "commit" stuff. For some hardware,
it's totally useless. For some other, like the Orinoco, it makes a
*big* difference.

	Jean