83xx: fix CFG_ENV_ADDR and CFG_ENV_SECT_SIZE definitions
Fix the definitions of CFG_ENV_ADDR and CFG_ENV_SECT_SIZE for all the 83xx boards. This change guarantees that the environment will be located on the first flash sector after the U-Boot image.
--- include/configs/MPC8313ERDB.h | 2 +- include/configs/MPC8323ERDB.h | 4 ++-- include/configs/MPC832XEMDS.h | 4 ++-- include/configs/MPC8349EMDS.h | 2 +- include/configs/MPC8349ITX.h | 2 +- include/configs/MPC8360EMDS.h | 4 ++-- 6 files changed, 9 insertions(+), 9 deletions(-)
include/configs/MPC8313ERDB.h +1 -1
@@ -326,7 +326,7 @@*/ #ifndef CFG_RAMBOOT #define CFG_ENV_IS_IN_FLASH 1 -#define CFG_ENV_ADDR (CFG_MONITOR_BASE + 0x40000) +#define CFG_ENV_ADDR (CFG_MONITOR_BASE + CFG_MONITOR_LEN)#define CFG_ENV_SECT_SIZE 0x10000/* 64K(one sector) for env */ #define CFG_ENV_SIZE 0x2000
include/configs/MPC8323ERDB.h +2 -2
@@ -354,8 +354,8 @@*/ #ifndef CFG_RAMBOOT #define CFG_ENV_IS_IN_FLASH 1 -#define CFG_ENV_ADDR (CFG_MONITOR_BASE + 0x40000) -#define CFG_ENV_SECT_SIZE 0x40000/* 256K(one sector) for env */ +#define CFG_ENV_ADDR (CFG_MONITOR_BASE + CFG_MONITOR_LEN) +#define CFG_ENV_SECT_SIZE 0x20000#define CFG_ENV_SIZE 0x2000#else #define CFG_NO_FLASH 1/* Flash is not usable now */
include/configs/MPC832XEMDS.h +2 -2
@@ -410,8 +410,8 @@*/ #ifndef CFG_RAMBOOT #define CFG_ENV_IS_IN_FLASH 1 -#define CFG_ENV_ADDR (CFG_MONITOR_BASE + 0x40000) -#define CFG_ENV_SECT_SIZE 0x40000/* 256K(one sector) for env */ +#define CFG_ENV_ADDR (CFG_MONITOR_BASE + CFG_MONITOR_LEN) +#define CFG_ENV_SECT_SIZE 0x20000#define CFG_ENV_SIZE 0x2000#else #define CFG_NO_FLASH 1/* Flash is not usable now */
include/configs/MPC8349EMDS.h +1 -1
@@ -456,7 +456,7 @@*/ #ifndef CFG_RAMBOOT #define CFG_ENV_IS_IN_FLASH 1 -#define CFG_ENV_ADDR (CFG_MONITOR_BASE + 0x40000) +#define CFG_ENV_ADDR (CFG_MONITOR_BASE + CFG_MONITOR_LEN)#define CFG_ENV_SECT_SIZE 0x20000/* 128K(one sector) for env */ #define CFG_ENV_SIZE 0x2000
include/configs/MPC8349ITX.h +1 -1
@@ -404,8 +404,8 @@ boards, we say we have two, but don't display a message if we find only one. */#ifndef CFG_RAMBOOT #define CFG_ENV_IS_IN_FLASH +#define CFG_ENV_ADDR (CFG_MONITOR_BASE + CFG_MONITOR_LEN)#define CFG_ENV_SECT_SIZE 0x10000/* 64K (one sector) for environment */ -#define CFG_ENV_ADDR (CFG_MONITOR_BASE + (4 * CFG_ENV_SECT_SIZE))#define CFG_ENV_SIZE 0x2000#else #define CFG_NO_FLASH /* Flash is not usable now */
include/configs/MPC8360EMDS.h +2 -2
@@ -443,8 +443,8 @@#ifndef CFG_RAMBOOT #define CFG_ENV_IS_IN_FLASH 1 -#define CFG_ENV_ADDR (CFG_MONITOR_BASE + 0x40000) -#define CFG_ENV_SECT_SIZE 0x40000/* 256K(one sector) for env */ +#define CFG_ENV_ADDR (CFG_MONITOR_BASE + CFG_MONITOR_LEN) +#define CFG_ENV_SECT_SIZE 0x20000#define CFG_ENV_SIZE 0x2000#else #define CFG_NO_FLASH 1/* Flash is not usable now */
Signature · 1 line
1.5.2.4
Review
Quoted 7 lines · Timur Tabi
Fix the definitions of
CFG_ENV_ADDRandCFG_ENV_SECT_SIZEfor all the 83xx boards. This change guarantees that the environment will be located on the first flash sector after the U-Boot image.Signed-off-by Timur Tabi <[email protected]>(quoted diff) +1
-0+ #define CFG_ENV_ADDR (CFG_MONITOR_BASE + CFG_MONITOR_LEN)
Hi Timur,
IMHO this would work only if the sum of CFG_MONITOR_BASE + CFG_MONITOR_LEN is sector aligned. Did you set CFG_MONITOR_LEN appropriately? (Just to avoid unexpected behaviour)
Best regards
Jens
Quoted 4 lines · Jens Gehrlein
IMHO this would work only if the sum of
CFG_MONITOR_BASE+CFG_MONITOR_LENis sector aligned. Did you setCFG_MONITOR_LENappropriately? (Just to avoid unexpected behaviour)
I'm under the assumption that CFG_MONITOR_LEN is always supposed to be a multiple of the sector size. Should I add comments to the header files to state that explicitly?
Signature · 2 lines
Timur Tabi Linux kernel developer at Freescale
Quoted 11 lines · Timur Tabi
Quoted 4 lines · Jens Gehrlein
IMHO this would work only if the sum of
CFG_MONITOR_BASE+CFG_MONITOR_LENis sector aligned. Did you setCFG_MONITOR_LENappropriately? (Just to avoid unexpected behaviour)I'm under the assumption that
CFG_MONITOR_LENis always supposed to be a multiple of the sector size. Should I add comments to the header files to state that explicitly?
It's just, because I saw implementations, where CFG_MONITOR_LEN was an arbitrary value. It depends on how the developer interpretes this define and how it is used in the cpu and board specific code.
Maybe a comment would help, but I leave the decision to you.
Best regards
Jens
Quoted 5 lines · In message <[email protected]> you wrote:
Fix the definitions of
CFG_ENV_ADDRandCFG_ENV_SECT_SIZEfor all the 83xx boards. This change guarantees that the environment will be located on the first flash sector after the U-Boot image.Signed-off-by Timur Tabi <[email protected]>
Quoted 8 lines
--- include/configs/MPC8313ERDB.h | 2 +- include/configs/MPC8323ERDB.h | 4 ++-- include/configs/MPC832XEMDS.h | 4 ++-- include/configs/MPC8349EMDS.h | 2 +- include/configs/MPC8349ITX.h | 2 +- include/configs/MPC8360EMDS.h | 4 ++-- 6 files changed, 9 insertions(+), 9 deletions(-)
The comment is clearly wrong. Thsi does not apply to *all* 83xx boards (there is a whole world outside of FSL, actually). Please fix and resubmit.
Best regards,
Wolfgang Denk
Signature · 4 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Time is an illusion perpetrated by the manufacturers of space.
Quoted 3 lines · Wolfgang Denk
The comment is clearly wrong. Thsi does not apply to *all* 83xx boards (there is a whole world outside of FSL, actually). Please fix and resubmit.
I said "all the 83xx boards", not "all 83xx" boards. Technically, there's a difference. However, I see your point and I will reword the comment.