On Saturday 30 June 2007 22:38:47 [EMAIL PROTECTED] wrote:
> These changes allow driver close routine to be called during module unload,
> to clean-up buffers and other software resources, flush queues etc. Also,
> hardware is reset to pristine state.
>
> Signed-off-by: Dhananjay Phadke <[EMAIL PROTECTED]>
> Signed-off-by: Milan Bag <[EMAIL PROTECTED]>
> Signed-off-by: Wen Xiong <[EMAIL PROTECTED]>
> off = netxen_nic_pci_set_window(adapter, memaddr);
> addr = pci_base_offset(adapter, off);
> writel(data, addr);
> + do {
> + if (readl(addr) == data)
> + break;
> + msleep_interruptible(100);
If you use msleep_interruptible(), I'd say you should check for
the return value of that call and probably abort firmware
processing here if a signal interrupted us.
> netxen_nic_reg_write(adapter, NETXEN_ROMUSB_ROM_ADDRESS, addr);
> netxen_nic_reg_write(adapter, NETXEN_ROMUSB_ROM_ABYTE_CNT, 3);
> @@ -856,10 +853,10 @@ int netxen_pinit_from_rom(struct netxen_
> netxen_nic_pci_change_crbwindow(adapter, 1);
> }
> if (init_delay == 1) {
> - ssleep(1);
> + ssleep(2);
Is it possible/desired to do some interruptible sleep here?
Two seconds uninterruptible sleep is probably a long time.
Even at init/boot.
> Index: netdev-2.6/drivers/net/netxen/netxen_nic_main.c
> ===================================================================
> --- netdev-2.6.orig/drivers/net/netxen/netxen_nic_main.c
> +++ netdev-2.6/drivers/net/netxen/netxen_nic_main.c
> @@ -511,6 +511,8 @@ netxen_nic_probe(struct pci_dev *pdev, c
> val |= 0x4;
> netxen_nic_write_w0(adapter, NETXEN_PCIE_REG(0x4), val);
> netxen_nic_read_w0(adapter, NETXEN_PCIE_REG(0x4), &val);
> + if (!(val & 0x4))
> + printk(KERN_ERR "NetXen: bit set failed\n");
That's the award for the most useless error message. :P
Can you give the user a hint what this bit is about?
> + if (adapter->portnum == 0) {
> + if (init_firmware_done) {
> + dma_watchdog_shutdown_request(adapter);
> + msleep(100);
> + i = 100;
> + while ((dma_watchdog_shutdown_poll_result(adapter) !=
> 1) && i) {
> + printk(KERN_INFO "dma_watchdog_shutdown_poll
> still in progress\n");
> + msleep(100);
> + i--;
> + }
> +
> + if (i == 0) {
> + printk(KERN_ERR "dma_watchdog_shutdown_request
> failed\n");
> + return;
> + }
> +
> + /* clear the register for future unloads/loads */
> + writel(0, NETXEN_CRB_NORMALIZE(adapter,
> NETXEN_CAM_RAM(0x1fc)));
> + printk(KERN_INFO "State: 0x%0x\n",
> + readl(NETXEN_CRB_NORMALIZE(adapter,
> CRB_CMDPEG_STATE)));
> +
> + /* leave the hw in the same state as reboot */
> + writel(0, NETXEN_CRB_NORMALIZE(adapter,
> CRB_CMDPEG_STATE));
> + if (netxen_pinit_from_rom(adapter, 0))
> + return;
> + udelay(500);
I guess we can do msleep(1) here, too, instead of the huge delay,
although it's twice as long.
Though, I could live with a 500us delay, too, if that's desired.
The rest looks pretty good to me.
--
Greetings Michael.
-
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
