On 11/21/2011 07:41 AM, Igor Grinberg wrote: > On 11/21/11 16:12, Tom Rini wrote: >> On Mon, Nov 21, 2011 at 12:04 AM, Igor Grinberg <grinb...@compulab.co.il> >> wrote: >>> On 11/20/11 16:26, Tom Rini wrote: >>>> On Sun, Nov 20, 2011 at 12:36 AM, Igor Grinberg <grinb...@compulab.co.il> >>>> wrote: >>>>> Hi Tom, >>>>> >>>>> On 11/19/11 00:48, Tom Rini wrote: >>>>>> A number of boards are populated with a PoP chip for both DDR and NAND >>>>>> memory. Other boards may simply use this as an easy way to identify >>>>>> board revs. So we provide a function that can be called early to reset >>>>>> the NAND chip and return the result of NAND_CMD_READID. All of this >>>>>> code is put into spl_id_nand.c and controlled via >>>>>> CONFIG_SPL_OMAP3_ID_NAND. >>>>>> >>>>>> Signed-off-by: Tom Rini <tr...@ti.com> >>>>>> --- >>>>>> arch/arm/cpu/armv7/omap3/Makefile | 3 + >>>>>> arch/arm/cpu/armv7/omap3/spl_id_nand.c | 87 >>>>>> +++++++++++++++++++++++++++ >>>>>> arch/arm/include/asm/arch-omap3/sys_proto.h | 1 + >>>>>> 3 files changed, 91 insertions(+), 0 deletions(-) >>>>>> create mode 100644 arch/arm/cpu/armv7/omap3/spl_id_nand.c >>>>>> >>>>>> diff --git a/arch/arm/cpu/armv7/omap3/Makefile >>>>>> b/arch/arm/cpu/armv7/omap3/Makefile >>>>>> index 8e85891..4b38e45 100644 >>>>>> --- a/arch/arm/cpu/armv7/omap3/Makefile >>>>>> +++ b/arch/arm/cpu/armv7/omap3/Makefile >>>>>> @@ -31,6 +31,9 @@ COBJS += board.o >>>>>> COBJS += clock.o >>>>>> COBJS += mem.o >>>>>> COBJS += sys_info.o >>>>>> +ifdef CONFIG_SPL_BUILD >>>>>> +COBJS-$(CONFIG_SPL_OMAP3_ID_NAND) += spl_id_nand.o >>>>>> +endif >>>>> >>>>> You haven't responded to my question on the above stuff. >>>>> Otherwise all the series look good to me. >>>> >>>> Missed that, sorry! >>>> >>>>> >>>>> Original version available at: >>>>> http://www.mail-archive.com/u-boot@lists.denx.de/msg68828.html >>>>> >>>>> Here is the relevant part: >>>>> >>>>>>>> diff --git a/arch/arm/cpu/armv7/omap3/Makefile >>>>>>>> b/arch/arm/cpu/armv7/omap3/Makefile >>>>>>>>>>> index 8e85891..772f3d4 100644 >>>>>>>>>>> --- a/arch/arm/cpu/armv7/omap3/Makefile >>>>>>>>>>> +++ b/arch/arm/cpu/armv7/omap3/Makefile >>>>>>>>>>> @@ -31,6 +31,9 @@ COBJS += board.o >>>>>>>>>>> COBJS += clock.o >>>>>>>>>>> COBJS += mem.o >>>>>>>>>>> COBJS += sys_info.o >>>>>>>>>>> +ifdef CONFIG_SPL_BUILD >>>>>>>>>>> +COBJS-$(CONFIG_SPL_OMAP3_POP_PROBE) += spl_pop_probe.o >>>>>>>>>>> +endif >>>>>>>>> >>>>>>>>> Can't CONFIG_SPL_OMAP3_..._PROBE symbol default to "no" >>>>>>>>> and depend on CONFIG_SPL_BUILD, so you don't need to enclose >>>>>>>>> it in #ifdef? >>>>>>> >>>>>>> But then it would build for both SPL and non-SPL cases. >>>>> >>>>> No, it should not. >>>>> What do you think of the following: >>>>> In the Makefile have only: >>>>> COBJS-$(CONFIG_SPL_OMAP3_POP_PROBE) += spl_pop_probe.o >>>>> >>>>> Then in the spl_pop_probe.c have this type of check: >>>>> #ifndef CONFIG_SPL_BUILD >>>>> # error CONFIG_SPL_OMAP3_POP_PROBE requires CONFIG_SPL_BUILD >>>>> #endif >>>>> >>>>> This way, you require the CONFIG_SPL_OMAP3_POP_PROBE symbol >>>>> be a part of the CONFIG_SPL_BUILD symbols group. >>>> >>>> Well, if we always link this, but then #error, U-Boot won't build :) >>> >>> No you do not always link this... please, read more carefully... >>> Only when CONFIG_SPL_OMAP3_POP_PROBE symbol is defined, the file will >>> be compiled, but if CONFIG_SPL_OMAP3_POP_PROBE defined without >>> CONFIG_SPL_BUILD being defined, then it will emit an error. >> >> So make the config file do: >> #ifdef CONFIG_SPL_BUILD >> #define CONFIG_SPL_OMAP3_POP_PROBE >> #endif >> ? That's now how the rest of the SPL code works. > > Well, yes I think it makes sense for all SPL related config options > to do something like: > #ifdef CONFIG_SPL_BUILD > #define CONFIG_SPL_OMAP3_POP_PROBE > #define CONFIG_SPL_... > #define CONFIG_SPL_... > #endif > > And the error message, I have proposed above, will prevent > people from doing stupid things, like defining > CONFIG_SPL_OMAP3_POP_PROBE without the CONFIG_SPL_BUILD. > At least for now, until we have Kbuild with dependencies and stuff...
Well, I guess the point I'd try and make is that it's not how SPL is done today. Really following the existing format would be (in the Makefile): ifdef CONFIG_SPL_BUILD ifdef CONFIG_SPL_OMAP3_ID_NAND COBJS-y += spl_id_nand.o endif endif I can see the point you're making but I'm asking if we need to change everyone around to your suggested way of building before we can merge these changes in? Thanks! -- Tom _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
