[PATCH] fixed pcnet32 multicast listen on big endian

[PATCH] fixed pcnet32 multicast listen on big endian

[Marcus Meissner]

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

--- linux-2.4.19/drivers/net/pcnet32.c.be	2003-02-05 07:59:27.000000000 +0100
+++ linux-2.4.19/drivers/net/pcnet32.c	2003-02-05 08:00:22.000000000 +0100
@@ -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;
 }

Re: [PATCH] fixed pcnet32 multicast listen on big endian

[Geert Uytterhoeven]

On Wed, 5 Feb 2003, Marcus Meissner wrote:
> 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
> 
> --- linux-2.4.19/drivers/net/pcnet32.c.be	2003-02-05 07:59:27.000000000 +0100
> +++ linux-2.4.19/drivers/net/pcnet32.c	2003-02-05 08:00:22.000000000 +0100
> @@ -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))
> +	);

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 -- geert@linux-m68k.org

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

Re: [PATCH] fixed pcnet32 multicast listen on big endian

[Geert Uytterhoeven]

On Sun, 23 Feb 2003, Geert Uytterhoeven wrote:
> On Wed, 5 Feb 2003, Marcus Meissner wrote:
> > 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
> > 
> > --- linux-2.4.19/drivers/net/pcnet32.c.be	2003-02-05 07:59:27.000000000 +0100
> > +++ linux-2.4.19/drivers/net/pcnet32.c	2003-02-05 08:00:22.000000000 +0100
> > @@ -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))
> > +	);
> 
> 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 -- geert@linux-m68k.org

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

Re: [PATCH] fixed pcnet32 multicast listen on big endian

[Jeff Garzik]

Geert Uytterhoeven wrote:
> On Sun, 23 Feb 2003, Geert Uytterhoeven wrote:
> 
>>On Wed, 5 Feb 2003, Marcus Meissner wrote:
>>
>>>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
>>>
>>>--- linux-2.4.19/drivers/net/pcnet32.c.be	2003-02-05 07:59:27.000000000 +0100
>>>+++ linux-2.4.19/drivers/net/pcnet32.c	2003-02-05 08:00:22.000000000 +0100
>>>@@ -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))
>>>+	);
>>
>>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

Re: [PATCH] fixed pcnet32 multicast listen on big endian

[Geert Uytterhoeven]

On Wed, 26 Feb 2003, Jeff Garzik wrote:
> Geert Uytterhoeven wrote:
> > On Sun, 23 Feb 2003, Geert Uytterhoeven wrote:
> >>On Wed, 5 Feb 2003, Marcus Meissner wrote:
> >>>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
> >>>
> >>>--- linux-2.4.19/drivers/net/pcnet32.c.be	2003-02-05 07:59:27.000000000 +0100
> >>>+++ linux-2.4.19/drivers/net/pcnet32.c	2003-02-05 08:00:22.000000000 +0100
> >>>@@ -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))
> >>>+	);
> >>
> >>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 :)

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 -- geert@linux-m68k.org

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