On Friday 22 June 2007 10:42:38 Mithlesh Thukral wrote: > diff --git a/drivers/net/netxen/netxen_nic_hw.c > b/drivers/net/netxen/netxen_nic_hw.c > index 4e958c9..f0df6fb 100644 > --- a/drivers/net/netxen/netxen_nic_hw.c > +++ b/drivers/net/netxen/netxen_nic_hw.c > @@ -378,6 +378,7 @@ int netxen_nic_hw_resources(struct netxe > crb_rcvpeg_state)); > while (state != PHAN_PEG_RCV_INITIALIZED && loops < 20) { > udelay(100); > + schedule(); > /* Window 1 call */ > state = readl(NETXEN_CRB_NORMALIZE(adapter, > recv_crb_registers
Better do msleep(1); instead of udelay+schedule. > @@ -700,7 +701,7 @@ void netxen_nic_pci_change_crbwindow(str > adapter->curr_window = 0; > } > > -void netxen_load_firmware(struct netxen_adapter *adapter) > +int netxen_load_firmware(struct netxen_adapter *adapter) > { > int i; > u32 data, size = 0; > @@ -712,15 +713,25 @@ void netxen_load_firmware(struct netxen_ > writel(1, NETXEN_CRB_NORMALIZE(adapter, NETXEN_ROMUSB_GLB_CAS_RST)); > > for (i = 0; i < size; i++) { > - if (netxen_rom_fast_read(adapter, flashaddr, (int *)&data) != > 0) { > - DPRINTK(ERR, > - "Error in netxen_rom_fast_read(). Will skip" > - "loading flash image\n"); > - return; > + while (netxen_rom_fast_read(adapter, flashaddr, (int *)&data) > != 0) { > + long timeout = 2 * HZ; > + while (timeout) { > + if (signal_pending(current)) { > + printk( "%s: Got a signal, exiting\n", > __FUNCTION__ ); > + return -1; > + } > + set_current_state(TASK_INTERRUPTIBLE); > + timeout = schedule_timeout(timeout); > + } You're opencoding msleep_interruptible() here? And this sleeps two seconds between each rom-read attempt. Is that really your intention? I'd say better attempt to read more often and sleep less each time. > off = netxen_nic_pci_set_window(adapter, memaddr); > addr = pci_base_offset(adapter, off); > writel(data, addr); > + while (readl(addr) != data) { > + mdelay(100); > + writel(data, addr); > + } Add a timeout. Else this will result in a system hang, if the hardware is faulty. > diff --git a/drivers/net/netxen/netxen_nic_init.c > b/drivers/net/netxen/netxen_nic_init.c > index 15f6dc5..8f5f4f8 100644 > --- a/drivers/net/netxen/netxen_nic_init.c > +++ b/drivers/net/netxen/netxen_nic_init.c > @@ -408,8 +408,12 @@ static inline int > do_rom_fast_read(struct netxen_adapter *adapter, int addr, int *valp) > { > if (jiffies > (last_schedule_time + (8 * HZ))) { > - last_schedule_time = jiffies; > - schedule(); > + if (last_schedule_time) { > + last_schedule_time = jiffies; > + schedule(); > + } else { > + last_schedule_time = jiffies; > + } Why this strange thing? I'd simply call cond_resched() instead of all this custom schedule timekeeping. That's best for system latency. > -void netxen_phantom_init(struct netxen_adapter *adapter, int pegtune_val) > +int netxen_phantom_init(struct netxen_adapter *adapter, int pegtune_val) > { > u32 val = 0; > - int loops = 0; > > if (!pegtune_val) { > - val = readl(NETXEN_CRB_NORMALIZE(adapter, CRB_CMDPEG_STATE)); > - while (val != PHAN_INITIALIZE_COMPLETE && > - val != PHAN_INITIALIZE_ACK && loops < 200000) { > - udelay(100); > - schedule(); > - val = > - readl(NETXEN_CRB_NORMALIZE > + do { > + long timeout = 10 * HZ; > + while (timeout) { > + if (signal_pending(current)) { > + printk(KERN_INFO"%s: Got a signal, > exiting\n", __FUNCTION__ ); > + printk(KERN_INFO"%s: val=0x%x, > pegtune_val=0x%x\n", __FUNCTION__, > + val, pegtune_val ); > + return -1; > + } > + set_current_state(TASK_INTERRUPTIBLE); > + timeout = schedule_timeout(timeout); > + } > + val = readl(NETXEN_CRB_NORMALIZE > (adapter, CRB_CMDPEG_STATE)); msleep_interruptible()? > @@ -1278,11 +1285,13 @@ int netxen_process_cmd_ring(unsigned lon > if (skb && (cmpxchg(&buffer->skb, skb, 0) == skb)) { > pci_unmap_single(pdev, frag->dma, frag->length, > PCI_DMA_TODEVICE); > + frag->dma = (u64) NULL; ??? Cast the NULL pointer to u64? > for (i = 1; i < buffer->frag_count; i++) { > DPRINTK(INFO, "getting fragment no %d\n", i); > frag++; /* Get the next frag */ > pci_unmap_page(pdev, frag->dma, frag->length, > PCI_DMA_TODEVICE); > + frag->dma = (u64) NULL; ??? > @@ -547,13 +546,7 @@ #endif > NETXEN_ROMUSB_GLB_PEGTUNE_DONE)); > /* Handshake with the card before we register the devices. */ > netxen_phantom_init(adapter, NETXEN_NIC_PEG_TUNE); > - > - /* leave the hw in the same state as reboot */ > - writel(0, NETXEN_CRB_NORMALIZE(adapter, CRB_CMDPEG_STATE)); > - netxen_pinit_from_rom(adapter, 0); > - udelay(500); > - netxen_load_firmware(adapter); > - netxen_phantom_init(adapter, NETXEN_NIC_PEG_TUNE); > + mdelay(2000); Two seconds delay? No! Too long, sleep. > @@ -653,30 +646,21 @@ static void __devexit netxen_nic_remove( > > netdev = adapter->netdev; > > - netxen_nic_disable_int(adapter); > - if (adapter->irq) > - free_irq(adapter->irq, adapter); > - > + unregister_netdev(netdev); > + mdelay(3000); Toooo long, sleep. > @@ -696,17 +680,73 @@ static void __devexit netxen_nic_remove( > } > } > > - unregister_netdev(netdev); > + if (adapter->flags & NETXEN_NIC_MSI_ENABLED) > + pci_disable_msi(pdev); > > vfree(adapter->cmd_buf_arr); > > + pci_disable_device(pdev); > + > + if (adapter->portnum == 0) { > + if (init_firmware_done) { > + dma_watchdog_shutdown_request(adapter); > + mdelay(100); > + i = 100; > + while ((dma_watchdog_shutdown_poll_result(adapter) != > 1) && i) { > + printk(KERN_INFO"dma_watchdog_shutdown_poll > still in progress\n"); > + mdelay(100); 0.1 seconds delay is too long IMO, too. Sleep! > + dma_watchdog_shutdown_request(adapter); > + mdelay(100); > + i = 100; > + while ((dma_watchdog_shutdown_poll_result(adapter) != 1) && i) { > + printk(KERN_INFO"dma_watchdog_shutdown_poll still in > progress\n"); > + mdelay(100); same... -- 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