Re: [PATCH 6/8] [RFC] qlge: Add mgmt port xface file qlge_mpi.c
From: Franco Fichtner <hidden>
Date: 2008-09-11 08:05:03
Ron Mercer wrote:
quoted hunk ↗ jump to hunk
Signed-off-by: Ron Mercer <redacted> --- drivers/net/qlge/qlge_mpi.c | 228 +++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 228 insertions(+), 0 deletions(-) create mode 100644 drivers/net/qlge/qlge_mpi.cdiff --git a/drivers/net/qlge/qlge_mpi.c b/drivers/net/qlge/qlge_mpi.c new file mode 100644 index 0000000..f235fc5 --- /dev/null +++ b/drivers/net/qlge/qlge_mpi.c@@ -0,0 +1,228 @@ +#include "qlge.h" + +/* + * Returns zero on success. + */ +static int ql_wait_mbx_cmd_cmplt(struct ql_adapter *qdev) +{ + int count = 50; + u32 temp; + + while (count) { + temp = ql_read32(qdev, STS); + if (temp & STS_PI) + return 0; + mdelay(5);
Do you really want to delay execution for 250ms in worst case (timeout) scenarios? A msleep() would be more graceful.
quoted hunk ↗ jump to hunk
+ count--; + } + return -ETIMEDOUT; +} + +static int ql_write_mbox_reg(struct ql_adapter *qdev, u32 reg, u32 data) +{ + int status = 0; + /* wait for reg to come ready */ + status = ql_wait_addr_reg(qdev, PROC_ADDR, PROC_ADDR_RDY); + if (status) + goto exit; + /* write the data to the data reg */ + ql_write32(qdev, PROC_DATA, data); + /* trigger the write */ + ql_write32(qdev, PROC_ADDR, reg); + /* wait for reg to come ready */ + status = ql_wait_addr_reg(qdev, PROC_ADDR, PROC_ADDR_RDY); + if (status) + goto exit;
These two lines above are redundant. You cold also avoid the goto with
opening a new block after the first if. Or how about just...
static int ql_write_mbox_reg(struct ql_adapter *qdev, u32 reg, u32 data)
{
/* wait for reg to come ready */
if (ql_wait_addr_reg(qdev_ PROC_ADDR, PROC_ADDR_RDY)) {
/* write the data to the data reg */
ql_write32(qdev, PROC_DATA, data);
/* trigger the write */
ql_write32(qdev, PROC_ADDR, reg);
}
/* wait for reg to come ready */
return ql_wait_addr_reg(qdev, PROC_ADDR, PROC_ADDR_RDY);
}
... omitting the need for the status variable altogether.
quoted hunk ↗ jump to hunk
+exit: + return status; +} + +static int ql_read_mbox_reg(struct ql_adapter *qdev, u32 reg, u32 *data) +{ + int status = 0;
Same here for the goto. And no need to initialize status with a constant which must be pulled from memory when there is no path using this default value anyway.
quoted hunk ↗ jump to hunk
+ /* wait for reg to come ready */ + status = ql_wait_addr_reg(qdev, PROC_ADDR, PROC_ADDR_RDY); + if (status) + goto exit; + /* set up for reg read */ + ql_write32(qdev, PROC_ADDR, reg | PROC_ADDR_R); + /* wait for reg to come ready */ + status = ql_wait_addr_reg(qdev, PROC_ADDR, PROC_ADDR_RDY); + if (status) + goto exit; + /* get the data */ + *data = ql_read32(qdev, PROC_DATA); +exit: + return status; +} + +static int ql_exec_mb_cmd(struct ql_adapter *qdev, struct mbox_params *mbcp) +{ + int i, status; + + /* + * Make sure there's nothing pending. + * This shouldn't happen. + */ + if (ql_read32(qdev, CSR) & CSR_HRI) + return -EBUSY; + + status = ql_sem_spinlock(qdev, SEM_PROC_REG_MASK); + if (status) + return -EBUSY; + /* + * Fill the outbound mailboxes. + */ + for (i = 0; i < mbcp->in_count; i++) { + status = + ql_write_mbox_reg(qdev, qdev->mailbox_in + i, + mbcp->mbox_in[i]); + if (status) + goto end; + } + /* + * Wake up the MPI firmware. + */ + ql_write32(qdev, CSR, CSR_CMD_SET_H2R_INT); +end: + ql_sem_unlock(qdev, SEM_PROC_REG_MASK); /* does flush too */ + return status; +} + +int ql_get_mb_sts(struct ql_adapter *qdev, struct mbox_params *mbcp) +{ + int i, status = 0;
Again, default value never used.
quoted hunk ↗ jump to hunk
+ + status = ql_sem_spinlock(qdev, SEM_PROC_REG_MASK); + if (status) + return -EBUSY; + for (i = 0; i < mbcp->out_count; i++) { + status = + ql_read_mbox_reg(qdev, qdev->mailbox_out + i, + &mbcp->mbox_out[i]); + if (status) { + QPRINTK(qdev, DRV, ERR, "Failed mailbox read.\n"); + break; + } + } + ql_sem_unlock(qdev, SEM_PROC_REG_MASK); /* does flush too */ + return status; +} + +static void ql_link_up(struct ql_adapter *qdev) +{ + struct mbox_params mbc; + struct mbox_params *mbcp = &mbc; + mbcp->out_count = 2; + + if (ql_get_mb_sts(qdev, mbcp)) + goto exit; + + qdev->link_status = mbcp->mbox_out[1]; + QPRINTK(qdev, DRV, ERR, "Link Up.\n"); + QPRINTK(qdev, DRV, ERR, "Link Status = 0x%.08x.\n", mbcp->mbox_out[1]); + if (!netif_carrier_ok(qdev->ndev)) { + QPRINTK(qdev, LINK, INFO, "Link is Up.\n"); + netif_carrier_on(qdev->ndev); + netif_wake_queue(qdev->ndev); + } +exit: + /* Clear the MPI firmware status. */ + ql_write32(qdev, CSR, CSR_CMD_CLR_R2PCI_INT); +} + +static void ql_link_down(struct ql_adapter *qdev) +{ + struct mbox_params mbc; + struct mbox_params *mbcp = &mbc; + mbcp->out_count = 3; + + if (ql_get_mb_sts(qdev, mbcp)) { + QPRINTK(qdev, DRV, ERR, "Firmware did not initialize!\n"); + goto exit; + } + + if (netif_carrier_ok(qdev->ndev)) { + QPRINTK(qdev, LINK, INFO, "Link is Down.\n"); + netif_carrier_off(qdev->ndev); + netif_stop_queue(qdev->ndev); + } + QPRINTK(qdev, DRV, ERR, "Link Down.\n"); + QPRINTK(qdev, DRV, ERR, "Link Status = 0x%.08x.\n", mbcp->mbox_out[1]); +exit: + /* Clear the MPI firmware status. */ + ql_write32(qdev, CSR, CSR_CMD_CLR_R2PCI_INT); +} + +static void ql_init_fw_done(struct ql_adapter *qdev) +{ + struct mbox_params mbc; + struct mbox_params *mbcp = &mbc; + mbcp->out_count = 2; + + if (ql_get_mb_sts(qdev, mbcp)) { + QPRINTK(qdev, DRV, ERR, "Firmware did not initialize!\n"); + goto exit; + } + QPRINTK(qdev, DRV, ERR, "Firmware initialized!\n"); + QPRINTK(qdev, DRV, ERR, "Firmware status = 0x%.08x.\n", + mbcp->mbox_out[0]); + QPRINTK(qdev, DRV, ERR, "Firmware Revision = 0x%.08x.\n", + mbcp->mbox_out[1]); +exit: + /* Clear the MPI firmware status. */ + ql_write32(qdev, CSR, CSR_CMD_CLR_R2PCI_INT); +} + +void ql_mpi_work(struct work_struct *work) +{ + struct ql_adapter *qdev = + container_of(work, struct ql_adapter, mpi_work.work); + struct mbox_params mbc; + struct mbox_params *mbcp = &mbc; + mbcp->out_count = 1; + + while (ql_read32(qdev, STS) & STS_PI) { + if (ql_get_mb_sts(qdev, mbcp)) { + QPRINTK(qdev, DRV, ERR, + "Could not read MPI, resetting ASIC!\n"); + ql_queue_asic_error(qdev); + } + + switch (mbcp->mbox_out[0]) { + case AEN_LINK_UP: + ql_link_up(qdev); + break; + case AEN_LINK_DOWN: + ql_link_down(qdev); + break; + case AEN_FW_INIT_DONE: +#ifdef PALLADIUM +/* Palladium Workaround Start */ + case 0: +/* Palladium Workaround End */ +#endif + ql_init_fw_done(qdev); + break; + case AEN_FW_INIT_FAIL: + break;
No breaking here...
+ case AEN_SYS_ERR: + break;
...and here necessary to make things more consistent with the lines below. (Having no default seems ok here.)
quoted hunk ↗ jump to hunk
+ case MB_CMD_STS_GOOD: + case MB_CMD_STS_INTRMDT: + case MB_CMD_STS_ERR: + case AEN_IDC_CMPLT: + case AEN_IDC_REQ: + break; + + } + } + ql_enable_completion_interrupt(qdev, 0); +} + +void ql_mpi_reset_work(struct work_struct *work) +{ + struct ql_adapter *qdev = + container_of(work, struct ql_adapter, mpi_reset_work.work); + printk(KERN_ERR "%s: Enter, qdev = %p..\n", __func__, qdev); +}
Another inconsistency: you're not using QPRINTK() like before. Is this intended? Franco