[U-Boot] net: asix: Fix AX88772B when used with DriverModel

Marek Vasut
2016-09-10T18:48:33Z

On 09/10/2016 07:24 PM, Joe Hershberger wrote:
> Hi Marek,
>
> On Sat, Sep 10, 2016 at 11:51 AM, Marek Vasut <marex@denx.de> wrote:
>> On 09/10/2016 06:28 PM, Joe Hershberger wrote:
>>> Hi Marek,
>>>
>>> On Sat, Sep 10, 2016 at 5:01 AM, Marek Vasut <marex@denx.de> wrote:
>>>> On 09/10/2016 03:34 AM, Marcel Ziswiler wrote:
>>>>> On Sat, 2016-09-10 at 02:18 +0200, Marcel Ziswiler wrote:
>>>>>> On Sat, 2016-09-10 at 01:23 +0200, Marek Vasut wrote:
>>>>>>>
>>>>>>> On 09/10/2016 01:13 AM, Marcel Ziswiler wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On Sat, 2016-09-10 at 01:04 +0200, Marek Vasut wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 09/09/2016 11:06 PM, Marcel Ziswiler wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Fri, 2016-09-09 at 13:57 -0500, Joe Hershberger wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Hi Joshua,
>>>>>>>>>>>
>>>>>>>>>>> https://patchwork.ozlabs.org/patch/666191/ was applied to
>>>>>>>>>>> u-
>>>>>>>>>>> boot-
>>>>>>>>>>> net.git.
>>>>>>>>>>>
>>>>>>>>>>> Thanks!
>>>>>>>>>>> -Joe
>>>>>>>>>> No, sorry, but this is really the wrong approach! As
>>>>>>>>>> discussed
>>>>>>>>>> before
>>>>>>>>>> rather than Joshua's patch the one from Alban should long
>>>>>>>>>> since
>>>>>>>>>> have
>>>>>>>>>> been applied:
>>>>>>>>>>
>>>>>>>>>> https://www.mail-archive.com/u-boot at lists.denx.de/msg221455.h
>>>>>>>>>> tm
>>>>>>>>>> l
>>>>>>>>>>
>>>>>>>>>> I will send a revert ASAP and hope Alban's patch will finally
>>>>>>>>>> make
>>>>>>>>>> its
>>>>>>>>>> way into master to fix this once and for all!
>>>>>>>>>>
>>>>>>>>> Can you, instead of sending a revert, just send a subsequent
>>>>>>>>> patch to
>>>>>>>>> fix this once and for all ?
>>>>>>>> Sure, I will just squash my revert and Alban's fix together and
>>>>>>>> send
>>>>>>>> that one along ASAP.
>>>>>>> Thanks
>>>>>> Don't thank me too early yet. While it works on Colibri T20 it
>>>>>> currently fails on Colibri T30. More network and/or USB brokenness...
>>>>>> Currently bisecting...
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks for taking care of this mess :)
>>>>>>>> You are very welcome.
>>>>>> How I do love U-Boot.
>>>>>
>>>>> And the winner is:
>>>>>
>>>>> commit aa7a648747d8c704a9a81c9e493d386930724e9d
>>>>> Author: Joe Hershberger <joe.hershberger@ni.com>
>>>>> Date: Mon Aug 15 14:42:15 2016 -0500
>>>>>
>>>>> net: Stop including NFS overhead in defragment max
>>>>>
>>>>
>>>> Uh oh, why is this aforementioned patch even correct ? And why don't we
>>>> just revert it ? And why didn't anyone notice any problems with it ?
>>>
>>> Before that patch, on at least some platforms, lots of memory was
>>> being wasted just because of trying to single-source the size of NFS
>>> overhead. That patch removed that.
>>>
>>> The comment from that patch: "If a board needs a specific different
>>> defragment size, that board can override this setting."
>>>
>>> That is the case here.
>>
>> Can we be sure that this patch will not break other board(s) ?
>
> It will likely affect 2 other boards in the same way...
>
> include/configs/apalis_t30.h: 56 #define CONFIG_TFTP_BLOCKSIZE 16384
> include/configs/colibri_imx7.h: 49 #define CONFIG_TFTP_BLOCKSIZE 16384
> include/configs/colibri_t30.h: 52 #define CONFIG_TFTP_BLOCKSIZE 16384

I _think_ you're mixing IP_PKTSIZE and CONFIG_TFTP_BLOCKSIZE (I might be
wrong, I'm no network stack expert). My biggest concern about the
aa7a648747d8c704a9a81c9e493d386930724e9d patch is that it might cause
silent memory corruption on a lot of systems. Are you positive this
is not the case, ever ?

--
Best regards,
Marek Vasut