[U-Boot] [PATCH] arm: omap: cm_t35: Fix: Re-add GPMC_NAND_ECC_LP_x8_LAYOUT

[U-Boot] [PATCH] arm: omap: cm_t35: Fix: Re-add GPMC_NAND_ECC_LP_x8_LAYOUT

[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.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Pekon Gupta <pekon@ti.com>
Cc: Nikita Kiryanov <nikita@compulab.co.il>
Cc: Igor Grinberg <grinberg@compulab.co.il>
Cc: Tom Rini <trini@ti.com>
---
 include/configs/cm_t35.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h
index cfc4f9b..aae05e0 100644
--- a/include/configs/cm_t35.h
+++ b/include/configs/cm_t35.h
@@ -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
-- 
1.8.5.5

[U-Boot] [PATCH] arm: omap: cm_t35: Fix: Re-add GPMC_NAND_ECC_LP_x8_LAYOUT

[Igor Grinberg]

On 03/11/14 18:04, Stefan Roese wrote:
> 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.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Pekon Gupta <pekon@ti.com>
> Cc: Nikita Kiryanov <nikita@compulab.co.il>
> Cc: Igor Grinberg <grinberg@compulab.co.il>
> Cc: Tom Rini <trini@ti.com>

Acked-by: Igor Grinberg <grinberg@compulab.co.il>

> ---
>  include/configs/cm_t35.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h
> index cfc4f9b..aae05e0 100644
> --- a/include/configs/cm_t35.h
> +++ b/include/configs/cm_t35.h
> @@ -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
> 

-- 
Regards,
Igor.

[U-Boot] arm: omap: cm_t35: Fix: Re-add GPMC_NAND_ECC_LP_x8_LAYOUT

[Tom Rini]

On Tue, Mar 11, 2014 at 05:04:45PM +0100, Stefan Roese wrote:

> 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.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Pekon Gupta <pekon@ti.com>
> Cc: Nikita Kiryanov <nikita@compulab.co.il>
> Cc: Igor Grinberg <grinberg@compulab.co.il>
> Cc: Tom Rini <trini@ti.com>
> Acked-by: Igor Grinberg <grinberg@compulab.co.il>

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

-- 
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>

[U-Boot] [PATCH] arm: omap: cm_t35: Fix: Re-add GPMC_NAND_ECC_LP_x8_LAYOUT

[Gupta, Pekon]

>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

[U-Boot] [PATCH] arm: omap: cm_t35: Fix: Re-add GPMC_NAND_ECC_LP_x8_LAYOUT

[Stefan Roese]

Hi Pekon,

On 13.03.2014 12:49, Gupta, Pekon wrote:
>> 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.

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

> 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.

> - 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