[U-Boot] [PATCH] ubi: reset relevant globals in ubi_exit()

[U-Boot] [PATCH] ubi: reset relevant globals in ubi_exit()

[Andrew Ruder]

Before calling ubi_init() the U-Boot wrapper calls ubi_mtd_param_parse()
to have the UBI driver add it to its mtd_dev_param[] array and increment
mtd_devs.  The first time ubi_init() is called, mtd_devs would be 1.

Before ubi_init() is called again (another partition is attached),
ubi_mtd_param_parse() is called again, incrementing mtd_devs again (now
2).  This results in ubi_init() now trying to attach the first partition
and the second partition.

Fix this by adding a section at the end of ubi_exit() where we reset any
globals that would need to be reset (in this case, just mtd_devs).

Test case:
$ ubi part <mtdname> ; ubi part <mtdname>

Before patch:

$ ubi part data
UBI: attaching mtd1 to ubi0
UBI: scanning is finished
UBI: attached mtd1 (name "mtd=4", size 4 MiB) to ubi0
[...]
UBI: available PEBs: 0, total reserved PEBs: 32, [...]
$ ubi part data
UBI: detaching mtd1 from ubi0
UBI: mtd1 is detached from ubi0
UBI: attaching mtd1 to ubi0
[...]
UBI: available PEBs: 0, total reserved PEBs: 32, [...]
** Note that this is where it tries to attach mtd1 again, fails, and
** then detaches everything as it errors out of ubi_init()
UBI: detaching mtd1 from ubi0
UBI: mtd1 is detached from ubi0
UBI init error 17
$

After patch:

$ ubi part data
UBI: attaching mtd1 to ubi0
UBI: scanning is finished
UBI: attached mtd1 (name "mtd=4", size 4 MiB) to ubi0
[...]
UBI: available PEBs: 0, total reserved PEBs: 32, [...]
$ ubi part data
UBI: detaching mtd1 from ubi0
UBI: mtd1 is detached from ubi0
UBI: attaching mtd1 to ubi0
UBI: scanning is finished
UBI: attached mtd1 (name "mtd=4", size 4 MiB) to ubi0
[...]
UBI: available PEBs: 0, total reserved PEBs: 32, [...]
$

Signed-off-by: Andrew Ruder <andrew.ruder@elecsyscorp.com>
Cc: Heiko Schocher <hs@denx.de>
Cc: Kyungmin Park <kmpark@infradead.org>
---
 drivers/mtd/ubi/build.c | 4 ++++
 1 file changed, 4 insertions(+)

Not sure this is the best place to make the change, but it is one of the least
obtrusive, IMO.  Please Cc: me on any responses as it will go directly to my inbox!

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 584cf5f..fc5cbce 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -1384,6 +1384,10 @@ void ubi_exit(void)
 	misc_deregister(&ubi_ctrl_cdev);
 	class_remove_file(ubi_class, &ubi_version);
 	class_destroy(ubi_class);
+#ifdef __UBOOT__
+	/* Reset any globals that the driver depends on being zeroed */
+	mtd_devs = 0;
+#endif
 }
 module_exit(ubi_exit);
 
-- 
2.1.1

[U-Boot] [PATCH] ubi: reset relevant globals in ubi_exit()

[Heiko Schocher]

Hello Andrew,

Am 13.11.2014 18:05, schrieb Andrew Ruder:
> Before calling ubi_init() the U-Boot wrapper calls ubi_mtd_param_parse()
> to have the UBI driver add it to its mtd_dev_param[] array and increment
> mtd_devs.  The first time ubi_init() is called, mtd_devs would be 1.
>
> Before ubi_init() is called again (another partition is attached),
> ubi_mtd_param_parse() is called again, incrementing mtd_devs again (now
> 2).  This results in ubi_init() now trying to attach the first partition
> and the second partition.
>
> Fix this by adding a section at the end of ubi_exit() where we reset any
> globals that would need to be reset (in this case, just mtd_devs).
>
> Test case:
> $ ubi part <mtdname> ; ubi part <mtdname>
>
> Before patch:
>
> $ ubi part data
> UBI: attaching mtd1 to ubi0
> UBI: scanning is finished
> UBI: attached mtd1 (name "mtd=4", size 4 MiB) to ubi0
> [...]
> UBI: available PEBs: 0, total reserved PEBs: 32, [...]
> $ ubi part data
> UBI: detaching mtd1 from ubi0
> UBI: mtd1 is detached from ubi0
> UBI: attaching mtd1 to ubi0
> [...]
> UBI: available PEBs: 0, total reserved PEBs: 32, [...]
> ** Note that this is where it tries to attach mtd1 again, fails, and
> ** then detaches everything as it errors out of ubi_init()
> UBI: detaching mtd1 from ubi0
> UBI: mtd1 is detached from ubi0
> UBI init error 17
> $
>
> After patch:
>
> $ ubi part data
> UBI: attaching mtd1 to ubi0
> UBI: scanning is finished
> UBI: attached mtd1 (name "mtd=4", size 4 MiB) to ubi0
> [...]
> UBI: available PEBs: 0, total reserved PEBs: 32, [...]
> $ ubi part data
> UBI: detaching mtd1 from ubi0
> UBI: mtd1 is detached from ubi0
> UBI: attaching mtd1 to ubi0
> UBI: scanning is finished
> UBI: attached mtd1 (name "mtd=4", size 4 MiB) to ubi0
> [...]
> UBI: available PEBs: 0, total reserved PEBs: 32, [...]
> $
>
> Signed-off-by: Andrew Ruder <andrew.ruder@elecsyscorp.com>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Kyungmin Park <kmpark@infradead.org>
> ---
>   drivers/mtd/ubi/build.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> Not sure this is the best place to make the change, but it is one of the least
> obtrusive, IMO.  Please Cc: me on any responses as it will go directly to my inbox!
>
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index 584cf5f..fc5cbce 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -1384,6 +1384,10 @@ void ubi_exit(void)
>   	misc_deregister(&ubi_ctrl_cdev);
>   	class_remove_file(ubi_class, &ubi_version);
>   	class_destroy(ubi_class);
> +#ifdef __UBOOT__
> +	/* Reset any globals that the driver depends on being zeroed */
> +	mtd_devs = 0;
> +#endif
>   }
>   module_exit(ubi_exit);

