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

[Joe Hershberger]

On Fri, Aug 12, 2016 at 4:55 PM, Stephen Warren <swarren at 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.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...

I'm working on it.

Cheers,
-Joe