[PATCH 1/6] Revert "Convert CONFIG_SYS_BR0_PRELIM et al to Kconfig"

[Tom Rini]

On Sun, May 01, 2022 at 12:17:51PM -0400, Tom Rini wrote:
> On Sun, May 01, 2022 at 05:33:19PM +0200, Pali Rohár wrote:
> > On Sunday 01 May 2022 11:14:21 Tom Rini wrote:
> > > On Sun, May 01, 2022 at 04:44:16PM +0200, Pali Rohár wrote:
> > > > On Sunday 01 May 2022 10:39:39 Tom Rini wrote:
> > > > > On Sun, May 01, 2022 at 04:23:52PM +0200, Pali Rohár wrote:
> > > > > 
> > > > > > This reverts commit c7fad78ec0ee41b72a58bebb61959570eb937ab1.
> > > > > > 
> > > > > > This commit made configuration, understanding, maintenance, debugging and
> > > > > > future development of the powerpc/mpc85xx Local Bus Controller on P1/P2
> > > > > > boards impossible.
> > > > > > 
> > > > > > All preliminary Base and Option registers depends on other code and C
> > > > > > macros generated at C compile time and they comes from the other macros.
> > > > > > 
> > > > > > For example, NOR base address and NOR options are set via macros
> > > > > > CONFIG_SYS_FLASH_BR_PRELIM and CONFIG_SYS_FLASH_OR_PRELIM. And then based
> > > > > > on other logic are filled correct values in to the correct macros
> > > > > > CONFIG_SYS_BR*_PRELIM and CONFIG_SYS_OR*_PRELIM.
> > > > > > 
> > > > > > These config options are not user configurable options and therefore
> > > > > > should not appear in menuconfig. Moreover for P1/P2 boards they have
> > > > > > nothing with DDR driver, so they should not appear in drivers/ddr.
> > > > > > 
> > > > > > This change was completely wrong direction, so revert it. It allows to
> > > > > > start fixing issues with FLASH, NOR, NAND and CPLD LBC configuration.
> > > > > > In current state it is impossible.
> > > > > > 
> > > > > > See also thread for more details:
> > > > > > https://lore.kernel.org/u-boot/20220426181740.o2n7xfg46ytljcdx@pali/t/#u
> > > > > > 
> > > > > > Signed-off-by: Pali Rohár <pali at kernel.org>
> > > > > 
> > > > > NAK.  We are not moving things back in to board config headers under
> > > > > CONFIG namespace.  Some other solution is required.
> > > > 
> > > > I spend time on this and I do not see any other solution. As explained
> > > > that commit just introduced more issues then what it brought, so it was
> > > > step backward, not forward. So please show other solution, if you do not
> > > > like this one.
> > > 
> > > Anything that I suggested in the previous thread about moving to board
> > > Kconfig files.
> > 
> > Kconfig does not support evaluating C macros from C header files. So it
> > would not work.
> 
> Document how to derive them using tools like 'printf' when adding more
> boards, which should not be a common case anyhow.
> 
> > > Or move it to some other header and out of CONFIG namespace.
> > 
> > This is board specific setting, used by drivers and arch code. So it
> > needs to be in some board location, like the config header file.
> 
> But all include/config/*.h files are destined to be removed, so they
> cannot live there.  Everything in the CONFIG namespace needs to be in
> Kconfig.  Nothing outside of CONFIG namespace belongs under
> include/configs/.
> 
> > > Or if dtoc (doc/develop/driver-model/of-plat.rst) isn't
> > > sufficient today to pull out the infos to use at build time, expand it
> > > to cover this case as it would be useful for large numbers of other
> > > cases.
> > 
> > This would mean to rewrite completely everything about LBC configuration
> > and its peripherals. I really do not have energy nor time for it.
> 
> Neither apparently has anyone else for the last far far too long.
> 
> > There are issues which needs to be fixed first, then some "code cleanup"
> > and "non-functional" changes could be done.
> > 
> > But that your commit c7fad78ec0ee41b72a58bebb61959570eb937ab1 make it
> > impossible to do anything with LBC, neither new development, nor doing
> > bug fixes. So it is in the worst state in which it can be.
> 
> How?  Derive the correct hex value and put it in.
> 
> > That commit has same effect as conservation of the code and putting it
> > into the state to disallow future development in this area. Because
> > everybody who wants to touch it, has to first do what you wrote above.
> 
> Yes.  The code is sub-standard and needs improvement.
> 
> > But this is such giant work which nobody is going to do, just for fixing
> > small bug, which is completely unrelated to that work. And that work is
> > only refactor/code cleanup which does not bring any functional value.
> > Nothing so fancy, that somebody would do in spare time.
> > 
> > So I hope that this was not your intension.
> 
> My intention is to get the PowerPC code up to modern U-Boot standards.
> Or, to get it removed since there's not enough interest in maintaining
> and updating it.
> 
> To re-iterate, I agree that it would be good to construct the values at
> build time using macros.  No one likes looking at raw magic values.  But
> for an otherwise dead end platform, documenting how these magic values
> are constructed (something under doc/arch/ or doc/board/) for anyone in
> the future doing more work is Good Enough for me.  Alternatively,
> re-working things so that instead of being pulled from
> include/configs/board-config.h they come from board/.../something.h is
> Good Enough, so long as they are NOT using the CONFIG prefix.  They can
> use CFG_ or just LBC_ or anything else that makes sense.

Getting back to this.  I see 13 more "LBC" CONFIG symbols that need
something done to them.  I'm sure most or all of them are in the same
situation as the PRELIM ones in this thread.  Have you attempted to move
them out of the CONFIG namespace by chance?  If not, I might start
looking soon at where to move them to, rather than migrate to Kconfig.
Thanks.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20220512/6d137905/attachment.sig>