[U-Boot] Porting UBI fixes (specially fastmap's) to U-Boot

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 10/08/2015 03:54 PM, Heiko Schocher wrote:
> Hello Richard,
>
> Am 08.10.2015 um 15:03 schrieb Richard Weinberger:
>> Hi!
>>
>> Am 08.10.2015 um 14:51 schrieb Heiko Schocher:
>>> Hello Ezequiel, Richard,
>>>
>>> Am 02.10.2015 um 18:27 schrieb Ezequiel Garcia:
>>>> Hello Heiko,
>>>>
>>>> According to Richard Weinberger, UBI fastmap is broken in U-Boot.
>>>> There are plenty
>>>> of fixes in Linux that we should pull in U-Boot to fix it.
>>>>
>>>> Maybe you are already aware of this and you have some work-in-progress
>>>> patches to share?
>>>> Otherwise, I might be able to spend some time working on this in the next weeks.
>>>
>>> I just tried to do this port ... but stumbled over some issues, I just try to
>>> looking at ... but running out of time for doing more ...
>>>
>>> - ubifastmap needs now scatterlist ... we have no support for this
>>>    currently in U-Boot.
>>
>> Huh?
>>
>> We have support for that in UBI. But the only user is UBI block.
>
> Ah... maybe I can drop it, great!
> Fixed, dropped.
>
>>> - I get some bogus compilerwarnings:
>>>
>>>    CC      drivers/mtd/ubi/fastmap-wl.o
>>> drivers/mtd/ubi/fastmap-wl.c:69:3: warning: implicit declaration of function 'wl_tree_add' [-Wimplicit-function-declaration]
>>>
>>> wl_tree_add is declared static here:
>>> http://lxr.free-electrons.com/source/drivers/mtd/ubi/wl.c#L152
>>>
>>> but called from fastmap-wl.c
>>> http://lxr.free-electrons.com/source/drivers/mtd/ubi/fastmap-wl.c#L64
>>
>> wl.c has a "#include "fastmap-wl.c".
>
> Ah, right ... got it, fixed.
>
>>> same for:
>>> find_mean_wl_entry
>>> self_check_in_wl_tree
>>> wl_get_wle
>>> find_wl_entry
>>> prot_queue_add
>>> schedule_ubi_work
>>> schedule_erase
>>>
>>> drivers/mtd/ubi/fastmap-wl.c: In function 'ubi_is_erase_work':
>>> drivers/mtd/ubi/fastmap-wl.c:340:1: warning: control reaches end of non-void function [-Wreturn-type]
>>>
>>> :-(
>>
>> ???
>>
>> /**
>>   * ubi_is_erase_work - checks whether a work is erase work.
>>   * @wrk: The work object to be checked
>>   */
>> int ubi_is_erase_work(struct ubi_work *wrk)
>> {
>>          return wrk->func == erase_worker;
>> }
>>
>>
>>> Does this warnings not occur when compiling linux with fastmap support?
>>
>> Nope. But I'm not using fancy new compilers/flags.
>
> Hmm.. have to look into it...
>
>>
>>> I used as base for porting ubi/ubifs from linux:
>>>
>>>     commit 64291f7db5bd8150a74ad2036f1037e6a0428df2
>>>      Author: Linus Torvalds <torvalds@linux-foundation.org>
>>>      Date:   Sun Aug 30 11:34:09 2015 -0700
>>>
>>>          Linux 4.2
>>
>> Just checked v4.2
>>
>>>
>>> @Richard: a first change for U-Boot wish:
>>>
>>> You used here and there sometimes "free" as an variable name ... for example:
>>> in "drivers/mtd/ubi/fastmap.c"
>>>
>>> static int scan_pool(struct ubi_device *ubi, struct ubi_attach_info *ai,
>>>                       int *pebs, int pool_size, unsigned long long *max_sqnum,
>>>                       struct list_head *free)
>>>
>>> compiling this under U-Boot drops:
>>>
>>>    CC      drivers/mtd/ubi/fastmap.o
>>> drivers/mtd/ubi/fastmap.c: In function 'scan_pool':
>>> drivers/mtd/ubi/fastmap.c:475:3: error: called object 'free' is not a function
>>
>> Which gcc warning-flag triggers that?
>
> Good question! I am not sure, if it is a flag ... why "called object" ...
>
>>> (line number differs from mainline, because I added some U-Boot specific
>>>   line above)
>>>
>>> Could we rename this var to something else ... like for example "pfree" ?
>>> I can prepare a patch for linux, if this would be Ok for you.
>>
>> Unless I'm mistaken this is fine in C.
>
> Yes, it looks fine, do not understand this ...
>
>> So, what exactly is the problem here?
>
> Compiling stops, as it is an "error" ... renaming this var, and error
> go away ...

I believe that in certain configs free is a #define on u-boot, which is
likely causing this problem.

Regards,

Hans