Suggestion: Revert commit c7fad78ec0ee ("Convert CONFIG_SYS_BR0_PRELIM et al to Kconfig")

Pali Rohár pali at kernel.org
Tue Apr 26 20:35:26 CEST 2022


On Tuesday 26 April 2022 14:23:48 Tom Rini wrote:
> On Tue, Apr 26, 2022 at 08:17:40PM +0200, Pali Rohár wrote:
> 
> > Hello! I would suggest to revert commit c7fad78ec0ee ("Convert
> > CONFIG_SYS_BR0_PRELIM et al to Kconfig").
> > 
> > The reason is that this commit made configuration, understanding,
> > maintenance and debugging of the powerpc/mpc85xx Local Bus Controller
> > hard, mainly impossible.
> > 
> > This commit completely removed usage of named macros, to easily check
> > address and size of the LBC memory banks. Removal was done also for
> > explaining comments of configuration options.
> > 
> > It is just a mess what this commit introduced and took me really long
> > time to debug and understand what is happening here... until I reverted
> > this commit manually in my tree.
> > 
> > Any opinions?
> > 
> > Btw, current values are wrong.
> 
> AFAICT, the current values match what was in use prior.

I'm not going to verify that these values really match as playing with
those magic numbers is a pain.

But some of these values were wrong even before that commit. And this
can be verified easier (just checking that size does not match to
expected value in DTS or documentation).

> But, these should probably not be in CONFIG namespace at all

Well, they do not belong to defconfig. These values are not user
configurable and are board wiring dependent. So should have never
appeared in menu config.

> and pulled from either device tree

This is probably the correct place. But you need to know e.g. values for
NAND or NOR for accessing these storages and therefore also for loading
and reading device tree blob from NAND/NOR.

So they cannot be stored in device tree (unless you parse device tree at
compile time and put the constants into appropriate u-boot bin location
available prior loading device tree blob).

> or some other non-board.h header file.

Unfortunately, these values define configuration for LocalBus Controller
which are board dependent and therefore must be in board header file or
some other board dependent code (exported to other u-boot subsystem,
powerpc/mpc init code and drivers).


More information about the U-Boot mailing list