[U-Boot] [PATCH] powerpc/p1022ds: Add support for NAND and NAND boot using SPL

[McClintock Matthew-B29882]

On Fri, Feb 15, 2013 at 6:34 PM, Scott Wood <scottwood at freescale.com> wrote:
> 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?

Because one of us was confused. At some point I switched over the
thinking we were talking about the ifdefs in dui.c. But, this clears
things up quite a bit - these ifdefs are not required and not sure why
there were ever put there.

>> 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?

Right, I tested without this but somehow lost the changes. Will fix in v2.

>
>
>> +/* 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?

Seems you are right about these bits. Not sure why this is or was ever
written like this (for reference this is several "internal patches"
squashed together). I'll remove the CONFIG_FSL_DIU_FB bits

>> +#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?

This should have been removed. It's since been fixed by Timur's other
patch to keep the BR/OR set when using the DIU.

>
>
>> @@ -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?

The two new lines are used in spl_minimal.c when communicating with
the PIXIS to determine what the bus_clk is for serial baud rate.

-M