[ptxdist] [PATCH] u-boot: Allow specification of padding byte for custom env images

[ptxdist] [PATCH] u-boot: Allow specification of padding byte for custom env images

[Sebastian Muxel]

The env image is currently always padded with the default bytes 0xFF.
New configuration U_BOOT_ENV_IMAGE_CUSTOM_PADDING_BYTES is added
to allow specification for custom env images.

Signed-off-by: Sebastian Muxel <sebastian.muxel@entner-electronics.com>
---
 platforms/u-boot.in | 9 +++++++++
 rules/u-boot.make   | 1 +
 2 files changed, 10 insertions(+)

diff --git a/platforms/u-boot.in b/platforms/u-boot.in
index 769237081..bcabc8b27 100644
--- a/platforms/u-boot.in
+++ b/platforms/u-boot.in
@@ -131,6 +131,15 @@ config U_BOOT_ENV_IMAGE_CUSTOM_SOURCE
 	  The file should have lines in the form var=value, one per line.
 	  Blank lines and lines starting with a # are ignored.
 
+config U_BOOT_ENV_IMAGE_CUSTOM_PADDING_BYTES
+	prompt "Bytes used to pad environment image"
+	hex
+	default "0xFF"
+	depends on U_BOOT_ENV_IMAGE_CUSTOM
+	help
+		Enter the byte used for padding the image to the specified 
+		U-Boot environment size.
+
 if !U_BOOT_ENV_IMAGE_NONE
 
 config U_BOOT_ENV_IMAGE_SIZE
diff --git a/rules/u-boot.make b/rules/u-boot.make
index 4bc7f3f78..6ef657529 100644
--- a/rules/u-boot.make
+++ b/rules/u-boot.make
@@ -123,6 +123,7 @@ ifdef PTXCONF_U_BOOT_ENV_IMAGE_CUSTOM
 		$(call ptx/ifdef,PTXCONF_U_BOOT_ENV_IMAGE_REDUNDANT,-r,) \
 		-s $(PTXCONF_U_BOOT_ENV_IMAGE_SIZE) \
 		-o $(U_BOOT_BUILD_DIR)/u-boot-env.img \
+		-p $(PTXCONF_U_BOOT_ENV_IMAGE_CUSTOM_PADDING_BYTES) \
 		$(U_BOOT_ENV_IMAGE_CUSTOM_SRC)
 endif
 	@$(call touch)
-- 
2.43.0

Re: [ptxdist] [PATCH] u-boot: Allow specification of padding byte for custom env images

[Christian Melki]

On 2/5/24 21:16, Sebastian Muxel wrote:
> [Du får inte e-post ofta från sebastian.muxel@entner-electronics.com. Läs om varför det här är viktigt på https://aka.ms/LearnAboutSenderIdentification ]
> 
> The env image is currently always padded with the default bytes 0xFF.
> New configuration U_BOOT_ENV_IMAGE_CUSTOM_PADDING_BYTES is added
> to allow specification for custom env images.
> 

Hi.

Just curious. In what situation would you need to alter the default
padding bytes? 0xff suits most (if not all) NVM types. Flash transition
layers usually just give an illusion of the traditional zero:ed block on
flash to block translation?

Regards,
Christian

> Signed-off-by: Sebastian Muxel <sebastian.muxel@entner-electronics.com>
> ---
>  platforms/u-boot.in | 9 +++++++++
>  rules/u-boot.make   | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/platforms/u-boot.in b/platforms/u-boot.in
> index 769237081..bcabc8b27 100644
> --- a/platforms/u-boot.in
> +++ b/platforms/u-boot.in
> @@ -131,6 +131,15 @@ config U_BOOT_ENV_IMAGE_CUSTOM_SOURCE
>           The file should have lines in the form var=value, one per line.
>           Blank lines and lines starting with a # are ignored.
> 
> +config U_BOOT_ENV_IMAGE_CUSTOM_PADDING_BYTES
> +       prompt "Bytes used to pad environment image"
> +       hex
> +       default "0xFF"
> +       depends on U_BOOT_ENV_IMAGE_CUSTOM
> +       help
> +               Enter the byte used for padding the image to the specified
> +               U-Boot environment size.
> +
>  if !U_BOOT_ENV_IMAGE_NONE
> 
>  config U_BOOT_ENV_IMAGE_SIZE
> diff --git a/rules/u-boot.make b/rules/u-boot.make
> index 4bc7f3f78..6ef657529 100644
> --- a/rules/u-boot.make
> +++ b/rules/u-boot.make
> @@ -123,6 +123,7 @@ ifdef PTXCONF_U_BOOT_ENV_IMAGE_CUSTOM
>                 $(call ptx/ifdef,PTXCONF_U_BOOT_ENV_IMAGE_REDUNDANT,-r,) \
>                 -s $(PTXCONF_U_BOOT_ENV_IMAGE_SIZE) \
>                 -o $(U_BOOT_BUILD_DIR)/u-boot-env.img \
> +               -p $(PTXCONF_U_BOOT_ENV_IMAGE_CUSTOM_PADDING_BYTES) \
>                 $(U_BOOT_ENV_IMAGE_CUSTOM_SRC)
>  endif
>         @$(call touch)
> --
> 2.43.0
> 
> 

Re: [ptxdist] [PATCH] u-boot: Allow specification of padding byte for custom env images

[Sebastian Muxel]

Hello

>Just curious. In what situation would you need to alter the default
>padding bytes? 0xff suits most (if not all) NVM types. Flash transition
>layers usually just give an illusion of the traditional zero:ed block on
>flash to block translation?

I had to tweak it to 0x00 after noticing that the env image generated by
our vendor supplied SDK has specified it in it's make process. Keeping
it at 0xFF wouldn't allow me to generate a firmware image. It could likely
be that it's a hard requirement of their image generation/flashing tool?
I honestly haven't put much further thought into it and assumed it's due
to the way NAND erasure works

Regards,
Sebastian

Re: [ptxdist] [PATCH] u-boot: Allow specification of padding byte for custom env images

[Christian Melki]

On 2/5/24 21:49, Sebastian Muxel wrote:
> [Du f?r inte e-post ofta fr?n sebastian.muxel@entner-electronics.com. L?s om varf?r det h?r ?r viktigt p? https://aka.ms/LearnAboutSenderIdentification ]
> 
> Hello
> 
>> Just curious. In what situation would you need to alter the default
>> padding bytes? 0xff suits most (if not all) NVM types. Flash transition
>> layers usually just give an illusion of the traditional zero:ed block on
>> flash to block translation?
> 
> I had to tweak it to 0x00 after noticing that the env image generated by
> our vendor supplied SDK has specified it in it's make process. Keeping
> it at 0xFF wouldn't allow me to generate a firmware image. It could likely
> be that it's a hard requirement of their image generation/flashing tool?
> I honestly haven't put much further thought into it and assumed it's due
> to the way NAND erasure works
> 

0x0 looks wrong to me w.r.t most flash media. Also not sure I understand
why 0xff would stop anything from generating a firmware image? That
said, I don't think it matters a lot. It just risks excessive erases the
first time a block is written on media that hasn't got a smart(er) FTL.
It's really not a common thing to want to change, hence the question. :)

For me it's usually the opposite. I want to make sure I don't get fills
with anything else than 0xff.

> Regards,
> Sebastian

Re: [ptxdist] [PATCH] u-boot: Allow specification of padding byte for custom env images

[Sebastian Muxel]

>0x0 looks wrong to me w.r.t most flash media. Also not sure I understand
>why 0xff would stop anything from generating a firmware image?

The tooling screams at me when i don't set it up this way. I'm 100%
certain it's a unfortunate requirement i had to comply with.

You could argue that the change is benefitial as it makes the interface
for mkenvimage more complete, even if the usecase is a rather unusual
one but i understand if it's too special of a case.

Regards,
Sebastian

Re: [ptxdist] [PATCH] u-boot: Allow specification of padding byte for custom env images

[Alexander Dahl]

Hello Sebastian,

regarding your patch itself, please check indentation in Kconfig help.
Maybe run `ptxdist lint` before submitting, IIRC that problem is
checked by the linter.  The indentation is strange there with tabs
first and two spaces then.

(more comments below)

Am Mon, Feb 05, 2024 at 09:49:16PM +0100 schrieb Sebastian Muxel:
> Hello
> 
> > Just curious. In what situation would you need to alter the default
> > padding bytes? 0xff suits most (if not all) NVM types. Flash transition
> > layers usually just give an illusion of the traditional zero:ed block on
> > flash to block translation?
> 
> I had to tweak it to 0x00 after noticing that the env image generated by
> our vendor supplied SDK has specified it in it's make process. Keeping
> it at 0xFF wouldn't allow me to generate a firmware image. It could likely
> be that it's a hard requirement of their image generation/flashing tool?

Interesting.  So this env image does not stay standalone, but will be
integrated in some other packaging?  I'm pretty sure U-Boot itself
does not care about those padding bytes at runtime.  Maybe it's a
thing the flashing tool checks?

> I honestly haven't put much further thought into it and assumed it's due
> to the way NAND erasure works

Probably not.  If you let a raw NAND flash chip erase itself with the
proper command, you get all 0xFF afterwards.  An intelligent flash
tool would recognize larger blocks of 0xFF in an image and then just
not touch the flash in that areas.  Writing 0xFF on already erased
blocks gains nothing.  Writing non 0xFF padding is just useless write
operations and will only decrease the lifetime of the flash.

Greets
Alex

Re: [ptxdist] *****POSSIBLE SPAM***** Re: [PATCH] u-boot: Allow specification of padding byte for custom env images

[Sebastian Muxel]

Hello Alexander,

>regarding your patch itself, please check indentation in Kconfig help.
>Maybe run `ptxdist lint` before submitting, IIRC that problem is
>checked by the linter.  The indentation is strange there with tabs
>first and two spaces then.

Thanks for the feedback, i'll keep it in mind with the next patch i'll
send in.

>Interesting.  So this env image does not stay standalone, but will be
>integrated in some other packaging?  I'm pretty sure U-Boot itself
>does not care about those padding bytes at runtime.  Maybe it's a
>thing the flashing tool checks?

It doesn't stay standalone, no, it's bundled together with various other images
into a "full-firmware" image (i.e. env, SPL, FIT image, config) which then get's
flashed onto the device. 

>Probably not.  If you let a raw NAND flash chip erase itself with the
>proper command, you get all 0xFF afterwards.  An intelligent flash
>tool would recognize larger blocks of 0xFF in an image and then just
>not touch the flash in that areas.  Writing 0xFF on already erased
>blocks gains nothing.  Writing non 0xFF padding is just useless write
>operations and will only decrease the lifetime of the flash.

The padding stops existing inside of the firmware image, so it doesn't
actually get to the point where it would try to write 0x00 onto the NAND
flash.

I honestly thought it's less of a obscure hacky parameter considering
it's one of the few mkenvimage provides but i'll take that it's not of
much use to most people.

Regards,
Sebastian