Re: [net-next v2 03/11] ixgbe: Use static inlines instead of macros

["Rustad, Mark D" <mark.d.rustad@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1697 bytes --]

On Jan 2, 2014, at 9:28 PM, Joe Perches <joe@perches.com> wrote:

> On Thu, 2014-01-02 at 21:18 -0800, Jeff Kirsher wrote:
>> From: Mark Rustad <mark.d.rustad@intel.com>
> []
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
> []
>> @@ -124,22 +124,31 @@ s32 ixgbe_reset_pipeline_82599(struct ixgbe_hw *hw);
>> #ifndef writeq
>> #define writeq(val, addr) writel((u32) (val), addr); \
>>     writel((u32) (val >> 32), (addr + 4));
>> #endif
> 
> This is unchanged, but it would be nicer with a do {} while.

Yes, it is definitely a trap for a macro to directly generate two statements like this. The #ifdef check seemed a little dubious to me when I first saw it, but at the time I chose to stick to what I had to do and then forgot to revisit this.

> #ifndef writeq
> #define writeq(val, addr)				\
> do {							\
> 	writel((u32)(val), addr);			\
> 	writel((u32)((val) >> 32), (addr + 4));		\
> } while (0)

Now that I have looked into it, I only fear becoming a "mucking foron" by touching it at all! :-) (grep the alpha arch code) Lacking an ARCH_HAS macro for detecting writeq, it does appear that this check is currently the way to do it, though it probably makes no sense whatsoever to compile this driver for any architecture that is not defining writeq. I wonder how many of these sequences were added in support of randconfig?

> Even then, this could be nicer as an inline too.

It does appear that the other places checking for writeq are being implemented with static inlines, so that should be the preferred method here as well.

-- 
Mark Rustad, Networking Division, Intel Corporation


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 841 bytes --]