[PATCH] mx6qsabrelite: increase offset for environment on SPI

[Denis Pynkin]

14.09.2020 17:11, Tom Rini wrote:
> On Mon, Sep 14, 2020 at 09:29:18AM -0400, Tom Rini wrote:
>> On Mon, Sep 14, 2020 at 04:20:20PM +0300, Denis Pynkin wrote:
>>> 14.09.2020 15:33, Tom Rini wroteт:
>>>> On Mon, Sep 14, 2020 at 08:03:33AM -0300, Fabio Estevam wrote:
>>>
>>>>>> The size of the binary created with the default U-boot config is much
>>>>>> greater than the default offset for environment `0x60000`. If the new
>>>>>> version is flashed onto SPI it overlaps with the stored environment.
>>>>>> This leads to U-Boot corruption in case of saving environment onto
>>>>>> SPI and non-bootable SabreLite board.
>>>>>>
>>>>>> Signed-off-by: Denis Pynkin <denis.pynkin at collabora.com>
>>>>>
>>>>> Reviewed-by: Fabio Estevam <festevam at gmail.com>
>>>>>
>>>>> In case you want to detect errors like this again in the future, you could add
>>>>> CONFIG_BOARD_SIZE_LIMIT that detects such overlaps in build-time.
>>>>>
>>>>> Please check commit 033f6ea5fa5f ("mx53loco: Fix U-Boot corruption
>>>>> after saving the environment")
>>>>>
>>>>> The addition of CONFIG_BOARD_SIZE_LIMIT can be a separate patch though.
>>>>
>>>> Hold on.  Can we shrink the board back down so that we don't need to
>>>> move the environment?  Moving the environment is bad as it will also
>>>> break existing users.  Thanks!
>>>
>>> I don't think so to be honest.
>>> Current sizes even for `u-boot-nodtb.imx` in my builds are:
>>> - 595308 (0x9156C) -- cross-compilation
>>> - 470780 (0x72EFC) -- native build
>>
>> I'd like to have one thread where we see what on earth is going on
>> there.  That's a rather crazy size difference and very troubling.
>>
>>> which is much larger than current offset 393216 (0x60000)
>>>
>>> The offset for environment `CONFIG_ENV_OFFSET=0x60000` were added in
>>> commit a09fea1 just a year ago.
>>> Not sure if it was tested with SabreLite board tbh -- this is a bulk
>>> commit aimed for ARMv8 and sizes of binaries produced with pre-a09fea1
>>> commit are also larger than the current offset.
>>
>> Ah, so here's what's going on then.  Commit a09fea1 wasn't about ARMv8,
>> it was about migrating options to the defconfig files.  In this case, it
>> should have been set to the value you're changing it to now, at least
>> from a read of the patch (include/configs/mx6sabre_common.h would set it
>> to 0xC0000 if CONFIG_ENV_IS_IN_MMC), but I thought I had done that
>> migration with my hack to make a tool that had the in-use value be
>> printed.  So I'm going to re-check that whole thing to see what else
>> might be wrong as well.  Thanks!
> 
> Ah, so, mx6qsabrelite falls in to the "nitrogen6x" family and not the
> rest of the "sabre" board families.  As such, it's always had the env
> offset of 0x60000.  Jumping back somewhat arbitrarily to v2014.10, I see
> with gcc-4.9 a size of 404480 on u-boot.imx which is still bigger.
> 
> The commit message isn't clear however, as environment is in MMC and not
> SPI, so SPI booting should be fine.  But MMC/eMMC is where this case can
> happen (I'm not sure which device is SD slot and which is eMMC on this
> hardware off-hand).  Thanks!

To my shame -- I didn't even thought about the booting from MMC, but yes
-- should be the same issue in case if U-Boot is placed on SD-card.

This device have 2MB SPI NOR flash there the U-Boot could be flashed
with an environment. We are using this approach, so I attempt to
describe the issue from this point of view.

Should I try to re-word the description? Or may I ask you to add a
better description?

-- 
wbr, Denis