| Message ID | 1404919091-29137-1-git-send-email-sr@denx.de |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Tom Rini |
| Headers | show |
Hi Stefan, >From: Stefan Roese [mailto:sr@denx.de] > >Commit a0a37183 (ARM: omap: merge GPMC initialization code for all >platform) broke NAND on OMAP3 based platforms. I noticed this while >testing the latest 2014.07-rc version on the TAO3530 board. NAND >detection did not work with this error message: > >NAND: nand: error: Unable to find NAND settings in GPMC Configuration - quitting > >As OMAP3 configs don't set CONFIG_NAND but CONFIG_NAND_CMD. the GPMC >was not initialized for NAND at all. This patch now fixes this issue. > Sorry couldn't understand this, why have users enabled CONFIG_NAND_CMD, if CONFIG_NAND itself is not enabled ? Otherwise, if TAO3530 board uses the on-die NAND, then it should enable NAND in its board profile via boards.cfg >Tested on TAO3530 board. with regards, pekon
Hi Pekon, On 09.07.2014 20:22, Gupta, Pekon wrote: >> Commit a0a37183 (ARM: omap: merge GPMC initialization code for all >> platform) broke NAND on OMAP3 based platforms. I noticed this while >> testing the latest 2014.07-rc version on the TAO3530 board. NAND >> detection did not work with this error message: >> >> NAND: nand: error: Unable to find NAND settings in GPMC Configuration - quitting >> >> As OMAP3 configs don't set CONFIG_NAND but CONFIG_NAND_CMD. the GPMC >> was not initialized for NAND at all. This patch now fixes this issue. >> > Sorry couldn't understand this, why have users enabled CONFIG_NAND_CMD, > if CONFIG_NAND itself is not enabled ? CONFIG_NAND doesn't seem to be a mandatory define if NAND is used. Many OMAP3 based boards don't enable this define but still use NAND. The "old" mem.c also did only check for CONFIG_CMD_NAND to configure the GPMC accordingly. Thats why I chose to change mem-common.c to accept this kind of configuration instead of changing all those config headers to enable CONFIG_NAND. > Otherwise, if TAO3530 board uses the on-die NAND, then it should enable > NAND in its board profile via boards.cfg Sorry, I don't understand what this has to do with on-die NAND. This patch is a clear fix to restore the original behavior with the "old" mem.c. Thanks, Stefan
On Thu, Jul 10, 2014 at 07:28:00AM +0200, Stefan Roese wrote: > Hi Pekon, > > On 09.07.2014 20:22, Gupta, Pekon wrote: > >>Commit a0a37183 (ARM: omap: merge GPMC initialization code for all > >>platform) broke NAND on OMAP3 based platforms. I noticed this while > >>testing the latest 2014.07-rc version on the TAO3530 board. NAND > >>detection did not work with this error message: > >> > >>NAND: nand: error: Unable to find NAND settings in GPMC Configuration - quitting > >> > >>As OMAP3 configs don't set CONFIG_NAND but CONFIG_NAND_CMD. the GPMC > >>was not initialized for NAND at all. This patch now fixes this issue. > >> > >Sorry couldn't understand this, why have users enabled CONFIG_NAND_CMD, > >if CONFIG_NAND itself is not enabled ? > > CONFIG_NAND doesn't seem to be a mandatory define if NAND is used. Exactly. Adding in CONFIG_NAND is something that am335x started doing because of the cases where we do, or do not, want to assume NAND exists.
>From: Tom Rini [mailto:tom.rini@gmail.com] On Behalf Of Rini, Tom >>On Thu, Jul 10, 2014 at 07:28:00AM +0200, Stefan Roese wrote: >> Hi Pekon, >> >>> On 09.07.2014 20:22, Gupta, Pekon wrote: >> >>Commit a0a37183 (ARM: omap: merge GPMC initialization code for all >> >>platform) broke NAND on OMAP3 based platforms. I noticed this while >> >>testing the latest 2014.07-rc version on the TAO3530 board. NAND >> >>detection did not work with this error message: >> >> >> >>NAND: nand: error: Unable to find NAND settings in GPMC Configuration - quitting >> >> >> >>As OMAP3 configs don't set CONFIG_NAND but CONFIG_NAND_CMD. the GPMC >> >>was not initialized for NAND at all. This patch now fixes this issue. >> >> >> >Sorry couldn't understand this, why have users enabled CONFIG_NAND_CMD, >> >if CONFIG_NAND itself is not enabled ? >> >> CONFIG_NAND doesn't seem to be a mandatory define if NAND is used. > >Exactly. Adding in CONFIG_NAND is something that am335x started doing >because of the cases where we do, or do not, want to assume NAND exists. > That is bad. We shouldn't have added any CONFIG just for sake of making one platform work, that to with very generic nomenclature. Anyways, given the fact we have two similar configs, Let's kill one of them completely. I think killing CONFIG_NAND would be easier, as it's used only in few TI specific am33xx boards. Agree ? with regards, pekon
On 12.07.2014 15:30, Gupta, Pekon wrote: >> From: Tom Rini [mailto:tom.rini@gmail.com] On Behalf Of Rini, Tom >>> On Thu, Jul 10, 2014 at 07:28:00AM +0200, Stefan Roese wrote: >>> Hi Pekon, >>> >>>> On 09.07.2014 20:22, Gupta, Pekon wrote: >>>>> Commit a0a37183 (ARM: omap: merge GPMC initialization code for all >>>>> platform) broke NAND on OMAP3 based platforms. I noticed this while >>>>> testing the latest 2014.07-rc version on the TAO3530 board. NAND >>>>> detection did not work with this error message: >>>>> >>>>> NAND: nand: error: Unable to find NAND settings in GPMC Configuration - quitting >>>>> >>>>> As OMAP3 configs don't set CONFIG_NAND but CONFIG_NAND_CMD. the GPMC >>>>> was not initialized for NAND at all. This patch now fixes this issue. >>>>> >>>> Sorry couldn't understand this, why have users enabled CONFIG_NAND_CMD, >>>> if CONFIG_NAND itself is not enabled ? >>> >>> CONFIG_NAND doesn't seem to be a mandatory define if NAND is used. >> >> Exactly. Adding in CONFIG_NAND is something that am335x started doing >> because of the cases where we do, or do not, want to assume NAND exists. >> > That is bad. We shouldn't have added any CONFIG just for sake of making > one platform work, that to with very generic nomenclature. Anyways, given > the fact we have two similar configs, Let's kill one of them completely. > I think killing CONFIG_NAND would be easier, as it's used only in few > TI specific am33xx boards. Agree ? Yes. It would be great if you send an patch for this to be applied after this release. And we should take my patch for this release to fix this problem quickly. Thanks, Stefan
>From: Stefan Roese [mailto:sr@denx.de] >>On 12.07.2014 15:30, Gupta, Pekon wrote: >>> From: Tom Rini [mailto:tom.rini@gmail.com] On Behalf Of Rini, Tom >>>> On Thu, Jul 10, 2014 at 07:28:00AM +0200, Stefan Roese wrote: >>>> Hi Pekon, >>>> >>>>> On 09.07.2014 20:22, Gupta, Pekon wrote: >>>>>> Commit a0a37183 (ARM: omap: merge GPMC initialization code for all >>>>>> platform) broke NAND on OMAP3 based platforms. I noticed this while >>>>>> testing the latest 2014.07-rc version on the TAO3530 board. NAND >>>>>> detection did not work with this error message: >>>>>> >>>>>> NAND: nand: error: Unable to find NAND settings in GPMC Configuration - quitting >>>>>> >>>>>> As OMAP3 configs don't set CONFIG_NAND but CONFIG_NAND_CMD. the GPMC >>>>>> was not initialized for NAND at all. This patch now fixes this issue. >>>>>> >>>>> Sorry couldn't understand this, why have users enabled CONFIG_NAND_CMD, >>>>> if CONFIG_NAND itself is not enabled ? >>>> >>>> CONFIG_NAND doesn't seem to be a mandatory define if NAND is used. >>> >>> Exactly. Adding in CONFIG_NAND is something that am335x started doing >>> because of the cases where we do, or do not, want to assume NAND exists. >>> >> That is bad. We shouldn't have added any CONFIG just for sake of making >> one platform work, that to with very generic nomenclature. Anyways, given >> the fact we have two similar configs, Let's kill one of them completely. >> I think killing CONFIG_NAND would be easier, as it's used only in few >> TI specific am33xx boards. Agree ? > >Yes. It would be great if you send an patch for this to be applied after >this release. And we should take my patch for this release to fix this >problem quickly. > Yes, absolutely, I don't want to stall your work. Acked-by: Pekon Gupta <pekon@ti.com> with regards, pekon
On Wed, Jul 09, 2014 at 05:18:09PM +0200, Stefan Roese wrote: > Commit a0a37183 (ARM: omap: merge GPMC initialization code for all > platform) broke NAND on OMAP3 based platforms. I noticed this while > testing the latest 2014.07-rc version on the TAO3530 board. NAND > detection did not work with this error message: > > NAND: nand: error: Unable to find NAND settings in GPMC Configuration - quitting > > As OMAP3 configs don't set CONFIG_NAND but CONFIG_NAND_CMD. the GPMC > was not initialized for NAND at all. This patch now fixes this issue. > > Tested on TAO3530 board. > > Signed-off-by: Stefan Roese <sr@denx.de> > Cc: Pekon Gupta <pekon@ti.com> > Cc: Tom Rini <trini@ti.com> > Acked-by: Pekon Gupta <pekon@ti.com> Please note that this introduces build failure on the tricorder platforms but is a bugfix. Thomas, tricorder is currently broken in that NAND doesn't work without this patch. With this patch we're now too big in SPL to fit in SRAM. I suspect, but this needs run-time testing that we can bump max size / say 6KB for stack, or better yet confirm that like am335x / am43xx / omap4+ we init DDR prior to putting CONFIG_SPL_STACK into sp and can use the whole of SRAM for SPL and have stack be an address in DDR (see http://patchwork.ozlabs.org/patch/371636/). Applied to u-boot-ti/master, thanks!
diff --git a/arch/arm/cpu/armv7/omap-common/mem-common.c b/arch/arm/cpu/armv7/omap-common/mem-common.c index 5bc7e1f..ba26cd1 100644 --- a/arch/arm/cpu/armv7/omap-common/mem-common.c +++ b/arch/arm/cpu/armv7/omap-common/mem-common.c @@ -89,7 +89,7 @@ void gpmc_init(void) }; u32 size = GPMC_SIZE_16M; u32 base = CONFIG_SYS_FLASH_BASE; -#elif defined(CONFIG_NAND) +#elif defined(CONFIG_NAND) || defined(CONFIG_CMD_NAND) /* configure GPMC for NAND */ const u32 gpmc_regs[GPMC_MAX_REG] = { M_NAND_GPMC_CONFIG1, M_NAND_GPMC_CONFIG2,
Commit a0a37183 (ARM: omap: merge GPMC initialization code for all platform) broke NAND on OMAP3 based platforms. I noticed this while testing the latest 2014.07-rc version on the TAO3530 board. NAND detection did not work with this error message: NAND: nand: error: Unable to find NAND settings in GPMC Configuration - quitting As OMAP3 configs don't set CONFIG_NAND but CONFIG_NAND_CMD. the GPMC was not initialized for NAND at all. This patch now fixes this issue. Tested on TAO3530 board. Signed-off-by: Stefan Roese <sr@denx.de> Cc: Pekon Gupta <pekon@ti.com> Cc: Tom Rini <trini@ti.com> --- arch/arm/cpu/armv7/omap-common/mem-common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)