[U-Boot] am33xx : bit-flip correction in oob
Belisko Marek
marek.belisko at gmail.com
Fri Apr 25 09:55:43 CEST 2014
Hi Pekon,
On Thu, Apr 24, 2014 at 1:53 PM, Gupta, Pekon <pekon at ti.com> wrote:
> Hi Marek,
>
>>From: Belisko Marek [mailto:marek.belisko at gmail.com]
>>On Wed, Apr 23, 2014 at 8:04 PM, Gupta, Pekon <pekon at ti.com> wrote:
>>>>From: Belisko Marek [mailto:marek.belisko at 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.belisko at 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
More information about the U-Boot
mailing list