Message ID | 1415898328-18819-1-git-send-email-andrew.ruder@elecsyscorp.com |
---|---|
State | Superseded |
Delegated to: | Heiko Schocher |
Headers | show |
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
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
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
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
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);
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!