From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Mon, 19 Oct 2015 16:53:07 +0200 Subject: [U-Boot] Porting UBI fixes (specially fastmap's) to U-Boot In-Reply-To: References: <561666CB.8070908@denx.de> <56166992.2020004@nod.at> <561683A5.6060000@denx.de> <56173C31.6010202@denx.de> <5617B37D.5050403@denx.de> <56247CFA.1010105@denx.de> Message-ID: <562503D3.3060800@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Ezequiel, Am 19.10.2015 um 15:47 schrieb Ezequiel Garcia: > On 19 October 2015 at 02:17, Heiko Schocher wrote: >> Hello Ezequiel, >> >> Am 17.10.2015 um 20:07 schrieb Ezequiel Garcia: >>> >>> Hi Heiko, >>> >>> On 9 October 2015 at 09:30, Heiko Schocher 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. Thanks for checking! > The patches seem to work fine here, so: > > Tested-by: Ezequiel Garcia Updated it to u-boot-ubi.git ubi_sync_with_linux >>> 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! Ah, ok. >> 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. Yes, correct. Patches are welcome ;-) bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany