On Fri, Aug 12, 2016 at 4:55 PM, Stephen Warren <swar...@wwwdotorg.org> wrote: > On 08/04/2016 03:48 PM, Joe Hershberger wrote: >> >> On Thu, Aug 4, 2016 at 4:34 PM, Joe Hershberger >> <joe.hershber...@gmail.com> wrote: >>> >>> On Thu, Aug 4, 2016 at 4:22 PM, Joe Hershberger >>> <joe.hershber...@gmail.com> wrote: >>>> >>>> Hi Stephen, >>>> >>>> On Tue, Aug 2, 2016 at 11:56 AM, Stephen Warren <swar...@wwwdotorg.org> >>>> wrote: >>>>> >>>>> On 08/01/2016 09:20 PM, Peter Chubb wrote: >>>>>> >>>>>> >>>>>> >>>>>> Hi Folks, >>>>>> Since patch 96350f729c42 "dm: tegra: net: Convert tegra boards to >>>>>> driver model for Ethernet" booting via dhcp has been broken on the >>>>>> Jetson TK1. > > ... > >>>>> I believe the correct approach is for the caller (network core code) to >>>>> provide cache-aligned buffers, rather than for each driver to align the >>>>> start/size when performing cache operations. Again, this is to ensure >>>>> that >>>>> cache operations don't affect any other data adjacent to the buffer. >>>>> Can you >>>>> see why the network core code isn't using cache-aligned buffers when >>>>> DM_ETH >>>>> is enabled? Perhaps DM_ETH isn't the trigger, but just changed >>>>> something >>>>> about the memory layout that exposed some other pre-existing issue. >>>> >>>> >>>> This should already be the case. The transmit buffer is setup like >>>> this: >>>> >>>> net_tx_packet = &net_pkt_buf[0] + (PKTALIGN - 1); >>>> net_tx_packet -= (ulong)net_tx_packet % PKTALIGN; >>>> >>>> in net/net.c:net_init(). >>>> >>>> The recv buffers are defined by the drivers and passed back to the >>>> core network code. In this case, the misalignment is caused by the >>>> rtl8169 driver... >>>> >>>> rtl8169_init_ring() does this: >>>> >>>> tpc->RxBufferRing[i] = &rxb[i * RX_BUF_SIZE]; >>>> >>>> ...but the size breaks alignment for all but the first entry: >>>> >>>> #define RX_BUF_SIZE 1536 /* Rx Buffer size */ >>>> >>>> This should be fixed by defining this instead: >>>> >>>> #define RX_BUF_SIZE ALIGN(1536, RTL8169_ALIGN) /* Rx >>>> Buffer size */ >>>> >>>> Also, there is an extra buffer that is memcpy'ed to, and then that is >>>> passed back to the core net code instead of the actual buffer that was >>>> recv'd into; I don't know why: >>>> >>>> static unsigned char rxdata[RX_BUF_LEN]; >>> >>> >>> I also noticed that the code to setup the tx ring buffers are >>> completely wrong. It's a good thing that NUM_TX_DESC is defined to be >>> 1. From rtl8169_init_ring(): >>> >>> for (i = 0; i < NUM_TX_DESC; i++) { >>> tpc->Tx_skbuff[i] = &txb[i]; >>> } >>> >>> That would set the buffer not only to be unaligned after the index 0, >>> but also to be one byte after the previous, thus they would all stomp >>> on each other. >>> >>> It should be: >>> >>> tpc->Tx_skbuff[i] = &txb[i * RX_BUF_SIZE]; >>> >>> It's probably also worth making another define for TX_BUF_SIZE even if >>> it is just defined to RX_BUF_SIZE to keep it from looking like a bug. >> >> >> Essentially this patch was incomplete; only ensuring that the first of >> the buffers in each ring were aligned: >> >> https://patchwork.ozlabs.org/patch/419406/ > > > Joe and Peter, do you expect to send a patch to fix these issues, or were > you hoping I would? If you're waiting on me, it won't be quick; too many > other cleanups and fixes stacked up right now...
I'm working on it. Cheers, -Joe _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot