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

From: Mark Rustad <mark.d.rustad@intel.com>

Kernel coding standard prefers static inline functions instead
of macros, so use them for register accessors. This is to prepare
for adding LER, Live Error Recovery, checks to those accessors.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h        |  5 +++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_common.h | 23 ++++++++++++++++-------
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c   |  4 ++--
 3 files changed, 23 insertions(+), 9 deletions(-)

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.

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

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


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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.