On Sun, Oct 20, 2024 at 06:57:35AM +0200, Marek Vasut wrote: > On 10/20/24 4:55 AM, Tom Rini wrote: > > On Sun, Oct 20, 2024 at 04:25:38AM +0200, Marek Vasut wrote: > > > On 10/20/24 2:27 AM, Tom Rini wrote: > > > > On Sun, Oct 20, 2024 at 12:09:03AM +0200, Marek Vasut wrote: > > > > > > > > > Select default U-Boot and SPL text base for the i.MX6 SoC. The U-Boot > > > > > and SPL text base is picked as the one used by various i.MX6 boards. > > > > > Update all the boards. > > > > > > > > > > Signed-off-by: Marek Vasut <[email protected]> > > > > [snip] > > > > > diff --git a/arch/arm/mach-imx/mx6/Kconfig > > > > > b/arch/arm/mach-imx/mx6/Kconfig > > > > > index 1f8022ee685..10b19984143 100644 > > > > > --- a/arch/arm/mach-imx/mx6/Kconfig > > > > > +++ b/arch/arm/mach-imx/mx6/Kconfig > > > > > @@ -15,6 +15,13 @@ config MX6 > > > > > select GPT_TIMER if !MX6UL && !MX6ULL > > > > > imply CMD_FUSE > > > > > +config TEXT_BASE > > > > > + default 0x17800000 > > > > > + > > > > > +config SPL_TEXT_BASE > > > > > + depends on SPL > > > > > + default 0x00908000 > > > > > > > > Please do these in Kconfig and common/spl/Kconfig with the other > > > > SoC-wide default values, thanks. > > > There are many examples which do it in arch/$ARCH/$SOC , imx7 does it > > > already, so ... why Kconfig instead ? > > > > > > $ git grep config.TEXT_BASE > > > Kconfig:config TEXT_BASE > > > arch/arm/mach-apple/Kconfig:config TEXT_BASE > > > arch/arm/mach-aspeed/Kconfig:config TEXT_BASE > > > arch/arm/mach-imx/mx7/Kconfig:config TEXT_BASE > > > arch/arm/mach-ipq40xx/Kconfig:config TEXT_BASE > > > arch/arm/mach-npcm/Kconfig:config TEXT_BASE > > > arch/arm/mach-owl/Kconfig:config TEXT_BASE > > > arch/arm/mach-rockchip/Kconfig:config TEXT_BASE > > > arch/arm/mach-rockchip/rk3308/Kconfig:config TEXT_BASE > > > arch/arm/mach-rockchip/rk3568/Kconfig:config TEXT_BASE > > > arch/arm/mach-rockchip/rk3588/Kconfig:config TEXT_BASE > > > arch/arm/mach-rockchip/rv1126/Kconfig:config TEXT_BASE > > > arch/arm/mach-socfpga/Kconfig:config TEXT_BASE > > > arch/arm/mach-stm32mp/Kconfig.13x:config TEXT_BASE > > > arch/arm/mach-stm32mp/Kconfig.15x:config TEXT_BASE > > > arch/arm/mach-stm32mp/Kconfig.25x:config TEXT_BASE > > > > There are many bad examples. In general (and excluding shoving stuff > > under board/.../Kconfig) I prefer those values to be in the top-level > > Kconfig that asks them rather than per-arch so that it's (a) hopefully > > clear that yes, SoCs can and should add a default when it's known and > > (b) because perhaps this will lead to someone thinking on the problem of > > how ugly it can be and what may be a better solution. Kconfig is > > certainly better than CML1 was/is, and better than our ad-hoc just > > define stuff in board.h files, but it's not perfect. OTOH, if someone > > wants to break down the options we've put (I've told people to put) in > > the top-level Kconfig and instead demonstrate and document a reasonable > > places to put them, OK. > > > > And if you really want to say that this should just match mx7 for today > > and move on for now, alright. > I am happy to do it either way, but we should figure out the best practice > ... which I was under the impression was to put architecture (or soc) > specific default values into arch/ , while the Kconfig symbol itself should > be in matching Kconfig somewhere else in the tree. Did this change recently > ?
As I was trying to say, I think the best practice is in the common Kconfig file and I've been saying that for years. But I also don't catch when people do something else, and that frankly makes it clear that while I think it's best, it's not obviously correct. -- Tom
signature.asc
Description: PGP signature
