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.

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.

In case of parallel NOR and parallel NAND booting there is no BootROM, no preboot environment, nothing. CPU does execute-in-place, maps the last page and starts execution of the last instruction on that page. It is U-Boot code which just does branch to _start symbol (which is also in the last page).

For NOR booting (P2020RDB-PC_defconfig) there is no SPL involved.

In case of SPI NOR or SD card booting, there is some trivial BootROM which takes pre-PBL header and it can be either generated by external NXP tool or now also U-Boot too (IIRC my patch for this was merged).

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.

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.

-- Tom

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.

On Friday 05 August 2022 11:54:53 Tom Rini wrote:

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

So I hope that you will do something with it. I already spent lot of time to fix and improve powerpc support, but the result is that my patches are on the list, mostly ignored; but changes which are breaking powerpc support are happily merging. In this state I'm loosing any motivation to continue development as it is needed to do again to find out what new is broken.

On Fri, 5 Aug 2022 18:20:19 -0400 Tom Rini trini@konsulko.com wrote:

On Fri, Aug 05, 2022 at 10:17:01PM +0200, Pali Rohár wrote:

...

On Friday 05 August 2022 11:54:53 Tom Rini wrote:

...

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.

So I hope that you will do something with it. I already spent lot of time to fix and improve powerpc support, but the result is that my patches are on the list, mostly ignored; but changes which are breaking powerpc support are happily merging. In this state I'm loosing any motivation to continue development as it is needed to do again to find out what new is broken.

I'm not planning to try and further fiddle with those symbols. A simple revert is not possible as CONFIG_SYS_EXTRA_OPTIONS is gone. I assume that Marek will be picking up your PowerPC patches at this point, so any further work you're doing in that area shouldn't be delayed. I'll put re-examining this on my TODO list, but it's below finishing my CONFIG_SYS_* audit, and then renaming stuff to CFG_SYS.

You should fix whatever platforms you have access to and ignore the rest, I feel likely to be removing most of them shortly at this point.

I shall try to fix this on our platform by "reverting" these to different names, so that there are no new CONFIG_ macros.

Marek

On Monday 08 August 2022 09:37:22 Tom Rini wrote:

On Mon, Aug 08, 2022 at 09:51:49AM +0200, Marek Behún wrote:

...

On Fri, 5 Aug 2022 18:20:19 -0400 Tom Rini trini@konsulko.com wrote:

...

On Fri, Aug 05, 2022 at 10:17:01PM +0200, Pali Rohár wrote:

...

On Friday 05 August 2022 11:54:53 Tom Rini wrote:

...

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.

So I hope that you will do something with it. I already spent lot of time to fix and improve powerpc support, but the result is that my patches are on the list, mostly ignored; but changes which are breaking powerpc support are happily merging. In this state I'm loosing any motivation to continue development as it is needed to do again to find out what new is broken.

I'm not planning to try and further fiddle with those symbols. A simple revert is not possible as CONFIG_SYS_EXTRA_OPTIONS is gone. I assume that Marek will be picking up your PowerPC patches at this point, so any further work you're doing in that area shouldn't be delayed. I'll put re-examining this on my TODO list, but it's below finishing my CONFIG_SYS_* audit, and then renaming stuff to CFG_SYS.

You should fix whatever platforms you have access to and ignore the rest, I feel likely to be removing most of them shortly at this point.

I shall try to fix this on our platform by "reverting" these to different names, so that there are no new CONFIG_ macros.

OK, thanks. I'll "fix" the corenet_ds platforms with a removal patch soon, since they've been orphaned for a long while.

-- Tom

Any progress on fixing this issue? Currently all this stuff is not working in u-boot master due to broken kconfig migration. And any continuation in kconfig migration just makes it worse and harder to fix later.

On Friday 26 August 2022 10:53:58 Tom Rini wrote:

On Wed, Aug 17, 2022 at 11:29:08AM +0200, Pali Rohár wrote:

...

On Monday 08 August 2022 09:37:22 Tom Rini wrote:

...

On Mon, Aug 08, 2022 at 09:51:49AM +0200, Marek Behún wrote:

...

On Fri, 5 Aug 2022 18:20:19 -0400 Tom Rini trini@konsulko.com wrote:

...

On Fri, Aug 05, 2022 at 10:17:01PM +0200, Pali Rohár wrote:

...

On Friday 05 August 2022 11:54:53 Tom Rini wrote: > 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.

So I hope that you will do something with it. I already spent lot of time to fix and improve powerpc support, but the result is that my patches are on the list, mostly ignored; but changes which are breaking powerpc support are happily merging. In this state I'm loosing any motivation to continue development as it is needed to do again to find out what new is broken.

I'm not planning to try and further fiddle with those symbols. A simple revert is not possible as CONFIG_SYS_EXTRA_OPTIONS is gone. I assume that Marek will be picking up your PowerPC patches at this point, so any further work you're doing in that area shouldn't be delayed. I'll put re-examining this on my TODO list, but it's below finishing my CONFIG_SYS_* audit, and then renaming stuff to CFG_SYS.

You should fix whatever platforms you have access to and ignore the rest, I feel likely to be removing most of them shortly at this point.

I shall try to fix this on our platform by "reverting" these to different names, so that there are no new CONFIG_ macros.

OK, thanks. I'll "fix" the corenet_ds platforms with a removal patch soon, since they've been orphaned for a long while.

Any progress on fixing this issue? Currently all this stuff is not working in u-boot master due to broken kconfig migration. And any continuation in kconfig migration just makes it worse and harder to fix later.

I've removed the corenet_ds platforms now.

-- Tom

I'm reminding this issue again. u-boot master branch is still broken.

On Sunday 09 October 2022 08:45:03 Tom Rini wrote:

On Sun, Oct 09, 2022 at 02:41:19PM +0200, Pali Rohár wrote:

...

On Friday 26 August 2022 10:53:58 Tom Rini wrote:

...

On Wed, Aug 17, 2022 at 11:29:08AM +0200, Pali Rohár wrote:

...

On Monday 08 August 2022 09:37:22 Tom Rini wrote:

...

On Mon, Aug 08, 2022 at 09:51:49AM +0200, Marek Behún wrote:

...

On Fri, 5 Aug 2022 18:20:19 -0400 Tom Rini trini@konsulko.com wrote:

> On Fri, Aug 05, 2022 at 10:17:01PM +0200, Pali Rohár wrote: > > On Friday 05 August 2022 11:54:53 Tom Rini wrote: > > > 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. > > > > So I hope that you will do something with it. I already spent lot of > > time to fix and improve powerpc support, but the result is that my > > patches are on the list, mostly ignored; but changes which are breaking > > powerpc support are happily merging. In this state I'm loosing any > > motivation to continue development as it is needed to do again to find > > out what new is broken. > > I'm not planning to try and further fiddle with those symbols. A > simple revert is not possible as CONFIG_SYS_EXTRA_OPTIONS is gone. I > assume that Marek will be picking up your PowerPC patches at this point, > so any further work you're doing in that area shouldn't be delayed. > I'll put re-examining this on my TODO list, but it's below finishing my > CONFIG_SYS_* audit, and then renaming stuff to CFG_SYS. > > You should fix whatever platforms you have access to and ignore the > rest, I feel likely to be removing most of them shortly at this point. >

I shall try to fix this on our platform by "reverting" these to different names, so that there are no new CONFIG_ macros.

OK, thanks. I'll "fix" the corenet_ds platforms with a removal patch soon, since they've been orphaned for a long while.

Any progress on fixing this issue? Currently all this stuff is not working in u-boot master due to broken kconfig migration. And any continuation in kconfig migration just makes it worse and harder to fix later.

I've removed the corenet_ds platforms now.

