[U-Boot] Porting UBI fixes (specially fastmap's) to U-Boot
Ezequiel Garcia
ezequiel at vanguardiasur.com.ar
Mon Oct 19 15:47:39 CEST 2015
On 19 October 2015 at 02:17, Heiko Schocher <hs at denx.de> wrote:
> Hello Ezequiel,
>
> Am 17.10.2015 um 20:07 schrieb Ezequiel Garcia:
>>
>> Hi Heiko,
>>
>> On 9 October 2015 at 09:30, Heiko Schocher <hs at denx.de> wrote:
>> [..]
>>>
>>>
>>>
>>> I just updated the "ubi_sync_with_linux" branch on u-boot-ubi.
>>>
>>> It seems UBI/UBIFS now work with the NAND on the aristainetos2
>>> board, but my stomach says, there are some subtile issues ...
>>>
>>> Still needs more testing, also not tested yet UBI on the SPI NOR on
>>> this board.
>>>
>>> If I "nand erase" the mtd device, and try "ubi part ..." it fails
>>>
>>> :-(
>>>
>>> But If I flash_eraseall under linux the device, and then make
>>> "ubi part..." in U-Boot, commmand succeeds ...
>>>
>>
>> Tested it on my custom AM335x board. Haven't used mainline U-Boot
>> but cherry-picked the following commits:
>>
>> linux, compat: add missing definitions for ubi
>> ubi/ubifs: some bugfixes
>> ubi,ubifs: sync with linux v4.2
>> ubi: reset mtd_devs when ubi part fail
>>
>> Commits apply cleanly except for "linux, compat: add missing
>> definitions for ubi"
>> which was trivial to backport anyway.
>>
>> U-Boot built fine, but there was a warning:
>>
>> drivers/mtd/ubi/fastmap.c: In function 'ubi_attach_fastmap':
>> drivers/mtd/ubi/fastmap.c:816:2: warning: pointer targets in passing
>> argument 3 of 'scan_pool' differ in signedness [-Wpointer-sign]
>> ret = scan_pool(ubi, ai, fmpl->pebs, pool_size, &max_sqnum, &free);
>>
>> Just sent this patch which should fix it:
>> http://patchwork.ozlabs.org/patch/531842/
>
>
> Thanks for fixing. With it, I see no compiler warning anymore.
> Applied to u-boot-ubi.git ubi_sync_with_linux
>
> Tried this branch on the aristainetos2 board with ubi/ubifs on nand and
> spi nor flash, worked fine, see log:
>
> http://xeidos.ddns.net/buildbot/builders/ari_ubi/builds/7
> http://xeidos.ddns.net/buildbot/builders/ari_ubi/builds/7/steps/shell/logs/tbotlog
>
>> The board seems to work fine, and I can even to "nand erase" and "ubi
>> part" without
>> any problem.
>
>
> Did you tried "ubi part" more than once in a row?
>
Yup, I can do "ubi part $mypart" several times. And can also do "nand
erase.part foo"
and then "ubi part foo" several times.
The patches seem to work fine here, so:
Tested-by: Ezequiel Garcia <ezequiel at vanguardiasur.com.ar>
>> However, I'm still seeing the same warning I had before, when my partition
>> is attached:
>>
>> ubi0: default fastmap pool size: 200
>> ubi0: default fastmap WL pool size: 100
>> ubi0: attaching mtd1
>> WARNING in drivers/mtd/ubi/fastmap.c line 846
>> ubi0: scanning is finished
>> ubi0: attached mtd1 (name "mtd=7", size 509 MiB)
>> ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 129024 bytes
>> ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 512
>> ubi0: VID header offset: 512 (aligned 512), data offset: 2048
>> ubi0: good PEBs: 4072, bad PEBs: 4, corrupted PEBs: 0
>> ubi0: user volume: 4, internal volumes: 1, max. volumes count: 128
>> ubi0: max/mean erase counter: 5/3, WL threshold: 4096, image sequence
>> number: 2068197800
>> ubi0: available PEBs: 3504, total reserved PEBs: 568, PEBs reserved
>> for bad PEB handling: 76
>>
>> @Richard, any ideas? Do you know which of the fixes should fix that?
>
After some further investigation, printing the counters as Richard suggested
I found it was a bug on my side :-( The Linux partition and the U-Boot partition
had different size (i.e. PEB count) and so Fastmap complained :-(
We trimmed the Linux partition size, and forgot to change U-Boot's
(or thought it wouldn't matter).
Sorry for the noise guys!
>
> Does this warning raise also in linux? I do not see it on the aristainetos2
> board.
>
> U-Boot code:
>
> /*
> * If fastmap is leaking PEBs (must not happen), raise a
> * fat warning and fall back to scanning mode.
> * We do this here because in ubi_wl_init() it's too late
> * and we cannot fall back to scanning.
> */
> #ifndef __UBOOT__
> if (WARN_ON(count_fastmap_pebs(ai) != ubi->peb_count -
> ai->bad_peb_count - fm->used_blocks))
> goto fail_bad;
> #else
> if (count_fastmap_pebs(ai) != ubi->peb_count -
> ai->bad_peb_count - fm->used_blocks) {
> WARN_ON(1);
> goto fail_bad;
> }
> #endif
>
> Hmm.. could not remember, why I did this U-Boot specific ...
> But that;s a good example, why I like the "#ifndef __UBOOT__"
> in Code ... I immediately see, if its linux or u-boot specific
> code ... maybe its worth to try the original linux code?
>
Linux WARN_xxx macros return a value, and so can be used inside
if statements. U-Boot on the other side, just prints a warning:
#define WARN_ON(x) if (x) {printf("WARNING in %s line %d\n" \
, __FILE__, __LINE__); }
AFAICS, the above ifndef __UBOOT__ is needed, unless you fix
your WARN_ON in U-Boot.
--
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
More information about the U-Boot
mailing list