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
