ubi: enable error reporting in initialization

UBI drivers error out if writebufsize is not filled in correctly. Grab this information from the CFI flash_info struct.

Signed-off-by Andrew Ruder<[email protected]>

Cc Wolfgang Denk<[email protected]>

Cc Heiko Schocher<[email protected]>

Cc Stefan Roese<[email protected]>

---
  drivers/mtd/cfi_mtd.c | 1 +
  1 file changed, 1 insertion(+)

drivers/mtd/cfi_mtd.c int cfi_mtd_init(void) · line 229

Quoted 9 lines

drivers/mtd/cfi_mtd.c +1 -0

index ac805ff..d4c9609 100644
@@ -226,6 +226,7 @@ int cfi_mtd_init(void)
 		mtd->flags		= MTD_CAP_NORFLASH;
 		mtd->size		= fi->size;
 		mtd->writesize		= 1;
+		mtd->writebufsize	= fi->buffer_size;

^ should be cfi

This is a typo ... your patch would not compile ... please just ack the patch [1], thanks!

Quoted 11 lines

drivers/mtd/cfi_mtd.c int cfi_mtd_init(void) · line 229

Quoted 9 lines

drivers/mtd/cfi_mtd.c +1 -0

index ac805ff..d4c9609 100644
@@ -226,6 +226,7 @@ int cfi_mtd_init(void)
  		mtd->flags		= MTD_CAP_NORFLASH;
  		mtd->size		= fi->size;
  		mtd->writesize		= 1;
+		mtd->writebufsize	= fi->buffer_size;

^ should be cfi

Quoted 2 lines

This is a typo ... your patch would not compile ... please just ack the patch [1], thanks!

I responded on the other thread with your other patch. I apologize for not seeing it originally. This patch, imo, is correct though and

definitely compiles - not a typo!

The UBI layer will disable much of its error reporting when it is compiled into the linux kernel to avoid stopping boot. We want this error reporting in U-Boot since we don't initialize the UBI layer until it is used and want the error reporting.

We force this by telling the UBI layer we are building as a module.

Signed-off-by Andrew Ruder<[email protected]>

Cc Wolfgang Denk<[email protected]>

Cc Heiko Schocher<[email protected]>

Cc Kyungmin Park<[email protected]>

---
  include/ubi_uboot.h | 2 ++
  1 file changed, 2 insertions(+)

The problem is in generally enabling this feature in the size impact ... This should be discussed if we want this for all boards ...

Ok, didn;t tried this with enabling "CONFIG_MTD_UBI_MODULE" ... But the name CONFIG_MTD_UBI_MODULE is not perfect, if we want just to enable error messages ...

Quoted 2 lines

The problem is in generally enabling this feature in the size impact ... This should be discussed if we want this for all boards ...

This change actually only triggers a few changes that affect whether or not ubi_init() returns error codes or not. For the purpose of discussion, I've added the only places where the CONFIG_MTD_UBI_MODULE is checked (ubi_is_module()).

drivers/mtd/ubi/build.c:

1: /* Attach MTD devices */
2: for (i = 0; i< mtd_devs; i++) {

3:           [...]
4:           mtd = open_mtd_device(p->name);
5:           if (IS_ERR(mtd)) {
6:               err = PTR_ERR(mtd);
7:               ubi_err("cannot open mtd %s, error %d", p->name, err);
8:               /* See comment below re-ubi_is_module(). */
9:               if (ubi_is_module())

10: goto out_detach;
11: continue;
12: }
13:
14: mutex_lock(&ubi_devices_mutex);
15: err = ubi_attach_mtd_dev(mtd, p->ubi_num,
16: p->vid_hdr_offs, p->max_beb_per1024);
17: mutex_unlock(&ubi_devices_mutex);
18: if (err< 0) {
19: ubi_err("cannot attach mtd%d", mtd->index);
20: put_mtd_device(mtd);
21:
22: /*

23:               * Originally UBI stopped initializing on any error.
24:               * However, later on it was found out that this
25:               * behavior is not very good when UBI is compiled into
26:               * the kernel and the MTD devices to attach are passed
27:               * through the command line. Indeed, UBI failure
28:               * stopped whole boot sequence.
29:               *
30:               * To fix this, we changed the behavior for the
31:               * non-module case, but preserved the old behavior for
32:               * the module case, just for compatibility. This is a
33:               * little inconsistent, though.
34:               */

35: if (ubi_is_module())
36: goto out_detach;>
Cheers,
Andy

37: }
38: }
39:
40: err = ubiblock_init();
41: if (err) {
42: ubi_err("block: cannot initialize, error %d", err);
43:
44: /* See comment above re-ubi_is_module(). */
45: if (ubi_is_module())
46: goto out_detach;
47: }
48:
49: return 0;
50:
51: out_detach:
52: [...]
53: return err;

Note that errors at lines 5, 18, and 40 are ignored (and the function returns 0) when not building UBI as a module. This means that when an error occurs in this function, U-Boot believes it has succeeded which leads to the corruption and lock up issues. No new error messages are enabled via this change, simply making ubi_init return a proper return code.

With and without this change (size check) on a ARM target:

% make -j5> /dev/null 2>&1&& wc -c u-boot.bin&& git revert --no-edit ca28e16192&& make -j5> /dev/null 2>&1&& wc -c u-boot.bin 383436 u-boot.bin
[elecsys_falcon/next c9a88c1] Revert "ubi: enable error reporting in initialization"

  1 file changed, 2 deletions(-)

383356 u-boot.bin
%

Note that it actually makes the size go down by 80 bytes on my target!

Quoted 3 lines

Ok, didn;t tried this with enabling "CONFIG_MTD_UBI_MODULE" ... But the name CONFIG_MTD_UBI_MODULE is not perfect, if we want just to enable error messages ...

Agree, didn't know what the general feelings were towards making changes to the UBI layer specifically. It would be nice if the function was called ubi_allow_init_errors() instead of ubi_is_module() and checked for either __UBOOT__ or the CONFIG_MTD_UBI_MODULE being defined.