On Sat, 2006-04-01 at 05:19 -0800, Linsys Contractor Amit S. Kale wrote: > +#include <linux/module.h> > +#include <linux/kernel.h> > +#include <linux/version.h> > + > +#include <linux/config.h> > +#include <linux/version.h> > + > +#define NetXen_CONF_X86 3 > + > +#if defined(CONFIG_MODVERSIONS) && !defined(MODVERSIONS) > +#define MODVERSIONS > +#endif > + > +#if defined(MODVERSIONS) && !defined(__GENKSYMS__) > +#include <config/modversions.h> > +#endif
This all looks like junk. Why do you have MODVERSIONS and stuff in here? Why version.h? Why twice? > +typedef char __int8_t; > +typedef short __int16_t; > +typedef int __int32_t; > + > +typedef 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; This all needs to go away. > +#define PRINTK_PREFIX "<1>" > +#define printf_0(A) printk(PRINTK_PREFIX A) > +#define printf_1(A,B) printk(PRINTK_PREFIX A, B) > +#define printf_2(A,B,C) printk(PRINTK_PREFIX A, B, C) > +#define printf_3(A,B,C,D) printk(PRINTK_PREFIX A, B, C, D) > +#define printf_4(A,B,C,D,E) printk(PRINTK_PREFIX A, B, C, D, E) > +#define printf_5(A,B,C,D,E,F) printk(PRINTK_PREFIX A, B, C, D, E, F) Yuck. Use gcc variadic macros instead. > +void DELAY(int A); Has to die, as I already mentioned. > +/* > + * We use the environmental controls to define/undefine various feature > + * control macros depending on the circumstances. > + * When compiling for a Linux host, we use the standard Linux configuration > + * method to simplify things here. > + */ > + > +#define NetXen_DELAY_HW 0 /* no delay */ > + > +#define NetXen_DELAY_HUMAN 0 /* humans read slow */ Ugh. > +#define UNUSED __attribute__((unused)) > +#define NOINLINE __attribute__((noinline)) Drop these. > +extern long pegDynamicMemStart; Not an appropriate variable name. > +/* > + * The basic unit of access when reading/writing control registers. > + */ > +typedef long native_t; /* most efficient integer on h/w */ Drop this. > +typedef __uint32_t netxen_crbword_t; /* single word in CRB space */ > +typedef __uint64_t netxen_dataword_t; /* single word in data space */ > +typedef __uint64_t netxen64ptr_t; /* a pointer that occupies 64 > bits */ Drop these. > +#define NetXen64PTR(P) ((netxen64ptr_t)((native_t)(P))) /* convert for > us */ Yuck. > +#define NetXen_ADDR_QDR_NET_MAX (0x00000003003fffffULL) You should move all the magic number definitions into a separate header file, so it's easier to read this code and find actual type names, struct definitions, functions, etc. > +int netxen_crb_read(unsigned long off, void *data); Flip the order of arguments to all these functions. > +#define NetXen_CRB_READ_VAL(ADDR) netxen_crb_read_val((ADDR)) This is silly. > +#define NetXen_CRB_READ(ADDR,VALUE) netxen_crb_read((ADDR),(netxen_crbword_t > *)(VALUE)) > +#define NetXen_CRB_READ_CHECK(ADDR, VALUE) \ > + do { \ > + if (netxen_crb_read(ADDR, VALUE)) return -1; \ > + } while(0) These are all gross, and obscure the operation of the code. > +typedef __uint8_t netxen_ethernet_macaddr_t[6]; Yuck. > +/* Nibble or Byte mode for phy interface (GbE mode only) */ > +typedef enum { > + NetXen_NIU_10_100_MB = 0, > + NetXen_NIU_1000_MB > +} netxen_niu_gbe_ifmode_t; This must not be a typedef. > +/* > + * NIU GB MAC Config Register 0 (applies to GB0, GB1, GB2, GB3) > + */ > +typedef struct { > + netxen_crbword_t > + tx_enable:1, /* 1:enable frame xmit, 0:disable */ > + tx_synched:1, /* R/O: xmit enable synched to xmit stream */ > + rx_enable:1, /* 1:enable frame recv, 0:disable */ > + rx_synched:1, /* R/O: recv enable synched to recv stream */ > + tx_flowctl:1, /* 1:enable pause frame generation, 0:disable */ > + rx_flowctl:1, /* 1:act on recv'd pause frames, 0:ignore */ > + rsvd1:2, > + loopback:1, /* 1:loop MAC xmits to MAC recvs, 0:normal */ > + rsvd2:7, > + tx_reset_pb:1, /* 1:reset frame xmit protocol blk, 0:no-op */ > + rx_reset_pb:1, /* 1:reset frame recv protocol blk, 0:no-op */ > + tx_reset_mac:1, /* 1:reset data/ctl multiplexer blk, 0:no-op */ > + rx_reset_mac:1, /* 1:reset ctl frames & timers blk, 0:no-op */ > + rsvd3:11, > + soft_reset:1; /* 1:reset the MAC and the SERDES, 0:no-op */ > +} netxen_niu_gb_mac_config_0_t; None of these struct should have typedefs. > +/* get/set the MAC address for a given MAC */ > +int netxen_niu_macaddr_get(int port, netxen_ethernet_macaddr_t *addr); > +int netxen_niu_macaddr_set(int port, netxen_ethernet_macaddr_t addr); Using typedefs is why you get nonsense like this, where the set routine passes a struct by value, instead of by reference. <b - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html