Hi Pekon, On Thu, Apr 24, 2014 at 1:53 PM, Gupta, Pekon <pe...@ti.com> wrote: > Hi Marek, > >>From: Belisko Marek [mailto:marek.beli...@gmail.com] >>On Wed, Apr 23, 2014 at 8:04 PM, Gupta, Pekon <pe...@ti.com> wrote: >>>>From: Belisko Marek [mailto:marek.beli...@gmail.com] >>>> >>>>CC-ing Pekon Gupta which add those changes in commit: >>>>6e562b1106ea6afc78752f50925e87e9dd14f8b4 >>>> >>>>On Tue, Apr 15, 2014 at 12:47 PM, Belisko Marek <marek.beli...@gmail.com> >>>>wrote: >>>>> Hi, >>>>> >>>>> we're running 2014.04-rc3 on custom am335x board (same configuration as >>>>> BBB). >>>>> When spl is loading u-boot from nand flash we can see a lot of >>>>> messages in console: >>>>> nand: bit-flip corrected @oob=0 >>>>> >>>>> It is always the same position (seems to be first byte in oob). >>>>> Anybody experienced same problem? >>>>> I've tested on 2 different flashes and result is same. >>>>> >>>>I was able to track down that read ecc from gpmc return always first >>>>byte as 0xFF (in omap_calculate_ecc()) >>> >>> This shouldn't be the case if the page is programmed. >>> Following is the expected ECC layout of BCH8 >>Thanks for reply and sorry for wrong explanation. I mean read_ecc[0] >>byte = 0xff. >>What I found also is that data in read_ecc was completely different >>from displayed when issue nand dump. >>I found patch which fixes issue [1]. The u-boot doesn't anymore print >>bit-flips in oob. I need to fix >>also spl nand_read_page as when loading u-boot from nand in spl still >>report issues. >> >>One more question. Shouldn;t code for bitflip in oob looks like this: >>--- a/drivers/mtd/nand/omap_gpmc.c >>+++ b/drivers/mtd/nand/omap_gpmc.c >>@@ -403,7 +403,7 @@ static int omap_correct_data_bch(struct mtd_info >>*mtd, uint8_t *dat, >> dat[byte_pos] ^= 1 << bit_pos; >> printf("nand: bit-flip corrected @data=%d\n", >> byte_pos); >> } else if (byte_pos < error_max) { >>- read_ecc[byte_pos - SECTOR_BYTES] = 1 << bit_pos; >>+ read_ecc[byte_pos - SECTOR_BYTES] ^= 1 << bit_pos; >> debug("nand: bit-flip corrected @oob=%d\n", byte_pos - >> SECTOR_BYTES); >> } else { >> > Yes absolutely. Please send a patch for this. > I think this got missed because read_ecc[] does not reach application layer. > Above layers are only interested in data portion, so even if the read_ecc[] > is uncorrected it doesn't matter to them. But yes this should be fixed. > Thanks for catching this. OK I'll do. > > >> >>[1] - http://e2e.ti.com/support/arm/sitara_arm/f/791/t/259699.aspx >> > Ahh.. Yes, I know of this issue.. > " The issue is mainly due to a NAND protocol violation in the omap driver > since the Random Data Output command (05h-E0h) expects to see only the column > address that should be addressed within the already loaded read page into the > read buffer. Only 2 address cycles with ALE active should be provided between > the 05h and E0h commands. The Page read command expects the full address > footprint (2bytes for column address + 3bytes for row address), but once the > page is loaded into the read buffer, Random Data Output should be used with > only 2bytes for column address." > > But the mentioned fix is not at right place. This issue is there in > nand_base.c: nand_command_lp() which is part of generic NAND > framework. So I suggest to use following patch instead. With original patch (or with your) it fix only u-boot but it doesn't fix loading u-boot from SPL as it using custom nand_read_page (in am335x_spl_bch.c) and not from nand_bases there must be other update to fix this issue. Probably something like: --- a/drivers/mtd/nand/am335x_spl_bch.c +++ b/drivers/mtd/nand/am335x_spl_bch.c @@ -64,14 +64,16 @@ static int nand_command(int block, int page, uint32_t offs, NAND_CTRL_ALE | NAND_CTRL_CHANGE); /* A[7:0] */ hwctrl(&mtd, (offs >> 8) & 0xff, NAND_CTRL_ALE); /* A[11:9] */ /* Row address */ - hwctrl(&mtd, (page_addr & 0xff), NAND_CTRL_ALE); /* A[19:12] */ - hwctrl(&mtd, ((page_addr >> 8) & 0xff), - NAND_CTRL_ALE); /* A[27:20] */ -#ifdef CONFIG_SYS_NAND_5_ADDR_CYCLE - /* One more address cycle for devices > 128MiB */ - hwctrl(&mtd, (page_addr >> 16) & 0x0f, - NAND_CTRL_ALE); /* A[31:28] */ -#endif + if (cmd != NAND_CMD_RNDOUT) { + hwctrl(&mtd, (page_addr & 0xff), NAND_CTRL_ALE); /* A[19:12] */ + hwctrl(&mtd, ((page_addr >> 8) & 0xff), NAND_CTRL_ALE); /* A[27:20] */ + + #ifdef CONFIG_SYS_NAND_5_ADDR_CYCLE + /* One more address cycle for devices > 128MiB */ + hwctrl(&mtd, (page_addr >> 16) & 0x0f, NAND_CTRL_ALE); /* A[31:28] */ + #endif + } + hwctrl(&mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
> > -------------- > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index 5d3232c..80256b5 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -674,7 +674,7 @@ static void nand_command_lp(struct mtd_info *mtd, > unsigned int command, > ctrl &= ~NAND_CTRL_CHANGE; > chip->cmd_ctrl(mtd, column >> 8, ctrl); > } > - if (page_addr != -1) { > + if (page_addr != -11 && command != NAND_CMD_RNDOUT) { ^^^small typo ;) > chip->cmd_ctrl(mtd, page_addr, ctrl); > chip->cmd_ctrl(mtd, page_addr >> 8, > NAND_NCE | NAND_ALE); > -------------- > > I think this is issue with Micron NAND also, because even the Micron > datasheet says that NAND_CMD_RNDOUT should take only 2cycles > of column address. But probably Micron devices don't crib on extra > address cycles, they just ignore it. I've tested it also on Micron flash and it doesn't report any bit-flips (without patch). > > > with regards, pekon BR, marek -- as simple and primitive as possible ------------------------------------------------- Marek Belisko - OPEN-NANDRA Freelance Developer Ruska Nova Ves 219 | Presov, 08005 Slovak Republic Tel: +421 915 052 184 skype: marekwhite twitter: #opennandra web: http://open-nandra.com _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot