sundance DFE-580TX DL10050B patch

sundance DFE-580TX DL10050B patch

[Philippe De Muyter]

Hello,

trying to use the bonding functionality with a DFE-580TX quad port board,
I discovered I had to change the sundance.c driver to write the station
address with word access; that's actually what the sundance technology st201
and the ic+ IP100 data sheets say, so that should be ok in the general
case.

Philippe

Philippe De Muyter  phdm@macqel.be  Tel +32 27029044
Macq Electronique SA  rue de l'Aeronef 2  B-1140 Bruxelles  Fax +32 27029077

--- drivers/net/sundance.c	Mon Mar 17 13:15:48 2003
+++ drivers/net/sundance.c	Thu Feb 13 11:56:43 2003
@@ -853,8 +853,10 @@
 	writel(np->rx_ring_dma, ioaddr + RxListPtr);
 	/* The Tx list pointer is written as packets are queued. */
 
-	for (i = 0; i < 6; i++)
-		writeb(dev->dev_addr[i], ioaddr + StationAddr + i);
+	/* Station address must be written as 16 bit words with the DL10050B chip. */
+	for (i = 0; i < 6; i += 2)
+		writew((dev->dev_addr[i + 1] << 8) + dev->dev_addr[i],
+			   ioaddr + StationAddr + i);
 
 	/* Initialize other registers. */
 	writew(dev->mtu + 14, ioaddr + MaxFrameSize);

Re: sundance DFE-580TX DL10050B patch

[Dave Jones]

On Mon, Mar 17, 2003 at 02:56:09PM +0100, Philippe De Muyter wrote:

 > +		writew((dev->dev_addr[i + 1] << 8) + dev->dev_addr[i],

Don't you want to OR those together instead of add them ?

		Dave

Re: sundance DFE-580TX DL10050B patch

[Philippe De Muyter]

Dave Jones wrote :
> On Mon, Mar 17, 2003 at 02:56:09PM +0100, Philippe De Muyter wrote:
> 
>  > +		writew((dev->dev_addr[i + 1] << 8) + dev->dev_addr[i],
> 
> Don't you want to OR those together instead of add them ?
> 
> 		Dave
> 
You're right.

Here it is :

--- drivers/net/sundance.c	Mon Mar 17 13:15:48 2003
+++ drivers/net/sundance.c	Thu Feb 13 11:56:43 2003
@@ -853,8 +853,10 @@
 	writel(np->rx_ring_dma, ioaddr + RxListPtr);
 	/* The Tx list pointer is written as packets are queued. */
 
-	for (i = 0; i < 6; i++)
-		writeb(dev->dev_addr[i], ioaddr + StationAddr + i);
+	/* Station address must be written as 16 bit words with the DL10050B chip. */
+	for (i = 0; i < 6; i += 2)
+		writew((dev->dev_addr[i + 1] << 8) | dev->dev_addr[i],
+			   ioaddr + StationAddr + i);
 
 	/* Initialize other registers. */
 	writew(dev->mtu + 14, ioaddr + MaxFrameSize);

Re: sundance DFE-580TX DL10050B patch

[Jeff Garzik]

On Mon, Mar 17, 2003 at 06:40:36PM +0100, Philippe De Muyter wrote:
> Dave Jones wrote :
> > On Mon, Mar 17, 2003 at 02:56:09PM +0100, Philippe De Muyter wrote:
> > 
> >  > +		writew((dev->dev_addr[i + 1] << 8) + dev->dev_addr[i],
> > 
> > Don't you want to OR those together instead of add them ?
> > 
> > 		Dave
> > 
> You're right.

No.

Adding and or'ing are exactly equivalent for the above case, where you
shift an 8-bit value left 8 bits, then add it to another 8-bit value.

The final answer may be obtained from examining the compiler's assembly
output, and see which combination ('or' or 'add') produces the best
code.

	Jeff

Re: sundance DFE-580TX DL10050B patch

[Philippe De Muyter]

Jeff Garzik wrote :
> On Mon, Mar 17, 2003 at 06:40:36PM +0100, Philippe De Muyter wrote:
> > Dave Jones wrote :
> > > On Mon, Mar 17, 2003 at 02:56:09PM +0100, Philippe De Muyter wrote:
> > > 
> > >  > +		writew((dev->dev_addr[i + 1] << 8) + dev->dev_addr[i],
> > > 
> > > Don't you want to OR those together instead of add them ?
> > > 
> > > 		Dave
> > > 
> > You're right.
> 
> No.
> 
> Adding and or'ing are exactly equivalent for the above case, where you
> shift an 8-bit value left 8 bits, then add it to another 8-bit value.

That was implied in Dave's remark and in my answer.

> 
> The final answer may be obtained from examining the compiler's assembly
> output, and see which combination ('or' or 'add') produces the best
> code.

We can't do that as that should be done for all the supported processors.
Furthermore, it is the compiler's job to produce the best machine code.

The only thing we can do to help the compiler is write simple code.  From
a logical point of view, oring is what is needed here, and from a hardware
point of view oring is cheaper than adding.

> 	Jeff

Now, back to the real problem :) .  Will you apply my patch ?

Philippe

Philippe De Muyter  phdm@macqel.be  Tel +32 27029044
Macq Electronique SA  rue de l'Aeronef 2  B-1140 Bruxelles  Fax +32 27029077