From mboxrd@z Thu Jan  1 00:00:00 1970
From: Scott Wood <scottwood@freescale.com>
Date: Fri, 15 Feb 2013 18:34:26 -0600
Subject: [U-Boot] [PATCH] powerpc/p1022ds: Add support for NAND and NAND
 boot using SPL
In-Reply-To: <1360877713-26317-1-git-send-email-msm@freescale.com> (from
	msm@freescale.com on Thu Feb 14 15:35:13 2013)
Message-ID: <1360974866.6960.9@snotra>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 02/14/2013 03:35:13 PM, Matthew McClintock wrote:
> +#if defined(CONFIG_SYS_BR2_PRELIM) && defined(CONFIG_SYS_OR2_PRELIM)
> +	/* for FPGA */
> +	set_lbc_br(2, CONFIG_SYS_BR2_PRELIM);
> +	set_lbc_or(2, CONFIG_SYS_OR2_PRELIM);
> +#else
> +#error CONFIG_SYS_BR2_PRELIM, CONFIG_SYS_OR2_PRELIM must be defined
> +#endif

As discussed internally, this if/else is pointless.  In internal  
discussion, you said it was moot, and then you post it again here?

> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 170a358..a1c7895 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -34,7 +34,9 @@ COBJS-$(CONFIG_EXYNOS_FB) += exynos_fb.o  
> exynos_fimd.o
>  COBJS-$(CONFIG_EXYNOS_MIPI_DSIM) += exynos_mipi_dsi.o  
> exynos_mipi_dsi_common.o \
>  				exynos_mipi_dsi_lowlevel.o
>  COBJS-$(CONFIG_EXYNOS_PWM_BL) += exynos_pwm_bl.o
> +ifndef CONFIG_SPL_BUILD
>  COBJS-$(CONFIG_FSL_DIU_FB) += fsl_diu_fb.o videomodes.o
> +endif
>  COBJS-$(CONFIG_S6E8AX0) += s6e8ax0.o
>  COBJS-$(CONFIG_S6E63D6) += s6e63d6.o
>  COBJS-$(CONFIG_LD9040) += ld9040.o

I thought we discussed internally that you don't need this?

> +/* Nand Flash */
> +#if defined(CONFIG_NAND_FSL_ELBC) && !defined(CONFIG_FSL_DIU_FB)
> +#define CONFIG_SYS_NAND_BASE		0xff800000
> +#ifdef CONFIG_PHYS_64BIT
> +#define CONFIG_SYS_NAND_BASE_PHYS	0xfff800000ull
> +#else
> +#define CONFIG_SYS_NAND_BASE_PHYS	CONFIG_SYS_NAND_BASE
> +#endif

CONFIG_FSL_DIU_FB is always defined, so when would we ever set  
CONFIG_SYS_NAND_BASE?

> +#define CONFIG_SYS_NAND_BASE_LIST     { CONFIG_SYS_NAND_BASE, }

...and here you use it outside the ifdef.  I think the only reason that  
this builds is that CONFIG_FSL_DIU_FB is defined *after* the above  
check.

Why are you introducing a new ifdefs on CONFIG_FSL_DIU_FB here in the  
first place?

> +#ifdef CONFIG_FSL_DIU_FB
> +#define CONFIG_SYS_BR1_PRELIM \
> +	(BR_PHYS_ADDR(0xe0000000) | BR_PS_16 | BR_V)
> +#define CONFIG_SYS_OR1_PRELIM	(OR_AM_128MB | 0xff7)
> +#endif

Here's another -- this one is dead code.  What does this have to do  
with NAND at all?

> @@ -177,6 +284,8 @@
>  #define PIXIS_LBMAP_SWITCH	7
>  #define PIXIS_LBMAP_MASK	0xF0
>  #define PIXIS_LBMAP_ALTBANK	0x20
> +#define PIXIS_SPD		0x07
> +#define PIXIS_SPD_SYSCLK_MASK	0x07
>  #define PIXIS_ELBC_SPI_MASK	0xc0
>  #define PIXIS_SPI		0x80

Relevance?

-Scott