Broken commit de47ff536363289f92f85ed1e4901724d238432d

[Tom Rini]

On Fri, Aug 05, 2022 at 05:51:35PM +0200, Pali Rohár wrote:
> On Friday 05 August 2022 11:44:00 Tom Rini wrote:
> > On Fri, Aug 05, 2022 at 05:12:59PM +0200, Pali Rohár wrote:
> > > On Friday 05 August 2022 11:03:40 Tom Rini wrote:
> > > > On Fri, Aug 05, 2022 at 04:59:35PM +0200, Pali Rohár wrote:
> > > > > On Friday 05 August 2022 10:47:31 Tom Rini wrote:
> > > > > > On Fri, Aug 05, 2022 at 04:21:24PM +0200, Pali Rohár wrote:
> > > > > > > On Wednesday 03 August 2022 12:13:18 Tom Rini wrote:
> > > > > > > > On Wed, Aug 03, 2022 at 06:00:13PM +0200, Pali Rohár wrote:
> > > > > > > > > On Tuesday 02 August 2022 06:58:26 Tom Rini wrote:
> > > > > > > > > > On Tue, Aug 02, 2022 at 11:13:38AM +0200, Pali Rohár wrote:
> > > > > > > > > > 
> > > > > > > > > > > Hello Tom!
> > > > > > > > > > > 
> > > > > > > > > > > Your commit de47ff536363289f92f85ed1e4901724d238432d ("Convert
> > > > > > > > > > > CONFIG_SYS_MPC85XX_NO_RESETVEC to Kconfig") seems to be broken.
> > > > > > > > > > 
> > > > > > > > > > I thought I had managed to mirror the TPL/SPL/full usage that was there
> > > > > > > > > > prior, but apparently some got missed.
> > > > > > > > > 
> > > > > > > > > Yea, conversion to Kconfig seems that was incorrect.
> > > > > > > > 
> > > > > > > > As the config files were just unclear, but you seem to understand what
> > > > > > > > it's supposed to be, a patch to clean it up would be most appreciated,
> > > > > > > > thanks.
> > > > > > > > 
> > > > > > > > -- 
> > > > > > > > Tom
> > > > > > > 
> > > > > > > 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.
> > > > > > 
> > > > > > Where is PBL in that case even then?
> > > > > 
> > > > > P2020 (and older) are pre-PBL boards, they do not support NXP PBL
> > > > > header.
> > > > 
> > > > Ah, OK, then it should just be removing TARGET_P2020RDB from the choice
> > > > on "Freescale PBL load location".
> > > > 
> > > > -- 
> > > > Tom
> > > 
> > > I just do not understand.
> > > 
> > > P10** and P20** do not support NXP PBL. They support only pre-PBL and
> > > for SD card pre-PBL support I added option FSL_PREPBL_ESDHC_BOOT_SECTOR.
> > > 
> > > But CONFIG_SDCARD is automatically set when SYS_EXTRA_OPTIONS contains
> > > "SDCARD" string and CONFIG_SDCARD is used then also in P10** and P20**
> > > SD-card version of SPL to load proper U-Boot.
> > 
> > So CONFIG_SDCARD was over-loaded? That's very frustrating. That's what
> > needs to be corrected then.
> 
> But it was correct, no? CONFIG_SDCARD ensures that U-Boot is compiled in
> mode in which can be booted from SD card. Or what do you have in mind as
> purpose of this symbol?
> 
> The issue is that your Kconfig migration changes enabled CONFIG_SDCARD
> also when building (parallel) NOR version of U-Boot.

To me, the biggest issue is that "CONFIG_SDCARD" exists. It's very much
non-descriptive and that for some platforms it means "we have NXP PBL"
and others means "we're booting from SDCARD". The former should be
renamed to be descriptive, and the latter should re-use CONFIG_SD_CARD
which is still a bad name, but what everyone else uses, so makes
renaming it later to something less bad easier.

-- 
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/20220805/50f85053/attachment.sig>