[U-Boot] [PATCH v2 1/5] at91: atmel_nand: extract HWECC initialization code into one function: atmel_hw_nand_init_param().

[Scott Wood]

On 08/16/2012 03:56 AM, Andreas Bießmann wrote:
> Dear Josh Wu,
> 
> On 16.08.2012 07:05, Josh Wu wrote:
>> Extract the hwecc initialization code into one function. It is a preparation for adding atmel PMECC support.
>>
>> Signed-off-by: Josh Wu <josh.wu at atmel.com>
>> ---
>>  drivers/mtd/nand/atmel_nand.c |  108 ++++++++++++++++++++++-------------------
>>  1 file changed, 57 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
>> index de66382..113da93 100644
>> --- a/drivers/mtd/nand/atmel_nand.c
>> +++ b/drivers/mtd/nand/atmel_nand.c
>> @@ -232,68 +232,19 @@ static int atmel_nand_correct(struct mtd_info *mtd, u_char *dat,
>>  static void atmel_nand_hwctl(struct mtd_info *mtd, int mode)
>>  {
>>  }
>> -#endif
>> -
>> -static void at91_nand_hwcontrol(struct mtd_info *mtd,
>> -					 int cmd, unsigned int ctrl)
>> -{
>> -	struct nand_chip *this = mtd->priv;
>> -
>> -	if (ctrl & NAND_CTRL_CHANGE) {
>> -		ulong IO_ADDR_W = (ulong) this->IO_ADDR_W;
>> -		IO_ADDR_W &= ~(CONFIG_SYS_NAND_MASK_ALE
>> -			     | CONFIG_SYS_NAND_MASK_CLE);
>> -
>> -		if (ctrl & NAND_CLE)
>> -			IO_ADDR_W |= CONFIG_SYS_NAND_MASK_CLE;
>> -		if (ctrl & NAND_ALE)
>> -			IO_ADDR_W |= CONFIG_SYS_NAND_MASK_ALE;
>> -
>> -#ifdef CONFIG_SYS_NAND_ENABLE_PIN
>> -		at91_set_gpio_value(CONFIG_SYS_NAND_ENABLE_PIN,
>> -				    !(ctrl & NAND_NCE));
>> -#endif
>> -		this->IO_ADDR_W = (void *) IO_ADDR_W;
>> -	}
>> -
>> -	if (cmd != NAND_CMD_NONE)
>> -		writeb(cmd, this->IO_ADDR_W);
>> -}
>>  
>> -#ifdef CONFIG_SYS_NAND_READY_PIN
>> -static int at91_nand_ready(struct mtd_info *mtd)
>> +int atmel_hw_nand_init_param(struct nand_chip *nand)
> 
> Grr ... just realized your kernel patch has the same named function. I
> would have named it with 'ecc' in ... nevertheless I would accept this.
> 
>>  {
>> -	return at91_get_gpio_value(CONFIG_SYS_NAND_READY_PIN);
>> -}
>> -#endif
>> -
>> -int board_nand_init(struct nand_chip *nand)
>> -{
>> -#ifdef CONFIG_ATMEL_NAND_HWECC
>>  	static int chip_nr = 0;
> 
> This seems to be an remnant. It seems this is a mixture between
> 'SELF_INIT' and older initialization by common nand code. Can you please
> adopt to the CONFIG_SYS_NAND_SELFINIT? -> please read doc/REDME.nand
> If I got this correctly you just need to move this static chip_nr plus
> mtd detection (nand_scan_ident()) into the board_nand_init() and call it
> _always_. Next step is to define CONFIG_SYS_NAND_SELFINIT in
> include/nand.h and fix compiler issues (board_nand_init then has no
> parameter).

nand_scan_tail() needs to be called at some point too (or just call
nand_scan() if you don't have anything to do in between).

> I dunno if this is ok to call the nand_scan_ident twice when hwecc is
> enabled. Scott, can you please comment?

It's not how the function is meant to be used.

-Scott