[U-Boot] [U-Boot, v3, 1/2] fs: fat: dynamically allocate memory for temporary buffer

[Marek Vasut]

On 2/21/19 9:40 AM, Chee, Tien Fong wrote:
> On Thu, 2019-02-21 at 09:29 +0100, Alexander Graf wrote:
>>
>> On 21.02.19 09:23, Chee, Tien Fong wrote:
>>>
>>> On Thu, 2019-02-21 at 08:45 +0100, Michal Simek wrote:
>>>>
>>>> Hi Tom,
>>>>
>>>> On 20. 02. 19 2:58, Tom Rini wrote:
>>>>>
>>>>>
>>>>> On Mon, Feb 11, 2019 at 02:56:19PM +0800, tien.fong.chee at intel.
>>>>> com
>>>>> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> From: Tien Fong Chee <tien.fong.chee at intel.com>
>>>>>>
>>>>>> Drop the statically allocated get_contents_vfatname_block and
>>>>>> dynamically allocate a buffer only if required. This saves
>>>>>> 64KiB of memory.
>>>>>>
>>>>>> Signed-off-by: Stefan Agner <stefan.ag... at toradex.com>
>>>>>> Signed-off-by: Tien Fong Chee <tien.fong.chee at intel.com>
>>>>> Applied to u-boot/master, thanks!
>>>> please remove this patch (better both of them because they were
>>>> in
>>>> series)
>>> I think patch 2/2 should be safe, because no memory size is
>>> changed.
>>> Basically, it just to release the allocated memory immediately when
>>> it's not required, so other can re-use it.
>>>
>>>>
>>>> because they are breaking at least ZynqMP SPL. It is also too
>>>> late in cycle to create random fix.
>>>>
>>>> You can't simply move 64KB from code to malloc without reflecting
>>>> this
>>>> by changing MALLOC space size.
>>>>
>>>> Other boards with SPL fat could be also affected by this if they
>>>> don't
>>>> allocate big malloc space.
>>> So, any suggestion to get the patch 1/2 accepted? inform all board
>>> maintainers to test it out?
>> You already received feedback that it does break ZynqMP, so the
>> current
>> approach won't work.
>>
>> How about you create a new kconfig option that allows you to say
>> whether
>> you want to use malloc or .bss for temporary data in the FAT driver.
>> You
>> can then have an _SPL_ version of that kconfig and check for it with
>> IS_ENABLED() which should automatically tell you the right answer
>> depending on whether you're in an SPL build or not.
>>
>> Then you can set the SPL version to default malloc and the non-SPL
>> version to default .bss.
> Marek and Tom rini,
> 
> Are you guys okay with Alex's suggestion?

I'm not a big fan of adding more and more ifdeffery.
Is there some other option ?

-- 
Best regards,
Marek Vasut