All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roland Dreier <rdreier@cisco.com>
To: Brice Goglin <brice@myri.com>
Cc: netdev@vger.kernel.org, Andrew Morton <akpm@osdl.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"Andrew J. Gallatin" <gallatin@myri.com>
Subject: Re: [PATCH 3/6] myri10ge - Driver header files
Date: Wed, 10 May 2006 14:57:57 -0700	[thread overview]
Message-ID: <adalkt97day.fsf@cisco.com> (raw)
In-Reply-To: <44625CD2.8040100@myri.com> (Brice Goglin's message of "Wed, 10 May 2006 23:36:18 +0200")

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.

  reply	other threads:[~2006-05-10 21:58 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-05-10 21:22 [PATCH 0/6] myri10ge - Myri-10G Ethernet driver Brice Goglin
2006-05-10 21:34 ` [PATCH 1/6] myri10ge - Revive pci_find_ext_capability Brice Goglin
2006-05-10 21:35 ` [PATCH 2/6] myri10ge - Add missing PCI IDs Brice Goglin
2006-05-10 21:52   ` Andi Kleen
2006-05-10 21:36 ` [PATCH 3/6] myri10ge - Driver header files Brice Goglin
2006-05-10 21:57   ` Roland Dreier [this message]
2006-05-10 22:00   ` Stephen Hemminger
2006-05-10 22:02   ` Francois Romieu
2006-05-10 21:40 ` [PATCH 4/6] myri10ge - First half of the driver Brice Goglin
2006-05-10 22:01   ` Stephen Hemminger
2006-05-10 22:06     ` Roland Dreier
2006-05-11 23:53       ` Brice Goglin
2006-05-10 22:04   ` Roland Dreier
2006-05-11 23:53     ` Brice Goglin
2006-05-10 23:13   ` Francois Romieu
2006-05-11 23:53     ` Brice Goglin
2006-05-12  6:47       ` Evgeniy Polyakov
2006-05-12 17:40         ` David S. Miller
2006-05-13 16:13       ` Francois Romieu
2006-05-15 17:02       ` Lee Revell
2006-05-15 17:39         ` Brice Goglin
2006-05-10 21:42 ` [PATCH 5/6] myri10ge - Second " Brice Goglin
2006-05-10 22:22   ` Stephen Hemminger
     [not found]     ` <200605111924.33125.netdev@axxeo.de>
2006-05-11 18:28       ` [PATCH] expose simplified skb_checksum_recalc Stephen Hemminger
2006-05-11 18:40         ` Ingo Oeser
2006-05-12 19:52         ` David S. Miller
2006-05-11 23:53     ` [PATCH 5/6] myri10ge - Second half of the driver Brice Goglin
2006-05-12  0:31       ` Herbert Xu
2006-05-10 21:43 ` [PATCH 6/6] myri10ge - Kconfig and Makefile Brice Goglin
2006-05-13 18:51   ` Adrian Bunk
2006-05-13 18:56     ` Brice Goglin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=adalkt97day.fsf@cisco.com \
    --to=rdreier@cisco.com \
    --cc=akpm@osdl.org \
    --cc=brice@myri.com \
    --cc=gallatin@myri.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.