[PATCH 0/5] CMD_SAVEENV ifdef cleanup

[Wolfgang Denk]

Dear Rasmus Villemoes,

In message <9c03710e-5eec-da6e-6c15-2f8a14cfcc36 at prevas.dk> you wrote:
>
> Can I ask whether you request changes to this patch series or if my
> answers to your various comments have been satisfactory?

I think you did no really answer to some of my concerns.

In Message <20200219132715.1F81A240036 at gemini.denx.de> I asked:

| Have you tested that this works?  How do the sizes of the
| images differe before and after applying your changes?

You replied:

...
Now also enable CONFIG_SPL_SAVEENV and SPL_FAT_WRITE, then with my
patches we get

| $ size u-boot spl/u-boot-spl
|    text    data     bss     dec     hex filename
|  407173   45308   98352  550833   867b1 u-boot
|   58298    3360   65860  127518   1f21e spl/u-boot-spl
| ....
| but without,
| 
| $ size u-boot spl/u-boot-spl
|    text    data     bss     dec     hex filename
|  407173   45308   98352  550833   867b1 u-boot
|   52659    3360     280   56299    dbeb spl/u-boot-spl

We can observe that

- the text size of the SPL grows from 52659 to 58298, i. e. by about
  5.5 kB or more than 10%
- the BSS size explodes from 280 to 65860 bytes, i. e. it grows from
  a few hndet bytes to more than 64 kB

I can see where the increase in text size is coming from - your
removal of #ifdef's now unconditionally includes some code that was
omitted before, for example functions env_fat_save(),
env_ext4_save(), env_sf_save(), plus a few variables.

It is not obvious to me but scary to see such an explosion of BSS
size.

It's difficult to comment here as it is not clear to me which exact
configuration you reported about, and it's also not clear if this is
a typical result, of if it's the only configuration you ever
tested.


Your patch description sounds as if it was just a #ifdef cleanup
without actual impact on the generated code, but the SPL size
differences above make it clear that it is not - or that your
testing has issues.

You also failed to comment on impact on other environment storage
configurations (NOR flash, NAND flash, UBI volume, ...).  If it's
only #ifdef changes without impact on function, then we should get
exactly the same images.  You did not comment if you have verified
that.

Best regards,

Wolfgang Denk

-- 
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
All easy problems have already been solved.