From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeroen Hofstee Date: Tue, 14 Jan 2014 21:38:37 +0100 Subject: [U-Boot] [PATCH] nand: fix reading after switching ecc In-Reply-To: <1389730875.24905.107.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> <52D599DE.1000806@myspectrum.nl> <1389730875.24905.107.camel@snotra.buserror.net> Message-ID: <52D5A04D.8090407@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 On 01/14/2014 09:21 PM, Scott Wood wrote: > On Tue, 2014-01-14 at 21:11 +0100, Jeroen Hofstee wrote: >> 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... > It doesn't matter if there are any such drivers at the moment. It's a > bad change, and a gratuitous difference from Linux with which we share > this common code. of course it does matter, you can chose not to support certain features, like subpage reads, interrupts, power management etc in a bootloader. As I said, moving it to gpmc is fine with me, so lets not making a long discussion of it. > > All the other cleanup required to change ECC modes is handled by the > OMAP driver; why shouldn't this be as well? If there are more architectures, who think it is brilliant to switch ecc it will become a mesh if not coped with in general. And yes omap might be the exception for now, so lets put it there. Regards, Jeroen