[ECOS] bugs in AT91 Ethernet driver

["Lambrecht Jürgen" <J.Lambrecht@TELEVIC.com>]

[-- Attachment #1: Type: text/plain, Size: 3739 bytes --]

Hello,

I found some bugs in the AT91 EMAC Ethernet driver - /packages/devs/eth/arm/at91/current/src/if_at91.c.
The bugs are present in at91_eth_recv(..) because the author did not understand how scatter-gather lists work I think. In Redboot, the driver works, and maybe also with LWIP, but not with freebsd.

With my fix, RX seems to work on my AT91SAM9260-EK based board.

TX only works for the first packet the freebsd stack sends out (a gratuitous ARP). The next packet it sends as response to an ARP request does not get out. I Monitor with Wireshark. Debugging in ecos is on to print the packet contents. 
The TX ARP packet is a bit strange: it is a broadcast instead of a unicast as it should be to the originator of the ARP request. And also SG[1] is printed - I thought the sg-list should always start with SG[0]?
I don't find a bug in the TX driver..
Maybe somebody else has an idea what could be wrong?

Kind regards,
Juergen

P.S.: in attachment a unix dif; that's not in the correct format for the ecos list I think - I need to take the time to look up how to....


This is my version of at91_eth_recv(..):

static void
at91_eth_recv(struct eth_drv_sc *sc,
              struct eth_drv_sg *sg_list,
              int sg_len)
{
   at91_eth_priv_t *priv = (at91_eth_priv_t *)sc->driver_private;
   int i;
   cyg_uint32 bytes_in_buffer;
   cyg_uint32 bytes_in_list = 0;
   cyg_uint32 bytes_needed_list = 0;
   cyg_uint32 buffer_pos = 0;
   cyg_uint8 * sg_buf;
   cyg_uint32 total_bytes = 0;
   /* buffer_pos is position in current 128B buffer */
   /* bytes_in_list is position in current sg_list */
   /* total bytes is total no. of packet bytes already copied */
   /* todo: check all cases */
   /*  1. packet in 1 rd-buf OK */
   /*  2. big packet in several rd-buf OK */
   for(i = 0;i<sg_len;i++)
   {
      while(bytes_in_list < sg_list[i].len) //freebsd - i=0: 14B
      {                                     //freebsd - i=1: 128B or remainder of packet
         bytes_needed_list = sg_list[i].len - bytes_in_list;

         if(priv->rbd[priv->curr_rbd_idx].sr & AT91_EMAC_RBD_SR_EOF)
         { /* This 128B buffer contains the End Of the Frame. */
            bytes_in_buffer =
		((priv->rbd[priv->curr_rbd_idx].sr & AT91_EMAC_RBD_SR_LEN_MASK)
		 - total_bytes); /* - buffer_pos; */
             //??? total_bytes already contains buffer_pos; ... 
         }
         else
         { /* AT91_EMAC_RX_BUFF_SIZE = 128B */
            bytes_in_buffer = AT91_EMAC_RX_BUFF_SIZE - buffer_pos;
         }

         sg_buf = (cyg_uint8 *)(sg_list[i].buf);

         if(bytes_needed_list < bytes_in_buffer)
         {
            if(sg_buf != NULL)
               memcpy(sg_buf, /* &sg_buf[bytes_in_list], */
		      &priv->rb[priv->curr_rbd_idx].rb[buffer_pos],
		      bytes_needed_list);
            bytes_in_list += bytes_needed_list;
            buffer_pos += bytes_needed_list;
            total_bytes += bytes_needed_list;
         }
         else
         {
            if(sg_buf != NULL)
              memcpy(sg_buf, /* wrong: &sg_buf[bytes_in_list], */
		     &priv->rb[priv->curr_rbd_idx].rb[buffer_pos],
		     bytes_in_buffer);
            bytes_in_list += bytes_in_buffer;
            total_bytes += bytes_in_buffer;

            /* Step our buffer on one */
            priv->rbd[priv->curr_rbd_idx].addr &= 
	      ~(AT91_EMAC_RBD_ADDR_OWNER_SW);
            priv->curr_rbd_idx++;
            if(priv->curr_rbd_idx >= CYGNUM_DEVS_ETH_ARM_AT91_RX_BUFS)
            {
               priv->curr_rbd_idx = 0;
            }
            buffer_pos = 0;
         }
      }
      bytes_in_list = 0; /* go to next list */
   }
}

[-- Attachment #2: diff.txt --]
[-- Type: text/plain, Size: 1628 bytes --]

42c42
< // Contributors: Juergen Lambrecht 
---
> // Contributors:  
44,45c44,45
< // Purpose:      BSD compatible network driver
< // Description:  HW network driver for AT91 EMAC block of AT91SAM uC's.
---
> // Purpose:
> // Description:
46,47d45
< // Limitations:  Jumbo frames are not supported because of
< //               AT91_EMAC_RBD_SR_LEN_MASK = 0xFFF
953c951
<    /* buffer_pos is position in current 128B buffer */
---
> 
954,957d951
<    /* bytes_in_list is position in current sg_list */
<    /* total bytes is total no. of packet bytes already copied */
<    /* todo: check all cases */
<    /* 1. packet in 1 rd-buf */
960,961c954,955
<       while(bytes_in_list < sg_list[i].len) //freebsd - i=0: 14B
<       {                                     //freebsd - i=1: 128B or remainder of packet
---
>       while(bytes_in_list < sg_list[i].len)
>       {
965,966c959,960
<          { /* This 128B buffer contains the End Of the Frame. */
<             bytes_in_buffer =
---
>          {
> 	      bytes_in_buffer = 
968c962
< 		 - total_bytes); /* - buffer_pos; */
---
> 		 - total_bytes) - buffer_pos;
969d962
<              //??? total_bytes already contains buffer_pos; ... 
972c965
<          { /* AT91_EMAC_RX_BUFF_SIZE = 128B */
---
>          {
981c974
<                memcpy(sg_buf, /* &sg_buf[bytes_in_list], */
---
>                memcpy(&sg_buf[bytes_in_list],
991c984
<               memcpy(sg_buf, /* wrong: &sg_buf[bytes_in_list], */
---
>               memcpy(&sg_buf[bytes_in_list],
1008d1000
<       bytes_in_list = 0; /* go to next list */

[-- Attachment #3: Type: text/plain, Size: 148 bytes --]

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss