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

[Alexander Graf]

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.

That should give you the fix you want, without the problems it
introduces for SPL (where malloc space is really constrained, and
discouraged to use because you can't check whether it fits at compile time).


Alex