On Thu, Dec 22, 2022 at 08:49:47AM +0100, Pali Rohár wrote: > On Wednesday 21 December 2022 21:54:15 Tom Rini wrote: > > 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. > > P2020RDB-PC_defconfig is for booting from FLASH NOR, not from SD card. > CONFIG_SDCARD for P1/P2 must be defined when booting from SD card. > > Before commit d433c74eecdce1e4952ef4e8c712a9289c0dfcc2 everything worked > fine and CONFIG_SDCARD was not defined for P2020RDB-PC_defconfig. After > commit d433c74eecdce1e4952ef4e8c712a9289c0dfcc2, symbol CONFIG_SDCARD is > defined. > > You can check it by adding into config.h: > > +#ifdef CONFIG_SDCARD > +#error > +#endif > > > 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 > > No, original code was OK. As is written in description > FSL_PREPBL_ESDHC_BOOT_SECTOR is for writing bootsector to SD card. And > it is optional as there is other way how to generate it, as described in > some doc/ file. > > But if you choose to compile u-boot for P2020RDB-PC_defconfig (NOR > FLASH) then there is no SD card booting and hence > FSL_PREPBL_ESDHC_BOOT_SECTOR for P2020RDB-PC_defconfig must never be > generated. So FSL_PREPBL_ESDHC_BOOT_SECTOR must depends on SDCARD. > > > 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. > > It is optional _user_ symbol. During compilation of sd card version of > u-boot, user can enable it. > > For turris 1.x board there is waiting patch on the list which uses it. > No review yet? > > > 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 > > Just fix the conversion process. The rule is simple: if config.h did > not have enabled CONFIG_SDCARD then after conversion config.h must not > have it enabled. > > Compare git checkout d433c74eecdce1e4952ef4e8c712a9289c0dfcc2~1 > and git checkout d433c74eecdce1e4952ef4e8c712a9289c0dfcc2 > > Because it worked fine before commit > d433c74eecdce1e4952ef4e8c712a9289c0dfcc2.
Thanks for explaining. Since you clearly understand what it should do, and I do not, please submit something to implement the correct questions in Kconfig. There was some overloading of a symbol before which I didn't understand, and still don't exactly. Otherwise, since I believe you're saying the Turris platform is fine, once Marek merges it (you will likely need to rebase on next, next week, once I finish merging all of the CONFIG migration stuff), I'll just delete the P1/P2 platforms sometime in the next release if no one actually cares to fix them. -- Tom
signature.asc
Description: PGP signature