Dear Rasmus Villemoes,
In message 9c03710e-5eec-da6e-6c15-2f8a14cfcc36@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@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