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

[Masahiro Yamada]

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.



> 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} ...


> #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.

Denali's document says
For 512B ECC sector size,
   ecc.bytes = Ceiling to next word (13 * ecc.strength)
For 1024B ECC sector size,
   ecc.bytes = Ceiling to next word (14 * ecc.strength)



And denali_setup_dma_sequence() function
(Why did you rename this function?)
did not work either.
I needed to fix it locally.


So bad news is this version itself does not work for me.
Good news is I could adjust it locally and confirmed some features
worked. (But I think I need more test.)

So, how will this situation work?
It turned out there are some differences between
two Denali hardwares and this driver works only for yours.

You merge it first, and (if you don't mind) shall I modify it
in a more generic way to run on both hardwares?

> If you want to run under SPL, there are some patches for that. Let me
> know if you need that. While for U-Boot, they are working fine. Probably

Thanks for your offering help.
But I am not sure if SOCFPGA and UniPhier can share a SPL nand driver.
(Actually I have locally our own Denali driver for SPL.
And I have Denali driver for main U-Boot, which is adjusted for
our SoCs, too.
But I don't mind to switch onto your driver if it works for me.)

> +#define CONFIG_SYS_NAND_USE_FLASH_BBT
> +#define CONFIG_SYS_NAND_REGS_BASE	SOCFPGA_NAND_REGS_ADDRESS
> +#define CONFIG_SYS_NAND_DATA_BASE	SOCFPGA_NAND_DATA_ADDRESS
> +#define CONFIG_SYS_NAND_BASE		CONFIG_SYS_NAND_REGS_BASE

Maybe
#define CONFIG_SYS_NAND_BASE	(SOCFPGA_NAND_DATA_ADDRESS + 0x10)
?


BTW, you changed all  denali->foo  to denali.foo.
It looks unnecessay to me.


Best Regards
Masahiro Yamada