Good catch, but wondering, why this not poped up in my tests, as I
did such a test ... Hmm... trying currently on the tqm5200s
board on a cfi nor flash [1] shows no error for me. But I just
activated ubi errormessages and broke the board ... so I must wait
for a debugger :-(

Beside of this, your patch seems feasible, I test it soon.

bye,
Heiko

[1] log on the tqm5200s board:
=> mtdparts

device nor0 <fc000000.flash>, # parts = 7
  #: name                size            offset          mask_flags
  0: firmware            0x00100000      0x00000000      0
  1: dtb                 0x00040000      0x00100000      0
  2: kernel              0x00240000      0x00140000      0
  3: small-fs            0x00280000      0x00380000      0
  4: initrd              0x00200000      0x00600000      0
  5: misc                0x00800000      0x00800000      0
  6: big-fs              0x01000000      0x01000000      0

active partition: nor0,0 - (firmware) 0x00100000 @ 0x00000000

defaults:
mtdids  : nor0=fc000000.flash
mtdparts: mtdparts=fc000000.flash:1m(firmware),256k(dtb),2304k(kernel),2560k(small-fs),2m(initrd),8m(misc),16m(big-fs)
=> ubi part misc
UBI: default fastmap pool size: 8
UBI: default fastmap WL pool size: 25
UBI: attaching mtd1 to ubi0
UBI: scanning is finished
UBI: empty MTD device detected
UBI: attached mtd1 (name "mtd=5", size 8 MiB) to ubi0
UBI: PEB size: 262144 bytes (256 KiB), LEB size: 262016 bytes
UBI: min./max. I/O unit sizes: 1/1, sub-page size 1
UBI: VID header offset: 64 (aligned 64), data offset: 128
UBI: good PEBs: 32, bad PEBs: 0, corrupted PEBs: 0
UBI: user volume: 0, internal volumes: 1, max. volumes count: 128
UBI: max/mean erase counter: 1/0, WL threshold: 4096, image sequence number: 0
UBI: available PEBs: 26, total reserved PEBs: 6, PEBs reserved for bad PEB handling: 0
=> ubi info l
Volume information dump:
         vol_id          2147479551
         reserved_pebs   2
         alignment       1
         data_pad        0
         vol_type        3
         name_len        13
         usable_leb_size 262016
         used_ebs        2
         used_bytes      524032
         last_eb_bytes   2
         corrupted       0
         upd_marker      0
         name            layout volume
=> ubi info
UBI: MTD device name:            "mtd=5"
UBI: MTD device size:            8 MiB
UBI: physical eraseblock size:   262144 bytes (256 KiB)
UBI: logical eraseblock size:    262016 bytes
UBI: number of good PEBs:        32
UBI: number of bad PEBs:         0
UBI: smallest flash I/O unit:    1
UBI: VID header offset:          64 (aligned 64)
UBI: data offset:                128
UBI: max. allowed volumes:       128
UBI: wear-leveling threshold:    4096
UBI: number of internal volumes: 1
UBI: number of user volumes:     0
UBI: available PEBs:             26
UBI: total number of reserved PEBs: 6
UBI: number of PEBs reserved for bad PEB handling: 0
UBI: max/mean erase counter: 1/0
=> ubi create test 400000
Creating dynamic volume test of size 4194304
=> ubi info l
Volume information dump:
         vol_id          0
         reserved_pebs   17
         alignment       1
         data_pad        0
         vol_type        3
         name_len        4
         usable_leb_size 262016
         used_ebs        17
         used_bytes      4454272
         last_eb_bytes   262016
         corrupted       0
         upd_marker      0
         name            test
Volume information dump:
         vol_id          2147479551
         reserved_pebs   2
         alignment       1
         data_pad        0
         vol_type        3
         name_len        13
         usable_leb_size 262016
         used_ebs        2
         used_bytes      524032
         last_eb_bytes   2
         corrupted       0
         upd_marker      0
         name            layout volume
=> ubi part misc
UBI: detaching mtd1 from ubi0
UBI: mtd1 is detached from ubi0
UBI: default fastmap pool size: 8
UBI: default fastmap WL pool size: 25
UBI: attaching mtd1 to ubi0
UBI: scanning is finished
UBI: attached mtd1 (name "mtd=5", size 8 MiB) to ubi0
UBI: PEB size: 262144 bytes (256 KiB), LEB size: 262016 bytes
UBI: min./max. I/O unit sizes: 1/1, sub-page size 1
UBI: VID header offset: 64 (aligned 64), data offset: 128
UBI: good PEBs: 32, bad PEBs: 0, corrupted PEBs: 0
UBI: user volume: 1, internal volumes: 1, max. volumes count: 128
UBI: max/mean erase counter: 2/1, WL threshold: 4096, image sequence number: 0
UBI: available PEBs: 9, total reserved PEBs: 23, PEBs reserved for bad PEB handling: 0
=> ubi part misc
UBI: detaching mtd1 from ubi0
UBI: mtd1 is detached from ubi0
UBI: default fastmap pool size: 8
UBI: default fastmap WL pool size: 25
UBI: attaching mtd1 to ubi0
UBI: scanning is finished
UBI: attached mtd1 (name "mtd=5", size 8 MiB) to ubi0
UBI: PEB size: 262144 bytes (256 KiB), LEB size: 262016 bytes
UBI: min./max. I/O unit sizes: 1/1, sub-page size 1
UBI: VID header offset: 64 (aligned 64), data offset: 128
UBI: good PEBs: 32, bad PEBs: 0, corrupted PEBs: 0
UBI: user volume: 1, internal volumes: 1, max. volumes count: 128
UBI: max/mean erase counter: 2/1, WL threshold: 4096, image sequence number: 0
UBI: available PEBs: 9, total reserved PEBs: 23, PEBs reserved for bad PEB handling: 0
=> ubi part misc
UBI: detaching mtd1 from ubi0
UBI: mtd1 is detached from ubi0
UBI: default fastmap pool size: 8
UBI: default fastmap WL pool size: 25
UBI: attaching mtd1 to ubi0
UBI: scanning is finished
UBI: attached mtd1 (name "mtd=5", size 8 MiB) to ubi0
UBI: PEB size: 262144 bytes (256 KiB), LEB size: 262016 bytes
UBI: min./max. I/O unit sizes: 1/1, sub-page size 1
UBI: VID header offset: 64 (aligned 64), data offset: 128
UBI: good PEBs: 32, bad PEBs: 0, corrupted PEBs: 0
UBI: user volume: 1, internal volumes: 1, max. volumes count: 128
UBI: max/mean erase counter: 2/1, WL threshold: 4096, image sequence number: 0
UBI: available PEBs: 9, total reserved PEBs: 23, PEBs reserved for bad PEB handling: 0
=>

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] ubi: reset relevant globals in ubi_exit()

