Re: [PATCH] ethtool_ops rev 4

[Jeff Garzik <jgarzik@pobox.com>]

Matthew Wilcox wrote:
> On Sat, Aug 02, 2003 at 06:34:46PM -0400, Jeff Garzik wrote:
> 
>>>diff -u include/linux/netdevice.h include/linux/netdevice.h
>>>--- include/linux/netdevice.h   31 Jul 2003 13:06:23 -0000
>>>+++ include/linux/netdevice.h   2 Aug 2003 18:37:16 -0000
>>>@@ -477,6 +477,10 @@
>>> */
>>>#define SET_NETDEV_DEV(net, pdev)      ((net)->class_dev.dev = (pdev))
>>>
>>>+static inline void set_ethtool_ops(struct net_device *dev, struct 
>>>ethtool_ops *
>>>ops)
>>>+{
>>>+       dev->ethtool_ops = ops;
>>>+}
>>
>>
>>It needs to be a macro for maximum flexibility.
> 
> 
> Nothing stops it being implemented as a macro in kcompat.  Having it as
> an inline function gives it argument typechecking which always gives me
> the warm fuzzies.

No, it _needs_ to be a macro for maximum flexibility.

Most importantly, kcompat code may use '#ifndef SET_ETHTOOL_OPS' as a 
trigger, to signal that compat code is needed.  No need for drivers to 
create tons of kernel-version-code ifdefs, just to test for when 
ethtool_ops appeared in 2.6, for when it starts appearing in 2.4 vendor 
backports, and (possibly) 2.4 itself.  Also, doing it at the cpp level 
allows compat code to #undef it, if it _really_ knows what its doing, 
and the situation calls for it.


>>Also, no need to convert in-kernel drivers over to using it...  Let 
>>driver authors use it or not as they choose.
> 
> 
> I took "Like pci_set_drvdata" as the most important part of your
> argument...  having everyone use it is no bad thing.

Certainly.  I have no real preferences either way, just noting that 
in-kernel drivers don't _need_ to use this macro.

	Jeff