wilc1000: fix a loop timeout condition
PATCH 1/1 wilc1000: fix a loop timeout condition
If the loop fails, the "while(trials--) {" loop will exit with "trials" set to -1. The test for that expects it to end with "trials" set to 0 so the warning message will not be printed.
Fix this by changing from a post-op to a pre-op. This does mean that we only make 99 attempts instead of 100 but that's okay.
--- drivers/net/wireless/microchip/wilc1000/wlan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/microchip/wilc1000/wlan.c +1 -1
@@ -575,7 +575,7 @@ void chip_allow_sleep(struct wilc *wilc) to_host_from_fw_bit =WILC_SPI_FW_TO_HOST_BIT ; } -while (trials--) { +while (--trials) { ret = hif_func->hif_read_reg (wilc, to_host_from_fw_reg, ®);if (ret)return ;
Signature · 1 line
2.30.2
Quoted 10 lines · Dan Carpenter
If the loop fails, the "while(trials--) {" loop will exit with "trials" set to -1. The test for that expects it to end with "trials" set to 0 so the warning message will not be printed.
Fix this by changing from a post-op to a pre-op. This does mean that we only make 99 attempts instead of 100 but that's okay.
Fixes f135a1571a05 ("wilc1000: Support chip sleep over SPI")Signed-off-by Dan Carpenter <[email protected]>
Thanks Dan.
Quoted 20 lines
--- drivers/net/wireless/microchip/wilc1000/wlan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/net/wireless/microchip/wilc1000/wlan.c +1
-2@@ -575,7 +575,7 @@ void chip_allow_sleep(struct wilc *wilc) to_host_from_fw_bit =WILC_SPI_FW_TO_HOST_BIT ; } -while (trials--) { +while (--trials) { ret = hif_func->hif_read_reg (wilc, to_host_from_fw_reg, ®);if (ret)return ; --2.30.2
Quoted 5 lines · On Fri, 2021-03-19 at 16:09 +0000, [email protected] wrote:
Quoted 4 lines · Dan Carpenter
If the loop fails, the "while(trials--) {" loop will exit with "trials" set to -1. The test for that expects it to end with "trials" set to 0 so the warning message will not be printed.
Quoted 7 lines
Quoted 5 lines
Fix this by changing from a post-op to a pre-op. This does mean that we only make 99 attempts instead of 100 but that's okay.
Fixes f135a1571a05 ("wilc1000: Support chip sleep over SPI")Signed-off-by Dan Carpenter <[email protected]>Thanks Dan.
Good catch, but wouldn't it be better to fix the time-out check condition instead? Something a long the lines of:
net/wireless/microchip/wilc1000/wlan.c renamed from net/wireless/microchip/wilc1000/wlan.c~ +2 -2
@@ -457,7 +457,7 @@u32 wakeup_reg, wakeup_bit;u32 to_host_from_fw_reg, to_host_from_fw_bit;u32 from_host_to_fw_reg, from_host_to_fw_bit; -u32 trials =100 ; +int trials =100 ;int ret;if (wilc->io_type ==WILC_HIF_SDIO ) {
@@ -483,7 +483,7 @@if ((reg & to_host_from_fw_bit) ==0 )break ; } -if (!trials) +if (trials <0 )pr_warn ("FW not responding\n" );/* Clear bit 1 */
This way, the loop could actually get executed the number of times indicated by the initialization of "trial" before issuing a warning message.
--david
Quoted 22 lines
Acked-by Ajay Singh <[email protected]>Quoted 20 lines
--- drivers/net/wireless/microchip/wilc1000/wlan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/net/wireless/microchip/wilc1000/wlan.c +1
-2@@ -575,7 +575,7 @@ void chip_allow_sleep(struct wilc *wilc) to_host_from_fw_bit =WILC_SPI_FW_TO_HOST_BIT ; } -while (trials--) { +while (--trials) { ret = hif_func->hif_read_reg (wilc, to_host_from_fw_reg, ®);if (ret)return ; --2.30.2
Quoted 39 lines · David Mosberger-Tang
Quoted 5 lines · On Fri, 2021-03-19 at 16:09 +0000, [email protected] wrote:
Quoted 4 lines · Dan Carpenter
If the loop fails, the "while(trials--) {" loop will exit with "trials" set to -1. The test for that expects it to end with "trials" set to 0 so the warning message will not be printed.
Quoted 7 lines
Quoted 5 lines
Fix this by changing from a post-op to a pre-op. This does mean that we only make 99 attempts instead of 100 but that's okay.
Fixes f135a1571a05 ("wilc1000: Support chip sleep over SPI")Signed-off-by Dan Carpenter <[email protected]>Thanks Dan.
Good catch, but wouldn't it be better to fix the time-out check condition instead? Something a long the lines of:
net/wireless/microchip/wilc1000/wlan.c renamed from net/wireless/microchip/wilc1000/wlan.c~ +2
-2@@ -457,7 +457,7 @@u32 wakeup_reg, wakeup_bit;u32 to_host_from_fw_reg, to_host_from_fw_bit;u32 from_host_to_fw_reg, from_host_to_fw_bit; -u32 trials =100 ; +int trials =100 ;int ret;if (wilc->io_type ==WILC_HIF_SDIO ) {@@ -483,7 +483,7 @@if ((reg & to_host_from_fw_bit) ==0 )break ; } -if (!trials) +if (trials <0 )pr_warn ("FW not responding\n" );/* Clear bit 1 */
Quoted 3 lines
This way, the loop could actually get executed the number of times indicated by the initialization of "trial" before issuing a warning message.
Those numbers are just made up... It doesn't matter either way.
regards,
dan carpenter
Quoted 10 lines · Dan Carpenter
If the loop fails, the "while(trials--) {" loop will exit with "trials" set to -1. The test for that expects it to end with "trials" set to 0 so the warning message will not be printed.
Fix this by changing from a post-op to a pre-op. This does mean that we only make 99 attempts instead of 100 but that's okay.
Fixes f135a1571a05 ("wilc1000: Support chip sleep over SPI")Signed-off-by Dan Carpenter <[email protected]>Acked-by Ajay Singh <[email protected]>
Patch applied to wireless-drivers-next.git, thanks.
2f51061edab9 wilc1000: fix a loop timeout condition
Signature · 3 lines
https://patchwork.kernel.org/project/linux-wireless/patch/YFS5gx/gi70zlIaO@mwanda/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches