Re: [PATCH 2.6.25] add bnx2x driver for BCM57710 - bnx2x.c

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Stephen Hemminger
Date: Thursday, November 15, 2007 - 4:03 pm

1. Please use dev_err() to help user figure out which board has problem:
Not:
		printk(KERN_ERR PFX "Cannot register net device\n");
Instead:
		dev_err(&pdev->dev, "cannot register net device\n");


2. Use new MAC_ADDR() rather than
	printk("node addr ");
	for (i = 0; i < 6; i++)
		printk("%2.2x", dev->dev_addr[i]);
	printk("\n");

3. The reset task logic needs more cleanup/protection.
Don't you want to cancel it on remove, and bp->in_reset_task is
not sufficient protection on SMP??
Could you not flush_scheduled_work(), instead use cancel_delayed_work_sync()?

4. Rather than hard coding mac address, could you use random_ether_address()
instead?

5. Current style police will complain about single line {}
	if (bp->phy_flags & PHY_XGXS_FLAG) {
		cmd->port = PORT_FIBRE;
	} else {
		cmd->port = PORT_TP;
	}
instead:
	if (bp->phy_flags & PHY_XGXS_FLAG)
		cmd->port = PORT_FIBRE;
	else
		cmd->port = PORT_TP;

6. The driver is using per-cpu tx queue, maybe it wants to have multi
queue instead?

7. bnx2x_get_stats() is uneeded. If you leave dev->get_stats() set to NULL
then register_netdev will handle it.


8. Spelling fixes:


spelling fix: s/managment/management/


...
management

default


why not just 

static const char *board_info[] = {
	"NetXtreme II BCM57710 XGb",
};


resolve??


enabling


-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[PATCH 2.6.25] add bnx2x driver for BCM57710, Eliezer Tamir, (Thu Nov 15, 12:19 pm)
Re: [PATCH 2.6.25] add bnx2x driver for BCM57710 - bnx2x.h, Eliezer Tamir, (Thu Nov 15, 12:45 pm)
Re: [PATCH 2.6.25] add bnx2x driver for BCM57710 - bnx2x.c, Eliezer Tamir, (Thu Nov 15, 12:46 pm)
Re: [PATCH 2.6.25] add bnx2x driver for BCM57710 - bnx2x.c, Stephen Hemminger, (Thu Nov 15, 4:03 pm)
Re: [PATCH 2.6.25] add bnx2x driver for BCM57710, David Miller, (Thu Nov 15, 4:16 pm)
Re: [PATCH 2.6.25] add bnx2x driver for BCM57710 - bnx2x.c, Eliezer Tamir, (Fri Nov 16, 12:55 am)