I'm reminding this issue again. u-boot master branch is still broken.

I don't really remember what the fix was at this point, but you should fix whatever boards you have and care about as it's a matter of selecting the correct option, yes?

See: https://lore.kernel.org/u-boot/20220805142124.6swsha6aj62f33e3@pali/ Broken is Kconfig migration which you done. I wrote in that email simple test case how to check if it is OK or not.

On Sun, 9 Oct 2022 12:32:02 -0400 Tom Rini trini@konsulko.com wrote:

On Sun, Oct 09, 2022 at 03:10:45PM +0200, Pali Rohár wrote:

...

On Sunday 09 October 2022 08:45:03 Tom Rini wrote:

...

On Sun, Oct 09, 2022 at 02:41:19PM +0200, Pali Rohár wrote:

...

On Friday 26 August 2022 10:53:58 Tom Rini wrote:

...

On Wed, Aug 17, 2022 at 11:29:08AM +0200, Pali Rohár wrote:

...

On Monday 08 August 2022 09:37:22 Tom Rini wrote: > On Mon, Aug 08, 2022 at 09:51:49AM +0200, Marek Behún wrote: > > On Fri, 5 Aug 2022 18:20:19 -0400 > > Tom Rini trini@konsulko.com wrote: > > > > > On Fri, Aug 05, 2022 at 10:17:01PM +0200, Pali Rohár wrote: > > > > On Friday 05 August 2022 11:54:53 Tom Rini wrote: > > > > > 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. > > > > > > > > So I hope that you will do something with it. I already spent lot of > > > > time to fix and improve powerpc support, but the result is that my > > > > patches are on the list, mostly ignored; but changes which are breaking > > > > powerpc support are happily merging. In this state I'm loosing any > > > > motivation to continue development as it is needed to do again to find > > > > out what new is broken. > > > > > > I'm not planning to try and further fiddle with those symbols. A > > > simple revert is not possible as CONFIG_SYS_EXTRA_OPTIONS is gone. I > > > assume that Marek will be picking up your PowerPC patches at this point, > > > so any further work you're doing in that area shouldn't be delayed. > > > I'll put re-examining this on my TODO list, but it's below finishing my > > > CONFIG_SYS_* audit, and then renaming stuff to CFG_SYS. > > > > > > You should fix whatever platforms you have access to and ignore the > > > rest, I feel likely to be removing most of them shortly at this point. > > > > > > > I shall try to fix this on our platform by "reverting" these to > > different names, so that there are no new CONFIG_ macros. > > OK, thanks. I'll "fix" the corenet_ds platforms with a removal patch > soon, since they've been orphaned for a long while.

Any progress on fixing this issue? Currently all this stuff is not working in u-boot master due to broken kconfig migration. And any continuation in kconfig migration just makes it worse and harder to fix later.

I've removed the corenet_ds platforms now.

I'm reminding this issue again. u-boot master branch is still broken.

I don't really remember what the fix was at this point, but you should fix whatever boards you have and care about as it's a matter of selecting the correct option, yes?

See: https://lore.kernel.org/u-boot/20220805142124.6swsha6aj62f33e3@pali/ Broken is Kconfig migration which you done. I wrote in that email simple test case how to check if it is OK or not.

Yes, but I'm not un-migrating the symbol. As I think you're the only person who cares about mpc85xx platforms right now, is your platform working, or is there something that needs to be changed so that it does work?

He's not the only one. I am still rebasing Pali's patches on top of current master. Please give me some more time.

On Wednesday 02 November 2022 00:14:21 Pali Rohár wrote:

On Monday 10 October 2022 14:20:20 Marek Behún wrote:

...

On Sun, 9 Oct 2022 12:32:02 -0400 Tom Rini trini@konsulko.com wrote:

...

On Sun, Oct 09, 2022 at 03:10:45PM +0200, Pali Rohár wrote:

...

On Sunday 09 October 2022 08:45:03 Tom Rini wrote:

...

On Sun, Oct 09, 2022 at 02:41:19PM +0200, Pali Rohár wrote:

...

On Friday 26 August 2022 10:53:58 Tom Rini wrote: > On Wed, Aug 17, 2022 at 11:29:08AM +0200, Pali Rohár wrote: > > On Monday 08 August 2022 09:37:22 Tom Rini wrote: > > > On Mon, Aug 08, 2022 at 09:51:49AM +0200, Marek Behún wrote: > > > > On Fri, 5 Aug 2022 18:20:19 -0400 > > > > Tom Rini trini@konsulko.com wrote: > > > > > > > > > On Fri, Aug 05, 2022 at 10:17:01PM +0200, Pali Rohár wrote: > > > > > > On Friday 05 August 2022 11:54:53 Tom Rini wrote: > > > > > > > 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. > > > > > > > > > > > > So I hope that you will do something with it. I already spent lot of > > > > > > time to fix and improve powerpc support, but the result is that my > > > > > > patches are on the list, mostly ignored; but changes which are breaking > > > > > > powerpc support are happily merging. In this state I'm loosing any > > > > > > motivation to continue development as it is needed to do again to find > > > > > > out what new is broken. > > > > > > > > > > I'm not planning to try and further fiddle with those symbols. A > > > > > simple revert is not possible as CONFIG_SYS_EXTRA_OPTIONS is gone. I > > > > > assume that Marek will be picking up your PowerPC patches at this point, > > > > > so any further work you're doing in that area shouldn't be delayed. > > > > > I'll put re-examining this on my TODO list, but it's below finishing my > > > > > CONFIG_SYS_* audit, and then renaming stuff to CFG_SYS. > > > > > > > > > > You should fix whatever platforms you have access to and ignore the > > > > > rest, I feel likely to be removing most of them shortly at this point. > > > > > > > > > > > > > I shall try to fix this on our platform by "reverting" these to > > > > different names, so that there are no new CONFIG_ macros. > > > > > > OK, thanks. I'll "fix" the corenet_ds platforms with a removal patch > > > soon, since they've been orphaned for a long while. > > > > Any progress on fixing this issue? Currently all this stuff is not > > working in u-boot master due to broken kconfig migration. And any > > continuation in kconfig migration just makes it worse and harder to fix > > later. > > I've removed the corenet_ds platforms now.

I'm reminding this issue again. u-boot master branch is still broken.

I don't really remember what the fix was at this point, but you should fix whatever boards you have and care about as it's a matter of selecting the correct option, yes?

See: https://lore.kernel.org/u-boot/20220805142124.6swsha6aj62f33e3@pali/ Broken is Kconfig migration which you done. I wrote in that email simple test case how to check if it is OK or not.

Yes, but I'm not un-migrating the symbol. As I think you're the only person who cares about mpc85xx platforms right now, is your platform working, or is there something that needs to be changed so that it does work?

He's not the only one. I am still rebasing Pali's patches on top of current master. Please give me some more time.

Gentle reminder.

What is the state?

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.

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.

On Thu, Dec 22, 2022 at 06:13:06PM +0100, Pali Rohár wrote:

On Thursday 22 December 2022 09:29:27 Tom Rini wrote:

...

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

In lot of projects it is common that developer who introduced broken commit, is responsible to either fix it or revert it. I really do not have a time and power to fix every one broken commit behind every one developer. I sent lot of other fixes and in this case I at least wrote what is broken. Moreover it my was not my decision to use Kconfig for this stuff, which is unsuitable tool here. So do not take me wrong but I do not have motivation to fight with another tool for things for which it was not designed and try to hunt bugs in it.

It should have been pretty simple migration from tool A to tool B as it does not introduce any new feature nor code change. It should produce same u-boot.bin binary before and after change. And also it is not needed to fully understand what which option means. And if binaries are not same then conversion was wrong. No need for HW, just compiling and unit testing.

I have rebased turris 1.x patches at least 3 times. This is not enough?? Why should I rebase it again? Note that it depends on P1/P2 because turris 1.x is based on P2.

Also I have sent tons of patches and fixes for P1/P2 platforms and the only thing which you are going to do is to delete this platform?

You should have wrote this statement year ago and I would never take care of P1/P2 and fixing it. This is absolutely rude decision to show active developers: go away, we do not care about you, your time and your contributions.

Had I realized 10 years ago that Freescale was overloading CONFIG_SDCARD they way they were, I'd have rejected the changes then. So yes, that's on me. But this also wasn't a simple migration, or things wouldn't be broken right now. That's part of the problem. And I can't functionally test things. Which is why I've repeatedly asked if you can pick up and "own" these parts of the system which you understand and can test, and I cannot. At the time I likely figured the size change on a few platforms was one of the cases where the migration fixed an inconsistency, rather than breakage, as that has also happened as these changes go in.

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. 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?

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)

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@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

On Fri, Dec 23, 2022 at 11:34:46PM +0100, Pali Rohár wrote:

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.

So, with the benefit of hindsight, I re-ran the before/after world build of the original bad migration, to see what changed where. That gives us: P2020RDB-PC P2020RDB-PC_36BIT P1020RDB-PC P1020RDB-PC_36BIT P1020RDB-PD P1010RDB-PA_36BIT_NOR P1010RDB-PA_NOR P1010RDB-PB_36BIT_NOR P1010RDB-PB_NOR T1024RDB_NAND T2080RDB_NAND T2080RDB_revD_NAND T2080QDS_NAND

And aside from p1_p2_rdb those differences are just print related. But, that excludes include/configs/. And then if we look at what sets CONFIG_SDCARD in include/configs/ there might be a few pad sizes that then get migrated wrong, but I'm not sure.

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)

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@kernel.org

Are you going to apply this fix to master branch?

...

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

Hi Pali,

On Thu, 22 Dec 2022 at 10:13, Pali Rohár pali@kernel.org wrote:

On Thursday 22 December 2022 09:29:27 Tom Rini wrote:

...

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

In lot of projects it is common that developer who introduced broken commit, is responsible to either fix it or revert it. I really do not have a time and power to fix every one broken commit behind every one developer. I sent lot of other fixes and in this case I at least wrote what is broken. Moreover it my was not my decision to use Kconfig for this stuff, which is unsuitable tool here. So do not take me wrong but I do not have motivation to fight with another tool for things for which it was not designed and try to hunt bugs in it.

It should have been pretty simple migration from tool A to tool B as it does not introduce any new feature nor code change. It should produce same u-boot.bin binary before and after change. And also it is not needed to fully understand what which option means. And if binaries are not same then conversion was wrong. No need for HW, just compiling and unit testing.

I have rebased turris 1.x patches at least 3 times. This is not enough?? Why should I rebase it again? Note that it depends on P1/P2 because turris 1.x is based on P2.

Also I have sent tons of patches and fixes for P1/P2 platforms and the only thing which you are going to do is to delete this platform?

You should have wrote this statement year ago and I would never take care of P1/P2 and fixing it. This is absolutely rude decision to show active developers: go away, we do not care about you, your time and your contributions.

I don't think it needs to be deleted, from my reading of what Tom said.

One thing to bear in mind is that the Kconfig migration has been going on for 6 years and Tom has actually managed to complete it. It will be a big win for the project. One thing that has made it so hard is that some board maintainers have moved on and don't take notice of migrations.

I know from bitter experience that converting to Kconfig and keeping everything the same is extremely tricky. Buildman has a -K option to show CONFIG differences largely for this reason. But in this case, we need to move things to CFG_ also...so...it is not easy.

Regards, Simon

On Wed, Dec 28, 2022 at 05:50:43PM +0100, Pali Rohár wrote:

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)

Yes, I'll look at this again. My first observation is that the exhaustive list of incorrectly migrated platforms listed there is ALSO the list of platforms that had SDCARD/SPIFLASH enabled, when they shouldn't have and now have NO_PBL set, So, that being set wrong meant that the part here was wrong. I think the answer might be to just fix those configs, and then also add a "depends on !MPC85XX_HAVE_RESET_VECTOR" to the *SYS_MPC85XX_NO_RESETVEC options.

Hi Pali,

On Wed, 28 Dec 2022 at 11:51, Pali Rohár pali@kernel.org wrote:

On Wednesday 28 December 2022 17:50:43 Pali Rohár wrote:

...

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

And there is one _interesting_ board:

$ for arch in `git grep 'config ARCH' arch/powerpc/cpu/mpc85xx/Kconfig | sed 's/.* //'`; do ./tools/moveconfig.py -f $arch ~MPC85XX_HAVE_RESET_VECTOR ~SYS_MPC85XX_NO_RESETVEC; done | grep -v ' matches|^$' socrates

(I do not know how to easily tell moveconfig.py to do OR, so I called in more times in loop).

socrates_defconfig is the only one mpc85xx board which does not have enabled reset vectors nor disabled reset vectors.

As it boots from flash too, it should have reset vector in its binary. I have looked into compiled u-boot ELF binary and reset vector is there. So good.

But in u-boot.bin binary it is not, so it is broken.

Checking with v2022.04 release and it is quite interesting. U-Boot build process produce more *.bin binaries:

-rw-r--r-- 1 pali pali 530613 dec 28 17:57 u-boot.bin -rw-r--r-- 1 pali pali 530613 dec 28 17:57 u-boot-dtb.bin -rwxr-xr-x 1 pali pali 524288 dec 28 17:57 u-boot-nodtb.bin -rw-r--r-- 1 pali pali 655360 dec 28 17:57 u-boot-socrates.bin

u-boot.bin and u-boot-dtb.bin are broken. u-boot-nodtb.bin is intermediate and u-boot-socrates.bin seems to be correct (contains reset vector at correct offset and also has DTB file in it).

U-Boot master build only broken u-boot.bin and does *not* build correct u-boot-socrates.bin.

I bisected this issue why u-boot-socrates.bin stopped being building. And reason is my commit 5af42eafd7e1 ("Makefile: Reduce usage of custom mpc85xx u-boot.bin target"). It is binman who built "correct" binary and that commit deselected it.

I looked again at CONFIG_MPC85XX_HAVE_RESET_VECTOR symbol and its meaning is currently "build only intermediate binaries for binman, separate one for u-boot main code, separate one for reset vector, separate for DTB and let binman to merge them via standard powerpc config binman file arch/powerpc/dts/u-boot.dtsi to output u-boot.bin."

From the code, this socrates is expected to build final binary u-boot-socrates.bin (not u-boot.bin) via binman, but not via common powerpc u-boot.dtsi, but rather via arch/powerpc/dts/socrates-u-boot.dtsi So it does not use CONFIG_MPC85XX_HAVE_RESET_VECTOR symbol (which is just for common powerpc binman stage).

I'm going to send a fix for socrates, so build process start again building u-boot-socrates.bin binary and let it to v2022.04 state.

The long term solution for socrates should be:

1. Stop building broken and unused u-boot.bin

Just a note on this...that file is produced by all boards. If a board needs to create another file, e.g. u-boot-rockchip.bin then that is fine. But the u-boot.bin file should remain.

2. Rename u-boot-socrates.bin to u-boot.bin

Please don't do that!

PS: Similarly is broken also keymile board, but for this one I had realized that is "custom" and months ago I sent patch for it for conversion to use common powerpc binman rule, even before commit 5af42eafd7e1 which broke both keymile and socrates: https://patchwork.ozlabs.org/project/uboot/patch/20220803112049.4287-1-pali@...

...

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)

Regards, Simon