[U-Boot] [PATCH v2 1/2] nand/denali: Adding Denali NAND driver support

[Chin Liang See <clsee@altera.com>]

Hi Masahiro,

On Fri, 2014-02-28 at 21:57 +0900, Masahiro Yamada wrote:
> Hello Chin,
> 
> 
> > > > > Where do you set nand->ecc.strength?
> > > > 
> > > > I believe this is only applicable for NAND_ECC_HW_SYNDROME mode. We are
> > > > using the NAND_ECC_HW (without the syndrome). Wonder you hit error
> > > > during run?
> > > 
> > > No, it must always be set for hardware ECC.  Note the lack of a break;
> > > before case NAND_ECC_HW_SYNDROME.
> > 
> > Good catch, thanks Scott!
> 
> ecc.strengh must be set for NAND_ECC_HW.
> 
> Otherwise, error message will be displayed and reboot.
> 
>    NAND:  NAND:  Denali NAND controller
>    BUG: failure at drivers/mtd/nand/nand_base.c:3224/nand_scan_tail()!
>    BUG!
>    resetting ...
> 
> You said this driver was tested against 3 different devices.
> 
> In that case, I can't understand how your board passed
> through nand_scan_tail().
> 
> Anyway, I modifed denali_nand_init() locally
> to set nand->ecc.strength.
> 


Actually this first revision of this patch was last year. It was tested
again old code base too that time. As I don't have the board with me
now, might need your help for new changes testing. I believe we can work
together to upstream this driver.


> 
> 
> > Hi Masahiro,
> > 
> > I rechecked my documentation and the value is 8.
> > The data sector size is 512 bytes while ECC sector size is 14 bytes.
> > With that, the controller able to auto correct up to 8 bits.
> > This is how a page will look like
> > 
> > 512 bytes data | 14 bytes ECC | 512 bytes data | 14 bytes ECC | 512
> > bytes data | 14 bytes ECC | 470 bytes data | 2 byte for bad block marker
> > | 42 bytes data | 14 bytes ECC | unused 
> > 
> > FYI, my documentation is located at
> > http://rocketboards.org/gitweb/?p=u-boot-socfpga.git;a=blob_plain;f=doc/README.SOCFPGA;hb=refs/heads/socfpga_v2013.01.01
> 
> For SOCFPAG, Denali controller IP is configured with
> 512 byte ECC sector size. OK.
> 
> I refer to "Denali NAND Flash Memory Controller User's Guide".
> 
> Accoding to it, Denali's IP has 2  choice for  ECC sector size:
> 512 byte or 1024 byte.
> 
> Panasonic's UniPhier SoCs adopt 1024 byte ECC sector size.
> (CONFIG_NAND_DENALI_ECC_SIZE = nand->ecc.size = 1024 for us.)
> ECC strength (nand->ecc.strength) is selectable from 8bit/16bit/24bit.
> (They correspond to  nand->ecc.bytes = 14, 28, 42, respectively)
> 
> So, In our SoC s
> 1024 byte data | {14 or 28 or 42 byte ECC} | 1024 byte data | 
> {14 or 28 or 42 byte ECC} ...


Hmmm from Denali user guide dated Jul 20 2009, 1024 can use correct bit
of 26 or 30 bits. 26 will yield 46 bytes of ECC sector size while 30
will yield 54 bytes sector size. 


> 
> 
> > #ifdef CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST
> > #define ECC_15BITS	26
> > static struct nand_ecclayout nand_15bit_oob = {
> > 	.eccbytes = ECC_15BITS,
> > };
> > #else
> > #define ECC_8BITS	14
> > static struct nand_ecclayout nand_8bit_oob = {
> > 	.eccbytes = ECC_8BITS,
> > };
> > #endif  /* CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST */
> 
> I'm afraid this part works only for 512 byte ECC sector.