[openib-general] [PATCH 9/10] Driver utility file - implements various utility macros

[Roland Dreier]

 > +#define hton8(x)	(x)
 > +#define hton16(x)	__cpu_to_be16(x)
 > +#define hton32(x)	__cpu_to_be32(x)
 > +#define hton64(x)	__cpu_to_be64(x)
 > +
 > +#define ntoh8(x)	(x)
 > +#define ntoh16(x)	__be16_to_cpu(x)
 > +#define ntoh32(x)	__be32_to_cpu(x)
 > +#define ntoh64(x)	__be64_to_cpu(x)

Please just use the standard cpu_to_beXX / beXX_to_cpu functions
directly (without the __).

 > +#define is_power_of2(value) (((value) & ((value - 1))) == 0)
 > +
 > +typedef unsigned long uintn;	/* __WORDSIZE/pointer sized integer */
 > +
 > +/* round down value to align, align must be a power of 2 */
 > +#ifndef ROUNDDOWNP2
 > +#define ROUNDDOWNP2(val, align)				\
 > +	(((uintn)(val)) & (~((uintn)(align)-1)))
 > +#endif
 > +/* round up value to align, align must be a power of 2 */
 > +#ifndef ROUNDUPP2
 > +#define ROUNDUPP2(val, align)						\
 > +	(((uintn)(val) + (uintn)(align) - 1) & (~((uintn)(align)-1)))
 > +#endif

If you need this stuff it should probably go in some common kernel
include.

 > +#if BITS_PER_LONG == 64
 > +#define PTR64(what)	((u64)(what))
 > +#define PTR(what)	((void *)(u64)(what))
 > +#elif BITS_PER_LONG == 32
 > +#define PTR64(what)	((u64)(u32)(what))
 > +#define PTR(what)	((void *)(u32)(what))
 > +#else
 > +#error "BITS_PER_LONG not 32 nor 64"
 > +#endif

umm.. what the heck is this trying to do?  If you want to cast a
pointer to an integer, just use 'unsigned long' to hold it.

 > +#endif
 > +#define __PRIN_PREFIX	"l"
 > +#define PRId64		__PRI64_PREFIX"d"
 > +#define PRIo64		__PRI64_PREFIX"o"
 > +#define PRIu64		__PRI64_PREFIX"u"
 > +#define PRIx64		__PRI64_PREFIX"x"
 > +#define PRIX64		__PRI64_PREFIX"X"
 > +#define PRIdN		__PRIN_PREFIX"d"
 > +#define PRIoN		__PRIN_PREFIX"o"
 > +#define PRIuN		__PRIN_PREFIX"u"
 > +#define PRIxN		__PRIN_PREFIX"x"

kernel style is just to use "%llx" or whatever for printing 64-bit
values, and cast them to unsigned long long to avoid warnings about
printf formats.

 > +/* source time is 100ths of a sec */
 > +#define CONV2JIFFIES(time) (((time) * HZ) / 100)
 > +#define CONV2USEC(time)    ((time) * 10000)

Why are you using such a wacky unit?  This looks really error-prone --
the conversions in <linux/jiffies.h> should be good enough I think.

 > +#ifndef min
 > +#define min(a,b)	((a)<(b)?(a):(b))
 > +#endif

Unneeded since the kernel _does_ have a better definition of min()
(type-safe, evaluations parameters only once, etc)

 - R.