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

McClintock Matthew-B29882
2013-02-18T19:07:31Z

On Fri, Feb 15, 2013 at 6:34 PM, Scott Wood <scottwood@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