| Message ID | 1321656491-19874-11-git-send-email-trini@ti.com |
|---|---|
| State | Accepted |
| Delegated to: | Sandeep Paulraj |
| Headers | show |
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 <trini@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. 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. > > COBJS-$(CONFIG_EMIF4) += emif4.o > COBJS-$(CONFIG_SDRC) += sdrc.o > diff --git a/arch/arm/cpu/armv7/omap3/spl_id_nand.c b/arch/arm/cpu/armv7/omap3/spl_id_nand.c > new file mode 100644 > index 0000000..0871fc9 > --- /dev/null > +++ b/arch/arm/cpu/armv7/omap3/spl_id_nand.c > @@ -0,0 +1,87 @@ > +/* > + * (C) Copyright 2011 > + * Texas Instruments, <www.ti.com> > + * > + * Author : > + * Tom Rini <trini@ti.com> > + * > + * Initial Code from: > + * Richard Woodruff <r-woodruff2@ti.com> > + * Jian Zhang <jzhang@ti.com> > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation; either version 2 of > + * the License, or (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, > + * MA 02111-1307 USA > + */ > + > +#include <common.h> > +#include <linux/mtd/nand.h> > +#include <asm/io.h> > +#include <asm/arch/sys_proto.h> > +#include <asm/arch/mem.h> > + > +static struct gpmc *gpmc_config = (struct gpmc *)GPMC_BASE; > + > +/* nand_command: Send a flash command to the flash chip */ > +static void nand_command(u8 command) > +{ > + writeb(command, &gpmc_config->cs[0].nand_cmd); > + > + if (command == NAND_CMD_RESET) { > + unsigned char ret_val; > + writeb(NAND_CMD_STATUS, &gpmc_config->cs[0].nand_cmd); > + do { > + /* Wait until ready */ > + ret_val = readl(&gpmc_config->cs[0].nand_dat); > + } while ((ret_val & NAND_STATUS_READY) != NAND_STATUS_READY); > + } > +} > + > +/* > + * Many boards will want to know the results of the NAND_CMD_READID command > + * in order to decide what to do about DDR initialization. This function > + * allows us to do that very early and to pass those results back to the > + * board so it can make whatever decisions need to be made. > + */ > +void identify_nand_chip(int *mfr, int *id) > +{ > + /* Make sure that we have setup GPMC for NAND correctly. */ > + writel(M_NAND_GPMC_CONFIG1, &gpmc_config->cs[0].config1); > + writel(M_NAND_GPMC_CONFIG2, &gpmc_config->cs[0].config2); > + writel(M_NAND_GPMC_CONFIG3, &gpmc_config->cs[0].config3); > + writel(M_NAND_GPMC_CONFIG4, &gpmc_config->cs[0].config4); > + writel(M_NAND_GPMC_CONFIG5, &gpmc_config->cs[0].config5); > + writel(M_NAND_GPMC_CONFIG6, &gpmc_config->cs[0].config6); > + > + /* > + * Enable the config. The CS size goes in bits 11:8. We set > + * bit 6 to enable the CS and the base address goes into bits 5:0. > + */ > + writel((GPMC_SIZE_128M << 8) | (GPMC_CS_ENABLE << 6) | > + ((NAND_BASE >> 24) & GPMC_BASEADDR_MASK), > + &gpmc_config->cs[0].config7); > + > + sdelay(2000); > + > + /* Issue a RESET and then READID */ > + nand_command(NAND_CMD_RESET); > + nand_command(NAND_CMD_READID); > + > + /* Set the address to read to 0x0 */ > + writeb(0x0, &gpmc_config->cs[0].nand_adr); > + > + /* Read off the manufacturer and device id. */ > + *mfr = readb(&gpmc_config->cs[0].nand_dat); > + *id = readb(&gpmc_config->cs[0].nand_dat); > +} > diff --git a/arch/arm/include/asm/arch-omap3/sys_proto.h b/arch/arm/include/asm/arch-omap3/sys_proto.h > index 80e167b..e5031d5 100644 > --- a/arch/arm/include/asm/arch-omap3/sys_proto.h > +++ b/arch/arm/include/asm/arch-omap3/sys_proto.h > @@ -40,6 +40,7 @@ void sdrc_init(void); > void do_sdrc_init(u32, u32); > void get_board_mem_timings(u32 *mcfg, u32 *ctrla, u32 *ctrlb, u32 *rfr_ctrl, > u32 *mr); > +void identify_nand_chip(int *mfr, int *id); > void emif4_init(void); > void gpmc_init(void); > void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,
On Sun, Nov 20, 2011 at 12:36 AM, Igor Grinberg <grinberg@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 <trini@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 :) I guess the reason to not #ifndef CONFIG_SPL_BUILD the whole file is that the normal style for SPL is to only include the file when building for SPL.
On 11/20/11 16:26, Tom Rini wrote: > On Sun, Nov 20, 2011 at 12:36 AM, Igor Grinberg <grinberg@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 <trini@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. > I guess the reason to not #ifndef CONFIG_SPL_BUILD the whole file is > that the normal style for SPL is to only include the file when > building for SPL. I don't understand the sentence above and the way it is related to my question.
On Mon, Nov 21, 2011 at 12:04 AM, Igor Grinberg <grinberg@compulab.co.il> wrote: > On 11/20/11 16:26, Tom Rini wrote: >> On Sun, Nov 20, 2011 at 12:36 AM, Igor Grinberg <grinberg@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 <trini@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.
On 11/21/11 16:12, Tom Rini wrote: > On Mon, Nov 21, 2011 at 12:04 AM, Igor Grinberg <grinberg@compulab.co.il> wrote: >> On 11/20/11 16:26, Tom Rini wrote: >>> On Sun, Nov 20, 2011 at 12:36 AM, Igor Grinberg <grinberg@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 <trini@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...
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 <grinberg@compulab.co.il> wrote: >>> On 11/20/11 16:26, Tom Rini wrote: >>>> On Sun, Nov 20, 2011 at 12:36 AM, Igor Grinberg <grinberg@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 <trini@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!
On 11/21/11 17:33, Tom Rini wrote: > 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 <grinberg@compulab.co.il> wrote: >>>> On 11/20/11 16:26, Tom Rini wrote: >>>>> On Sun, Nov 20, 2011 at 12:36 AM, Igor Grinberg <grinberg@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 <trini@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 This is bad! We don't want the code to look like the above crap, do we? Because next thing will be even worth: ifdef CONFIG_SPL_BUILD ifdef CONFIG_SPL_OMAP3_ID_NAND ifdef CONFIG_SPL_OMAP3_ID_NAND_SHIT... COBJS-y += spl_id_nand_shit...o endif 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! Ok. I understand your point. No, I don't think we should. The real question is, do we want it look like the above crap? If not, then please, do it right in this patch and all the rest can be changed later. Also would be nice to make all future patches do the right thing.
On 11/22/2011 07:33 AM, Igor Grinberg wrote: > On 11/21/11 17:33, Tom Rini wrote: >> 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 <grinberg@compulab.co.il> wrote: >>>>> On 11/20/11 16:26, Tom Rini wrote: >>>>>> On Sun, Nov 20, 2011 at 12:36 AM, Igor Grinberg <grinberg@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 <trini@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 > > This is bad! > We don't want the code to look like the above crap, do we? > Because next thing will be even worth: > ifdef CONFIG_SPL_BUILD > ifdef CONFIG_SPL_OMAP3_ID_NAND > ifdef CONFIG_SPL_OMAP3_ID_NAND_SHIT... > COBJS-y += spl_id_nand_shit...o > endif > 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! > > Ok. I understand your point. No, I don't think we should. > The real question is, do we want it look like the above crap? > If not, then please, do it right in this patch and all the rest > can be changed later. > Also would be nice to make all future patches do the right thing. OK, will do. Thanks!
On 11/22/2011 07:51 AM, Tom Rini wrote: > On 11/22/2011 07:33 AM, Igor Grinberg wrote: >> On 11/21/11 17:33, Tom Rini wrote: >>> 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 <grinberg@compulab.co.il> wrote: >>>>>> On 11/20/11 16:26, Tom Rini wrote: >>>>>>> On Sun, Nov 20, 2011 at 12:36 AM, Igor Grinberg <grinberg@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 <trini@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 >> >> This is bad! >> We don't want the code to look like the above crap, do we? >> Because next thing will be even worth: >> ifdef CONFIG_SPL_BUILD >> ifdef CONFIG_SPL_OMAP3_ID_NAND >> ifdef CONFIG_SPL_OMAP3_ID_NAND_SHIT... >> COBJS-y += spl_id_nand_shit...o >> endif >> 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! >> >> Ok. I understand your point. No, I don't think we should. >> The real question is, do we want it look like the above crap? >> If not, then please, do it right in this patch and all the rest >> can be changed later. >> Also would be nice to make all future patches do the right thing. > > OK, will do. Thanks! Well, there's a problem. spl/Makefile both sets CONFIG_SPL_BUILD and then says "here's a bunch of core stuff" we need. So... we can't hide most CONFIG choices under a CONFIG_SPL_BUILD check. We can in the Makefiles however do more: ifdef CONFIG_SPL_BUILD COBJS-$(CONFIG_SPL_...) += spl_foo.o endif than we do today.
On 11/22/11 17:39, Tom Rini wrote: > On 11/22/2011 07:51 AM, Tom Rini wrote: >> On 11/22/2011 07:33 AM, Igor Grinberg wrote: >>> On 11/21/11 17:33, Tom Rini wrote: >>>> 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 <grinberg@compulab.co.il> wrote: >>>>>>> On 11/20/11 16:26, Tom Rini wrote: >>>>>>>> On Sun, Nov 20, 2011 at 12:36 AM, Igor Grinberg <grinberg@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 <trini@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 >>> >>> This is bad! >>> We don't want the code to look like the above crap, do we? >>> Because next thing will be even worth: >>> ifdef CONFIG_SPL_BUILD >>> ifdef CONFIG_SPL_OMAP3_ID_NAND >>> ifdef CONFIG_SPL_OMAP3_ID_NAND_SHIT... >>> COBJS-y += spl_id_nand_shit...o >>> endif >>> 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! >>> >>> Ok. I understand your point. No, I don't think we should. >>> The real question is, do we want it look like the above crap? >>> If not, then please, do it right in this patch and all the rest >>> can be changed later. >>> Also would be nice to make all future patches do the right thing. >> >> OK, will do. Thanks! > > Well, there's a problem. spl/Makefile both sets CONFIG_SPL_BUILD and > then says "here's a bunch of core stuff" we need. So... we can't hide > most CONFIG choices under a CONFIG_SPL_BUILD check. Why? What's the problem? Is a board config file gets included before the CONFIG_SPL_BUILD gets exported? And then the "sub" symbol does not get defined? Is that what's going on? or am I missing something? > We can in the > Makefiles however do more: > ifdef CONFIG_SPL_BUILD > COBJS-$(CONFIG_SPL_...) += spl_foo.o > endif > than we do today. And it will turn into a crap... and spread over all the U-Boot code... This is a problem! What I propose here is to use the same model as Linux uses - one independent config option per feature, which can be selected by a board config file. Is it impossible right now?
On 11/23/2011 12:39 AM, Igor Grinberg wrote: > On 11/22/11 17:39, Tom Rini wrote: >> On 11/22/2011 07:51 AM, Tom Rini wrote: >>> On 11/22/2011 07:33 AM, Igor Grinberg wrote: >>>> On 11/21/11 17:33, Tom Rini wrote: >>>>> 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 <grinberg@compulab.co.il> wrote: >>>>>>>> On 11/20/11 16:26, Tom Rini wrote: >>>>>>>>> On Sun, Nov 20, 2011 at 12:36 AM, Igor Grinberg <grinberg@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 <trini@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 >>>> >>>> This is bad! >>>> We don't want the code to look like the above crap, do we? >>>> Because next thing will be even worth: >>>> ifdef CONFIG_SPL_BUILD >>>> ifdef CONFIG_SPL_OMAP3_ID_NAND >>>> ifdef CONFIG_SPL_OMAP3_ID_NAND_SHIT... >>>> COBJS-y += spl_id_nand_shit...o >>>> endif >>>> 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! >>>> >>>> Ok. I understand your point. No, I don't think we should. >>>> The real question is, do we want it look like the above crap? >>>> If not, then please, do it right in this patch and all the rest >>>> can be changed later. >>>> Also would be nice to make all future patches do the right thing. >>> >>> OK, will do. Thanks! >> >> Well, there's a problem. spl/Makefile both sets CONFIG_SPL_BUILD and >> then says "here's a bunch of core stuff" we need. So... we can't hide >> most CONFIG choices under a CONFIG_SPL_BUILD check. > > Why? What's the problem? > Is a board config file gets included before the CONFIG_SPL_BUILD > gets exported? And then the "sub" symbol does not get defined? Correct. Give the change you're proposing a try on devkit8000, you'll see what I mean. > Is that what's going on? or am I missing something? > >> We can in the >> Makefiles however do more: >> ifdef CONFIG_SPL_BUILD >> COBJS-$(CONFIG_SPL_...) += spl_foo.o >> endif >> than we do today. > > And it will turn into a crap... and spread over all the U-Boot code... > This is a problem! > > What I propose here is to use the same model as > Linux uses - one independent config option per feature, > which can be selected by a board config file. > Is it impossible right now? Yes, needs a good bit of thinking.
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 COBJS-$(CONFIG_EMIF4) += emif4.o COBJS-$(CONFIG_SDRC) += sdrc.o diff --git a/arch/arm/cpu/armv7/omap3/spl_id_nand.c b/arch/arm/cpu/armv7/omap3/spl_id_nand.c new file mode 100644 index 0000000..0871fc9 --- /dev/null +++ b/arch/arm/cpu/armv7/omap3/spl_id_nand.c @@ -0,0 +1,87 @@ +/* + * (C) Copyright 2011 + * Texas Instruments, <www.ti.com> + * + * Author : + * Tom Rini <trini@ti.com> + * + * Initial Code from: + * Richard Woodruff <r-woodruff2@ti.com> + * Jian Zhang <jzhang@ti.com> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +#include <common.h> +#include <linux/mtd/nand.h> +#include <asm/io.h> +#include <asm/arch/sys_proto.h> +#include <asm/arch/mem.h> + +static struct gpmc *gpmc_config = (struct gpmc *)GPMC_BASE; + +/* nand_command: Send a flash command to the flash chip */ +static void nand_command(u8 command) +{ + writeb(command, &gpmc_config->cs[0].nand_cmd); + + if (command == NAND_CMD_RESET) { + unsigned char ret_val; + writeb(NAND_CMD_STATUS, &gpmc_config->cs[0].nand_cmd); + do { + /* Wait until ready */ + ret_val = readl(&gpmc_config->cs[0].nand_dat); + } while ((ret_val & NAND_STATUS_READY) != NAND_STATUS_READY); + } +} + +/* + * Many boards will want to know the results of the NAND_CMD_READID command + * in order to decide what to do about DDR initialization. This function + * allows us to do that very early and to pass those results back to the + * board so it can make whatever decisions need to be made. + */ +void identify_nand_chip(int *mfr, int *id) +{ + /* Make sure that we have setup GPMC for NAND correctly. */ + writel(M_NAND_GPMC_CONFIG1, &gpmc_config->cs[0].config1); + writel(M_NAND_GPMC_CONFIG2, &gpmc_config->cs[0].config2); + writel(M_NAND_GPMC_CONFIG3, &gpmc_config->cs[0].config3); + writel(M_NAND_GPMC_CONFIG4, &gpmc_config->cs[0].config4); + writel(M_NAND_GPMC_CONFIG5, &gpmc_config->cs[0].config5); + writel(M_NAND_GPMC_CONFIG6, &gpmc_config->cs[0].config6); + + /* + * Enable the config. The CS size goes in bits 11:8. We set + * bit 6 to enable the CS and the base address goes into bits 5:0. + */ + writel((GPMC_SIZE_128M << 8) | (GPMC_CS_ENABLE << 6) | + ((NAND_BASE >> 24) & GPMC_BASEADDR_MASK), + &gpmc_config->cs[0].config7); + + sdelay(2000); + + /* Issue a RESET and then READID */ + nand_command(NAND_CMD_RESET); + nand_command(NAND_CMD_READID); + + /* Set the address to read to 0x0 */ + writeb(0x0, &gpmc_config->cs[0].nand_adr); + + /* Read off the manufacturer and device id. */ + *mfr = readb(&gpmc_config->cs[0].nand_dat); + *id = readb(&gpmc_config->cs[0].nand_dat); +} diff --git a/arch/arm/include/asm/arch-omap3/sys_proto.h b/arch/arm/include/asm/arch-omap3/sys_proto.h index 80e167b..e5031d5 100644 --- a/arch/arm/include/asm/arch-omap3/sys_proto.h +++ b/arch/arm/include/asm/arch-omap3/sys_proto.h @@ -40,6 +40,7 @@ void sdrc_init(void); void do_sdrc_init(u32, u32); void get_board_mem_timings(u32 *mcfg, u32 *ctrla, u32 *ctrlb, u32 *rfr_ctrl, u32 *mr); +void identify_nand_chip(int *mfr, int *id); void emif4_init(void); void gpmc_init(void); void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,
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 <trini@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