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

Reply via email to