From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26576 invoked by alias); 31 May 2008 23:13:08 -0000 Received: (qmail 26568 invoked by uid 22791); 31 May 2008 23:13:07 -0000 X-Spam-Check-By: sourceware.org Received: from d5152C2DE.access.telenet.be (HELO lx-dmz.televic.com) (81.82.194.222) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 31 May 2008 23:12:50 +0000 Received: (qmail 32019 invoked from network); 31 May 2008 23:12:47 -0000 Received: from nt-email.televic.com (10.0.0.9) by lx-dmz.televic.com with SMTP; 31 May 2008 23:12:47 -0000 Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Date: Sat, 31 May 2008 23:13:00 -0000 Message-ID: <369C2E4EDB94C34881A8178BEA192A1205247E@nt-email.TELEVIC.COM> From: =?iso-8859-1?Q?Lambrecht_J=FCrgen?= To: "Andrew Lunn" Cc: X-IsSubscribed: yes Mailing-List: contact ecos-discuss-help@ecos.sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: ecos-discuss-owner@ecos.sourceware.org Subject: RE: [ECOS] bugs in AT91 Ethernet driver X-SW-Source: 2008-05/txt/msg00134.txt.bz2 > -----Original Message----- > From: Andrew Lunn [mailto:andrew@lunn.ch] > Sent: zaterdag 31 mei 2008 13:44 > To: Lambrecht J=FCrgen > Cc: ecos-discuss@sources.redhat.com > Subject: Re: [ECOS] bugs in AT91 Ethernet driver >=20 > Hi Joergen (;-), my name is written with an 'u' with an umlaut (2 dots) on the u - and= that umlaut-u gets screwed up often.. That's why I often type my name as J= uergen because it is a German rule to put an 'e' after a letter that should= have an umlaut but when you can/want not put one.) >=20 > To make your patch easier to discuss, here it is again, but slightly > modified. >=20 > --- if_at91.c 23 Mar 2007 19:02:09 -0000 1.2 > +++ if_at91.c 31 May 2008 11:31:39 -0000 > @@ -937,6 +937,7 @@ >=20 > for(i =3D 0;i { > + bytes_in_list =3D 0; > while(bytes_in_list < sg_list[i].len) > { > bytes_needed_list =3D sg_list[i].len - bytes_in_list; >=20 > You have this at the end. I put it at the beginning where it think it > more logically belongs. We have just started a new SG buffer, so we > currently have zero bytes in it. This i agree with. >=20 indeed, putting it at the beginning is more clear. > @@ -945,7 +946,7 @@ > { > bytes_in_buffer =3D > ((priv->rbd[priv->curr_rbd_idx].sr & > AT91_EMAC_RBD_SR_LEN_MASK) > - - total_bytes) - buffer_pos; > + - total_bytes); > } > else > { >=20 > Your comment is correct. total_bytes already includes buffer_pos, so > including it again results in the driver discarding some bytes from > the end of the packet. >=20 > @@ -957,7 +958,7 @@ > if(bytes_needed_list < bytes_in_buffer) > { > if(sg_buf !=3D NULL) > - memcpy(&sg_buf[bytes_in_list], > + memcpy(&sg_buf, > &priv->rb[priv->curr_rbd_idx].rb[buffer_pos], > bytes_needed_list); > bytes_in_list +=3D bytes_needed_list; > @@ -967,7 +968,7 @@ > else > { > if(sg_buf !=3D NULL) > - memcpy(&sg_buf[bytes_in_list], > + memcpy(&sg_buf, > &priv->rb[priv->curr_rbd_idx].rb[buffer_pos], > bytes_in_buffer); > bytes_in_list +=3D bytes_in_buffer; >=20 > These two changes i don't understand. I think they are wrong. It could > be you have not found out this is wrong because of other problems indeed Because 'bytes_in_list' was not reset, I thought this was an error. Only af= terwards I found the 'bytes_in_list' error. > means you have not got past the small ARP packets. The [bytes_in_list] indeed, not got past the small ARP packets. > is to handle when the bytes left in a receiver buffer are less than > the number of bytes left in the SG buffer. It needs to copy as many > bytes are available from one receive buffer and the move onto the next > receive buffer and copy as many bytes as needed into the sg buffer to > fill it up. Only then will it move onto the next sg buffer. >=20 indeed I knew this already, but I have made the error-i-thouht-it-was myself when = writing my own fpga-memory-mapped Ethernet driver a while ago. Then I thought sg_list was only 1 buffer and the 'i' was to iterate over th= e bytes.... Now I know better. > Andrew Thanks Andrew. This item is closed for the RX part. For the TX part, see my other reply. Kind regards, Juergen -- Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss