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

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

[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.

Thanks!
-- 
Ezequiel Garc?a, VanguardiaSur
www.vanguardiasur.com.ar

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

[Richard Weinberger]

Hi!

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.

BTW: it is not broken in terms of you broke it.
It is just the old fastmap version. In v4.1 fastmap saw
a massive update. Sadly this changes are a way to big for the stable tree.

> 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'll happily help you!

Thanks,
//richard

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

[Heiko Schocher]

Hello,

Am 02.10.2015 um 20:46 schrieb Richard Weinberger:
> Hi!
>
> 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.

Thanks for pointing!

> BTW: it is not broken in terms of you broke it.

Puuh ;-)

In what way is it broken? If we update to 4.1, will ubi work (mount
an existing UBI partition with fastmap) on existing boards?

> It is just the old fastmap version. In v4.1 fastmap saw
> a massive update. Sadly this changes are a way to big for the stable tree.

What do you mean with "for the stable tree" ?

>> 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'll happily help you!

I am next week in Dublin on the ELCE so I have no time to look into
this issue ... maybe the week after ...

But you can try to update to 4.1 mainline linux (or maybe 4.2?)
I hope it is not too complicated ... I am sorry, I have no patches
prepared for this

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[Richard Weinberger]

Hi!

Am 03.10.2015 um 06:27 schrieb Heiko Schocher:
>>> 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.
> 
> Thanks for pointing!
> 
>> BTW: it is not broken in terms of you broke it.
> 
> Puuh ;-)
> 
> In what way is it broken? If we update to 4.1, will ubi work (mount
> an existing UBI partition with fastmap) on existing boards?

The on-flash layout did not change, so it should work.

The "old" fastmap code had a number of issues, mostly related to powercuts
and some logical issues.
In 4.1 it saw a huge amount of rework and fixes, including UBI core code.

>> It is just the old fastmap version. In v4.1 fastmap saw
>> a massive update. Sadly this changes are a way to big for the stable tree.
> 
> What do you mean with "for the stable tree" ?

I was under the assumption that you are porting all UBI fixes to u-boot which are marked for stable.
As the fastmap rework turned out to be much more than a few single fixes they are not
in the Linux stable tree.

>>> 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'll happily help you!
> 
> I am next week in Dublin on the ELCE so I have no time to look into
> this issue ... maybe the week after ...

I'm also at ELCE, maybe we can discuss this also in person.

Thanks,
//richard

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

[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.

- 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

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]

:-(

Does this warnings not occur when compiling linux with fastmap support?

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


@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

(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.

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[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.

> - 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".

> 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.

> 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?

> (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.
So, what exactly is the problem here?

Thanks,
//richard

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

[Heiko Schocher]

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 ...

Currently I have a (with warnings) compiled U-Boot with ubi/ubifs
with linux 4.2 base ... now testing (or tomorrow)

Thanks for your help!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[Hans de Goede]

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

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

[Ezequiel Garcia]

Heiko,

On 8 October 2015 at 11:54, Heiko Schocher <hs@denx.de> wrote:
[..]
>
> Currently I have a (with warnings) compiled U-Boot with ubi/ubifs
> with linux 4.2 base ... now testing (or tomorrow)
>

If you share a branch, I can help with some tests here.

Thanks a lot for the quick work!
-- 
Ezequiel Garc?a, VanguardiaSur
www.vanguardiasur.com.ar

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

[Richard Weinberger]

Am 08.10.2015 um 16:58 schrieb Hans de Goede:
>> 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.

Ahhh!
As soon you #define something the symbol is burned.

That said, I have no problem with renaming the variable.
It does not hurt me and if it makes your life easier... :-)

Thanks,
//richard

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

[Heiko Schocher]

Hello Richard, Hans,

Am 08.10.2015 um 18:04 schrieb Richard Weinberger:
> Am 08.10.2015 um 16:58 schrieb Hans de Goede:
>>> 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.

Thanks for clarifing this.

> Ahhh!
> As soon you #define something the symbol is burned.
>
> That said, I have no problem with renaming the variable.
> It does not hurt me and if it makes your life easier... :-)

Yes, this makes it easier, patches sent to ml:

[PATCH] UBI: rename free variable
http://lists.infradead.org/pipermail/linux-mtd/2015-October/062434.html

[PATCH] UBIFS: rename free variable
http://lists.infradead.org/pipermail/linux-mtd/2015-October/062435.html

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[Heiko Schocher]

Hello Ezequiel,

Am 08.10.2015 um 17:54 schrieb Ezequiel Garcia:
> Heiko,
>
> On 8 October 2015 at 11:54, Heiko Schocher <hs@denx.de> wrote:
> [..]
>>
>> Currently I have a (with warnings) compiled U-Boot with ubi/ubifs
>> with linux 4.2 base ... now testing (or tomorrow)
>>
>
> If you share a branch, I can help with some tests here.

I pushed my current develop state to:

http://git.denx.de/?p=u-boot/u-boot-ubi.git;a=shortlog;h=refs/heads/ubi_sync_with_linux

! Only for Testing !

I tested it on the aristainetos2 board, with fastmap enabled. I was
enable to attach the ubi partition on nand flash with the "ubi part .."
command. If I have attached a partition and again call "ubi part ..."
u-boot hangs currently ... more testing debugging hopefully I can do
today. so there is some work for debugging. It would be nice, if you
can look into this issue (and of course try ubi on your hw), thanks!

And maybe you find time to get rid of the compilerwarnings ;-)

> Thanks a lot for the quick work!

You are welcome

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[Heiko Schocher]

Hello Ezequiel,

Am 09.10.2015 um 06:01 schrieb Heiko Schocher:
> Hello Ezequiel,
>
> Am 08.10.2015 um 17:54 schrieb Ezequiel Garcia:
>> Heiko,
>>
>> On 8 October 2015 at 11:54, Heiko Schocher <hs@denx.de> wrote:
>> [..]
>>>
>>> Currently I have a (with warnings) compiled U-Boot with ubi/ubifs
>>> with linux 4.2 base ... now testing (or tomorrow)
>>>
>>
>> If you share a branch, I can help with some tests here.
>
> I pushed my current develop state to:
>
> http://git.denx.de/?p=u-boot/u-boot-ubi.git;a=shortlog;h=refs/heads/ubi_sync_with_linux
>
> ! Only for Testing !
>
> I tested it on the aristainetos2 board, with fastmap enabled. I was
> enable to attach the ubi partition on nand flash with the "ubi part .."
> command. If I have attached a partition and again call "ubi part ..."
> u-boot hangs currently ... more testing debugging hopefully I can do
> today. so there is some work for debugging. It would be nice, if you
> can look into this issue (and of course try ubi on your hw), thanks!
>
> And maybe you find time to get rid of the compilerwarnings ;-)
>
>> Thanks a lot for the quick work!
>
> You are welcome

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 ...

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[Heiko Schocher]

Hello Ezequiel,

Am 09.10.2015 um 14:30 schrieb Heiko Schocher:
> Hello Ezequiel,
>
> Am 09.10.2015 um 06:01 schrieb Heiko Schocher:
>> Hello Ezequiel,
>>
>> Am 08.10.2015 um 17:54 schrieb Ezequiel Garcia:
>>> Heiko,
>>>
>>> On 8 October 2015 at 11:54, Heiko Schocher <hs@denx.de> wrote:
>>> [..]
>>>>
>>>> Currently I have a (with warnings) compiled U-Boot with ubi/ubifs
>>>> with linux 4.2 base ... now testing (or tomorrow)
>>>>
>>>
>>> If you share a branch, I can help with some tests here.
>>
>> I pushed my current develop state to:
>>
>> http://git.denx.de/?p=u-boot/u-boot-ubi.git;a=shortlog;h=refs/heads/ubi_sync_with_linux
>>
>> ! Only for Testing !
>>
>> I tested it on the aristainetos2 board, with fastmap enabled. I was
>> enable to attach the ubi partition on nand flash with the "ubi part .."
>> command. If I have attached a partition and again call "ubi part ..."
>> u-boot hangs currently ... more testing debugging hopefully I can do
>> today. so there is some work for debugging. It would be nice, if you
>> can look into this issue (and of course try ubi on your hw), thanks!
>>
>> And maybe you find time to get rid of the compilerwarnings ;-)
>>
>>> Thanks a lot for the quick work!
>>
>> You are welcome
>
> 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 ...

