Re: [PATCH 2/9] NetXen 1G/10G driver

[Francois Romieu]

Linsys Contractor Amit S. Kale <[EMAIL PROTECTED]> :
> diff -Naru linux-2.6.16_orig/drivers/net/netxen/netxen_nic_ethtool.c 
> linux-2.6.16/drivers/net/netxen/netxen_nic_ethtool.c
> --- linux-2.6.16_orig/drivers/net/netxen/netxen_nic_ethtool.c 1969-12-31 
> 16:00:00.000000000 -0800
> +++ linux-2.6.16/drivers/net/netxen/netxen_nic_ethtool.c      2006-03-24 
> 14:13:57.000000000 -0800
[...]
> +static const struct netxen_nic_stats netxen_nic_gstrings_stats[] = {
> +        { "rcvd bad skb",     NetXen_NIC_STAT(stats.rcvdbadskb) },
> +        { "xmit called",      NetXen_NIC_STAT(stats.xmitcalled) },
> +        { "xmited frames",    NetXen_NIC_STAT(stats.xmitedframes) },
> +        { "xmit finished",    NetXen_NIC_STAT(stats.xmitfinished) },
> +        { "bad skb len",      NetXen_NIC_STAT(stats.badskblen) },
> +        { "no cmd desc",      NetXen_NIC_STAT(stats.nocmddescriptor) },
> +        { "polled",           NetXen_NIC_STAT(stats.polled) },
> +        { "uphappy",          NetXen_NIC_STAT(stats.uphappy) },
> +        { "updropped",        NetXen_NIC_STAT(stats.updropped) },
> +        { "uplcong",          NetXen_NIC_STAT(stats.uplcong) },
> +        { "uphcong",          NetXen_NIC_STAT(stats.uphcong) },
> +        { "upmcong",          NetXen_NIC_STAT(stats.upmcong) },
> +        { "updunno",          NetXen_NIC_STAT(stats.updunno) },
> +        { "skb freed",        NetXen_NIC_STAT(stats.skbfreed) },
> +        { "tx dropped",       NetXen_NIC_STAT(stats.txdropped) },
> +        { "tx null skb",      NetXen_NIC_STAT(stats.txnullskb) },
> +        { "csummed",          NetXen_NIC_STAT(stats.csummed) },
> +        { "no rcv",           NetXen_NIC_STAT(stats.no_rcv) },
> +        { "rx bytes",         NetXen_NIC_STAT(stats.rxbytes) },
> +        { "tx bytes",         NetXen_NIC_STAT(stats.txbytes) },
> +};

Please use "_" instead of " " (see drivers/net/).

[...]
> +static int
> +netxen_nic_get_eeprom_len(struct net_device *netdev)
> +{
> +        struct netxen_port *port = netdev->priv;

s/netdev->priv/netdev_priv(netdev)/

Btw it is quite common to use 'dev' instead of 'netdev'. Your call.

[...]
> +static void
> +netxen_nic_get_regs(struct net_device *netdev, struct ethtool_regs *regs, 
> void *p)
> +{
> +        struct netxen_port        *port = netdev->priv;
> +        struct netxen_adapter_s   *adapter = port->adapter;
> +        netxen_crbword_t           mode, *regs_buff = p;
> +        unsigned long           flags;
> +        void                   *addr;
> +
> +        memset(p, 0, NetXen_NIC_REGS_LEN);
> +        regs->version = (1 << 24) | (adapter->ahw.revision_id << 16) | 
> +                            adapter->ahw.device_id;
> +        write_lock_irqsave(&adapter->adapter_lock, flags);
> +        // which mode
> +        NetXen_NIC_LOCKED_READ_REG(NetXen_NIU_MODE, &regs_buff[0]);
> +        mode = regs_buff[0];
> +
> +        // Common registers to all the modes
> +        NetXen_NIC_LOCKED_READ_REG(NetXen_NIU_STRAP_VALUE_SAVE_HIGHER, 
> &regs_buff[2]);
> +        switch(mode) {
> +        case 4: {//XGB Mode
> +                NetXen_NIC_LOCKED_READ_REG(NetXen_NIU_XG_SINGLE_TERM, 
> &regs_buff[3]);
> +                NetXen_NIC_LOCKED_READ_REG(NetXen_NIU_XG_DRIVE_HI, 
> &regs_buff[4]);
> +                NetXen_NIC_LOCKED_READ_REG(NetXen_NIU_XG_DRIVE_LO, 
> &regs_buff[5]);
[snip]

No need to unroll the loop: please use some data array + a for loop.

[...]
> +static int
> +netxen_nic_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
> +{
> +        // can't implement as WOL register is not known !!
> +        return 0;

If it can not be implemented, it should return something < 0, say -EINVAL.

-- 
Ueimor
-
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