Skip to contentSkip to conversation

fixed pcnet32 multicast listen on big endian

PATCH In review Marcus Meissner 3 participants 5 messages Feb 5, 2003 – Feb 26, 2003 Network drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fixed pcnet32 multicast listen on big endian

Hi folks,

This fixes multicast listen for pcnet32 on at least powerpc and powerpc64 kernels.

The mcast_table is in memory referenced by the card and so it needs to be accessed in little endian mode.

Ciao, Marcus

drivers/net/pcnet32.c renamed from drivers/net/pcnet32.c.be +3 -1
@@ -1534,7 +1534,9 @@
 	
 	crc = ether_crc_le(6, addrs);
 	crc = crc >> 26;
-	mcast_table [crc >> 4] |= 1 << (crc & 0xf);
+	mcast_table [crc >> 4] = le16_to_cpu(
+		le16_to_cpu(mcast_table [crc >> 4]) | (1 << (crc & 0xf))
+	);
     }
     return;
 }
#1 lore raw
Quoted 11 lines · Marcus Meissner

This fixes multicast listen for pcnet32 on at least powerpc and powerpc64 kernels.

The mcast_table is in memory referenced by the card and so it needs to be accessed in little endian mode.

Ciao, Marcus

drivers/net/pcnet32.c line 1537

Quoted 5 lines
drivers/net/pcnet32.c renamed from drivers/net/pcnet32.c.be +1 -1
@@ -1534,7 +1534,9 @@
 
 	crc = ether_crc_le(6, addrs);
 	crc = crc >> 26;
-	mcast_table [crc >> 4] |= 1 << (crc & 0xf);
+	mcast_table [crc >> 4] = le16_to_cpu(
                                 ^^^^^^^^^^^

drivers/net/pcnet32.c line 1539

Quoted 2 lines
drivers/net/pcnet32.c renamed from drivers/net/pcnet32.c.be +2 -0
+		le16_to_cpu(mcast_table [crc >> 4]) | (1 << (crc & 0xf))
+	);

Shouldn't the first conversion be `cpu_to_le16'?

Gr{oetje,eeting}s,

Geert (reading bk-commits)

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

#2 lore raw
Quoted 18 lines · Geert Uytterhoeven
Quoted 11 lines · Marcus Meissner

This fixes multicast listen for pcnet32 on at least powerpc and powerpc64 kernels.

The mcast_table is in memory referenced by the card and so it needs to be accessed in little endian mode.

Ciao, Marcus

drivers/net/pcnet32.c line 1537

Quoted 5 lines
drivers/net/pcnet32.c renamed from drivers/net/pcnet32.c.be +1 -1
@@ -1534,7 +1534,9 @@
 
 	crc = ether_crc_le(6, addrs);
 	crc = crc >> 26;
-	mcast_table [crc >> 4] |= 1 << (crc & 0xf);
+	mcast_table [crc >> 4] = le16_to_cpu(
                                 ^^^^^^^^^^^
Quoted 4 lines

drivers/net/pcnet32.c line 1539

Quoted 2 lines
drivers/net/pcnet32.c renamed from drivers/net/pcnet32.c.be +2 -0
+		le16_to_cpu(mcast_table [crc >> 4]) | (1 << (crc & 0xf))
+	);

Shouldn't the first conversion be `cpu_to_le16'?

Ugh, a quick grep shows that this driver _always_ uses `le*_to_cpu()' to convert from CPU to little endian.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

#3 lore raw
Quoted 23 lines · Geert Uytterhoeven
Quoted 21 lines · Geert Uytterhoeven
Quoted 11 lines · Marcus Meissner

This fixes multicast listen for pcnet32 on at least powerpc and powerpc64 kernels.

The mcast_table is in memory referenced by the card and so it needs to be accessed in little endian mode.

Ciao, Marcus

drivers/net/pcnet32.c line 1537

Quoted 5 lines
drivers/net/pcnet32.c renamed from drivers/net/pcnet32.c.be +1 -1
@@ -1534,7 +1534,9 @@
 
 	crc = ether_crc_le(6, addrs);
 	crc = crc >> 26;
-	mcast_table [crc >> 4] |= 1 << (crc & 0xf);
+	mcast_table [crc >> 4] = le16_to_cpu(
                                ^^^^^^^^^^^
Quoted 8 lines
Quoted 4 lines

drivers/net/pcnet32.c line 1539

Quoted 2 lines
drivers/net/pcnet32.c renamed from drivers/net/pcnet32.c.be +2 -0
+		le16_to_cpu(mcast_table [crc >> 4]) | (1 << (crc & 0xf))
+	);

Shouldn't the first conversion be `cpu_to_le16'?

Ugh, a quick grep shows that this driver _always_ uses `le*_to_cpu()' to convert from CPU to little endian.

Cosmetically you are correct, and I prefer it to be changed eventually.

However programatically, it has no effect, because those cpu_to_foo and foo_to_cpu functions either swap, or they don't. Direction doesn't matter terribly much :)

Jeff

#4 lore raw
Quoted 28 lines · Jeff Garzik
Quoted 27 lines · Geert Uytterhoeven
Quoted 24 lines · Geert Uytterhoeven
Quoted 11 lines · Marcus Meissner

This fixes multicast listen for pcnet32 on at least powerpc and powerpc64 kernels.

The mcast_table is in memory referenced by the card and so it needs to be accessed in little endian mode.

Ciao, Marcus

drivers/net/pcnet32.c line 1537

Quoted 5 lines
drivers/net/pcnet32.c renamed from drivers/net/pcnet32.c.be +1 -1
@@ -1534,7 +1534,9 @@
 
 	crc = ether_crc_le(6, addrs);
 	crc = crc >> 26;
-	mcast_table [crc >> 4] |= 1 << (crc & 0xf);
+	mcast_table [crc >> 4] = le16_to_cpu(
                                ^^^^^^^^^^^

drivers/net/pcnet32.c line 1539

Quoted 2 lines
drivers/net/pcnet32.c renamed from drivers/net/pcnet32.c.be +2 -0
+		le16_to_cpu(mcast_table [crc >> 4]) | (1 << (crc & 0xf))
+	);

Shouldn't the first conversion be `cpu_to_le16'?

Quoted 8 lines
Quoted 2 lines

Ugh, a quick grep shows that this driver _always_ uses `le*_to_cpu()' to convert from CPU to little endian.

Cosmetically you are correct, and I prefer it to be changed eventually.

However programatically, it has no effect, because those cpu_to_foo and foo_to_cpu functions either swap, or they don't. Direction doesn't matter terribly much :)

That's true.

BTW, you save one swap by changing the code to

    mcast_table [crc >> 4] |= cpu_to_le16(1 << (crc & 0xf));

Because the order of bitwise or and swap doesn't matter.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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