ping?

Did you had time for some tests?

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[Ezequiel Garcia]

On 16 October 2015 at 04:56, Heiko Schocher <hs@denx.de> wrote:
> Hello Ezequiel,
>
>
> Am 09.10.2015 um 14:30 schrieb Heiko Schocher:
>>
>> Hello Ezequiel,
>>
>> Am 09.10.2015 um 06:01 schrieb Heiko Schocher:
>>>
>>> Hello Ezequiel,
>>>
>>> Am 08.10.2015 um 17:54 schrieb Ezequiel Garcia:
>>>>
>>>> Heiko,
>>>>
>>>> On 8 October 2015 at 11:54, Heiko Schocher <hs@denx.de> wrote:
>>>> [..]
>>>>>
>>>>>
>>>>> Currently I have a (with warnings) compiled U-Boot with ubi/ubifs
>>>>> with linux 4.2 base ... now testing (or tomorrow)
>>>>>
>>>>
>>>> If you share a branch, I can help with some tests here.
>>>
>>>
>>> I pushed my current develop state to:
>>>
>>>
>>> http://git.denx.de/?p=u-boot/u-boot-ubi.git;a=shortlog;h=refs/heads/ubi_sync_with_linux
>>>
>>> ! Only for Testing !
>>>
>>> I tested it on the aristainetos2 board, with fastmap enabled. I was
>>> enable to attach the ubi partition on nand flash with the "ubi part .."
>>> command. If I have attached a partition and again call "ubi part ..."
>>> u-boot hangs currently ... more testing debugging hopefully I can do
>>> today. so there is some work for debugging. It would be nice, if you
>>> can look into this issue (and of course try ubi on your hw), thanks!
>>>
>>> And maybe you find time to get rid of the compilerwarnings ;-)
>>>
>>>> Thanks a lot for the quick work!
>>>
>>>
>>> You are welcome
>>
>>
>> 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 ...
>
>
> ping?
>

Just got back from a trip...

> Did you had time for some tests?
>

...so it will take me some time to get to this. Still it's
on the top of my list, so I'll try to work on this as soon
as possible.
-- 
Ezequiel Garc?a, VanguardiaSur
www.vanguardiasur.com.ar

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

[Ezequiel Garcia]

Hi Heiko,

On 9 October 2015 at 09:30, Heiko Schocher <hs@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/

The board seems to work fine, and I can even to "nand erase" and "ubi
part" without
any problem.

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?
-- 
Ezequiel Garc?a, VanguardiaSur
www.vanguardiasur.com.ar

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

[Richard Weinberger]

Am 17.10.2015 um 20:07 schrieb Ezequiel Garcia:
> 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?

As written on IRC, please try attaching _exactly_ the same image using Linux,
boot from TFTP and disable UBI in u-boot completely.
And please add debug prints to the WARN_ON(), I'd love to see which counters don't match.

Thanks,
//richard

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

[Heiko Schocher]

Hello Ezequiel,

Am 17.10.2015 um 20:07 schrieb Ezequiel Garcia:
> Hi Heiko,
>
> On 9 October 2015 at 09:30, Heiko Schocher <hs@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?

> 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?

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?

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[Ezequiel Garcia]

On 19 October 2015 at 02:17, Heiko Schocher <hs@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@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@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

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

