Broken commit de47ff536363289f92f85ed1e4901724d238432d

[Pali Rohár]

And back to this issue...

On Tuesday 02 August 2022 11:13:38 Pali Rohár wrote:
> Hello Tom!
> 
> Your commit de47ff536363289f92f85ed1e4901724d238432d ("Convert
> CONFIG_SYS_MPC85XX_NO_RESETVEC to Kconfig") seems to be broken.
> 
> If you look at P1020RDB-PD_defconfig file change in this commit there is:
> 
> --- a/configs/P1020RDB-PD_defconfig
> +++ b/configs/P1020RDB-PD_defconfig
> @@ -9,6 +9,7 @@ CONFIG_MPC85xx=y
>  # CONFIG_CMD_ERRATA is not set
>  CONFIG_TARGET_P1020RDB_PD=y
>  CONFIG_MPC85XX_HAVE_RESET_VECTOR=y
> +CONFIG_SYS_MPC85XX_NO_RESETVEC=y
>  CONFIG_MP=y
>  CONFIG_FIT=y
>  CONFIG_FIT_VERBOSE=y
> 
> Which does not make sense to me.
> 
> First thing is that CONFIG_MPC85XX_HAVE_RESET_VECTOR and
> CONFIG_SYS_MPC85XX_NO_RESETVEC are exclusive options. You can either
> disable generating of reset vector in image or enable it. What is
> expected from the result when you ask Kconfig to both enable and disable
> it? First specified option win? Or last specified win? Or random of
> those two options win? It is not really clear for me.

Experiments proved that CONFIG_SYS_MPC85XX_NO_RESETVEC wins over
CONFIG_MPC85XX_HAVE_RESET_VECTOR. So problematic commit
de47ff536363289f92f85ed1e4901724d238432d effectively disabled reset
vectors in more defconfig files.

> Second thing is that reset vector is required for (parallel) NOR booting
> and your change is adding CONFIG_SYS_MPC85XX_NO_RESETVEC=y to defconfig
> for NOR, which to my guess make image non-bootable and broken.

And this is truth. CONFIG_SYS_MPC85XX_NO_RESETVEC=y in defconfig broke
booting from parallel FLASH NOR memory. Without reset vector, u-boot
from FLASH cannot be booted.

When I manually disabled CONFIG_SYS_MPC85XX_NO_RESETVEC for P2020 then
together with CONFIG_SDCARD fix, I was able to boot U-Boot from FLASH.

So kconfig conversion in commit de47ff536363289f92f85ed1e4901724d238432d
was done incorrectly. Because in 2022.04 CONFIG_SYS_MPC85XX_NO_RESETVEC
was really not enabled in config.h for FLASH defconfigs.

> And seems that other defconfig files in that change have similar issues.

Tom, would you fix this commit de47ff536363289f92f85ed1e4901724d238432d
too? I do not know how you did that kconfig conversion but fix could be
straightforward. By boolean logic CONFIG_MPC85XX_HAVE_RESET_VECTOR xor
CONFIG_SYS_MPC85XX_NO_RESETVEC can be defined. Not both at the same
time.

By moveconfig.py following defconfigs are affected:

$ ./tools/moveconfig.py -f MPC85XX_HAVE_RESET_VECTOR SYS_MPC85XX_NO_RESETVEC
9 matches
P1020RDB-PC P1010RDB-PB_36BIT_NOR P1010RDB-PA_36BIT_NOR P1020RDB-PC_36BIT P1010RDB-PA_NOR P1010RDB-PB_NOR P2020RDB-PC P2020RDB-PC_36BIT P1020RDB-PD

All of these defconfigs (by their names) boot from FLASH nor, so they
must have reset vector included and so *NO_RESETVEC* must *not* be
enabled. (Hope it is clear even with too many negations)