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...
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
