From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeroen Hofstee Date: Tue, 14 Jan 2014 21:11:10 +0100 Subject: [U-Boot] [PATCH] nand: fix reading after switching ecc In-Reply-To: <1389637125.24905.32.camel@snotra.buserror.net> References: <1389447588-17529-1-git-send-email-jeroen@myspectrum.nl> <20980858CB6D3A4BAE95CA194937D5E73EA61001@DBDE04.ent.ti.com> <1389637125.24905.32.camel@snotra.buserror.net> Message-ID: <52D599DE.1000806@myspectrum.nl> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Scott, Pekon, On 01/13/2014 07:18 PM, Scott Wood wrote: > >> >>> The omap_gpmc allows switching ecc at runtime. Since >>> the NAND_SUBPAGE_READ flag is only set, it is kept when >>> switching to hw ecc, which is not correct. This leads to >>> calling chip->ecc.read_subpage which is not a valid >>> pointer. Therefore also clear the flag so reading in >>> hw mode works again. >>> >>> Cc: Scott Wood >>> Cc: Pekon Gupta >>> Cc: Nikita Kiryanov >>> Signed-off-by: Jeroen Hofstee >>> --- >>> drivers/mtd/nand/nand_base.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c >>> index 1ce55fd..0762a19 100644 >>> --- a/drivers/mtd/nand/nand_base.c >>> +++ b/drivers/mtd/nand/nand_base.c >>> @@ -3354,6 +3354,8 @@ int nand_scan_tail(struct mtd_info *mtd) >>> /* Large page NAND with SOFT_ECC should support subpage reads */ >>> if ((chip->ecc.mode == NAND_ECC_SOFT) && (chip->page_shift > 9)) >>> chip->options |= NAND_SUBPAGE_READ; >>> + else >>> + chip->options &= ~NAND_SUBPAGE_READ; > NACK; this breaks NAND_SUBPAGE_READ with hardware ECC for drivers that > support it. There is something to argue in favour of that in general, but there are no such drivers in u-boot at the moment though (well at least not doing it on purpose). I don't mind moving it to gpmc, but for correctness, it doesn't break things at the moment... >> I don't think it's good to add OMAP specific changes to nand_base.c. >> It's better if you can add this as part of omap_select_ecc_scheme() in omap_gpmc.c > Yes, clear it from the OMAP switching code if OMAP can't do subpage > reads with hardware ECC. The gpmc will fail in hw ecc mode when trying to do subpage reads. Pekon any suggestion for the elm mode, or should this bit just be cleared unconditionally? Regards, Jeroen