wireless, wavelan: spin off by 1
PATCH 1/1 wireless, wavelan: spin off by 1
spin can reach -1 after the loop, so 0 is still success.
---
drivers/net/wireless/wavelan_cs.c +2 -3
@@ -838,9 +838,8 @@ wv_82593_cmd(struct net_device * dev, }while (((status &SR3_EXEC_STATE_MASK ) !=SR3_EXEC_IDLE ) && (spin-- >0 )); -/* If the interrupt hasn't be posted */ -if (spin <=0 ) - { +/* If the interrupt hasn't been posted */ +if (spin <0 ) {#ifdef DEBUG_INTERRUPT_ERROR printk (KERN_INFO "wv_82593_cmd: %s timeout (previous command), status 0x%02x\n" , str, status);
Quoted 2 lines · Roel Kluin
spin can reach -1 after the loop, so 0 is still success.
You are probably technically right, but it does not matter in
practice. The chip should answer way faster than this timeout, and we would loose only 0.1% of the overall timeout value. If the chip answer was that close to the timeout, because of variation, we would fail half the time and would need a bigger timeout anyway.
A better way would be to not test spin, but to test the status
register itself. That way, it's less ambiguous.
Anyway, such low level code is tricky, and I personally would
not want any change without thourough testing with the hardware. We know the curent code work, and I don't have time to test, so I would vote to not change the code.
Regards,
Jean
Quoted 18 lines
Signed-off-by Roel Kluin <[email protected]>---
drivers/net/wireless/wavelan_cs.c +2
-3@@ -838,9 +838,8 @@ wv_82593_cmd(struct net_device * dev, }while (((status &SR3_EXEC_STATE_MASK ) !=SR3_EXEC_IDLE ) && (spin-- >0 )); -/* If the interrupt hasn't be posted */ -if (spin <=0 ) - { +/* If the interrupt hasn't been posted */ +if (spin <0 ) {#ifdef DEBUG_INTERRUPT_ERROR printk (KERN_INFO "wv_82593_cmd: %s timeout (previous command), status 0x%02x\n" , str, status);