On Sat, 2006-04-01 at 05:20 -0800, Linsys Contractor Amit S. Kale wrote:

> +#define NO_EPG_PROXY      1
> +
> +#define HOST_CPU_64_BIT   1

What?

> +#include <linux/skbuff.h>
> +#ifdef NETIF_F_HW_VLAN_TX
> +#include <linux/if_vlan.h>
> +#endif
> +#ifndef MODULE
> +#include <asm/i387.h>
> +#endif

?????

> +#define BAR_0            0
> +#define PCI_DMA_64BIT    0xffffffffffffffffULL
> +#define PCI_DMA_32BIT    0x00000000ffffffffULL

Yuck.

> +extern int netxen_nic_debug;
> +#define netxen_nic_debug_on  do { netxen_nic_debug = 1; } while(0)
> +#define netxen_nic_debug_off do { netxen_nic_debug = 0; } while(0)

Yuck.

> +/* only works for sizes that are powers of 2 */
> +#define NetXen_ROUNDUP(i, size) ((i) = (((i) + (size) - 1) & ~((size) - 1)))

Why is this defined in two headers?  Use ALIGN.

> +#ifndef    TAILQ_FIRST
> +#define    TAILQ_FIRST(head)    ((head)->tqh_first)
> +#define    TAILQ_EMPTY(head)    ((head)->tqh_first == NULL)
> +#endif

Why aren't you using a struct list_head for this?


> +/*
> + * One hardware_context{} per adapter
> + * contains interrupt info as well shared hardware info.
> + */
> +typedef struct _hardware_context {

Should have netxen_ prefix.  No typedefs.

> +typedef struct netdev_list_s netdev_list_t;
> +struct netdev_list_s {
> +        netdev_list_t *next;
> +        struct net_device *netdev;
> +};

Yuck.  Use <linux/list.h> instead.

> +#define    PORT_UP              0
> +#define    PORT_DOWN            1
> +#define    PORT_INITIALIAZED    2
> +#define    PORT_SUSPEND         3

The likelihood of name collisions with generic macro names like this is
very high.  Please fix them all.


> +#ifndef readq
> +static inline __uint64_t readq(void __iomem *addr)
> +{
> +        return readl(addr) |
> +               (((__uint64_t)readl(addr+4)) << 32LL);
> +}
> +#endif
> +
> +#ifndef writeq
> +static inline void writeq(__uint64_t val, void __iomem *addr)
> +{
> +        writel(((__uint32_t)(val)), (addr));
> +        writel(((__uint32_t)(val >> 32)), (addr + 4));
> +}
> +#endif
> +
> +
> +/*
> + * Following macros require the mapped addresses to access
> + * the Phantom memory.
> + */
> +#define NetXen_NIC_PCI_READ_8(ADDR)         readb((ADDR))
> +#define NetXen_NIC_PCI_READ_16(ADDR)        readw((ADDR))
> +#define NetXen_NIC_PCI_READ_32(ADDR)        readl((ADDR))
> +#define NetXen_NIC_PCI_READ_64(ADDR)        readq((ADDR))
> +
> +#define NetXen_NIC_PCI_WRITE_8(DATA, ADDR)  writeb(DATA, (ADDR))
> +#define NetXen_NIC_PCI_WRITE_16(DATA, ADDR) writew(DATA, (ADDR))
> +#define NetXen_NIC_PCI_WRITE_32(DATA, ADDR) writel(DATA, (ADDR))
> +#define NetXen_NIC_PCI_WRITE_64(DATA, ADDR) writeq(DATA, (ADDR))
> +
> +#define NetXen_NIC_HW_BLOCK_WRITE_64(DATA_PTR, ADDR, NUM_WORDS)        \
> +        do {                                                        \
> +                int i;                                              \
> +                u64 *a = (u64 *) (DATA_PTR);                        \
> +                u64 *b = (u64 *) (ADDR);                            \
> +                for (i=0; i< (NUM_WORDS); i++) {                    \
> +                        writeq(readq(a), b);                        \
> +                        b++;                                        \
> +                        a++;                                        \
> +                }                                                   \
> +        } while (0)
> +
> +#define NetXen_NIC_HW_BLOCK_READ_64(DATA_PTR, ADDR, NUM_WORDS)           \
> +        do {                                                          \
> +                int i;                                                \
> +                u64 *a = (u64 *) (DATA_PTR);                          \
> +                u64 *b = (u64 *) (ADDR);                              \
> +                for (i=0; i< (NUM_WORDS); i++) {                      \
> +                        writeq(readq(b), a);                          \
> +                        b++;                                          \
> +                        a++;                                          \
> +                }                                                     \
> +        } while (0)

This must all die.

> +#define NetXen_DEBUG_PVT_32_ADDR(A)                   \
> +        (unsigned int)(A)
> +#define NetXen_DEBUG_PVT_32_VALUE(V)                  \
> +        *((unsigned int *)(V))
> +#define NetXen_DEBUG_PVT_32_VALUE_LIT(V)              \
> +        (unsigned int)(V)
> +#define NetXen_DEBUG_PVT_64_ADDR_LO(A)                \
> +        (unsigned int)((unsigned long)(A) & 0xffffffff)
> +#define NetXen_DEBUG_PVT_64_ADDR_HI(A)                \
> +        (unsigned int)((((unsigned long)(A) >> 16) >> 16) & 0xffffffff)
> +#define NetXen_DEBUG_PVT_64_VALUE_LO(V)               \
> +        (unsigned int)(*(__uint64_t *)(V) & 0xffffffff)
> +#define NetXen_DEBUG_PVT_64_VALUE_HI(V)               \
> +        (unsigned int)(((*(__uint64_t *)(V) >> 16) >> 16) & 0xffffffff)
> +#define NetXen_DEBUG_PVT_64_VALUE_LIT_LO(LV)          \
> +        (unsigned int)((LV) & 0xffffffff)
> +#define NetXen_DEBUG_PVT_64_VALUE_LIT_HI(LV)          \
> +        (unsigned int)(((LV) >> 32) & 0xffffffff)

???

> +#if defined(CONFIG_X86_64)
> +typedef unsigned long uptr_t;
> +#else
> +typedef unsigned uptr_t;
> +#endif

Just use ptrdiff_t.

> +#define NetXen_NIC_LOCKED_READ_REG(X, Y)   \
> +        addr = (void *)(adapter->ahw.pci_base + X);     \
> +        *(uint32_t *)Y = NetXen_NIC_PCI_READ_32(addr);

What if someone tries to use this macro inside an if statement without
curly braces around it?

        <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

Reply via email to