[U-Boot] [PATCH] mtd: zynq: nand: Move board_nand_init() function to board.c

[Michal Simek]

On 8.11.2017 10:24, Wilson Lee wrote:
> Hi Michal,
> 
> On Wed, 2017-11-08 at 08:54 +0100, Michal Simek wrote:
>> On 8.11.2017 03:44, Wilson Lee wrote:
>>>
>>> Putting board_nand_init() function inside NAND driver was not
>>> appropriate
>>> due to it doesn't allow board vendor to customise their NAND
>>> initialization code such as adding NAND lock/unlock code.
>>>
>>> This commit was to move the board_nand_init() function from NAND
>>> driver
>>> to board.c file. This allow customization of board_nand_init()
>>> function.
>>>
>>> Signed-off-by: Wilson Lee <wilson.lee at ni.com>
>>> Cc: Joe Hershberger <joe.hershberger at ni.com>
>>> Cc: Keng Soon Cheah <keng.soon.cheah at ni.com>
>>> Cc: Chen Yee Chew <chen.yee.chew at ni.com>
>>> Cc: Albert Aribaud <albert.u.boot at aribaud.net>
>>> Cc: Michal Simek <michal.simek at xilinx.com>
>>> Cc: Siva Durga Prasad Paladugu <siva.durga.paladugu at xilinx.com>
>>> Cc: Scott Wood <oss at buserror.net>
>>> ---
>>>  arch/arm/mach-zynq/include/mach/nand.h |  9 +++++++++
>>>  board/xilinx/zynq/board.c              | 10 ++++++++++
>>>  drivers/mtd/nand/zynq_nand.c           | 12 +-----------
>>>  3 files changed, 20 insertions(+), 11 deletions(-)
>>>  create mode 100644 arch/arm/mach-zynq/include/mach/nand.h
>>>
>>> diff --git a/arch/arm/mach-zynq/include/mach/nand.h
>>> b/arch/arm/mach-zynq/include/mach/nand.h
>>> new file mode 100644
>>> index 0000000..61ef45f
>>> --- /dev/null
>>> +++ b/arch/arm/mach-zynq/include/mach/nand.h
>>> @@ -0,0 +1,9 @@
>>> +/*
>>> + * Copyright (C) 2017 National Instruments Corp.
>>> + *
>>> + * SPDX-License-Identifier:	GPL-2.0+
>>> + */
>>> +
>>> +#include <nand.h>
>>> +
>>> +void zynq_nand_init(void);
>>> diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
>>> index 90ef542..8bcb830 100644
>>> --- a/board/xilinx/zynq/board.c
>>> +++ b/board/xilinx/zynq/board.c
>>> @@ -9,6 +9,7 @@
>>>  #include <fpga.h>
>>>  #include <mmc.h>
>>>  #include <zynqpl.h>
>>> +#include <asm/arch/nand.h>
>>>  #include <asm/arch/hardware.h>
>>>  #include <asm/arch/sys_proto.h>
>>>  
>>> @@ -156,3 +157,12 @@ int dram_init(void)
>>>  	return 0;
>>>  }
>>>  #endif
>>> +
>>> +static struct nand_chip nand_chip[CONFIG_SYS_MAX_NAND_DEVICE];
>>> +void board_nand_init(void)
>>> +{
>>> +	struct nand_chip *nand = &nand_chip[0];
>>> +
>>> +	if (zynq_nand_init(nand, 0))
>>> +		puts("ZYNQ NAND init failed\n");
>>> +}
>> This is probably not enough for platforms which don't enable NAND.
>>
>>>
>>> diff --git a/drivers/mtd/nand/zynq_nand.c
>>> b/drivers/mtd/nand/zynq_nand.c
>>> index dec2c41..9be6856 100644
>>> --- a/drivers/mtd/nand/zynq_nand.c
>>> +++ b/drivers/mtd/nand/zynq_nand.c
>>> @@ -1006,7 +1006,7 @@ static int zynq_nand_device_ready(struct
>>> mtd_info *mtd)
>>>  	return 0;
>>>  }
>>>  
>>> -static int zynq_nand_init(struct nand_chip *nand_chip, int devnum)
>>> +int zynq_nand_init(struct nand_chip *nand_chip, int devnum)
>>>  {
>>>  	struct zynq_nand_info *xnand;
>>>  	struct mtd_info *mtd;
>>> @@ -1191,13 +1191,3 @@ fail:
>>>  	free(xnand);
>>>  	return err;
>>>  }
>>> -
>>> -static struct nand_chip nand_chip[CONFIG_SYS_MAX_NAND_DEVICE];
>>> -
>>> -void board_nand_init(void)
>>> -{
>>> -	struct nand_chip *nand = &nand_chip[0];
>>> -
>>> -	if (zynq_nand_init(nand, 0))
>>> -		puts("ZYNQ NAND init failed\n");
>>> -}
>>>
>> What's the sequence which you want to add?
> 
> Actually we wish to perform a NAND unlock right after the NAND was
> begin initialized. So that, we can do some write operation (ubifs table
> R/W) when we mount it as ubifs.
> 
>> Isn't it better just to include this there or create a hook here?
> 
> I was thinking for adding a hook here before. But, it will be more make
> sense to me if we correct the NAND driver by moving board_nand to
> board.c file.

ok. It means please put there some ifdefs around and can you also please
mark board_nand_init as weak in board.c that boards which include full
board.c file can overwrite it by own function? At least I hope that we
reduce number of duplications.

Thanks,
Michal