[PATCH v2] drivers: net: xgene: Rewrite loop in xgene_enet_ecc_init()

[Geert Uytterhoeven]

drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c: In function =E2=80=98=
xgene_enet_ecc_init=E2=80=99:
drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c:126: warning: =E2=80=98=
data=E2=80=99 may be used uninitialized in this function

Depending on the arbitrary value on the stack, the loop may terminate
too early, and cause a bogus -ENODEV failure.

Signed-off-by: Geert Uytterhoeven <***@linux-m68k.org>
---
v2: Rewrite the loop instead of pre-initializing data.

drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c b/driver=
s/net/ethernet/apm/xgene/xgene_enet_sgmac.c
index e6d24c2101982444..2a497b38ed420495 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c
@@ -127,17 +127,15 @@ static int xgene_enet_ecc_init(struct xgene_enet_=
pdata *p)
int i;
=20
xgene_enet_wr_diag_csr(p, ENET_CFG_MEM_RAM_SHUTDOWN_ADDR, 0);
- for (i =3D 0; i < 10 && data !=3D ~0U ; i++) {
+ for (i =3D 0; i < 10; i++) {
usleep_range(100, 110);
data =3D xgene_enet_rd_diag_csr(p, ENET_BLOCK_MEM_RDY_ADDR);
+ if (data =3D=3D ~0U)
+ return 0;
}
=20
- if (data !=3D ~0U) {
- netdev_err(ndev, "Failed to release memory from shutdown\n");
- return -ENODEV;
- }
-
- return 0;
+ netdev_err(ndev, "Failed to release memory from shutdown\n");
+ return -ENODEV;
}
=20
static void xgene_enet_config_ring_if_assoc(struct xgene_enet_pdata *p=
)
--=20
1.9.1

[David Miller]

From: Geert Uytterhoeven <***@linux-m68k.org>
Date: Wed, 22 Oct 2014 09:39:41 +0200

drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c:126: warning: ‘data’ may be used uninitialized in this function
Depending on the arbitrary value on the stack, the loop may terminate
too early, and cause a bogus -ENODEV failure.
---
v2: Rewrite the loop instead of pre-initializing data.

I hate to be a pest, but like the other patch of your's I think
a do { } while() works best here because the intent is clearly
to

[Geert Uytterhoeven]

Post by David Miller
Date: Wed, 22 Oct 2014 09:39:41 +0200

Post by Geert Uytterhoeven
drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c: In function =E2=80=
drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c:126: warning: =E2=80=

=98data=E2=80=99 may be used uninitialized in this function

Post by David Miller

Post by Geert Uytterhoeven
Depending on the arbitrary value on the stack, the loop may terminat=

e

Post by David Miller

Post by Geert Uytterhoeven
too early, and cause a bogus -ENODEV failure.
---
v2: Rewrite the loop instead of pre-initializing data.

I hate to be a pest, but like the other patch of your's I think
a do { } while() works best here because the intent is clearly
to run the loop at least once, right?

I wanted to avoid checking for "data !=3D ~0U" twice: once to abort the=
loop,
and once to check if a timeout happened.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ***@linux-=
m68k.org

In personal conversations with technical people, I call myself a hacker=
=2E But
when I'm talking to journalists I just say "programmer" or something li=
ke that.
-- Linus Torvalds

[David Miller]

From: Geert Uytterhoeven <***@linux-m68k.org>
Date: Wed, 22 Oct 2014 21:50:06 +0200

Post by David Miller
Date: Wed, 22 Oct 2014 09:39:41 +0200

drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c:126: warning: ‘data’ may be used uninitialized in this function
Depending on the arbitrary value on the stack, the loop may terminate
too early, and cause a bogus -ENODEV failure.
---
v2: Rewrite the loop instead of pre-initializing data.

I hate to be a pest, but like the other patch of your's I think
a do { } while() works best here because the intent is clearly
to run the loop at least once, right?

I wanted to avoid checking for "data != ~0U" twice: once to abort the loop,
and once to check if a timeout happened.

Hmmm:

do {
usleep_range(...);
data = ...();
if (data == ~0)
return 0;
} while (++i < 10);

netdev_err(...);
return -ENOD

[Geert Uytterhoeven]

Post by David Miller
Date: Wed, 22 Oct 2014 21:50:06 +0200

Post by David Miller
Date: Wed, 22 Oct 2014 09:39:41 +0200

Post by Geert Uytterhoeven
drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c: In function =E2=
drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c:126: warning: =E2=

=80=98data=E2=80=99 may be used uninitialized in this function

Post by David Miller

Post by David Miller

Post by Geert Uytterhoeven
Depending on the arbitrary value on the stack, the loop may termin=

ate

Post by David Miller

Post by David Miller

Post by Geert Uytterhoeven
too early, and cause a bogus -ENODEV failure.
---
v2: Rewrite the loop instead of pre-initializing data.

I hate to be a pest, but like the other patch of your's I think
a do { } while() works best here because the intent is clearly
to run the loop at least once, right?

I wanted to avoid checking for "data !=3D ~0U" twice: once to abort =

the loop,

Post by David Miller

and once to check if a timeout happened.

do {
usleep_range(...);
data =3D ...();
if (data =3D=3D ~0)
return 0;
} while (++i < 10);
netdev_err(...);
return -ENODEV;
Why would you have to check data twice?

--=20
Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ***@linux-=
m68k.org

In personal conversations with technical people, I call myself a hacker=
=2E But
when I'm talking to journalists I just say "programmer" or something li=
ke that.
-- Linus Torvalds

[Geert Uytterhoeven]

Post by David Miller
Date: Wed, 22 Oct 2014 21:50:06 +0200

Post by David Miller
Date: Wed, 22 Oct 2014 09:39:41 +0200

Post by Geert Uytterhoeven
drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c: In function =E2=
drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c:126: warning: =E2=

=80=98data=E2=80=99 may be used uninitialized in this function

Post by David Miller

Post by David Miller

Post by Geert Uytterhoeven
Depending on the arbitrary value on the stack, the loop may termin=

ate

Post by David Miller

Post by David Miller

Post by Geert Uytterhoeven
too early, and cause a bogus -ENODEV failure.
---
v2: Rewrite the loop instead of pre-initializing data.

I hate to be a pest, but like the other patch of your's I think
a do { } while() works best here because the intent is clearly
to run the loop at least once, right?

I wanted to avoid checking for "data !=3D ~0U" twice: once to abort =

the loop,

Post by David Miller

and once to check if a timeout happened.

do {
usleep_range(...);
data =3D ...();
if (data =3D=3D ~0)
return 0;
} while (++i < 10);
netdev_err(...);
return -ENODEV;
Why would you have to check data twice?

Yes, that would work to.

=46eel free to do s/for (i =3D 0; i < 10; i++)/do/ and s/}/} while (++i=
< 10);/

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ***@linux-=
m68k.org

In personal conversations with technical people, I call myself a hacker=
=2E But
when I'm talking to journalists I just say "programmer" or something li=
ke that.
-- Linus Torvalds