[U-Boot] tegra124 jetson-tk1 ethernet problems.

[Stephen Warren]

On 08/04/2016 03:48 PM, Joe Hershberger wrote:
> On Thu, Aug 4, 2016 at 4:34 PM, Joe Hershberger
> <joe.hershberger at gmail.com> wrote:
>> On Thu, Aug 4, 2016 at 4:22 PM, Joe Hershberger
>> <joe.hershberger at gmail.com> wrote:
>>> Hi Stephen,
>>>
>>> On Tue, Aug 2, 2016 at 11:56 AM, Stephen Warren <swarren at 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...