On 15.12.2015 23:20, Joe Hershberger wrote: > On Fri, Dec 11, 2015 at 6:03 AM, Michal Simek <michal.si...@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
fixed > >> free, pong buffer is checked too and return if both are free. >> >> Signed-off-by: Michal Simek <michal.si...@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? Unfortunately yes. Linux driver uses the same logic. https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/xilinx/xilinx_emaclite.c#n414 > >> --- >> 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". done. > >> 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(®s->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 = ®s->rx_ping; >> + ack = ®s->rx_ping_rsr; >> + } else { >> + debug("Data not found in rx_ping buffer\n"); >> + /* Pong buffer is not available - return >> immediatelly */ > > immediatelly -> immediately fixed. > >> + 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(®s->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 = ®s->rx_pong; >> + ack = ®s->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. yep > >> +#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? There are a lot of versions of this IP. It is BE/LE driver and there could be HW issues too. But look below. > >> case 0x806: > > Use PROT_ARP from include/net.h done. > >> 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; Based on http://image.slidesharecdn.com/arp-140623072259-phpapp01/95/arp-6-638.jpg?cb=1403508219 + 10 padding and +4 crc. I see that tsec driver does something with crc but maybe crc are completely ignored by U-Boot On the other hand linux is using just ETH_FCS_LEN (4). I have added +4 for now. > >> - debug("ARP Packet\n"); >> + debug("ARP Packet %x\n", length); >> break; >> case 0x800: > > Use PROT_IP from include/net.h fixed. > >> 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? look below. > > If this is reading the total_length field, it seems that it will not > handle packet fragments well. Isn't it fragmentation done on one layer up? IP packet header is there all the time. >> + 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. I have tried it and it is working fine. I am not reading just 5 words but I think it is better to read the full align ARP packet size and then do another read for the rest. Please look at v2 with this change I think you will like it more than this one. > 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. I haven't seen any problem like this and I expect none has played with it too. Thanks, Michal _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
