From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Ruder Date: Wed, 5 Nov 2014 08:27:02 -0600 Subject: [U-Boot] [PATCH 1/2] ubi: enable error reporting in initialization In-Reply-To: <5459CA57.9090009@denx.de> References: <1415117278-1255-1-git-send-email-andrew.ruder@elecsyscorp.com> <5459CA57.9090009@denx.de> Message-ID: <20141105142702.GA12864@og3k> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Wed, Nov 05, 2014 at 07:57:27AM +0100, Heiko Schocher wrote: > 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; 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! > 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. Cheers, Andy