[PATCH] wl12xx: loop off by one.

[PATCH] wl12xx: loop off by one.

[Roel Kluin]

with `while (loop++ < INIT_LOOP)' `loop' reaches -1 after the loop.

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/drivers/net/wireless/wl12xx/boot.c b/drivers/net/wireless/wl12xx/boot.c
index 48ac08c..8899905 100644
--- a/drivers/net/wireless/wl12xx/boot.c
+++ b/drivers/net/wireless/wl12xx/boot.c
@@ -247,7 +247,7 @@ int wl12xx_boot_run_firmware(struct wl12xx *wl)
 		}
 	}
 
-	if (loop >= INIT_LOOP) {
+	if (loop > INIT_LOOP) {
 		wl12xx_error("timeout waiting for the hardware to "
 			     "complete initialization");
 		return -EIO;

Re: [PATCH] wl12xx: loop off by one.

[Kalle Valo]

Roel Kluin <roel.kluin@gmail.com> writes:

> with `while (loop++ < INIT_LOOP)' `loop' reaches -1 after the loop.

Sorry, my brains are on vacation mode right now and I can't understand
how loop can reach -1. Here's the code:

    loop = 0;
    while (loop++ < INIT_LOOP) {
          ...
    }

    if (loop >= INIT_LOOP) {
       wl1251_error("timeout waiting for the hardware to "
                    "complete initialization");
       return -EIO;
    }

What am I missing?

Also the patch won't apply to wireless-testing because main.c is renamed
to wl1251_main.c.

-- 
Kalle Valo

Re: [PATCH] wl12xx: loop off by one.

[Roel Kluin]

With `while (loop++ < INIT_LOOP)' `loop' becomes INIT_LOOP + 1 after
the loop.

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
>> with `while (loop++ < INIT_LOOP)' `loop' reaches -1 after the loop.
>
> Sorry, my brains are on vacation mode right now and I can't understand
> how loop can reach -1.

The confusion is my fault. The changelog should have as above. The patch
is correct though.

> Here's the code:

>    loop = 0;
>    while (loop++ < INIT_LOOP) {
>          ...
>    }
>
>    if (loop >= INIT_LOOP) {
>       wl1251_error("timeout waiting for the hardware to "
>                    "complete initialization");
>       return -EIO;
>    }

In the last iteration `loop' is `INIT_LOOP -1', the test occurs and then
the increment to `INIT_LOOP'. If just then the break occurs:
`if (interrupt & wl->chip.intr_init_complete)' evaluates to true, then
we error out, although there was no timeout. This could be very unlikely
to occur.

> Also the patch won't apply to wireless-testing because main.c is renamed
> to wl1251_main.c.

Ok, this is against your tree

diff --git a/drivers/net/wireless/wl12xx/wl1251_boot.c b/drivers/net/wireless/wl12xx/wl1251_boot.c
index d8a155d..b2ae71c 100644
--- a/drivers/net/wireless/wl12xx/wl1251_boot.c
+++ b/drivers/net/wireless/wl12xx/wl1251_boot.c
@@ -247,7 +247,7 @@ int wl1251_boot_run_firmware(struct wl1251 *wl)
 		}
 	}
 
-	if (loop >= INIT_LOOP) {
+	if (loop > INIT_LOOP) {
 		wl1251_error("timeout waiting for the hardware to "
 			     "complete initialization");
 		return -EIO;

Re: [PATCH] wl12xx: loop off by one.

[Kalle Valo]

Roel Kluin <roel.kluin@gmail.com> writes:

> With `while (loop++ < INIT_LOOP)' `loop' becomes INIT_LOOP + 1 after
> the loop.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
>>> with `while (loop++ < INIT_LOOP)' `loop' reaches -1 after the loop.
>>
>> Sorry, my brains are on vacation mode right now and I can't understand
>> how loop can reach -1.
>
> The confusion is my fault. The changelog should have as above. The patch
> is correct though.

Ah. Thanks for explaining this to me, I understand this now. Good catch,
thanks for fixing this.

Acked-by: Kalle Valo <kalle.valo@nokia.com>

John, please take this patch. Please holler if we need to resend it.

-- 
Kalle Valo