- Remove useless comments. Hint: if the comment does not add more
information than the next line of code does without the comment,
just leave it out.
- Replace HZ with 100 - see the comment for an explanation. Arguably
one could use 250, 300 or 1000 as well. Or 42, for that matter.
- Replace hand-rolled memset with memset and avoid the macro to make the
code more obvious to a reader.
- Replace "(req_cnt + 2)" with 3 and add a comment.
The function still strikes me as horribly wrong. Releasing and
re-acquiring a lock held by the caller is never a good sign. An
arbitrary wait-loop using busy spinning is just Plain Wrong(tm). The
device has interrupts, so why do we implement polling in a function
called from the interrupt handler? Bonkers!
Signed-off-by: Joern Engel <joern@xxxxxxxxx>
---
drivers/scsi/qla2xxx/qla_iocb.c | 36 +++++++++++++++++-------------------
1 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index c822967..941d413 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -569,13 +569,21 @@ qla2x00_req_pkt(scsi_qla_host_t *vha)
{
struct qla_hw_data *ha = vha->hw;
device_reg_t __iomem *reg = ha->iobase;
- request_t *pkt = NULL;
- uint32_t *dword_ptr, timer;
- uint16_t req_cnt = 1, cnt;
+ request_t *pkt;
+ uint32_t timer;
+ uint16_t cnt;
- /* Wait 1 second for slot. */
- for (timer = HZ; timer; timer--) {
- if ((req_cnt + 2) >= vha->req->cnt) {
+ /*
+ * Wait 200-900us for a slot. 200us for 100 udelay(2), another 700us seem
+ * to be used up elsewhere in this loop - or in the calling code around.
+ * An older comment and the former usage of HZ indicates that the intention
+ * was to wait for 1s, but the code never did that. Given that the actual
+ * code, rather than the original intention has see a lot of testing, keep
+ * it at 200us.
+ */
+ for (timer = 100; timer; timer--) {
+ /* XXX Where does the magical value of 3 come from? */
+ if (3 >= vha->req->cnt) {
/* Calculate number of free request entries. */
if (IS_FWI2_CAPABLE(ha))
cnt = (uint16_t)RD_REG_DWORD(®->isp24.req_q_out);
@@ -591,28 +599,19 @@ qla2x00_req_pkt(scsi_qla_host_t *vha)
}
/* If room for request in request ring. */
- if ((req_cnt + 2) < vha->req->cnt) {
+ /* XXX Where does the magical value of 3 come from? */
+ if (3 < vha->req->cnt) {
vha->req->cnt--;
pkt = vha->req->ring_ptr;
- /* Zero out packet. */
- dword_ptr = (uint32_t *)pkt;
- for (cnt = 0; cnt < REQUEST_ENTRY_SIZE / 4; cnt++)
- *dword_ptr++ = 0;
-
- /* Set system defined field. */
+ memset(pkt, 0, sizeof(*pkt));
pkt->sys_define = (uint8_t)vha->req->ring_index;
-
- /* Set entry count. */
pkt->entry_count = 1;
-
return pkt;
}
- /* Release ring specific lock */
spin_unlock_irq(&ha->hardware_lock);
- /* 2 us */
udelay(2);
/*
* Check for pending interrupts, during init we issue marker directly
@@ -620,7 +619,6 @@ qla2x00_req_pkt(scsi_qla_host_t *vha)
if (!vha->marker_needed && !vha->flags.init_done)
qla2x00_poll(vha->req->rsp);
- /* Reaquire ring specific lock */
spin_lock_irq(&ha->hardware_lock);
}
--
1.7.7.3
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
