boot/uboot: add config option for uboot environment padding byte
PATCH 1/1 boot/uboot: add config option for uboot environment padding byte
Updated the help text for the size of the uboot environment. The size of the uboot environment, which is given in bytes, needs to be exactly correct in order to calculate the CRC checksum as expected and re-calculated by uboot during the target boot process. Otherwise the environment is ignored during boot due to CRC mismatch.
Additionally, add the missing option to specify the padding byte which is used by the mkenvimage -p parameter.
--- boot/uboot/Config.in | 13 +++++++++++-- boot/uboot/uboot.mk | 10 ++++++---- 2 files changed, 17 insertions(+), 6 deletions(-)
boot/uboot/Config.in +11 -2
@@ -413,8 +413,17 @@ config BR2_TARGET_UBOOT_ENVIMAGE_SOURCE config BR2_TARGET_UBOOT_ENVIMAGE_SIZE string "Size of environment" help - Size of envronment, can be prefixed with 0x for hexadecimal - values. + Size of environment in bytes, can be prefixed with 0x for + hexadecimal values. Needs to match exactly for correct CRC + checksum calculation as re-calculated by uboot during each + target boot. If a CRC mismatch occurs during boot the + environment will be ignored. + +config BR2_TARGET_UBOOT_ENVIMAGE_PADDING_BYTE + hex "Padding byte" + default 0x00 + help + The byte used for padding at the end of the environment image. config BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT bool "Environment has two copies"
boot/uboot/uboot.mk +6 -4
@@ -256,10 +256,12 @@ define UBOOT_INSTALL_IMAGES_CMDS ) $(if $(BR2_TARGET_UBOOT_ENVIMAGE), cat $(call qstrip,$(BR2_TARGET_UBOOT_ENVIMAGE_SOURCE)) | \ - $(HOST_DIR)/bin/mkenvimage -s $(BR2_TARGET_UBOOT_ENVIMAGE_SIZE) \ - $(if $(BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT),-r) \ - $(if $(filter BIG,$(BR2_ENDIAN)),-b) \ - -o $(BINARIES_DIR)/uboot-env.bin -) + $(HOST_DIR)/bin/mkenvimage \ + $(if $(BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT),-r) \ + $(if $(filter BIG,$(BR2_ENDIAN)),-b) \ + -p $(BR2_TARGET_UBOOT_ENVIMAGE_PADDING_BYTE) \ + -s $(BR2_TARGET_UBOOT_ENVIMAGE_SIZE) \ + -o $(BINARIES_DIR)/uboot-env.bin -) $(if $(BR2_TARGET_UBOOT_BOOT_SCRIPT), $(HOST_DIR)/bin/mkimage -C none -A $(MKIMAGE_ARCH) -T script \ -d $(call qstrip,$(BR2_TARGET_UBOOT_BOOT_SCRIPT_SOURCE)) \
Signature · 1 line
2.7.4
PATCH 1/1 boot/uboot: add config option for uboot environment padding byte
Hello,
boot/uboot/Config.in config BR2_TARGET_UBOOT_ENVIMAGE_SOURCE · line 417
Quoted 7 lines · Johannes Schmitz
config
BR2_TARGET_UBOOT_ENVIMAGE_SIZEboot/uboot/Config.in +2
-2string "Size of environment" help - Size of envronment, can be prefixed with 0x for hexadecimal - values. + Size of environment in bytes, can be prefixed with 0x for + hexadecimal values. Needs to match exactly for correct CRC
"Needs to match exactly" with what ?
It needs to match with the size of the environment declared in the U-Boot configuration.
boot/uboot/Config.in config BR2_TARGET_UBOOT_ENVIMAGE_SOURCE · line 426
Quoted 5 lines
boot/uboot/Config.in +5
-0+config BR2_TARGET_UBOOT_ENVIMAGE_PADDING_BYTE + hex "Padding byte" + default 0x00 + help + The byte used for padding at the end of the environment image.
I'm still not sure what is the point of customizing the padding byte. Does someone has a use-case for that ? If not, I'm not sure it makes sense to add a Buildroot option for it.
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 boot/uboot: add config option for uboot environment padding byte
boot/uboot/Config.in config BR2_TARGET_UBOOT_ENVIMAGE_SOURCE · line 426
Quoted 5 lines
boot/uboot/Config.in +5
-0+config BR2_TARGET_UBOOT_ENVIMAGE_PADDING_BYTE + hex "Padding byte" + default 0x00 + help + The byte used for padding at the end of the environment image.
I'm still not sure what is the point of customizing the padding byte. Does someone has a use-case for that ? If not, I'm not sure it makes sense to add a Buildroot option for it.
I find it definitely useful for debugging. I wouldn't have been able to isolate the issue with the size and CRC without this option because if you want to compare with hexdump you want everything to exactly match to see what's wrong. Without this, the next person might go down the same way like myself. The default of uboot when saving the environment is 0x00 and the default of mkenvimage is 0xFF. My idea is to make it the same to (0x00) to make the developers life easier. My experience is that any of these details takes some amount of your time to sort out so a less confusing default can safe time for everyone. On the other hand people might want to deliberately set it to 0xFF to exactly check the boarders of the env partition on the SD card, especially during development of genimage.cfg. Furthermore it makes you understand quicker what happens during env image generation. If you hide the option from the user/platform developer again makes life harder. I am speaking from the point of view of a buildroot newcomer. Now of course I now what's going on but would that option have been there it would have saved me one afternoon.
Regards
Johannes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20171212/22cc0c96/attachment.html>
PATCH 1/1 boot/uboot: add config option for uboot environment padding byte
Hello,
Quoted 8 lines · Johannes Schmitz
I find it definitely useful for debugging. I wouldn't have been able to isolate the issue with the size and CRC without this option because if you want to compare with hexdump you want everything to exactly match to see what's wrong. Without this, the next person might go down the same way like myself. The default of uboot when saving the environment is
0x00and the default of mkenvimage is0xFF. My idea is to make it the same to (0x00) to make the developers life easier. My experience is that any of these details takes some amount of your time to sort out so a less confusing default can
Quoted 8 lines
safe time for everyone. On the other hand people might want to deliberately set it to
0xFFto exactly check the boarders of the env partition on the SD card, especially during development of genimage.cfg. Furthermore it makes you understand quicker what happens during env image generation. If you hide the option from the user/platform developer again makes life harder. I am speaking from the point of view of a buildroot newcomer. Now of course I now what's going on but would that option have been there it would have saved me one afternoon.
Hm, ok. I'm still not super convinced, but the code/complexity isn't big enough to really argue more than that :)
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 boot/uboot: add config option for uboot environment padding byte
In reply to Thomas PetazzoniThomas, Johannes, All,
On 2017-12-12 06:27 +0100, Thomas Petazzoni spake thusly:
Quoted 1 line · Johannes Schmitz
[--SNIP--]
Quoted 9 lines
boot/uboot/Config.in config BR2_TARGET_UBOOT_ENVIMAGE_SOURCE · line 426
Quoted 5 lines
boot/uboot/Config.in +5
-0+config BR2_TARGET_UBOOT_ENVIMAGE_PADDING_BYTE + hex "Padding byte" + default 0x00 + help + The byte used for padding at the end of the environment image.I'm still not sure what is the point of customizing the padding byte. Does someone has a use-case for that ? If not, I'm not sure it makes sense to add a Buildroot option for it.
As far as I understand, this is because the U-Boot on Johannes' board would expect the padding byte to be 0x00 when the default for mkenvimage is 0xFF.
I have no idea why U-Boot expects 0x00 in Johannes' case, though. The only thing I can think of is that the 'default' byte value on flash after a page-erase depends on the technology: for NOR, the value after erase ix 0x00, while for NAND, it is 0xFF. So maybe that's the reason...
But any case, as you said in a further reply, adding that new option does not add much complexity.
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 boot/uboot: add config option for uboot environment padding byte
I slightly changed the help text in the last version of the patch. It seems to actually work with other padding bytes if the size is correct. The problem was a combination of those two things.
It's really hard to compare with hexdump if not all the bytes are exactly identical. So setting the padding byte to the same value as uboot save command uses helped me to finally debug and understand the problem.
There was another suggestion to slightly extend the help text regarding the size a bit which I yet have to implement.
Regards
Johannes
Am 13.12.2017 7:23 nachm. schrieb "Yann E. MORIN" <[email protected]>:
Thomas, Johannes, All,
On 2017-12-12 06:27 +0100, Thomas Petazzoni spake thusly:
Quoted 1 line · Johannes Schmitz
[--SNIP--]
Quoted 9 lines
boot/uboot/Config.in config BR2_TARGET_UBOOT_ENVIMAGE_SOURCE · line 426
Quoted 5 lines
boot/uboot/Config.in +5
-0+config BR2_TARGET_UBOOT_ENVIMAGE_PADDING_BYTE + hex "Padding byte" + default 0x00 + help + The byte used for padding at the end of the environment image.I'm still not sure what is the point of customizing the padding byte. Does someone has a use-case for that ? If not, I'm not sure it makes sense to add a Buildroot option for it.
As far as I understand, this is because the U-Boot on Johannes' board would expect the padding byte to be 0x00 when the default for mkenvimage is 0xFF.
I have no idea why U-Boot expects 0x00 in Johannes' case, though. The only thing I can think of is that the 'default' byte value on flash after a page-erase depends on the technology: for NOR, the value after erase ix 0x00, while for NAND, it is 0xFF. So maybe that's the reason...
But any case, as you said in a further reply, adding that new option does not add much complexity.
Regards,
Yann E. MORIN.
--
.-----------------.--------------------.------------------.-
-------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^-
-------------------'
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20171213/7e8dd683/attachment.html>
PATCH 1/1 boot/uboot: add config option for uboot environment padding byte
In reply to Yann E. MORINHello,
Quoted 7 lines · Yann E. MORIN
Quoted 3 lines
I'm still not sure what is the point of customizing the padding byte. Does someone has a use-case for that ? If not, I'm not sure it makes sense to add a Buildroot option for it.
As far as I understand, this is because the U-Boot on Johannes' board would expect the padding byte to be
0x00when the default for mkenvimage is0xFF.
No, U-Boot does not care at all what the padding byte is. Johannes board works just fine with the default padding byte of 0xff. All what matters for U-Boot is that the CRC is correct.
Obviously if your environment image has a size of X bytes, its CRC is calculated over X bytes. Then if the U-Boot configuration says the environment has Y bytes, it calculates the CRC on Y bytes. So if X != Y, you're screwed, as the CRCs won't match.
Johannes problem was *not* a padding byte problem, but *only* a wrong environment size.
Quoted 1 line
I have no idea why U-Boot expects
0x00in Johannes' case, though.
It does not. Johannes' U-Boot works fine with 0xff as the padding byte.
Quoted 6 lines
The only thing I can think of is that the 'default' byte value on flash after a page-erase depends on the technology: for NOR, the value after erase ix
0x00, while for NAND, it is0xFF. So maybe that's the reason...But any case, as you said in a further reply, adding that new option does not add much complexity.
But it serves no purpose whatsoever :-)
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
A message that is not in the archive (sent off-list or not yet received): CAMbDF3J=j94Ze6LVJWnA7N453JMOP60-SzjLBdrB6n5s_aSkPw@mail.gmail.com
PATCH 1/1 boot/uboot: add config option for uboot environment padding byte
Am 14.12.2017 7:13 vorm. schrieb "Thomas Petazzoni" < [email protected]>:
Hello,
Quoted 3 lines · Yann E. MORIN
But any case, as you said in a further reply, adding that new option does not add much complexity.
But it serves no purpose whatsoever :-)
I disagree with this but maybe it's my electrical engineering background, I always want full control down to the physical layer.
And mostly there is a use case even if I don't know all of them myself.
I have a question though, is 32k the default uboot environment size? Then I could also set this as a default. Otherwise again it is a good argument for keeping the padding option in for debugging purposes.
Regards
Johannes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20171214/a3be6725/attachment.html>
PATCH 1/1 boot/uboot: add config option for uboot environment padding byte
Hello,
Quoted 3 lines · Johannes Schmitz
I disagree with this but maybe it's my electrical engineering background, I always want full control down to the physical layer.
And mostly there is a use case even if I don't know all of them myself.
If Buildroot starts to support use cases that we don't even understand when they are needed, then we are going on a very, very slippery road.
Quoted 3 lines
I have a question though, is 32k the default uboot environment size? Then I could also set this as a default. Otherwise again it is a good argument for keeping the padding option in for debugging purposes.
No, there is nothing like a default environment size. Each board has its own value in its U-boot configuration file.
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 boot/uboot: add config option for uboot environment padding byte
Quoted 11 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Thomas" == Thomas Petazzoni <[email protected]> writes:
Hello,
Quoted 3 lines · Johannes Schmitz
I disagree with this but maybe it's my electrical engineering background, I always want full control down to the physical layer.
And mostly there is a use case even if I don't know all of them myself.If Buildroot starts to support use cases that we don't even understand when they are needed, then we are going on a very, very slippery road.
I agree. I was marked this patch as rejected in patchwork.
Signature · 1 line
Bye, Peter Korsgaard