Broken commit d433c74eecdce1e4952ef4e8c712a9289c0dfcc2

[Tom Rini]

On Fri, Dec 16, 2022 at 10:56:39PM +0100, Pali Rohár wrote:
> On Friday 05 August 2022 16:21:24 Pali Rohár wrote:
> > Broken is also commit d433c74eecdce1e4952ef4e8c712a9289c0dfcc2. Seems
> > that all kconfig migration changes done after that commit are broken.
> > 
> > I really do not have energy to investigate what and how was broken due
> > to incorrect kconfig migration.
> > 
> > 
> > I did simple test. Applied following change:
> > 
> > diff --git a/include/configs/p1_p2_rdb_pc.h b/include/configs/p1_p2_rdb_pc.h
> > index a6523753d5ca..489f24df0ab1 100644
> > --- a/include/configs/p1_p2_rdb_pc.h
> > +++ b/include/configs/p1_p2_rdb_pc.h
> > @@ -624,3 +624,7 @@ __stringify(__PCIE_RST_CMD)"\0"
> >  "bootm $norbootaddr - $norfdtaddr"
> >  
> >  #endif /* __CONFIG_H */
> > +
> > +#ifdef CONFIG_SDCARD
> > +#error
> > +#endif
> > 
> > And then called:
> > 
> > make CROSS_COMPILE=powerpc-linux-gnuspe- P2020RDB-PC_defconfig u-boot.bin
> > 
> > And it failed, even when this defconfig file is not SD card builds.
> 
> Tom, that commit d433c74eecdce1e4952ef4e8c712a9289c0dfcc2 is yours.
> Could you please look at it? Because it is a regressions which made P1
> and P2 broken. Based on the past experience it really does not make
> sense to wait for somebody who promised to do something as same
> situation is just repeating.
> 
> Above diff is a simple check to verify if code conversion is correct or
> not. If _before_ conversion CONFIG_SDCARD was not defined then also
> _after_ conversion this macro must not be defined. Right?

I dug through all of this again. I thought I understood what the right
answer was again for a moment, but I don't. You, however, understand
what platforms don't use PBL and what they use instead. I understand
half of the fix, which is to change:
choice
        prompt "Freescale PBL load location"
        depends on RAMBOOT_PBL || ((TARGET_P1010RDB_PA || TARGET_P1010RDB_PB \
                || TARGET_P1020RDB_PC || TARGET_P1020RDB_PD || TARGET_P2020RDB) \
                && !CMD_NAND)

To, I think:
choice
        prompt "Freescale PBL load location"
        depends on RAMBOOT_PBL

Then introduce some new, not "SDCARD" symbol, for the P1/P2 platforms
that don't use PBL but instead the FSL_PREPBL_ESDHC_BOOT_SECTOR logic
you introduced before.

I say I almost thought I had it because I thought this would work:
diff --git a/arch/powerpc/cpu/mpc85xx/Kconfig b/arch/powerpc/cpu/mpc85xx/Kconfig
index 24d3f1f20c25..59740b173b11 100644
--- a/arch/powerpc/cpu/mpc85xx/Kconfig
+++ b/arch/powerpc/cpu/mpc85xx/Kconfig
@@ -15,7 +15,7 @@ config CMD_ERRATA
 config FSL_PREPBL_ESDHC_BOOT_SECTOR
 	bool "Generate QorIQ pre-PBL eSDHC boot sector"
 	depends on MPC85xx
-	depends on SDCARD
+	depends on TARGET_P1020RDB_PC || TARGET_P1020RDB_PD || TARGET_P2020RDB
 	help
 	  With this option final image would have prepended QorIQ pre-PBL eSDHC
 	  boot sector suitable for SD card images. This boot sector instruct
diff --git a/boot/Kconfig b/boot/Kconfig
index 4a001bcee851..d1c9c5f25067 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -725,8 +725,7 @@ config RAMBOOT_PBL
 
 choice
 	prompt "Freescale PBL load location"
-	depends on RAMBOOT_PBL || ((TARGET_P1010RDB_PA || TARGET_P1010RDB_PB \
-		|| TARGET_P1020RDB_PC || TARGET_P1020RDB_PD || TARGET_P2020RDB) \
+	depends on RAMBOOT_PBL || ((TARGET_P1010RDB_PA || TARGET_P1010RDB_PB) \
 		&& !CMD_NAND)
 
 config SDCARD

But no one enables CONFIG_FSL_PREPBL_ESDHC_BOOT_SECTOR. But maybe that
just needs to be enabled on the platforms in question, and then the
first dependency change above is just dropping the SDCARD line? I really
don't know, and I'd be equally happy to just remove all of the P1*/P2*
boards if they don't boot and no one cares to fix them.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20221221/84a0f166/attachment.sig>