Re: [PATCH 3/6] myri10ge - Driver header files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



A few quick obvious comments:

 > +#ifdef MYRI10GE_MCP
 > +typedef signed char          int8_t;
 > +typedef signed short        int16_t;
 > +typedef signed int          int32_t;
 > +typedef signed long long    int64_t;
 > +typedef unsigned char       uint8_t;
 > +typedef unsigned short     uint16_t;
 > +typedef unsigned int       uint32_t;
 > +typedef unsigned long long uint64_t;
 > +#endif

What's this doing?  If you must use uintXX_t types the kernel already
has them.  Although it would be nicer to use u8, u16, etc.

 > +/* 8 Bytes */
 > +typedef struct
 > +{
 > +  uint32_t high;
 > +  uint32_t low;
 > +} mcp_dma_addr_t;

All of these typedefs are unnecessary.  In the kernel it's strongly
preferred to just do

struct mcp_dma_addr {
	u32 high;
        u32 low;
};

and then use "struct mcp_dma_addr" instead of "mcp_dma_addr_t".

Similarly for enums.  Just use "enum whatever" instead of "whatever_t".

BTW, indentation is busted in these headers too (two spaces instead of a tab).

 - R.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux