Skip to contentSkip to conversation

pico-imx7d: Increase the CONFIG_ENV_OFFSET size

PATCH Superseded Fabio Estevam 3 participants 6 messages Nov 30, 2018 Arm i.MX and Layerscape

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Fabio Estevam

author reviewer
#0 lore raw

PATCH v2 1/1 pico-imx7d: Increase the CONFIG_ENV_OFFSET size

U-Boot binary has grown in such a way that it goes beyond the reserved area for the environment variables.

Running "saveenv" causes U-Boot to hang because of this overlap.

Fix this problem by increasing the CONFIG_ENV_OFFSET size.

Signed-off-by Fabio Estevam <[email protected]>

---
Changes since v1:
- Reword the commit log (Stefano)

 include/configs/pico-imx7d.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/configs/pico-imx7d.h +1 -1
index 2bc42a0..120111d 100644
@@ -134,7 +134,7 @@
 /* FLASH and environment organization */
 #define CONFIG_ENV_SIZE			SZ_8K
 
-#define CONFIG_ENV_OFFSET			(8 * SZ_64K)
+#define CONFIG_ENV_OFFSET			(12 * SZ_64K)
 #define CONFIG_SYS_FSL_USDHC_NUM		2
 
 #define CONFIG_SYS_MMC_ENV_DEV			0
Signature · 1 line
2.7.4
#1 lore raw

PATCH v2 1/1 pico-imx7d: Increase the CONFIG_ENV_OFFSET size

Hi Fabio,

Quoted 20 lines · Fabio Estevam

U-Boot binary has grown in such a way that it goes beyond the reserved area for the environment variables.

Running "saveenv" causes U-Boot to hang because of this overlap.

Fix this problem by increasing the CONFIG_ENV_OFFSET size.

Signed-off-by Fabio Estevam <[email protected]>

---
Changes since v1:
- Reword the commit log (Stefano)

  include/configs/pico-imx7d.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
include/configs/pico-imx7d.h +0 -0
index 2bc42a0..120111d 100644

include/configs/pico-imx7d.h line 137

Quoted 5 lines
include/configs/pico-imx7d.h +1 -1
@@ -134,7 +134,7 @@
  /* FLASH and environment organization */
  #define CONFIG_ENV_SIZE			SZ_8K
 
-#define CONFIG_ENV_OFFSET			(8 * SZ_64K)
+#define CONFIG_ENV_OFFSET			(12 * SZ_64K)

I find this hard to read: (12 * SZ_64K). Wouldn't (768 * SZ_1K) be better?

And I found using CONFIG_BOARD_SIZE_LIMIT quite handy, as it detects such increased image growths beyond its limit upon build time. Perhaps you could add this as well? This should do it for this board:

#define CONFIG_BOARD_SIZE_LIMIT CONFIG_ENV_OFFSET

Thanks,
Stefan

author reviewer
#2 lore raw

PATCH v2 1/1 pico-imx7d: Increase the CONFIG_ENV_OFFSET size

Hi Stefan,

Quoted 5 lines · Stefan Roese

include/configs/pico-imx7d.h line 137

Quoted 2 lines
include/configs/pico-imx7d.h +1 -1
-#define CONFIG_ENV_OFFSET                    (8 * SZ_64K)
+#define CONFIG_ENV_OFFSET                    (12 * SZ_64K)

I find this hard to read: (12 * SZ_64K). Wouldn't (768 * SZ_1K) be better?

Yes, this is a good idea, but it does not work in conjunction with your suggestion below.

Quoted 5 lines

And I found using CONFIG_BOARD_SIZE_LIMIT quite handy, as it detects such increased image growths beyond its limit upon build time. Perhaps you could add this as well? This should do it for this board:

#define CONFIG_BOARD_SIZE_LIMIT CONFIG_ENV_OFFSET

Thanks for this suggestion.

I was not aware of CONFIG_BOARD_SIZE_LIMIT, which is very helpful!

The only issue I see with this approach is that it does not accept expressions like (768 * SZ_1K):

SYM u-boot.sym
/bin/sh: 1: printf: (768 * SZ_1K): expected numeric value u-boot-nodtb.bin exceeds file size limit:
limit: 0 bytes
actual: 482952 bytes
excess: 482952 bytes
Makefile:1040: recipe for target 'u-boot-nodtb.bin' failed

So I will hardcode CONFIG_ENV_OFFSET and use CONFIG_BOARD_SIZE_LIMIT:

#define CONFIG_ENV_OFFSET 786432
/* Detect overlap between U-Boot image and environment area in build-time */ #define CONFIG_BOARD_SIZE_LIMIT CONFIG_ENV_OFFSET

IMHO it is better to get an error in build time when U-Boot and env area overlaps than providing a pretty CONFIG_ENV_OFFSET format :-)

Thanks!

#3 lore raw

PATCH v2 1/1 pico-imx7d: Increase the CONFIG_ENV_OFFSET size

Hi Fabio,

Quoted 15 lines · Fabio Estevam
Quoted 5 lines · Stefan Roese

include/configs/pico-imx7d.h line 137

Quoted 2 lines
include/configs/pico-imx7d.h +1 -1
-#define CONFIG_ENV_OFFSET                    (8 * SZ_64K)
+#define CONFIG_ENV_OFFSET                    (12 * SZ_64K)

