Broken commit d433c74eecdce1e4952ef4e8c712a9289c0dfcc2

Fri Dec 23 23:34:46 CET 2022

On Friday 23 December 2022 22:39:00 Pali Rohár wrote:
> On Friday 23 December 2022 14:18:32 Tom Rini wrote:
> > On Fri, Dec 23, 2022 at 08:10:14PM +0100, Pali Rohár wrote:
> > > On Thursday 22 December 2022 13:22:50 Tom Rini wrote:
> > > > On Thu, Dec 22, 2022 at 06:56:40PM +0100, Pali Rohár wrote:
> > > > > On Thursday 22 December 2022 12:33:32 Tom Rini wrote:
> > > > > > I'm sorry you're frustrated here. I'm also frustrated here because the
> > > > > > #ifdef games that PowerPC used, in a number of places have been very
> > > > > > hard to un-wrap so that we can have something other than a home-grown
> > > > > > build system.
> > > > > 
> > > > > I was already trying to reduce it too, some patches I sent, some other I
> > > > > was preparing and some other are part of turris 1.x platform, which is
> > > > > waiting there for 6 months. I planned to apply removal of MMC symbols to
> > > > > other P1/P2 boards, like it is in turris patch, but after turris patch
> > > > > is merged... which did not happen yet.
> > > > 
> > > > Right, and thanks for what you've done already.
> > > > 
> > > > > > And I've tried to take your other feedback in to
> > > > > > consideration, which has resulted in a large number of symbols being
> > > > > > moved to CFG_... instead of Kconfig, as you're right, it wasn't the
> > > > > > right mechanism for them.
> > > > > > 
> > > > > > So, is it really just the 3 platforms that use p1_p2_rdb.h that need
> > > > > > neither SDCARD nor SPIFLASH for the NAND and no extra suffix defconfigs?
> > > > > > Or is it the P1010RDB ones too?
> > > > > 
> > > > > It applies for e500 v1/v2 cores, which is cpu/mpc85xx in u-boot and
> > > > > predates P3 platform. If there are not some suspicious symbol names then
> > > > > it should match any board which uses ARCH_MPC85??? or ARCH_P1?? or
> > > > > ARCH_P2020 symbol.
> > > > > 
> > > > > P2040 and T???? do not have e500 v1/v2 cores (they have e500mc or e5500
> > > > > or e6500), so you can ignore these.
> > > > 
> > > > OK.
> > > > 
> > > > > Is there any tool which can list all defconfig files which defines some
> > > > > of those symbols, including transitionally?
> > > > 
> > > > With the caveat of only symbols in Kconfig, tools/moveconfig.py -b will
> > > > make a database that you can consult with -f. That takes both SYMBOL and
> > > > ~SYMBOL to list configs with SYMBOL enabled or disabled, respectively.
> > > > And you can chain them together. For example:
> > > > $ ./tools/moveconfig.py -f ARCH_P1010 SDCARD
> > > > 8 matches
> > > > P1010RDB-PB_36BIT_NOR P1010RDB-PA_NOR P1010RDB-PB_SDCARD P1010RDB-PB_36BIT_SDCARD P1010RDB-PA_36BIT_NOR P1010RDB-PA_36BIT_SDCARD P1010RDB-PB_NOR P1010RDB-PA_SDCARD
> > > > $ ./tools/moveconfig.py -f ARCH_P1010 ~SDCARD
> > > > 8 matches
> > > > P1010RDB-PA_NAND P1010RDB-PB_SPIFLASH P1010RDB-PB_36BIT_SPIFLASH P1010RDB-PA_36BIT_NAND P1010RDB-PA_36BIT_SPIFLASH P1010RDB-PA_SPIFLASH P1010RDB-PB_NAND P1010RDB-PB_36BIT_NAND
> > > 
> > > Ok, I tried to build database and list of the affected defconfig files:
> > > (all except T, P3+ and P204x)
> > > 
> > > $ for arch in `git grep 'config ARCH' arch/powerpc/cpu/mpc85xx/Kconfig | sed 's/.* //' | grep -v 'ARCH_T\|ARCH_P[345]\|ARCH_P204'`; do ./tools/moveconfig.py -f $arch SDCARD; done | grep -v ' matches\|^$'
> > > P1010RDB-PB_36BIT_NOR P1010RDB-PA_SDCARD P1010RDB-PB_NOR P1010RDB-PB_SDCARD P1010RDB-PA_36BIT_NOR P1010RDB-PB_36BIT_SDCARD P1010RDB-PA_NOR P1010RDB-PA_36BIT_SDCARD
> > > P1020RDB-PC P1020RDB-PD_SDCARD P1020RDB-PC_SDCARD P1020RDB-PC_36BIT P1020RDB-PD P1020RDB-PC_36BIT_SDCARD
> > > P2020RDB-PC_SDCARD P2020RDB-PC_36BIT P2020RDB-PC_36BIT_SDCARD P2020RDB-PC
> > > 
> > > So it means that these defconfigs are broken and CONFIG_SDCARD for them
> > > must be turned off:
> > > 
> > > P1010RDB-PB_36BIT_NOR
> > > P1010RDB-PB_NOR
> > > P1010RDB-PA_36BIT_NOR
> > > P1010RDB-PA_NOR
> > > P1020RDB-PC
> > > P1020RDB-PC_36BIT
> > > P1020RDB-PD
> > > P2020RDB-PC_36BIT
> > > P2020RDB-PC
> > > 
> > > 
> > > These defconfigs have already CONFIG_SDCARD turned off:
> > > 
> > > $ for arch in `git grep 'config ARCH' arch/powerpc/cpu/mpc85xx/Kconfig | sed 's/.* //' | grep -v 'ARCH_T\|ARCH_P[345]\|ARCH_P204'`; do ./tools/moveconfig.py -f $arch ~SDCARD; done | grep -v ' matches\|^$'
> > > socrates
> > > MPC8548CDS MPC8548CDS_legacy MPC8548CDS_36BIT
> > > P1010RDB-PB_36BIT_NAND P1010RDB-PB_NAND P1010RDB-PA_36BIT_SPIFLASH P1010RDB-PB_SPIFLASH P1010RDB-PA_36BIT_NAND P1010RDB-PA_NAND P1010RDB-PB_36BIT_SPIFLASH P1010RDB-PA_SPIFLASH
> > > P1020RDB-PC_NAND P1020RDB-PC_36BIT_SPIFLASH P1020RDB-PD_SPIFLASH P1020RDB-PC_SPIFLASH P1020RDB-PD_NAND P1020RDB-PC_36BIT_NAND
> > > P2020RDB-PC_SPIFLASH P2020RDB-PC_36BIT_SPIFLASH P2020RDB-PC_NAND P2020RDB-PC_36BIT_NAND
> > > qemu-ppce500
> > > 
> > > And seems that no of them is sd card related (hopefully).
> > 
> > Thanks!  Does the following look reasonable to you
> > (CONFIG_FSL_FIXED_MMC_LOCATION was enabled due to SDCARD)? If so I'll
> > make a proper patch next:
> 
> Just to note that possibility of booting pre-PBL devices from SD or SPI
> is described in document AN3659 - Booting from On-Chip ROM (eSDHC or eSPI):
> https://www.nxp.com/docs/en/application-note/AN3659.pdf
> In P2020 documentation it is named "On-chip boot ROM-eSDHC".
> 
> > 
> > 
> > diff --git a/boot/Kconfig b/boot/Kconfig
> > index 4a001bcee851..424ad0e466da 100644
> > --- a/boot/Kconfig
> > +++ b/boot/Kconfig
> > @@ -724,16 +724,19 @@ config RAMBOOT_PBL
> >  	  For more details refer to doc/README.pblimage
> >  
> >  choice
> > -	prompt "Freescale PBL load location"
> > +	prompt "Freescale PBL (or predecessor) load location"
> >  	depends on RAMBOOT_PBL || ((TARGET_P1010RDB_PA || TARGET_P1010RDB_PB \
> >  		|| TARGET_P1020RDB_PC || TARGET_P1020RDB_PD || TARGET_P2020RDB) \
> >  		&& !CMD_NAND)