[Andrew Ruder]

On 11/14/2014 12:20 AM, Heiko Schocher wrote:
> Good catch, but wondering, why this not poped up in my tests, as I
> did such a test ...

Are you on 2014.10?  I don't think this issue existed on the 2014.07-rc3 
I was using earlier.

- Andy

[U-Boot] [PATCH] ubi: reset relevant globals in ubi_exit()

[Heiko Schocher]

Hello Andrew,

Am 14.11.2014 14:31, schrieb Andrew Ruder:
> On 11/14/2014 12:20 AM, Heiko Schocher wrote:
>> Good catch, but wondering, why this not poped up in my tests, as I
>> did such a test ...
>
> Are you on 2014.10?  I don't think this issue existed on the 2014.07-rc3
> I was using earlier.

Yes, I tested currently with current mainline ...

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] ubi: reset relevant globals in ubi_exit()

[Heiko Schocher]

Hello Andrew,

Am 17.11.2014 07:21, schrieb Heiko Schocher:
> Hello Andrew,
>
> Am 14.11.2014 14:31, schrieb Andrew Ruder:
>> On 11/14/2014 12:20 AM, Heiko Schocher wrote:
>>> Good catch, but wondering, why this not poped up in my tests, as I
>>> did such a test ...
>>
>> Are you on 2014.10?  I don't think this issue existed on the 2014.07-rc3
>> I was using earlier.
>
> Yes, I tested currently with current mainline ...

Could you please test on your hw with current ML? I could not see
this problem, but maybe I have another setup ... thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany