Skip to contentSkip to conversation

arm: omap: cm_t35: Fix: Re-add GPMC_NAND_ECC_LP_x8_LAYOUT

PATCH In review Stefan Roese 4 participants 5 messages Mar 11, 2014 – Mar 13, 2014

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 arm: omap: cm_t35: Fix: Re-add GPMC_NAND_ECC_LP_x8_LAYOUT

Patch a7e36fc9 (mtd: nand: omap: remove unused #defines from common omap_gpmc.h) removed some MTD related defines. Including GPMC_NAND_ECC_LP_x8_LAYOUT. But this define is also needed for the memory controller configuration (only the x8 defines are needed, the x16 defines are the default). Without it the NAND subsystem is not configured correctly and booting into U-Boot does not work.

Cc Pekon Gupta <[email protected]>
Cc Nikita Kiryanov <[email protected]>
Cc Igor Grinberg <[email protected]>
Cc Tom Rini <[email protected]>
---
 include/configs/cm_t35.h | 2 ++
 1 file changed, 2 insertions(+)
include/configs/cm_t35.h +2 -0
index cfc4f9b..aae05e0 100644
@@ -158,6 +158,8 @@
 							/* CS0 */
 #define CONFIG_SYS_MAX_NAND_DEVICE	1		/* Max number of NAND */
 							/* devices */
+#define GPMC_NAND_ECC_LP_x8_LAYOUT
+
 /* Environment information */
 #define CONFIG_BOOTDELAY		3
 #define CONFIG_ZERO_BOOTDELAY_CHECK
Signature · 1 line
1.8.5.5
#1 lore raw

PATCH 1/1 arm: omap: cm_t35: Fix: Re-add GPMC_NAND_ECC_LP_x8_LAYOUT

Quoted 7 lines · Stefan Roese

Patch a7e36fc9 (mtd: nand: omap: remove unused #defines from common omap_gpmc.h) removed some MTD related defines. Including GPMC_NAND_ECC_LP_x8_LAYOUT. But this define is also needed for the memory controller configuration (only the x8 defines are needed, the x16 defines are the default). Without it the NAND subsystem is not configured correctly and booting into U-Boot does not work.

Quoted 5 lines
Cc Pekon Gupta <[email protected]>
Cc Nikita Kiryanov <[email protected]>
Cc Igor Grinberg <[email protected]>
Cc Tom Rini <[email protected]>
Quoted 18 lines
---
 include/configs/cm_t35.h | 2 ++
 1 file changed, 2 insertions(+)
include/configs/cm_t35.h +2 -0
index cfc4f9b..aae05e0 100644
@@ -158,6 +158,8 @@
 							/* CS0 */
 #define CONFIG_SYS_MAX_NAND_DEVICE	1		/* Max number of NAND */
 							/* devices */
+#define GPMC_NAND_ECC_LP_x8_LAYOUT
+
 /* Environment information */
 #define CONFIG_BOOTDELAY		3
 #define CONFIG_ZERO_BOOTDELAY_CHECK
Signature · 2 lines
Regards,
Igor.
#2 lore raw
In reply to Stefan Roese
Quoted 7 lines · Stefan Roese

Patch a7e36fc9 (mtd: nand: omap: remove unused #defines from common omap_gpmc.h) removed some MTD related defines. Including GPMC_NAND_ECC_LP_x8_LAYOUT. But this define is also needed for the memory controller configuration (only the x8 defines are needed, the x16 defines are the default). Without it the NAND subsystem is not configured correctly and booting into U-Boot does not work.

Quoted 6 lines
Cc Pekon Gupta <[email protected]>
Cc Nikita Kiryanov <[email protected]>
Cc Igor Grinberg <[email protected]>
Cc Tom Rini <[email protected]>

Applied to u-boot-ti/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140312/5cc7e5fe/attachment.pgp>
#3 lore raw

PATCH 1/1 arm: omap: cm_t35: Fix: Re-add GPMC_NAND_ECC_LP_x8_LAYOUT

In reply to Stefan Roese
Quoted 9 lines

From: Stefan Roese [mailto:sr at denx.de]

Patch a7e36fc9 (mtd: nand: omap: remove unused #defines from common omap_gpmc.h) removed some MTD related defines. Including GPMC_NAND_ECC_LP_x8_LAYOUT. But this define is also needed for the memory controller configuration (only the x8 defines are needed, the x16 defines are the default). Without it the NAND subsystem is not configured correctly and booting into U-Boot does not work.

The root cause of this problem is in ..

	arch/arm/cpu/armv7/omap3/mem.c

#if defined(GPMC_NAND_ECC_SP_x8_LAYOUT) || defined(GPMC_NAND_ECC_LP_x8_LAYOUT) where based on GPMC_NAND_ECC_LP_x8_LAYOUT, different
controller configuration is selected. Unless you need a urgent fix,

- I would suggest to use [1] instead. I'll try to get a cleaner version of [1] incorporating other comments from Scott, soon.

- Alternatively, you can also look at [2], which is more robust but was still under discussion.

[1] http://lists.denx.de/pipermail/u-boot/2013-December/168507.html

[2]
http://lists.denx.de/pipermail/u-boot/2013-September/163882.html
http://lists.denx.de/pipermail/u-boot/2013-September/163879.html

with regards, pekon

#4 lore raw

PATCH 1/1 arm: omap: cm_t35: Fix: Re-add GPMC_NAND_ECC_LP_x8_LAYOUT

Hi Pekon,

Quoted 9 lines · Pekon
Quoted 9 lines

From: Stefan Roese [mailto:sr at denx.de]

Patch a7e36fc9 (mtd: nand: omap: remove unused #defines from common omap_gpmc.h) removed some MTD related defines. Including GPMC_NAND_ECC_LP_x8_LAYOUT. But this define is also needed for the memory controller configuration (only the x8 defines are needed, the x16 defines are the default). Without it the NAND subsystem is not configured correctly and booting into U-Boot does not work.

Quoted 5 lines

The root cause of this problem is in ..

	arch/arm/cpu/armv7/omap3/mem.c

#if defined(GPMC_NAND_ECC_SP_x8_LAYOUT) || defined(GPMC_NAND_ECC_LP_x8_LAYOUT) where based on GPMC_NAND_ECC_LP_x8_LAYOUT, different
controller configuration is selected.

I know as I introduced this code a while ago for this x8 NAND target. ;)

Quoted 1 line

Unless you need a urgent fix,

Yes, I need an urgent fix. This board does not boot anymore. Note that Tom already pulled my patch into his TI repo. So this fix is on its way right now.

Quoted 5 lines

- I would suggest to use [1] instead. I'll try to get a cleaner version of [1] incorporating other comments from Scott, soon.

- Alternatively, you can also look at [2], which is more robust but was still under discussion.

Yes, your patches look more elegant. I suggest to move to them once the discussion has settled. And then remove these x8_LAYOUT defines completely when they are really not needed any more. But please not sooner.

Thanks,
Stefan