[U-Boot] [PATCH 1/2] mtd: denali: improve nand_read_oob and fix nand_write_oob

[Masahiro Yamada]

Hi Chin,



> > [1] Fix denali_write_oob() handler.
> > 
> > As for v7, "nand markbad" did not work at all.
> > 
> > With this patch, it works.
> 
> Actually this was tested before.
> Here is the test result (as this was written as SOCFPGA documentation)
> 
> SOCFPGA_CYCLONE5 # nand markbad 0
> Bad block table written to 0x00003ffe0000, version 0x02
> Bad block table written to 0x00003ffc0000, version 0x02
> block 0x00000000 successfully marked as bad
> SOCFPGA_CYCLONE5 # nand bad
> 
> Device 0 bad blocks:
> 00000000
> 3ff80000
> 3ffa0000
> 3ffc0000
> 3ffe0000
> SOCFPGA_CYCLONE5 #
> 

The sutuation is different on my board.

When I run "nand markbad" with version 7, timeout error occurrs.

This is the log.

  => nand markbad 0
  Denali timeout with interrupt status 00000800
  Denali timeout with interrupt status 00000800
  block 0x00000000 NOT marked as bad! ERROR 0


I guess you test it on v2013.01 code base
with NAND_BBT_USE_FLASH enabled.

In that case, as you mentioned below,
nand_write_oob() is never called.
(But it is called on the latest release)

I'm gessing that's why you did not notice this bug in
nand_write_oob() function.



> > 
> > [3] Remove false comment
> > 
> >  /* Writes OOB data to the device.
> >   * This code unused under normal U-Boot console as normally page write raw
> >   * to be used for write oob data with main data.
> >   */
> >   static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
> > 
> > This comment is telling a lie.
> > write_oob_data() is called from "nand markbad" command.
> > It must be deleted.
> 
> Actually this comment is correct during integration into 2013 release. I
> set the option to use NAND_USE_FLASH_BBT where it will not call
> nand_do_write_oob. Do note that we will always write bad block info into
> bad block table which are located at last 4 blocks of the NAND flash.

Yes, this comment _was_ correct on v2013.01 release
only when you set NAND_BBT_USE_FLASH.

But it isn't on the latest release. (Precisely, after the commit
dfe64e2c)

On the latest code, bad block info are written back into
both oob area of  the bad block and flash-based bad block table.
It means write_oob_data() is always used under normal U-Boot usage.


> In latest code, the function "nand_do_write_oob" will be called if
> write_oob is set (where NAND_BBT_NO_OOB_BBM is clear). From the
> description of NAND_BBT_NO_OOB_BBM, user need to set it if we don't want
> the bad block table located at OOB. 

Yes, exactly.

> For our case, it should be true as we are using HW ECC. Our data might
> span into OOB region. Definitely we don't want this bad block table
> overwritten our data then. Wonder is this match with your understanding.
> 
> In short, the function write_oob_data shouldn't be called for our case.

I know SOCFPGA git repository (git://rocketboards.org/u-boot-socfpga.git)
is based on v2013.01 release.

This often caused problems under our review process.
A big change happened to NAND infrastucture  after v2013.04 release.
Check  commit dfe64e2c (mtd: resync with Linux-3.7.1).
With this commit, many NAND-related files were updated.

I think you tested your code based on v2013.01 and posted it to the ML.
It  often does not work on the current u-boot/master.

The behavior of bad-block-marking is one difference.

Before, I also pointed out "nand->ecc.strength" was missing
from your code.


And I notice another bug.
denali.c  v7  adds NAND_BBT_USE_FLASH to nand->options.
But on the latest code, it must be added nand->bbt_options.
Otherwise, nand_scan_tail() fails.

I think BBT option was added to nand->options on v2013.01.
But it was moved to nand->bbt_options.


If you had tested it on the latest release, you could have noticed
the bugs soon.
I strongly hope that SOCFPGA repository would be rebased on the latest
release.


Best Regards
Masahiro Yamada