[Heiko Schocher]

Hello Ezequiel,

Am 19.10.2015 um 15:47 schrieb Ezequiel Garcia:
> On 19 October 2015 at 02:17, Heiko Schocher <hs@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@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.

Thanks for checking!

> The patches seem to work fine here, so:
>
> Tested-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>

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

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

[Richard Weinberger]

Am 19.10.2015 um 15:47 schrieb Ezequiel Garcia:
> 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!

Good to know. :)

Let me find a way to detect and report this kind of user error
better.
The WARN_ON() is only meant for bad internal errors.

> 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.

+1

Thanks,
//richard

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

[Ezequiel Garcia]

On 19 October 2015 at 17:22, Richard Weinberger <richard@nod.at> wrote:
> Am 19.10.2015 um 15:47 schrieb Ezequiel Garcia:
>> 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!
>
> Good to know. :)
>
> Let me find a way to detect and report this kind of user error
> better.
> The WARN_ON() is only meant for bad internal errors.
>

Sure. In case it's not clear, let me clarify what the issue was: the
MTD partition
has one size in Linux (4072 EB) and another size in U-Boot (4076 EB).

When the map was built by Linux's Fastmap, it contained a number of PEBs that
conflicted with the number of PEBs U-Boot's Fastmap code was expecting
(becase Linux EB != U-Boot EB).

Not sure how you would detect such a mismatch, but it was a very good
idea to print a noisy warning :-)

We were effectively running without fastmap so far, becasue neither U-Boot
nor Linux could find a proper fastmap and attach the UBI partition using it.

Hope it's clear.
-- 
Ezequiel Garc?a, VanguardiaSur
www.vanguardiasur.com.ar

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

[Richard Weinberger]

Am 19.10.2015 um 23:40 schrieb Ezequiel Garcia:
> On 19 October 2015 at 17:22, Richard Weinberger <richard@nod.at> wrote:
>> Am 19.10.2015 um 15:47 schrieb Ezequiel Garcia:
>>> 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!
>>
>> Good to know. :)
>>
>> Let me find a way to detect and report this kind of user error
>> better.
>> The WARN_ON() is only meant for bad internal errors.
>>
> 
> Sure. In case it's not clear, let me clarify what the issue was: the
> MTD partition
> has one size in Linux (4072 EB) and another size in U-Boot (4076 EB).
> 
> When the map was built by Linux's Fastmap, it contained a number of PEBs that
> conflicted with the number of PEBs U-Boot's Fastmap code was expecting
> (becase Linux EB != U-Boot EB).
> 
> Not sure how you would detect such a mismatch, but it was a very good
> idea to print a noisy warning :-)

I think it would make sense to drop the WARN_ON() and replace it to a
warning using ubi_err() which explains what possible went wrong.
i.e. MTD partition layout mismatch, an internal error, etc...

> We were effectively running without fastmap so far, becasue neither U-Boot
> nor Linux could find a proper fastmap and attach the UBI partition using it.

Yeah, U-Boot's fast decided to fall back to scanning mode and had to drop the
existing fastmap and therefore Linux also had to do a full scan too.

Thanks,
//richard

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

[Heiko Schocher]

Hello Richard

Am 19.10.2015 um 23:48 schrieb Richard Weinberger:
> Am 19.10.2015 um 23:40 schrieb Ezequiel Garcia:
>> On 19 October 2015 at 17:22, Richard Weinberger <richard@nod.at> wrote:
>>> Am 19.10.2015 um 15:47 schrieb Ezequiel Garcia:
>>>> 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!
>>>
>>> Good to know. :)
>>>
>>> Let me find a way to detect and report this kind of user error
>>> better.
>>> The WARN_ON() is only meant for bad internal errors.
>>>
>>
>> Sure. In case it's not clear, let me clarify what the issue was: the
>> MTD partition
>> has one size in Linux (4072 EB) and another size in U-Boot (4076 EB).
>>
>> When the map was built by Linux's Fastmap, it contained a number of PEBs that
>> conflicted with the number of PEBs U-Boot's Fastmap code was expecting
>> (becase Linux EB != U-Boot EB).
>>
>> Not sure how you would detect such a mismatch, but it was a very good
>> idea to print a noisy warning :-)
>
> I think it would make sense to drop the WARN_ON() and replace it to a
> warning using ubi_err() which explains what possible went wrong.
> i.e. MTD partition layout mismatch, an internal error, etc...

Yep.

>> We were effectively running without fastmap so far, becasue neither U-Boot
>> nor Linux could find a proper fastmap and attach the UBI partition using it.
>
> Yeah, U-Boot's fast decided to fall back to scanning mode and had to drop the
> existing fastmap and therefore Linux also had to do a full scan too.

Didn;t got this, why is fastmap not working in U-Boot?
On the aristainetos2 board, I use fastmap, see:
http://xeidos.ddns.net/buildbot/builders/ari_ubi/builds/7/steps/shell/logs/tbotlog
search for "attached by fastmap" ...

nand 1024MiB:
05:02:32,905:INFO   :tbotlib   # write 1: ubi part ubi 4096
[...]
05:02:34,289:INFO   :tbotlib   # read 1: ubi0: attached by fastmap
[...]
05:02:34,529:INFO   :tbotlib   # read 1: ubi0: available PEBs: 3877, total reserved PEBs: 207, PEBs 
reserved for bad PEB handling: 68

1024MiB ubi artition attached in 0.624 s ...

SPI Nor flash:
05:03:14,380:INFO   :tbotlib   # write 1: ubi part rescue-system 64
[...]
05:03:16,010:INFO   :tbotlib   # read 1: ubi0: attached by fastmap
[...]
05:03:16,215:INFO   :tbotlib   # read 1: ubi0: available PEBs: 10, total reserved PEBs: 231, PEBs 
reserved for bad PEB handling: 0

15MiB Nor UBI partition in ~2 s ...

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[Richard Weinberger]

Am 20.10.2015 um 06:00 schrieb Heiko Schocher:
> Hello Richard
> 
> Am 19.10.2015 um 23:48 schrieb Richard Weinberger:
>> Am 19.10.2015 um 23:40 schrieb Ezequiel Garcia:
>>> On 19 October 2015 at 17:22, Richard Weinberger <richard@nod.at> wrote:
>>>> Am 19.10.2015 um 15:47 schrieb Ezequiel Garcia:
>>>>> 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!
>>>>
>>>> Good to know. :)
>>>>
>>>> Let me find a way to detect and report this kind of user error
>>>> better.
>>>> The WARN_ON() is only meant for bad internal errors.
>>>>
>>>
>>> Sure. In case it's not clear, let me clarify what the issue was: the
>>> MTD partition
>>> has one size in Linux (4072 EB) and another size in U-Boot (4076 EB).
>>>
>>> When the map was built by Linux's Fastmap, it contained a number of PEBs that
>>> conflicted with the number of PEBs U-Boot's Fastmap code was expecting
>>> (becase Linux EB != U-Boot EB).
>>>
>>> Not sure how you would detect such a mismatch, but it was a very good
>>> idea to print a noisy warning :-)
>>
>> I think it would make sense to drop the WARN_ON() and replace it to a
>> warning using ubi_err() which explains what possible went wrong.
>> i.e. MTD partition layout mismatch, an internal error, etc...
> 
> Yep.
> 
>>> We were effectively running without fastmap so far, becasue neither U-Boot
>>> nor Linux could find a proper fastmap and attach the UBI partition using it.
>>
>> Yeah, U-Boot's fast decided to fall back to scanning mode and had to drop the
>> existing fastmap and therefore Linux also had to do a full scan too.
> 
> Didn;t got this, why is fastmap not working in U-Boot?

No, the above explanation is why fastmap was used in Ezequiel's setup.

Thanks,
//richard