I find this hard to read: (12 * SZ_64K). Wouldn't (768 * SZ_1K) be better?

Yes, this is a good idea, but it does not work in conjunction with your suggestion below.

Quoted 4 lines

And I found using CONFIG_BOARD_SIZE_LIMIT quite handy, as it detects such increased image growths beyond its limit upon build time. Perhaps you could add this as well? This should do it for this board:

Quoted 5 lines
Quoted 1 line

#define CONFIG_BOARD_SIZE_LIMIT CONFIG_ENV_OFFSET

Thanks for this suggestion.

I was not aware of CONFIG_BOARD_SIZE_LIMIT, which is very helpful!

Yes, I also only found out about this a few months ago. Always something new to learn... ;)

Quoted 11 lines

The only issue I see with this approach is that it does not accept expressions like (768 * SZ_1K):

SYM u-boot.sym
/bin/sh: 1: printf: (768 * SZ_1K): expected numeric value u-boot-nodtb.bin exceeds file size limit:

   limit:  0 bytes
   actual: 482952 bytes
   excess: 482952 bytes

Makefile:1040: recipe for target 'u-boot-nodtb.bin' failed

Quoted 8 lines

So I will hardcode CONFIG_ENV_OFFSET and use CONFIG_BOARD_SIZE_LIMIT:

#define CONFIG_ENV_OFFSET 786432
/* Detect overlap between U-Boot image and environment area in build-time */ #define CONFIG_BOARD_SIZE_LIMIT CONFIG_ENV_OFFSET

IMHO it is better to get an error in build time when U-Boot and env area overlaps than providing a pretty CONFIG_ENV_OFFSET format :-)

I agree. I'm using hex values like this 0x80000, which look better to my eyes (personal taste). Perhaps the Makefile logic handling BOARD_LIMIT_SIZE can be improved to support other expressions as well.

Thanks,
Stefan

#4 lore raw

PATCH v2 1/1 pico-imx7d: Increase the CONFIG_ENV_OFFSET size

In reply to Fabio Estevam

Dear Fabio,

Quoted 3 lines · In message <CAOMZO5B3Dbub=XVMLKgDUQhkS7to8YsJeqsBmJE56y+itA2aMg@mail.gmail.com>…
Quoted 3 lines

I find this hard to read: (12 * SZ_64K). Wouldn't (768 * SZ_1K) be better?

...

Quoted 2 lines

The only issue I see with this approach is that it does not accept expressions like (768 * SZ_1K):

This is why I always prefer to use expressions that are both readable and computable, like (768 << 10).

Quoted 1 line

#define CONFIG_ENV_OFFSET 786432

Would

#define CONFIG_ENV_OFFSET (768 << 10)

or@least

#define CONFIG_ENV_OFFSET (768 * 1024)

work?

Quoted 2 lines

/* Detect overlap between U-Boot image and environment area in build-time */ #define CONFIG_BOARD_SIZE_LIMIT CONFIG_ENV_OFFSET

Note this is only correct if the U-Boot image gets written at offset zero - is this the case?

Quoted 2 lines

IMHO it is better to get an error in build time when U-Boot and env area overlaps than providing a pretty CONFIG_ENV_OFFSET format :-)

Agreed!

[Please ignore my previous message - working through the list sequentially is not optimal...]

Best regards,

Wolfgang Denk

Signature · 6 lines
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
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
"My name is Linus Torvalds, you messed with my kernel, prepare to die"
                                                  - Linus Torvalds in
      <[email protected]>
author reviewer
#5 lore raw

PATCH v2 1/1 pico-imx7d: Increase the CONFIG_ENV_OFFSET size

Hi Wolfgang,

Quoted 7 lines · Wolfgang Denk

This is why I always prefer to use expressions that are both readable and computable, like (768 << 10).

Quoted 1 line

#define CONFIG_ENV_OFFSET 786432

Would

Quoted 7 lines

#define CONFIG_ENV_OFFSET (768 << 10)

or at least

#define CONFIG_ENV_OFFSET (768 * 1024)

work?

CONFIG_BOARD_SIZE_LIMIT does not accept such definitions and it fails like this:

/bin/sh: 1: printf: (768 << 10): expected numeric value u-boot-nodtb.bin exceeds file size limit:
limit: 0 bytes
actual: 482952 bytes
excess: 482952 bytes

That is the reason I used #define CONFIG_ENV_OFFSET 786432

Quoted 5 lines
Quoted 2 lines

/* Detect overlap between U-Boot image and environment area in build-time */ #define CONFIG_BOARD_SIZE_LIMIT CONFIG_ENV_OFFSET

Note this is only correct if the U-Boot image gets written at offset zero - is this the case?

That is a good point!

In this case SPL is flashed at offset 1k and u-boot.img is flashed at offset 69k.

So I guess I should hardcode CONFIG_BOARD_SIZE_LIMIT to 768k - 69k = 699k = 715776.

#define CONFIG_ENV_OFFSET 715776
/* Detect overlap between U-Boot image and environment area in build-time */ #define CONFIG_BOARD_SIZE_LIMIT CONFIG_ENV_OFFSET