[U-Boot] [PATCH 6/7 v8] NAND: TPL : introduce the TPL based on the SPL

[Zhang Ying-B40530]

-----Original Message-----
From: Wood Scott-B07421 
Sent: Wednesday, July 17, 2013 2:07 AM
To: Zhang Ying-B40530
Cc: Wood Scott-B07421; u-boot at lists.denx.de; afleming at gmail.com; Xie Xiaobo-R63061
Subject: Re: [PATCH 6/7 v8] NAND: TPL : introduce the TPL based on the SPL

On 07/16/2013 05:04:55 AM, Zhang Ying-B40530 wrote:
> 
> 
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, July 16, 2013 7:56 AM
> To: Zhang Ying-B40530
> Cc: u-boot at lists.denx.de; afleming at gmail.com; Xie Xiaobo-R63061;  
> Zhang Ying-B40530
> Subject: Re: [PATCH 6/7 v8] NAND: TPL : introduce the TPL based on  
> the SPL
> 

> How about something like:
> 
> # $@ is output, $(1) and $(2) are inputs, $(3) is padded intermediate,
> $(4) is pad-to
> SPL_PAD_APPEND = \
>                  $(OBJCOPY) ${OBJCFLAGS} --pad-to=$(4) -I binary -O  
> binary \
>                          $(1) $(obj)$(3); \
>                  cat $(obj)$(3) $(obj)$(2) > $@; \
>                  rm $(obj)$(3)
> 
> $(obj)u-boot-with-spl.bin: $(obj)spl/u-boot-spl.bin  
> $(obj)tpl/u-boot-with-tpl.bin
>                  $(call
> SPL_PAD_APPEND,$<,u-boot-with-tpl.bin,spl/u-boot-spl-pad.bin,$(CONFIG_SPL_PAD_TO))
> 
> $(obj)u-boot-with-tpl.bin: $(obj)tpl/u-boot-tpl.bin $(obj)u-boot.bin
>                  $(call
> SPL_PAD_APPEND,$<,u-boot.bin,tpl/u-boot-tpl-pad.bin,$(CONFIG_TPL_PAD_TO))
> [Zhang Ying]
> According to your advice, how to do for those don't have TPL?

They would use the rule for u-boot-with-spl.bin, and the TPL rule would  
be ignored.  No ifdef needed.
[Zhang Ying] 
But, Don't u-boot-with-spl.bin contain tpl/u-boot-with-tpl.bin in the following rule?

$(obj)u-boot-with-spl.bin: $(obj)spl/u-boot-spl.bin  $(obj)tpl/u-boot-with-tpl.bin
          $(call SPL_PAD_APPEND,$<,u-boot-with-tpl.bin,spl/u-boot-spl-pad.bin,$(CONFIG_SPL_PAD_TO))


> > diff --git a/include/nand.h b/include/nand.h index 228d871..2aa7238
> > 100644
> > --- a/include/nand.h
> > +++ b/include/nand.h
> > @@ -153,6 +153,9 @@ int nand_unlock(nand_info_t *meminfo, loff_t
> > start, size_t length,  int nand_get_lock_status(nand_info_t  
> *meminfo,
> > loff_t offset);
> >
> >  int nand_spl_load_image(uint32_t offs, unsigned int size, void  
> *dst);
> > +#ifdef CONFIG_TPL_BUILD
> > +int nand_load_image(uint32_t offs, unsigned int uboot_size, void
> > *vdst);
> > +#endif
> >  void nand_deselect(void);
> >
> 
> Don't ifdef prototypes.  Plus, some other platforms may want this in  
> other configurations.
> [Zhang Ying]
> Remove ifdef?  If remove, there was error:
> cmd_nand.c:889:12: error: conflicting types for 'nand_load_image'.

Hmm...  In that case, please rename the function when built static.

> The function nand_load_image is defined in two files:
> 1. /common/cmd_nand.c:
> static int nand_load_image(cmd_tbl_t *cmdtp, nand_info_t *nand,
>                            ulong offset, ulong addr, char *cmd)
> 2. drivers/mtd/nand/fsl_elbc_spl.c
> #ifndef CONFIG_TPL_BUILD
> static
> #endif
> int nand_load_image(uint32_t offs, unsigned int uboot_size, void  
> *vdst)
> 
> This function only is called by outside in TPL. So, there should  
> ifdef.

You're defining a function called "nand_load_image" ind  
include/nand.h.  There's nothing eLBC-specific about that.  It is  
entirely possible that another implementation will want to export that  
function in an ordinary SPL -- or even possibly in the main U-Boot  
image.

In fact, there's already a common definition for this, which is  
nand_spl_load_image().  Use that.
[Zhang Ying] 
There has a prolem:
nand_spl_simple.c: In function 'nand_init':
nand_spl_simple.c:265:2: error: too many arguments to function 'board_nand_init'

This because the functionality "board_nand_init" is called and parameter is not null, 
but board_nand_init prototype is declared to "void board_nand_init(void)".

In "include/nand.h":

#ifdef CONFIG_SYS_NAND_SELF_INIT
void board_nand_init(void);
int nand_register(int devnum);
#else
extern int board_nand_init(struct nand_chip *nand);
#endif

There need ifdef CONFIG_SYS_NAND_SELF_INIT in nand_init(), it's ok?

#ifdef CONFIG_SYS_NAND_SELF_INIT
        board_nand_init();
#else
        board_nand_init(&nand_chip);
#endif