Re: [U-Boot] [PATCH] omap3: igep00x0: fix NAND_BOOT for igep_0020 and igep_0030
(re-sending as earlier mail got moderated).
Hi Enric,
From: Enric Balletbo Serra [mailto:eballetbo@gmail.com] 2014-04-03 9:09 GMT+02:00 Pekon Gupta pekon@ti.com:
fixes commit e37e954eba3edb5015a0a02880d57517f57425d8 OMAP3: igep00x0: Convert to ti_omap3_common.h.
Above commit introduced compilation error when building igep0020 and igep0030 boards for NAND boot. It removed 'CONFIG_SYS_NAND_U_BOOT_START' which tells SPL/MLO offset of u-boot.img in NAND.
Signed-off-by: Pekon Gupta pekon@ti.com
include/configs/omap3_igep00x0.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/include/configs/omap3_igep00x0.h b/include/configs/omap3_igep00x0.h index 8b8583a..4e87d6b 100644 --- a/include/configs/omap3_igep00x0.h +++ b/include/configs/omap3_igep00x0.h @@ -199,6 +199,8 @@ #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 #define CONFIG_NAND_OMAP_ECCSCHEME OMAP_ECC_HAM1_CODE_HW +#define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE +#define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
These two defines are defined in ti_armv7_common.h. The ti_armv7_common.h is included by ti_omap3_common.h, and omap3_igep00x0.h includes this file. So should not be necessary define this. I tried to build current mainline using igep0020_nand_config and built without errors for me. Which branch are you using ?
My bad, I din't check that one of my local commits had removed above two defines from "ti_armv7_common.h", because these are board-specific configs, and should be in individual board-configs. I got misled because one of your commits (e37e954) removes these two defines and has title "OMAP3: igep00x0: Convert to ti_omap3_common.h."
Sorry about noise, please ignore this patch. I'll merge these changes with my other patch, where I move out board-specific configs from "ti_armv7_common.h"
with regards, pekon
On Thu, Apr 03, 2014 at 09:12:56AM +0000, Gupta, Pekon wrote:
(re-sending as earlier mail got moderated).
Hi Enric,
From: Enric Balletbo Serra [mailto:eballetbo@gmail.com] 2014-04-03 9:09 GMT+02:00 Pekon Gupta pekon@ti.com:
fixes commit e37e954eba3edb5015a0a02880d57517f57425d8 OMAP3: igep00x0: Convert to ti_omap3_common.h.
Above commit introduced compilation error when building igep0020 and igep0030 boards for NAND boot. It removed 'CONFIG_SYS_NAND_U_BOOT_START' which tells SPL/MLO offset of u-boot.img in NAND.
Signed-off-by: Pekon Gupta pekon@ti.com
include/configs/omap3_igep00x0.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/include/configs/omap3_igep00x0.h b/include/configs/omap3_igep00x0.h index 8b8583a..4e87d6b 100644 --- a/include/configs/omap3_igep00x0.h +++ b/include/configs/omap3_igep00x0.h @@ -199,6 +199,8 @@ #define CONFIG_SYS_NAND_ECCSIZE 512 #define CONFIG_SYS_NAND_ECCBYTES 3 #define CONFIG_NAND_OMAP_ECCSCHEME OMAP_ECC_HAM1_CODE_HW +#define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE +#define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
These two defines are defined in ti_armv7_common.h. The ti_armv7_common.h is included by ti_omap3_common.h, and omap3_igep00x0.h includes this file. So should not be necessary define this. I tried to build current mainline using igep0020_nand_config and built without errors for me. Which branch are you using ?
My bad, I din't check that one of my local commits had removed above two defines from "ti_armv7_common.h", because these are board-specific configs, and should be in individual board-configs. I got misled because one of your commits (e37e954) removes these two defines and has title "OMAP3: igep00x0: Convert to ti_omap3_common.h."
Sorry about noise, please ignore this patch. I'll merge these changes with my other patch, where I move out board-specific configs from "ti_armv7_common.h"
Well hold up. Where things need to be in NAND depends on where the SoC is going to check for an initial loader, followed by where it's going to check for redundant copies (if applicable). So you'll have a bit of a job convincing me that we don't want to default to a likely right location (I forget off-hand if bigger NAND pages would push the location higher or not or if we'd just sacrifice total number of possible redundant copies).
Hi Tom,
From: Tom Rini [mailto:tom.rini@gmail.com] On Behalf Of Rini, Tom
On Thu, Apr 03, 2014 at 09:12:56AM +0000, Gupta, Pekon wrote:
[...]
Sorry about noise, please ignore this patch. I'll merge these changes with my other patch, where I move out board-specific configs from "ti_armv7_common.h"
Well hold up. Where things need to be in NAND depends on where the SoC is going to check for an initial loader, followed by where it's going to check for redundant copies (if applicable). So you'll have a bit of a job convincing me that we don't want to default to a likely right location (I forget off-hand if bigger NAND pages would push the location higher or not or if we'd just sacrifice total number of possible redundant copies).
Some CONFIGS_SYS_NAND_xx like (CONFIG_SYS_NAND_U_BOOT_OFFS) are dependent on:
(1) NAND device connected to the board. NAND partitions need to be multiple of block-sizes, so a NAND device having blocksize=128K _may_ have different offset than NAND device with blocksize=256k. Example: Both AM335x_beaglebone and AM335x_EVM use same NAND layout, but they use NAND devices with different block-sizes.
(2) Board users are free to use custom MTD NAND partition layout. And CONFIG_SYS_NAND_U_BOOF_OFFS is dependent on this partition table So, when 'MTDPARTS_DEFAULT' is defined in individual board configs, so this CONFIG_SYS_NAND_U_BOOT_OFFS should also be present in individual board configs.
For other configs which enable generic _driver_ features (like CONFIG_CMD_NAND) can still remain in ti_arm_common.h hope that makes sense ?
with regards, pekon
participants (2)
-
Gupta, Pekon -
Tom Rini