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

[Chin Liang See]

Hi Scott,

On Thu, 2014-02-27 at 16:32 -0600, Scott Wood wrote:
> On Thu, 2014-02-27 at 15:02 -0600, Chin Liang See wrote:
> > Hi Masahiro,
> > 
> > On Thu, 2014-02-27 at 23:35 +0900, Masahiro Yamada wrote:
> > > Hello Chin,
> > > 
> > > 
> > > > +
> > > > +	nand->ecc.mode = NAND_ECC_HW;
> > > > +	nand->ecc.size = CONFIG_NAND_DENALI_ECC_SIZE;
> > > > +	nand->ecc.read_oob = denali_read_oob;
> > > > +	nand->ecc.write_oob = denali_write_oob;
> > > > +	nand->ecc.read_page = denali_read_page;
> > > > +	nand->ecc.read_page_raw = denali_read_page_raw;
> > > > +	nand->ecc.write_page = denali_write_page;
> > > > +	nand->ecc.write_page_raw = denali_write_page_raw;
> > > > +#ifdef CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST
> > > > +	/* 15bit ECC */
> > > > +	nand->ecc.bytes = 26;
> > > > +	nand->ecc.layout = &nand_15bit_oob;
> > > > +#else	/* 8bit ECC */
> > > > +	nand->ecc.bytes = 14;
> > > > +	nand->ecc.layout = &nand_8bit_oob;
> > > > +#endif
> > > > +	nand->ecc.calculate = denali_ecc_calculate;
> > > > +	nand->ecc.correct  = denali_ecc_correct;
> > > > +	nand->ecc.hwctl  = denali_ecc_hwctl;
> > > 
> > > You set nand->ecc.mode = NAND_ECC_HW,
> > > but it looks like you don't set  nand->ecc.strength.
> > > 
> > > So, I think initialization will fail in nand_scan_tail() function.
> > > 
> > > Here,
> > > 
> > >                 if (mtd->writesize >= chip->ecc.size) {
> > >                         if (!chip->ecc.strength) {
> > >                                 pr_warn("Driver must set ecc.strength when using hardware ECC\n");
> > >                                 BUG();
> > >                         }
> > >                         break;
> > > 
> > > 
> > > 
> > > 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!


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

Chin Liang

> 
> -Scott
> 
> 
>