All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Garzik <jeff@garzik.org>
To: "Linsys Contractor Amit S. Kale" <amitkale@unminc.com>
Cc: netdev@vger.kernel.org, sanjeev@netxen.com,
	unmproj@linsyssoft.com, Andrew Morton <akpm@osdl.org>
Subject: Re: [PATCH 2.6.17 6/9] NetXen: hw initialization routines
Date: Wed, 05 Jul 2006 12:12:26 -0400	[thread overview]
Message-ID: <44ABE4EA.6090603@garzik.org> (raw)
In-Reply-To: <Pine.LNX.4.64.0607050638260.27969@dut46>

Linsys Contractor Amit S. Kale wrote:

> +    while (state != PHAN_INITIALIZE_COMPLETE && loops < 200000) {
> +        udelay(100);
> +        /* Window 1 call */
> +        read_lock(&adapter->adapter_lock);
> +        state = readl(NETXEN_CRB_NORMALIZE(adapter, CRB_CMDPEG_STATE));
> +        read_unlock(&adapter->adapter_lock);
> +
> +        loops++;
> +    }
> +    if (loops >= 200000) {
> +        printk(KERN_ERR "Cmd Peg initialization not complete:%x.\n",
> +               state);
> +        err = -EIO;
> +        return err;
> +    }

worst case udelay() spins CPU for far too long, locking out other tasks

Good thing this driver isn't used in latency-senstitive applications, or 
medical equipment!


> +void initialize_adapter_sw(struct netxen_adapter *adapter)
> +{
> +    int ctxid, ring;
> +    u32 i;
> +    u32 num_rx_bufs = 0;
> +    struct netxen_rcv_desc_ctx *rcv_desc;
> +    struct netxen_ring_context *ctx;
> +
> +    DPRINTK(INFO, "initializing some queues: %p\n", adapter);
> +    for (ctxid = 0; ctxid < MAX_RING_CTX; ++ctxid) {
> +        ctx = &adapter->ring_ctx[ctxid];
> +        ctx->free_cmd_count = ctx->max_tx_desc_count;
> +        for (ring = 0; ring < NUM_RCV_DESC_RINGS; ring++) {
> +            struct netxen_rx_buffer *rx_buf;
> +            rcv_desc = &ctx->recv_ctx.rcv_desc[ring];
> +            rcv_desc->rcv_free = rcv_desc->max_rx_desc_count;
> +            rcv_desc->begin_alloc = 0;
> +            rx_buf = rcv_desc->rx_buf_arr;
> +            num_rx_bufs = rcv_desc->max_rx_desc_count;
> +            /*
> +             * Now go through all of them, set reference handles
> +             * and put them in the queues.
> +             */
> +            for (i = 0; i < num_rx_bufs; i++) {
> +                rx_buf->ref_handle = i;
> +                rx_buf->state = NETXEN_BUFFER_FREE;
> +
> +                DPRINTK(INFO, "Rx buf:ctx%d i(%d) rx_buf:"
> +                    "%p\n", ctxid, i, rx_buf);
> +                rx_buf++;
> +            }
> +        }
> +    }
> +    DPRINTK(INFO, "initialized buffers for %s and %s\n",
> +        "adapter->free_cmd_buf_list", "adapter->free_rxbuf");
> +}
> +
> +int initialize_adapter_hw(struct netxen_adapter *adapter)
> +{
> +    u32 value = 0;
> +
> +    if (netxen_nic_get_board_info(adapter) != 0)
> +        printk("%s: Error getting board config info.\n",
> +            netxen_nic_driver_name);
> +
> +    switch (adapter->ahw.board_type) {
> +    case NETXEN_NIC_GBE:
> +        adapter->ahw.max_ports = 4;
> +        break;
> +
> +    case NETXEN_NIC_XGBE:
> +        adapter->ahw.max_ports = 1;
> +        break;
> +
> +    default:
> +        printk(KERN_ERR "%s: Unknown board type\n",
> +            netxen_nic_driver_name);
> +    }
> +
> +    return value;
> +}
> +
> +/*
> + * netxen_decode_crb_addr(0 - utility to translate from internal 
> Phantom CRB
> + * address to external PCI CRB address.
> + */
> +unsigned long netxen_decode_crb_addr(unsigned long addr)
> +{
> +    int i;
> +    unsigned long base_addr, offset, pci_base;
> +
> +    crb_addr_transform_setup();
> +
> +    pci_base = NETXEN_ADDR_ERROR;
> +    base_addr = addr & 0xfff00000;
> +    offset = addr & 0x000fffff;
> +
> +    for (i = 0; i < NETXEN_MAX_CRB_XFORM; i++) {
> +        if (crb_addr_xform[i] == base_addr) {
> +            pci_base = i << 20;
> +            break;
> +        }
> +    }
> +    if (pci_base == NETXEN_ADDR_ERROR)
> +        return pci_base;
> +    else
> +        return (pci_base + offset);
> +}
> +
> +static long rom_max_timeout = 10000;
> +static long rom_lock_timeout = 100000000;
> +
> +int rom_lock(struct netxen_adapter *adapter)
> +{
> +    int iter;
> +    int done = 0, timeout = 0;
> +
> +    while (!done) {
> +        /* acquire semaphore2 from PCI HW block */
> +        netxen_nic_read_w0(adapter, NETXEN_PCIE_REG(PCIE_SEM2_LOCK),
> +                   &done);
> +        if (done == 1)
> +            break;
> +        if (timeout >= rom_lock_timeout)
> +            return -1;
> +
> +        timeout++;
> +        /*
> +         * Yield CPU
> +         */
> +        if (!in_atomic())
> +            schedule();
> +        else {
> +            for (iter = 0; iter < 20; iter++)
> +                cpu_relax();    /*This a nop instr on i386 */

when is this function ever used in atomic context?

> +int netxen_wait_rom_done(struct netxen_adapter *adapter)
> +{
> +    long timeout = 0;
> +    long done = 0;
> +
> +    while (done == 0) {
> +        done = netxen_nic_reg_read(adapter, NETXEN_ROMUSB_GLB_STATUS);
> +        done &= 2;
> +        timeout++;
> +        if (timeout >= rom_max_timeout) {
> +            printk("Timeout reached  waiting for rom done");
> +            return -1;
> +        }
> +    }
> +    return 0;

how long does this spin the cpu?


> +int do_rom_fast_read(struct netxen_adapter *adapter, int addr, int *valp)
> +{
> +    netxen_nic_reg_write(adapter, NETXEN_ROMUSB_ROM_ADDRESS, addr);
> +    netxen_nic_reg_write(adapter, NETXEN_ROMUSB_ROM_ABYTE_CNT, 3);
> +    udelay(100);        /* prevent bursting on CRB */

likely PCI posting bug


> +    netxen_nic_reg_write(adapter, NETXEN_ROMUSB_ROM_DUMMY_BYTE_CNT, 0);
> +    netxen_nic_reg_write(adapter, NETXEN_ROMUSB_ROM_INSTR_OPCODE, 0xb);
> +    if (netxen_wait_rom_done(adapter)) {
> +        printk("Error waiting for rom done\n");
> +        return -1;
> +    }
> +    /* reset abyte_cnt and dummy_byte_cnt */
> +    netxen_nic_reg_write(adapter, NETXEN_ROMUSB_ROM_ABYTE_CNT, 0);
> +    udelay(100);        /* prevent bursting on CRB */

ditto



> +    netxen_nic_reg_write(adapter, NETXEN_ROMUSB_ROM_DUMMY_BYTE_CNT, 0);
> +
> +    *valp = netxen_nic_reg_read(adapter, NETXEN_ROMUSB_ROM_RDATA);
> +    return 0;
> +}
> +
> +int netxen_rom_fast_read(struct netxen_adapter *adapter, int addr, int 
> *valp)
> +{
> +    int ret;
> +
> +    if (rom_lock(adapter) != 0)
> +        return -1;
> +
> +    ret = do_rom_fast_read(adapter, addr, valp);
> +    rom_unlock(adapter);
> +    return ret;
> +}
> +
> +int pinit_from_rom(struct netxen_adapter *adapter, int verbose)
> +{
> +    int addr, val, status;
> +    int n, i;
> +    int init_delay = 0;
> +    struct crb_addr_pair *buf;
> +    unsigned long off;
> +    unsigned long flags;
> +
> +    /* resetall */
> +    status = netxen_nic_get_board_info(adapter);
> +    if (status)
> +        printk("%s: pinit_from_rom: Error getting board info\n",
> +               netxen_nic_driver_name);
> +
> +    netxen_crb_writelit_adapter(adapter, NETXEN_ROMUSB_GLB_SW_RESET,
> +                    0xffffffff);
> +
> +    if (verbose) {
> +        int val;
> +        if (netxen_rom_fast_read(adapter, 0x4008, &val) == 0)
> +            printk("P2 ROM board type: 0x%08x\n", val);
> +        else
> +            printk("Could not read board type\n");
> +        if (netxen_rom_fast_read(adapter, 0x400c, &val) == 0)
> +            printk("P2 ROM board  num: 0x%08x\n", val);
> +        else
> +            printk("Could not read board number\n");
> +        if (netxen_rom_fast_read(adapter, 0x4010, &val) == 0)
> +            printk("P2 ROM chip   num: 0x%08x\n", val);
> +        else
> +            printk("Could not read chip number\n");
> +    }
> +
> +    if (netxen_rom_fast_read(adapter, 0, &n) == 0 && (n & 
> 0x800000000ULL)) {
> +        n &= ~0x80000000ULL;

kill magic numbers, use named constants


> +        if (n < 1024) {
> +            if (verbose)
> +                printk("%s: %d CRB init values found"
> +                       " in ROM.\n", netxen_nic_driver_name, n);
> +        } else {
> +            printk("%s:n=0x%x Error! NetXen card flash not"
> +                   " initialized.\n", __FUNCTION__, n);
> +            return -1;
> +        }
> +        buf = kcalloc(n, sizeof(struct crb_addr_pair), GFP_KERNEL);
> +        if (buf == NULL) {
> +            printk("%s: pinit_from_rom: Unable to calloc memory.\n",
> +                   netxen_nic_driver_name);
> +            return -1;
> +        }
> +        for (i = 0; i < n; i++) {
> +            if (netxen_rom_fast_read(adapter, 8 * i + 4, &val) != 0
> +                || netxen_rom_fast_read(adapter, 8 * i + 8,
> +                            &addr) != 0)
> +                goto error_ret;
> +
> +            buf[i].addr = addr;
> +            buf[i].data = val;
> +
> +            if (verbose)
> +                printk("%s: PCI:     0x%08x == 0x%08x\n",
> +                       netxen_nic_driver_name, (unsigned int)
> +                       netxen_decode_crb_addr((unsigned long)
> +                                  addr), val);
> +        }
> +        for (i = 0; i < n; i++) {
> +
> +            off =
> +                netxen_decode_crb_addr((unsigned long)buf[i].addr) +
> +                NETXEN_PCI_CRBSPACE;
> +            /* skipping cold reboot MAGIC */
> +            if (off == NETXEN_CAM_RAM(0x1fc))
> +                continue;
> +
> +            /* After writing this register, HW needs time for CRB */
> +            /* to quiet down (else crb_window returns 0xffffffff) */
> +            if (off == NETXEN_ROMUSB_GLB_SW_RESET) {
> +                init_delay = 1;
> +                /* hold xdma in reset also */
> +                buf[i].data = 0x8000ff;
> +            }
> +
> +            if (ADDR_IN_WINDOW1(off)) {
> +                read_lock(&adapter->adapter_lock);
> +                writel(buf[i].data,
> +                       NETXEN_CRB_NORMALIZE(adapter, off));
> +                read_unlock(&adapter->adapter_lock);
> +            } else {
> +                write_lock_irqsave(&adapter->adapter_lock,
> +                           flags);
> +                netxen_nic_pci_change_crbwindow(adapter, 0);
> +                off = adapter->ahw.pci_base + off;
> +                writel(buf[i].data, (void *)off);
> +                netxen_nic_pci_change_crbwindow(adapter, 1);
> +                write_unlock_irqrestore(&adapter->adapter_lock,
> +                            flags);
> +            }
> +            if (init_delay == 1) {
> +                msleep(1000);

ssleep()

> +                init_delay = 0;
> +            }
> +            msleep(1);
> +        }
> +        kfree(buf);
> +
> +        /* disable_peg_cache_all */
> +
> +        /* unreset_net_cache */
> +        netxen_nic_hw_read_wx(adapter, NETXEN_ROMUSB_GLB_SW_RESET, &val,
> +                      4);
> +        netxen_crb_writelit_adapter(adapter, NETXEN_ROMUSB_GLB_SW_RESET,
> +                        (val & 0xffffff0f));
> +        /* p2dn replyCount */
> +        netxen_crb_writelit_adapter(adapter,
> +                        NETXEN_CRB_PEG_NET_D + 0xec, 0x1e);
> +        /* disable_peg_cache 0 */
> +        netxen_crb_writelit_adapter(adapter,
> +                        NETXEN_CRB_PEG_NET_D + 0x4c, 8);
> +        /* disable_peg_cache 1 */
> +        netxen_crb_writelit_adapter(adapter,
> +                        NETXEN_CRB_PEG_NET_I + 0x4c, 8);
> +
> +        /* peg_clr_all */
> +
> +        /* peg_clr 0 */
> +        netxen_crb_writelit_adapter(adapter, NETXEN_CRB_PEG_NET_0 + 0x8,
> +                        0);
> +        netxen_crb_writelit_adapter(adapter, NETXEN_CRB_PEG_NET_0 + 0xc,
> +                        0);
> +        /* peg_clr 1 */
> +        netxen_crb_writelit_adapter(adapter, NETXEN_CRB_PEG_NET_1 + 0x8,
> +                        0);
> +        netxen_crb_writelit_adapter(adapter, NETXEN_CRB_PEG_NET_1 + 0xc,
> +                        0);
> +        /* peg_clr 2 */
> +        netxen_crb_writelit_adapter(adapter, NETXEN_CRB_PEG_NET_2 + 0x8,
> +                        0);
> +        netxen_crb_writelit_adapter(adapter, NETXEN_CRB_PEG_NET_2 + 0xc,
> +                        0);
> +        /* peg_clr 3 */
> +        netxen_crb_writelit_adapter(adapter, NETXEN_CRB_PEG_NET_3 + 0x8,
> +                        0);
> +        netxen_crb_writelit_adapter(adapter, NETXEN_CRB_PEG_NET_3 + 0xc,
> +                        0);
> +    }
> +    return 0;
> +      error_ret:
> +    return -1;
> +}
> +
> +void phantom_init(struct netxen_adapter *adapter)
> +{
> +    u32 val = 0;
> +    int loops = 0;
> +
> +    read_lock(&adapter->adapter_lock);
> +    netxen_nic_hw_read_wx(adapter, NETXEN_ROMUSB_GLB_PEGTUNE_DONE, 
> &val, 4);
> +    writel(1,
> +           NETXEN_CRB_NORMALIZE(adapter, NETXEN_ROMUSB_GLB_PEGTUNE_DONE));
> +
> +    if (0 == val) {
> +        while (val != PHAN_INITIALIZE_COMPLETE && loops < 2000000) {
> +            udelay(100);

locks up CPU for too long



> +            val =
> +                readl(NETXEN_CRB_NORMALIZE
> +                  (adapter, CRB_CMDPEG_STATE));
> +            loops++;
> +        }
> +        if (val != PHAN_INITIALIZE_COMPLETE)
> +            printk("WARNING: Initial boot wait loop failed...\n");
> +    }
> +    read_unlock(&adapter->adapter_lock);
> +}
> +
> +void load_firmware(struct netxen_adapter *adapter)
> +{
> +    int i;
> +    long data, size = 0;
> +    long flashaddr = NETXEN_FLASH_BASE, memaddr = NETXEN_PHANTOM_MEM_BASE;
> +
> +    size = (16 * 1024) / 4;
> +    read_lock(&adapter->adapter_lock);
> +    writel(1, NETXEN_CRB_NORMALIZE(adapter, NETXEN_ROMUSB_GLB_CAS_RST));
> +    read_unlock(&adapter->adapter_lock);
> +
> +    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;
> +        }
> +        netxen_nic_pci_mem_write(adapter, memaddr, &data, 4);
> +        flashaddr += 4;
> +        memaddr += 4;
> +    }
> +    udelay(100);

PCI posting


> +    read_lock(&adapter->adapter_lock);
> +    /* make sure Casper is powered on */
> +    writel(0x3fff,
> +           NETXEN_CRB_NORMALIZE(adapter, 
> NETXEN_ROMUSB_GLB_CHIP_CLK_CTRL));
> +    writel(0, NETXEN_CRB_NORMALIZE(adapter, NETXEN_ROMUSB_GLB_CAS_RST));
> +    read_unlock(&adapter->adapter_lock);
> +
> +    udelay(10000);

delay far too long


> +int netxen_nic_rx_has_work(struct netxen_adapter *adapter)
> +{
> +    int ctxid;
> +
> +    for (ctxid = 0; ctxid < MAX_RING_CTX; ++ctxid) {
> +        struct netxen_ring_context *ctx = &adapter->ring_ctx[ctxid];
> +        struct netxen_recv_context *recv_ctx = &(ctx->recv_ctx);
> +        u32 consumer;
> +        struct status_desc_t *desc_head;
> +        struct status_desc_t *desc;    /* used to read status desc here */
> +
> +        consumer = recv_ctx->status_rx_consumer;
> +        desc_head = recv_ctx->rcv_status_desc_head;
> +        desc = &desc_head[consumer];
> +
> +        if ((desc->owner & STATUS_OWNER_HOST))
> +            return 1;
> +    }
> +
> +    return 0;
> +}

put function near user, and mark 'static', to enable possibility of inlining



> +void netxen_watchdog_task(unsigned long v)
> +{
> +    int port_num;
> +    struct netxen_port *port;
> +    struct net_device *netdev;
> +    struct netxen_adapter *adapter = (struct netxen_adapter *)v;
> +    unsigned long flags;
> +
> +    if (adapter->driver_mismatch) {
> +        /* We return without turning on the netdev queue as there
> +         * was a mismatch in driver and firmware version
> +         */
> +        return;

huh?  when will this ever happen?


> +    for (port_num = 0; port_num < adapter->ahw.max_ports; port_num++) {
> +        port = adapter->port[port_num];
> +        netdev = port->netdev;
> +
> +        if ((netdev->flags & IFF_UP) && !netif_carrier_ok(netdev)) {

don't test IFF_UP, you likely want netif_running(), etc.


> +            printk(KERN_INFO "%s port %d, %s carrier is now ok\n",
> +                   netxen_nic_driver_name, port_num, netdev->name);
> +            netif_carrier_on(netdev);
> +        }
> +
> +        if (netif_queue_stopped(netdev))


Calling netif_wake_queue() without doing something useful with TX queue 
is bogus



> +inline void
> +netxen_process_rcv(struct netxen_adapter *adapter, int ctxid,
> +           struct status_desc_t *desc)

inline without static is likely wrong


> +{
> +    struct netxen_port *port = adapter->port[desc->port];
> +    struct pci_dev *pdev = port->pdev;
> +    struct net_device *netdev = port->netdev;
> +    int index = desc->reference_handle;
> +    struct netxen_recv_context *recv_ctx =
> +        &(adapter->ring_ctx[ctxid].recv_ctx);
> +    struct netxen_rx_buffer *buffer;
> +    struct sk_buff *skb;
> +    u32 length = desc->total_length;
> +    u32 desc_ctx;
> +    struct netxen_rcv_desc_ctx *rcv_desc;
> +    int ret;
> +
> +    desc_ctx = desc->type;
> +    if (unlikely(desc_ctx >= NUM_RCV_DESC_RINGS)) {
> +        printk("%s: %s Bad Rcv descriptor ring\n",
> +               netxen_nic_driver_name, netdev->name);
> +        return;
> +    }
> +
> +    rcv_desc = &recv_ctx->rcv_desc[desc_ctx];
> +    buffer = &rcv_desc->rx_buf_arr[index];
> +
> +    pci_unmap_single(pdev, buffer->dma, rcv_desc->dma_size,
> +             PCI_DMA_FROMDEVICE);
> +
> +    skb = (struct sk_buff *)buffer->skb;
> +
> +    if (unlikely(skb == NULL)) {
> +        /*
> +         * This should not happen and if it does, it is serious,
> +         * catch it
> +         */

When will this EVER happen?

Kill all this code...


Got tired of reviewing at this point.

Please fix the mentioned issues and repost --as one big patch-- (i.e. 
requires a URL).

	Jeff



  reply	other threads:[~2006-07-05 16:12 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-07-05 13:15 [PATCH 2.6.17 0/9] NetXen: ethernet nic driver Linsys Contractor Amit S. Kale
2006-07-05 13:20 ` [PATCH 2.6.17 1/9] NetXen: Makefile and ethtool interface Linsys Contractor Amit S. Kale
2006-07-05 15:34   ` Jeff Garzik
2006-07-06 13:50     ` Pradeep Dalvi
2006-07-05 13:29 ` [PATCH 2.6.17 2/9] NetXen: Main header file Linsys Contractor Amit S. Kale
2006-07-05 15:46   ` Jeff Garzik
2006-07-05 13:31 ` [PATCH 2.6.17 3/9] NetXen: Registers info " Linsys Contractor Amit S. Kale
2006-07-05 15:51   ` Jeff Garzik
2006-07-05 13:34 ` [PATCH 2.6.17 4/9] NetXen: hardware access routines Linsys Contractor Amit S. Kale
2006-07-05 16:00   ` Jeff Garzik
2006-07-05 13:38 ` [PATCH 2.6.17 5/9] NetXen: hardware access header file Linsys Contractor Amit S. Kale
2006-07-05 16:04   ` Jeff Garzik
2006-07-05 13:40 ` [PATCH 2.6.17 6/9] NetXen: hw initialization routines Linsys Contractor Amit S. Kale
2006-07-05 16:12   ` Jeff Garzik [this message]
2006-07-05 13:42 ` [PATCH 2.6.17 7/9] NetXen: ioctl interface and intr routines Linsys Contractor Amit S. Kale
2006-07-05 13:44 ` [PATCH 2.6.17 8/9] NetXen: Driver main file Linsys Contractor Amit S. Kale
2006-07-05 13:47 ` [PATCH 2.6.17 9/9] NetXen: niu handling and CRB reg definitions Linsys Contractor Amit S. Kale

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=44ABE4EA.6090603@garzik.org \
    --to=jeff@garzik.org \
    --cc=akpm@osdl.org \
    --cc=amitkale@unminc.com \
    --cc=netdev@vger.kernel.org \
    --cc=sanjeev@netxen.com \
    --cc=unmproj@linsyssoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.