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

From: Tom Rini <trini@konsulko.com>
To: "Pali Rohár" <pali@kernel.org>
Cc: Priyanka Jain <priyanka.jain@nxp.com>,
	Sinan Akman <sinan@writeme.com>,
	u-boot@lists.denx.de
Subject: Re: Suggestion: Revert commit c7fad78ec0ee ("Convert CONFIG_SYS_BR0_PRELIM et al to Kconfig")
Date: Tue, 26 Apr 2022 14:47:40 -0400	[thread overview]
Message-ID: <20220426184740.GE7271@bill-the-cat> (raw)
In-Reply-To: <20220426183526.2wf3buag5pv2rilf@pali>

[-- Attachment #1: Type: text/plain, Size: 2174 bytes --]

On Tue, Apr 26, 2022 at 08:35:26PM +0200, Pali Rohár wrote:
> 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.

Right.  It's been a while since I linked it, but:
https://patchwork.ozlabs.org/project/uboot/patch/1500407318-7977-1-git-send-email-trini@konsulko.com/
is what I use for migrating non-obvious values to Kconfig.  So I used
that to print out all of these, I'm pretty sure before and after.

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

So some bitrot, probably then, sigh.

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

So they shouldn't be asked and should be:
config SYS_FOO
	hex
	default 0xBEEF

in the board Kconfig files.  And the help part of
drivers/ddr/fsl/Kconfig updated to explain where/how to figure out or
find the appropriate values.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]