[U-Boot] [PATCH 10/12] net: emaclite: Use indirect access in emaclite_recv

Joe Hershberger joe.hershberger at gmail.com
Tue Dec 15 23:20:52 CET 2015


On Fri, Dec 11, 2015 at 6:03 AM, Michal Simek <michal.simek at xilinx.com> wrote:
> When IP is configured with pong buffers, IP is receiving packets to ping
> and then to pong buffer and than ping again.
> Origin logic in the driver remains there that when ping buffer is

Origin -> Original

> free, pong buffer is checked too and return if both are free.
>
> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> ---
>
> Do you know macros which I could use for addressing certain fields in
> ethernet and IP packet? The code is there because copying huge amount of
> data is causing performance penalty.

So you're saying the IP will not tell you the length of the frame
received? You have to pull it out of the packet data?

> ---
>  drivers/net/xilinx_emaclite.c | 90 ++++++++++++++++++++++++-------------------
>  1 file changed, 51 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/net/xilinx_emaclite.c b/drivers/net/xilinx_emaclite.c
> index e97ce2ce31f3..b5ff4f099251 100644
> --- a/drivers/net/xilinx_emaclite.c
> +++ b/drivers/net/xilinx_emaclite.c
> @@ -22,11 +22,6 @@
>
>  #define ENET_ADDR_LENGTH       6

Use ARP_HLEN from include/net.h

>
> -/* EmacLite constants */
> -#define XEL_BUFFER_OFFSET      0x0800  /* Next buffer's offset */
> -#define XEL_RSR_OFFSET         0x17FC  /* Rx status */
> -#define XEL_RXBUFF_OFFSET      0x1000  /* Receive Buffer */
> -
>  /* Xmit complete */
>  #define XEL_TSR_XMIT_BUSY_MASK         0x00000001UL
>  /* Xmit interrupt enable bit */
> @@ -86,7 +81,7 @@ struct emaclite_regs {
>  };
>
>  struct xemaclite {
> -       u32 nextrxbuffertouse;  /* Next RX buffer to read from */
> +       bool nextrxbuffertouse; /* Next RX buffer to read from */

When using a boolean for this sort of thing it is good to give it a
more clear name, such as "use_rx_pong_buffer_next".

>         u32 txpp;               /* TX ping pong buffer */
>         u32 rxpp;               /* RX ping pong buffer */
>         int phyaddr;
> @@ -455,45 +450,63 @@ static int emaclite_recv(struct eth_device *dev)
>  {
>         u32 length;
>         u32 reg;
> -       u32 baseaddress;
> +       u32 *addr, *ack;
>         struct xemaclite *emaclite = dev->priv;
> -
> -       baseaddress = dev->iobase + emaclite->nextrxbuffertouse;
> -       reg = in_be32 (baseaddress + XEL_RSR_OFFSET);
> -       debug("Testing data at address 0x%x\n", baseaddress);
> -       if ((reg & XEL_RSR_RECV_DONE_MASK) == XEL_RSR_RECV_DONE_MASK) {
> -               if (emaclite->rxpp)
> -                       emaclite->nextrxbuffertouse ^= XEL_BUFFER_OFFSET;
> +       struct emaclite_regs *regs = emaclite->regs;
> +       u32 attempt = 0;
> +
> +try_again:
> +       if (!emaclite->nextrxbuffertouse) {
> +               reg = in_be32(&regs->rx_ping_rsr);
> +               debug("Testing data at rx_ping\n");
> +               if ((reg & XEL_RSR_RECV_DONE_MASK) == XEL_RSR_RECV_DONE_MASK) {
> +                       debug("Data found in rx_ping buffer\n");
> +                       addr = &regs->rx_ping;
> +                       ack = &regs->rx_ping_rsr;
> +               } else {
> +                       debug("Data not found in rx_ping buffer\n");
> +                       /* Pong buffer is not available - return immediatelly */

immediatelly -> immediately

> +                       if (!emaclite->rxpp)
> +                               return -1;
> +
> +                       /* Try pong buffer if this is first attempt */
> +                       if (attempt++)
> +                               return -1;
> +                       emaclite->nextrxbuffertouse =
> +                                                 !emaclite->nextrxbuffertouse;
> +                       goto try_again;
> +               }
>         } else {
> -
> -               if (!emaclite->rxpp) {
> -                       debug("No data was available - address 0x%x\n",
> -                                                               baseaddress);
> -                       return 0;
> +               reg = in_be32(&regs->rx_pong_rsr);
> +               debug("Testing data at rx_pong\n");
> +               if ((reg & XEL_RSR_RECV_DONE_MASK) == XEL_RSR_RECV_DONE_MASK) {
> +                       debug("Data found in rx_pong buffer\n");
> +                       addr = &regs->rx_pong;
> +                       ack = &regs->rx_pong_rsr;
>                 } else {
> -                       baseaddress ^= XEL_BUFFER_OFFSET;
> -                       reg = in_be32 (baseaddress + XEL_RSR_OFFSET);
> -                       if ((reg & XEL_RSR_RECV_DONE_MASK) !=
> -                                               XEL_RSR_RECV_DONE_MASK) {
> -                               debug("No data was available - address 0x%x\n",
> -                                               baseaddress);
> -                               return 0;
> -                       }
> +                       debug("Data not found in rx_pong buffer\n");
> +                       /* Try ping buffer if this is first attempt */
> +                       if (attempt++)
> +                               return -1;
> +                       emaclite->nextrxbuffertouse =
> +                                                 !emaclite->nextrxbuffertouse;
> +                       goto try_again;
>                 }
>         }
> +
> +#define        ETH_TYPE_OFFSET         3

This is offset in count of 32-bit words.

> +#define        IP_LENGTH_OFFSET        4
>         /* Get the length of the frame that arrived */
> -       switch(((ntohl(in_be32 (baseaddress + XEL_RXBUFF_OFFSET + 0xC))) &
> -                       0xFFFF0000 ) >> 16) {
> +       switch (((ntohl(in_be32(addr + ETH_TYPE_OFFSET))) & 0xFFFF0000) >> 16) {

Why not read 16 bits? Not possible in hardware?

>                 case 0x806:

Use PROT_ARP from include/net.h

>                         length = 42 + 20; /* FIXME size of ARP */

Not sure what this is trying to include, but that seems like it is too big.

>From net.h:
#define ETHER_HDR_SIZE  (sizeof(struct ethernet_hdr))
which equals 14 bytes.
#define ARP_HDR_SIZE    (8+20)

So it should add up to 14 + 8 + 20 = 42;

I would use:

                         length = ETHER_HDR_SIZE + ARP_HDR_SIZE;

> -                       debug("ARP Packet\n");
> +                       debug("ARP Packet %x\n", length);
>                         break;
>                 case 0x800:

Use PROT_IP from include/net.h

>                         length = 14 + 14 +
> -                       (((ntohl(in_be32 (baseaddress + XEL_RXBUFF_OFFSET +
> -                                               0x10))) & 0xFFFF0000) >> 16);
> -                       /* FIXME size of IP packet */
> -                       debug ("IP Packet\n");
> +                             (((ntohl(in_be32(addr + IP_LENGTH_OFFSET))) &
> +                              0xFFFF0000) >> 16);

Why not read 16 bits?

If this is reading the total_length field, it seems that it will not
handle packet fragments well.

> +                       debug("IP Packet %x\n", length);
>                         break;

Can you afford to read the first to read the first 5 words of the
packet into a buffer and use the structs in net.h to overlay the
buffer to access the data? That would be a lot cleaner to look at.

You also need to somehow track fragmented packets. Not sure how to do
it universally. You sure there's no way to get the frame size from the
hardware? That seems very limiting.

>                 default:
>                         debug("Other Packet\n");
> @@ -501,15 +514,14 @@ static int emaclite_recv(struct eth_device *dev)
>                         break;
>         }
>
> -       xemaclite_alignedread((u32 *) (baseaddress + XEL_RXBUFF_OFFSET),
> -                       etherrxbuff, length);
> +       xemaclite_alignedread(addr, etherrxbuff, length);
>
>         /* Acknowledge the frame */
> -       reg = in_be32 (baseaddress + XEL_RSR_OFFSET);
> +       reg = in_be32(ack);
>         reg &= ~XEL_RSR_RECV_DONE_MASK;
> -       out_be32 (baseaddress + XEL_RSR_OFFSET, reg);
> +       out_be32(ack, reg);
>
> -       debug("Packet receive from 0x%x, length %dB\n", baseaddress, length);
> +       debug("Packet receive from 0x%p, length %dB\n", addr, length);
>         net_process_received_packet((uchar *)etherrxbuff, length);
>         return length;
>
> --
> 1.9.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot


More information about the U-Boot mailing list