And it should depends on CPU/ARCH, not at list of boards... as this is
bootrom/CPU feature, not board feature. So maybe on CONFIG_MPC85xx? But
needs to be ensured that SDCARD symbol is not enabled in defconfigs
where it is not.

> >  
> >  config SDCARD
> > -	bool "Freescale PBL is found on SD card"
> > +	bool "Freescale PBL (or similar) is found on SD card"
> >  
> >  config SPIFLASH
> > -	bool "Freescale PBL is found on SPI flash"
> > +	bool "Freescale PBL (or similar) is found on SPI flash"
> > +
> > +config NO_PBL
> > +	bool "Freescale PBL (or similar) is not used in this case"
> >  
> >  endchoice
> >  
> 
> Ok! I did runtime tests with P2020RDB-PC_defconfig on P2020 based board.
> 
> 
> 1. directly on v2022.04 tag
> 
> ==> works fine
> 
> 
> 2. merged problematic commit d433c74eecdc with v2022.04 tag
> 
> $ ll u-boot.bin
> -rw-r--r-- 1 pali pali 151781376 dec 23 21:24 u-boot.bin
> 
> ==> not possible to flash --> binary toooooo big (due to broken commit)
> 
> 
> 3. merged problematic commit d433c74eecdc with v2022.04 tag and applying
> above boot/Kconfig change and adding CONFIG_NO_PBL=y into file
> P2020RDB-PC_defconfig. Also I added to include/configs/p1_p2_rdb_pc.h
> check:
> 
> #ifdef CONFIG_SDCARD
> #error "CONFIG_SDCARD is defined"
> #endif
> 
> Whole steps:
> $ git checkout v2022.04
> $ git merge d433c74eecdc
> (resolve merge conflict for doc/)
> $ git checkout --theirs doc/usage/index.rst
> $ git add doc/usage/index.rst
> $ git commit
> apply change
> add CONFIG_NO_PBL=y
> $ make CROSS_COMPILE=powerpc-linux-gnuspe- P2020RDB-PC_defconfig
> $ make CROSS_COMPILE=powerpc-linux-gnuspe- -j12
> 
> Binary now has correct size (it must have exactly 768 kB)
> 
> $ ll u-boot.bin
> -rw-r--r-- 1 pali pali 786432 dec 23 21:33 u-boot.bin
> 
> U-Boot 2022.04-00286-g72336d158369-dirty (Dec 23 2022 - 21:18:23 +0100)
> 
> CPU0:  P2020E, Version: 2.1, (0x80ea0021)
> Core:  e500, Version: 5.1, (0x80211051)
> Clock Configuration:
>        CPU0:1200 MHz, CPU1:1200 MHz, 
>        CCB:600  MHz,
>        DDR:400  MHz (800 MT/s data rate) (Asynchronous), LBC:600  MHz
> L1:    D-cache 32 KiB enabled
>        I-cache 32 KiB enabled
> Model: fsl,P2020RDB-PC
> Board: P2020RDB-PC CPLD: V4.1 PCBA: V4.0
> rom_loc: nor upper bank
> SD/MMC : 4-bit Mode
> eSPI : Enabled
> DRAM:  Detected UDIMM 9905594-003.A00G  
> 2 GiB (DDR3, 64-bit, CL=6, ECC off)
> Core:  11 devices, 9 uclasses, devicetree: embed
> No address specified for VSC7385 microcode.
> Flash: 16 MiB
> L2:    512 KiB enabled
> MMC:   fsl_esdhc: Internal clock never stabilised.
> FSL_SDHC: 0
> Loading Environment from Flash... OK
> In:    serial
> Out:   serial
> Err:   serial
> Net:   eTSEC1
> Error: eTSEC1 address not set.
> , eTSEC2
> Error: eTSEC2 address not set.
> , eTSEC3
> Error: eTSEC3 address not set.
> 
> => 
> 
> ==> So works fine!
> 
> 
> 4. u-boot master with this patch
> 
> ==> does not work, no output on console; but SDCARD version is working,
> so clearly some other commit broke non-SDCARD version too.
> 
> So go ahead and apply this fix to master branch. You can add my
> Tested-by: Pali Rohár <pali at kernel.org>
> 
> > diff --git a/configs/P1010RDB-PA_36BIT_NOR_defconfig b/configs/P1010RDB-PA_36BIT_NOR_defconfig
> > index dcf74f5d6af5..deff04bf881d 100644
> > --- a/configs/P1010RDB-PA_36BIT_NOR_defconfig
> > +++ b/configs/P1010RDB-PA_36BIT_NOR_defconfig
> > @@ -19,7 +19,7 @@ CONFIG_FIT=y
> >  CONFIG_FIT_VERBOSE=y
> >  CONFIG_OF_BOARD_SETUP=y
> >  CONFIG_OF_STDOUT_VIA_ALIAS=y
> > -CONFIG_FSL_FIXED_MMC_LOCATION=y
> > +CONFIG_NO_PBL=y
> >  CONFIG_BOOTDELAY=10
> >  CONFIG_USE_BOOTCOMMAND=y
> >  CONFIG_BOOTCOMMAND="setenv bootargs root=/dev/ram rw console=$consoledev,$baudrate $othbootargs ramdisk_size=$ramdisk_size;tftp $ramdiskaddr $ramdiskfile;tftp $loadaddr $bootfile;tftp $fdtaddr $fdtfile;bootm $loadaddr $ramdiskaddr $fdtaddr"
> > diff --git a/configs/P1010RDB-PA_NOR_defconfig b/configs/P1010RDB-PA_NOR_defconfig
> > index 537d8bf576b0..98486a0ae2ff 100644
> > --- a/configs/P1010RDB-PA_NOR_defconfig
> > +++ b/configs/P1010RDB-PA_NOR_defconfig
> > @@ -18,7 +18,7 @@ CONFIG_FIT=y
> >  CONFIG_FIT_VERBOSE=y
> >  CONFIG_OF_BOARD_SETUP=y
> >  CONFIG_OF_STDOUT_VIA_ALIAS=y
> > -CONFIG_FSL_FIXED_MMC_LOCATION=y
> > +CONFIG_NO_PBL=y
> >  CONFIG_BOOTDELAY=10
> >  CONFIG_USE_BOOTCOMMAND=y
> >  CONFIG_BOOTCOMMAND="setenv bootargs root=/dev/ram rw console=$consoledev,$baudrate $othbootargs ramdisk_size=$ramdisk_size;tftp $ramdiskaddr $ramdiskfile;tftp $loadaddr $bootfile;tftp $fdtaddr $fdtfile;bootm $loadaddr $ramdiskaddr $fdtaddr"
> > diff --git a/configs/P1010RDB-PB_36BIT_NOR_defconfig b/configs/P1010RDB-PB_36BIT_NOR_defconfig
> > index 92a7e0966936..41a1a852b986 100644
> > --- a/configs/P1010RDB-PB_36BIT_NOR_defconfig
> > +++ b/configs/P1010RDB-PB_36BIT_NOR_defconfig
> > @@ -19,7 +19,7 @@ CONFIG_FIT=y
> >  CONFIG_FIT_VERBOSE=y
> >  CONFIG_OF_BOARD_SETUP=y
> >  CONFIG_OF_STDOUT_VIA_ALIAS=y
> > -CONFIG_FSL_FIXED_MMC_LOCATION=y
> > +CONFIG_NO_PBL=y
> >  CONFIG_BOOTDELAY=10
> >  CONFIG_USE_BOOTCOMMAND=y
> >  CONFIG_BOOTCOMMAND="setenv bootargs root=/dev/ram rw console=$consoledev,$baudrate $othbootargs ramdisk_size=$ramdisk_size;tftp $ramdiskaddr $ramdiskfile;tftp $loadaddr $bootfile;tftp $fdtaddr $fdtfile;bootm $loadaddr $ramdiskaddr $fdtaddr"
> > diff --git a/configs/P1010RDB-PB_NOR_defconfig b/configs/P1010RDB-PB_NOR_defconfig
> > index 3e16470608e5..16c076bc9aaa 100644
> > --- a/configs/P1010RDB-PB_NOR_defconfig
> > +++ b/configs/P1010RDB-PB_NOR_defconfig
> > @@ -18,7 +18,7 @@ CONFIG_FIT=y
> >  CONFIG_FIT_VERBOSE=y
> >  CONFIG_OF_BOARD_SETUP=y
> >  CONFIG_OF_STDOUT_VIA_ALIAS=y
> > -CONFIG_FSL_FIXED_MMC_LOCATION=y
> > +CONFIG_NO_PBL=y
> >  CONFIG_BOOTDELAY=10
> >  CONFIG_USE_BOOTCOMMAND=y
> >  CONFIG_BOOTCOMMAND="setenv bootargs root=/dev/ram rw console=$consoledev,$baudrate $othbootargs ramdisk_size=$ramdisk_size;tftp $ramdiskaddr $ramdiskfile;tftp $loadaddr $bootfile;tftp $fdtaddr $fdtfile;bootm $loadaddr $ramdiskaddr $fdtaddr"
> > diff --git a/configs/P1020RDB-PC_36BIT_defconfig b/configs/P1020RDB-PC_36BIT_defconfig
> > index ce0ba0e37574..e60c92cedae9 100644
> > --- a/configs/P1020RDB-PC_36BIT_defconfig
> > +++ b/configs/P1020RDB-PC_36BIT_defconfig
> > @@ -21,7 +21,7 @@ CONFIG_FIT=y
> >  CONFIG_FIT_VERBOSE=y
> >  CONFIG_OF_BOARD_SETUP=y
> >  CONFIG_OF_STDOUT_VIA_ALIAS=y
> > -CONFIG_FSL_FIXED_MMC_LOCATION=y
> > +CONFIG_NO_PBL=y
> >  CONFIG_BOOTDELAY=10
> >  CONFIG_USE_BOOTCOMMAND=y
> >  CONFIG_BOOTCOMMAND="setenv bootargs root=/dev/$bdev rw rootdelay=30 console=$consoledev,$baudrate $othbootargs;usb start;ext2load usb 0:1 $loadaddr /boot/$bootfile;ext2load usb 0:1 $fdtaddr /boot/$fdtfile;bootm $loadaddr - $fdtaddr"
> > diff --git a/configs/P1020RDB-PC_defconfig b/configs/P1020RDB-PC_defconfig
> > index aae886b75c4e..f0f0eee65d29 100644
> > --- a/configs/P1020RDB-PC_defconfig
> > +++ b/configs/P1020RDB-PC_defconfig
> > @@ -20,7 +20,7 @@ CONFIG_FIT=y
> >  CONFIG_FIT_VERBOSE=y
> >  CONFIG_OF_BOARD_SETUP=y
> >  CONFIG_OF_STDOUT_VIA_ALIAS=y
> > -CONFIG_FSL_FIXED_MMC_LOCATION=y
> > +CONFIG_NO_PBL=y
> >  CONFIG_BOOTDELAY=10
> >  CONFIG_USE_BOOTCOMMAND=y
> >  CONFIG_BOOTCOMMAND="setenv bootargs root=/dev/$bdev rw rootdelay=30 console=$consoledev,$baudrate $othbootargs;usb start;ext2load usb 0:1 $loadaddr /boot/$bootfile;ext2load usb 0:1 $fdtaddr /boot/$fdtfile;bootm $loadaddr - $fdtaddr"
> > diff --git a/configs/P1020RDB-PD_defconfig b/configs/P1020RDB-PD_defconfig
> > index 5fecb6684735..8b7f9310b1a4 100644
> > --- a/configs/P1020RDB-PD_defconfig
> > +++ b/configs/P1020RDB-PD_defconfig
> > @@ -20,7 +20,7 @@ CONFIG_FIT=y
> >  CONFIG_FIT_VERBOSE=y
> >  CONFIG_OF_BOARD_SETUP=y
> >  CONFIG_OF_STDOUT_VIA_ALIAS=y
> > -CONFIG_FSL_FIXED_MMC_LOCATION=y
> > +CONFIG_NO_PBL=y
> >  CONFIG_BOOTDELAY=10
> >  CONFIG_USE_BOOTCOMMAND=y
> >  CONFIG_BOOTCOMMAND="setenv bootargs root=/dev/$bdev rw rootdelay=30 console=$consoledev,$baudrate $othbootargs;usb start;ext2load usb 0:1 $loadaddr /boot/$bootfile;ext2load usb 0:1 $fdtaddr /boot/$fdtfile;bootm $loadaddr - $fdtaddr"
> > diff --git a/configs/P2020RDB-PC_36BIT_defconfig b/configs/P2020RDB-PC_36BIT_defconfig
> > index b1dbca6e7ea3..1640f2379863 100644
> > --- a/configs/P2020RDB-PC_36BIT_defconfig
> > +++ b/configs/P2020RDB-PC_36BIT_defconfig
> > @@ -21,7 +21,7 @@ CONFIG_FIT=y
> >  CONFIG_FIT_VERBOSE=y
> >  CONFIG_OF_BOARD_SETUP=y
> >  CONFIG_OF_STDOUT_VIA_ALIAS=y
> > -CONFIG_FSL_FIXED_MMC_LOCATION=y
> > +CONFIG_NO_PBL=y
> >  CONFIG_BOOTDELAY=10
> >  CONFIG_USE_BOOTCOMMAND=y
> >  CONFIG_BOOTCOMMAND="setenv bootargs root=/dev/$bdev rw rootdelay=30 console=$consoledev,$baudrate $othbootargs;usb start;ext2load usb 0:1 $loadaddr /boot/$bootfile;ext2load usb 0:1 $fdtaddr /boot/$fdtfile;bootm $loadaddr - $fdtaddr"
> > diff --git a/configs/P2020RDB-PC_defconfig b/configs/P2020RDB-PC_defconfig
> > index 1ee46f9fbe9f..ca9f44728845 100644
> > --- a/configs/P2020RDB-PC_defconfig
> > +++ b/configs/P2020RDB-PC_defconfig
> > @@ -20,7 +20,7 @@ CONFIG_FIT=y
> >  CONFIG_FIT_VERBOSE=y
> >  CONFIG_OF_BOARD_SETUP=y
> >  CONFIG_OF_STDOUT_VIA_ALIAS=y
> > -CONFIG_FSL_FIXED_MMC_LOCATION=y
> > +CONFIG_NO_PBL=y
> >  CONFIG_BOOTDELAY=10
> >  CONFIG_USE_BOOTCOMMAND=y
> >  CONFIG_BOOTCOMMAND="setenv bootargs root=/dev/$bdev rw rootdelay=30 console=$consoledev,$baudrate $othbootargs;usb start;ext2load usb 0:1 $loadaddr /boot/$bootfile;ext2load usb 0:1 $fdtaddr /boot/$fdtfile;bootm $loadaddr - $fdtaddr"
> > 
> > -- 
> > Tom
> 
> 