On Thu, 21 Apr 2016 12:48:50 +0200 Heiko Schocher <h...@denx.de> wrote:
> Hello Boris, > > Am 21.04.2016 um 12:25 schrieb Boris Brezillon: > > Hi Heiko, > > > > On Thu, 21 Apr 2016 12:09:34 +0200 > > Heiko Schocher <h...@denx.de> wrote: > > > >> Hello Boris, > >> > >> Am 21.04.2016 um 10:58 schrieb Boris Brezillon: > >>> On Tue, 2 Feb 2016 11:54:35 +0100 > >>> Heiko Schocher <h...@denx.de> wrote: > >>> > >>>> Set free_count to zero before walking through ai->erase list > >>>> in wl_init(). > >>>> > >>>> As U-Boot has no workqueue/threads, it immediately calls > >>>> erase_worker(), which increase for each erased block > >>>> free_count. Without this patch, free_count gets after > >>>> this initialized to zero in wl_init(), so the free_count > >>>> variable always has the maybe wrong value 0. > >>>> > >>>> Detected this behaviour on the dxr2 board, where the > >>>> UBI fastmap gets not written when attaching/dettaching > >>>> on an empty NAND. It drops instead the error message: > >>>> > >>>> could not find any anchor PEB > >>>> > >>>> With this patch, fastmap gets written on dettach. > >>> > >>> I ran into the same problem, and produced the exact same patch to > >>> fix it, so > >>> > >>> Reviewed-by: Boris Brezillon <boris.brezil...@free-electrons.com> > >> > >> Thanks! > >> > >> I did not yet found time, to investigate this problem deeper, > >> sorry. > >> > >> The real reason to me seems, on an empty nand flash, we call > >> scan_all() which calls scan_peb() which calls ubi_io_read_ec_hdr() > >> which returns UBI_IO_FF as the nand is empty. > >> > >> This adds the PEB to the erase list, and here comes the difference > >> between U-Boot and linux, we have no threads in U-Boot, so we call > >> the erase_worker function immediately ... which increments the > >> "ubi->free_count" variable ... after that it get set to > >> "ubi->free_count = 0" ... which leads into the error we see ... > >> > >> No idea, if the correct fix not would be to move this erase_worker > >> call after the attach phase ended, as Richard suggested, or if this > >> fix is also valid ... > > > > I discussed that with Richard, and I think moving the ->free_count > > assignment before iterating over the ->erase list is a good solution. > > Ah! Ok, than its fine for me too. > > > I know the Linux code is assuming that the UBI thread is not launched > > yet when we call ubi_wl_init(), but to me it seems a bit risky to rely > > on this assumption (what if we do the UBI thread creation a bit > > earlier for some reason?). And, of course, as you explained, uboot does > > not know anything about threads, so all UBI works are executed > > synchronously, which makes this implementation buggy in uboot. > > Hmm... is it also a valid fix for linux then? Well, it's not required, but it's making the code more future proof IMO. So again, I'll let Richard decide on this aspect. -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
