From mboxrd@z Thu Jan 1 00:00:00 1970 From: Igor Grinberg Date: Mon, 21 Nov 2011 16:41:10 +0200 Subject: [U-Boot] [PATCH v4 10/14] OMAP3 SPL: Add identify_nand_chip function In-Reply-To: References: <1321656491-19874-1-git-send-email-trini@ti.com> <1321656491-19874-11-git-send-email-trini@ti.com> <4EC8ADF4.8050702@compulab.co.il> <4EC9F7ED.50904@compulab.co.il> Message-ID: <4ECA6306.10501@compulab.co.il> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 11/21/11 16:12, Tom Rini wrote: > On Mon, Nov 21, 2011 at 12:04 AM, Igor Grinberg wrote: >> On 11/20/11 16:26, Tom Rini wrote: >>> On Sun, Nov 20, 2011 at 12:36 AM, Igor Grinberg 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 >>>>> --- >>>>> 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 at 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... -- Regards, Igor.