[U-Boot] [PATCH V6 2/5] omap-common: add nand spl support

[Aneesh V <aneesh@ti.com>]

Hi Simon,

On Thursday 28 July 2011 02:08 PM, Simon Schwarz wrote:
> Add NAND support for the new SPL structure.
>
> Signed-off-by: Simon Schwarz<simonschwarzcor@gmail.com>
> ---
> This patch didn't exist before V2!
>
> V2 changes:
> ADD Some define-barriers for OMAP3 to only use NAND
> ADD nand_load_image() - inits the OMAP gpmc, loads the images - parses the
> 	header
> CHG cosmetic
> ADD do_reset() implementation for omap-common spl
> ADD nand_copy_image to nand.h
> ADD CPP barriers for mmc and nand support. The parts depending on library
> 	support are only compiled if the respective library is included.
>
> V3 changes:
> ADD Comment why setup_clocks_for_console() isn't called for OMAP3
> CHG cosmetic (deleted empty line)
> CHG rename of NAND_MODE_HW to NAND_MODE_HW_ECC
> DEL NAND_MODE_SW. Not used.
>
> V4 changes:
> CHG cosmetic - style problems
>
> V5 changes:
> CHG renamed nand_copy_image to nand_spl_load_image
> CHG offs paramter of nand_spl_load_image is of type loff_t now
>
> V6 changes:
> ADD call to nand_deselect after loading the images
> ADD nand_deselect to nand.h
>
> Transition from V1 to V2 also includes that this patch is now based on
> 	- the new SPL layout by Aneesh V and Daniel Schwierzeck
>    	- the OMAP4 SPL patches by Aneesh V
> ---
>   arch/arm/cpu/armv7/omap-common/spl.c |   47 ++++++++++++++++++++++++++++++++++
>   arch/arm/include/asm/omap_common.h   |    1 +
>   include/nand.h                       |    3 ++
>   3 files changed, 51 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/omap-common/spl.c b/arch/arm/cpu/armv7/omap-common/spl.c
> index d177652..7ec5c7c 100644
> --- a/arch/arm/cpu/armv7/omap-common/spl.c
> +++ b/arch/arm/cpu/armv7/omap-common/spl.c
> @@ -26,6 +26,7 @@
>   #include<asm/u-boot.h>
>   #include<asm/utils.h>
>   #include<asm/arch/sys_proto.h>
> +#include<nand.h>
>   #include<mmc.h>
>   #include<fat.h>
>   #include<timestamp_autogenerated.h>
> @@ -107,6 +108,7 @@ static void parse_image_header(const struct image_header *header)
>   	}
>   }
>
> +#ifdef CONFIG_SPL_MMC_SUPPORT
>   static void mmc_load_image_raw(struct mmc *mmc)
>   {
>   	u32 image_size_sectors, err;
> @@ -140,7 +142,9 @@ end:
>   		hang();
>   	}
>   }
> +#endif /* CONFIG_SPL_MMC_SUPPORT */

here..

>
> +#ifdef CONFIG_SPL_MMC_SUPPORT
>   static void mmc_load_image_fat(struct mmc *mmc)
>   {
>   	s32 err;
> @@ -173,7 +177,9 @@ end:
>   		hang();
>   	}
>   }
> +#endif /* CONFIG_SPL_MMC_SUPPORT */

and here..

You start the same the #ifdef again immediately after the #endif. Why
don't you club them together into just one #ifdef block.

Actually, since we have garbage collection of un-used functions, I
think doing the calls under #ifdef should be enough, which you have
taken care in board_init_r(). That may help to avoid some #ifdef
clutter.

>
> +#ifdef CONFIG_SPL_MMC_SUPPORT
>   static void mmc_load_image(void)
>   {
>   	struct mmc *mmc;
> @@ -206,6 +212,28 @@ static void mmc_load_image(void)
>   		hang();
>   	}
>   }
> +#endif /* CONFIG_SPL_MMC_SUPPORT */
> +
> +#ifdef CONFIG_SPL_NAND_SUPPORT
> +static void nand_load_image(void)
> +{
> +	gpmc_init();
> +	nand_init();
> +	nand_spl_load_image(CONFIG_SYS_NAND_U_BOOT_OFFS,
> +		CONFIG_SYS_NAND_U_BOOT_SIZE,
> +		(uchar *)CONFIG_SYS_NAND_U_BOOT_DST);
> +#ifdef CONFIG_NAND_ENV_DST
> +	nand_spl_load_image(CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE,
> +		(uchar *)CONFIG_NAND_ENV_DST);
> +#ifdef CONFIG_ENV_OFFSET_REDUND
> +	nand_spl_load_image(CONFIG_ENV_OFFSET_REDUND, CONFIG_ENV_SIZE,
> +		(uchar *)CONFIG_NAND_ENV_DST + CONFIG_ENV_SIZE);
> +#endif
> +#endif
> +	nand_deselect();
> +	parse_image_header((struct image_header *)CONFIG_SYS_NAND_U_BOOT_DST);
> +}
> +#endif /* CONFIG_SPL_NAND_SUPPORT */
>
>   void jump_to_image_no_args(void)
>   {
> @@ -228,10 +256,17 @@ void board_init_r(gd_t *id, ulong dummy)
>   	boot_device = omap_boot_device();
>   	debug("boot device - %d\n", boot_device);
>   	switch (boot_device) {
> +#ifdef CONFIG_SPL_MMC_SUPPORT
>   	case BOOT_DEVICE_MMC1:
>   	case BOOT_DEVICE_MMC2:
>   		mmc_load_image();
>   		break;
> +#endif
> +#ifdef CONFIG_SPL_NAND_SUPPORT
> +	case BOOT_DEVICE_NAND:
> +		nand_load_image();
> +		break;
> +#endif
>   	default:
>   		printf("SPL: Un-supported Boot Device - %d!!!\n", boot_device);
>   		hang();
> @@ -259,7 +294,11 @@ void preloader_console_init(void)
>   	gd->flags |= GD_FLG_RELOC;
>   	gd->baudrate = CONFIG_BAUDRATE;
>
> +/* Console clock for OMAP3 is already initialized by per_clocks_enable()
> + * called in board.c by s_init() */
> +#ifndef CONFIG_OMAP34XX
>   	setup_clocks_for_console();
> +#endif

Please do one of the solutions Andreas suggested instead of having that
ifndef.

br,
Aneesh