Re: [PATCH] - e1000_ethtool.c - convert macros to functions

["Kok, Auke" <auke-jan.h.kok@intel.com>]

Joe Perches wrote:
> On Wed, 2007-10-31 at 14:30 -0700, Kok, Auke wrote:
>> Joe Perches wrote:
>> that's not a bad idea, however see below:
>> can't we keep the macro here (and just make it call the function instead of
>> expanding). the resulting code is much more lenghty and contains all these logic
>> traps that the previous code didn't have.
>> just have the macro expand to `if (reg_pattern_test(...)) return 1)` and you don't
>> need to change any of the calling lines.
> 
> You could define something like:
> 
> #define REG_PATTERN_TEST(reg, mask, write) \
> 	if (reg_pattern_test(adapter, data, \
> 			     E1000_REG(&adapter->hw, reg), \
> 			     mask, write)) \
> 		return 1;
> 
> But isn't the macro with an embedded return a bit too obfuscating?

in this case it saves a bunch of time reading and makes the code actually more
readable and robust IMO. Now we're explicitly mobming out on an error. with your
patch we need to assure that for every call to reg_pattern_test we check the
return value.

>>> +#define E1000_READ_REG_ARRAY(a, reg, offset)		\
>>> +	(readl((a)->hw_addr +				\
>>> +	       (((a)->mac_type >= e1000_82543)		\
>>> +		? E1000_##reg : E1000_82542_##reg) +	\
>>> +	       ((offset) << 2)))
>> did you have to change these macro's ?
> 
> No.  Your choice to keep/remove.
> I did want to use the E1000_REG or a new E1000_REG_ADDR macro.
> 
>> also, I'm a bit inclined to prefer a patch for e1000e for now as we're about to
>> move the pci-express hardware over, but we can certainly merge something like this
>> in e1000 after the move as well.
> 
> Simple enough.
> 
> When is e1000e scheduled to be part of defconfig?

I don't know about defconfig, but you can already get e1000 in 2.6.24rc1...

Auke


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/