On Sat, 2006-04-01 at 05:49 -0800, Linsys Contractor Amit S. Kale wrote:
> + for (ctx = 0; ctx < MAX_RCV_CTX; ++ctx) {
Why did this patch suddenly start in the middle?
> + if (adapter->is_up != ADAPTER_UP_MAGIC) {
> + printk("NetXen adapter %d\n", opencount);
> + adapter->is_up = ADAPTER_UP_MAGIC;
> + if (netxen_nic_init_port (port) != 0) {
> + printk(KERN_ERR "%s: Failed to initialize port %d\n",
> + netxen_nic_driver_name, port->portnum);
> + return -EIO;
> + }
> + netxen_nic_init_niu(adapter);
> + /* setup all the resources for the Phantom... */
> + /* this include the descriptors for rcv, tx, and status */
> + netxen_nic_clear_stats(adapter);
> + err = netxen_nic_hw_resources (adapter);
> + if (err) {
> + DPRINTK(1, ERR, "Error in setting hw resources:"
> + "%d\n", err);
> + return err;
> + }
This init routine doesn't make any attempt to clean up after itself. If
anything fails, you've left the card and the kernel in a broken state.
> +#ifndef SINGLE_DMA_BUF
Lose the ifdefs.
> +#ifndef SINGLE_DMA_BUF
> +//#if !defined(CONFIG_PCI_MSI)
> + if(adapter->activePorts == 1) {
> + read_lock(&adapter->adapter_lock);
> + netxen_nic_enable_int(adapter);
> + read_unlock(&adapter->adapter_lock);
> + }
> +//#endif
> +#endif
This is unreadable.
> + /* Window 1 call */
> + read_lock(&adapter->adapter_lock);
> + state = NetXen_NIC_PCI_READ_32(CRB_NORMALIZE(adapter,
> + CRB_CMDPEG_STATE));
> + read_unlock(&adapter->adapter_lock);
Why are you acquiring locks around all these PCI accesses?
> + addr = pci_alloc_consistent(adapter->ahw.pdev,
> + sizeof(cmdDescType0_t)*
> + adapter->MaxTxDescCount,
> + (dma_addr_t *)&hw->cmdDesc_physAddr);
This variable naming is all junk.
> + if (addr == NULL) {
> + DPRINTK(1,ERR, "bad return from pci_alloc_consistent\n");
> + err = -ENOMEM;
> + return err;
> + }
One return at the end of functions, with proper cleanup code.
My eyes glazed over around this point, and I stopped reading.
<b
-
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
