The way to use Kconfig in U-Boot

Simon Glass sjg at chromium.org
Thu May 28 05:08:26 CEST 2020 

Hi Michal,

On Tue, 26 May 2020 at 00:35, Michal Simek <michal.simek at xilinx.com> wrote:
>
> On 25. 05. 20 17:29, Tom Rini wrote:
> > On Mon, May 25, 2020 at 08:57:27AM -0600, Simon Glass wrote:
> >> Hi Michal,
> >>
> >> On Mon, 25 May 2020 at 03:16, Michal Simek <monstr at monstr.eu> wrote:
> >>>
> >>> On 22. 05. 20 19:58, Tom Rini wrote:
> >>>> On Fri, May 22, 2020 at 11:38:58PM +0800, Bin Meng wrote:
> >>>>> Hi Simon,
> >>>>>
> >>>>> On Fri, May 22, 2020 at 10:10 PM Simon Glass <sjg at chromium.org> wrote:
> >>>>>>
> >>>>>> Hi Bin,
> >>>>>>
> >>>>>> On Fri, 22 May 2020 at 08:00, Tom Rini <trini at konsulko.com> wrote:
> >>>>>>>
> >>>>>>> On Fri, May 22, 2020 at 09:53:07PM +0800, Bin Meng wrote:
> >>>>>>>> Hi Simon,
> >>>>>>>>
> >>>>>>>> On Fri, May 22, 2020 at 9:38 PM Simon Glass <sjg at chromium.org> wrote:
> >>>>>>>>>
> >>>>>>>>> Hi,
> >>>>>>>>>
> >>>>>>>>> On Fri, 22 May 2020 at 06:32, Tom Rini <trini at konsulko.com> wrote:
> >>>>>>>>>>
> >>>>>>>>>> On Fri, May 22, 2020 at 06:20:39PM +0800, Bin Meng wrote:
> >>>>>>>>>>> Hi,
> >>>>>>>>>>>
> >>>>>>>>>>> Kconfig is a flexible language and there are different ways to set a
> >>>>>>>>>>> value for a specific platform.
> >>>>>>>>>>>
> >>>>>>>>>>> We can either:
> >>>>>>>>>>>
> >>>>>>>>>>> - Use Kconfig overriding functionality
> >>>>>>>>>>> - Use Kconfig conditional set syntax like "default xxx if FOO"
> >>>>>>>>>>>
> >>>>>>>>>>> Based on current Kconfig files hierarchy, in the root directory we
> >>>>>>>>>>> have the following come at the very beginning:
> >>>>>>>>>>>
> >>>>>>>>>>> # Allow defaults in arch-specific code to override any given here
> >>>>>>>>>>> source "arch/Kconfig"
> >>>>>>>>>>>
> >>>>>>>>>>> Based on this I thought our original design was to use the overriding.
> >>>>>>>>>>>
> >>>>>>>>>>> But it seems not everyone is consistent on doing such. For example, we
> >>>>>>>>>>> have a bunch of unmaintainable (IMO) Kconfig options like this:
> >>>>>>>>>>>
> >>>>>>>>>>> config SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR
> >>>>>>>>>>> hex "Address on the MMC to load U-Boot from"
> >>>>>>>>>>> depends on SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR
> >>>>>>>>>>> default 0x50 if ARCH_SUNXI
> >>>>>>>>>>> default 0x75 if ARCH_DAVINCI
> >>>>>>>>>>> default 0x8a if ARCH_MX6 || ARCH_MX7
> >>>>>>>>>>> default 0x100 if ARCH_UNIPHIER
> >>>>>>>>>>> default 0x140 if ARCH_MVEBU
> >>>>>>>>>>> default 0x200 if ARCH_SOCFPGA || ARCH_AT91
> >>>>>>>>>>> default 0x300 if ARCH_ZYNQ || ARCH_KEYSTONE || OMAP34XX || OMAP44XX || \
> >>>>>>>>>>>          OMAP54XX || AM33XX || AM43XX || ARCH_K3
> >>>>>>>>>>> default 0x4000 if ARCH_ROCKCHIP
> >>>>>>>>>>> help
> >>>>>>>>>>>   Address on the MMC to load U-Boot from, when the MMC is being used
> >>>>>>>>>>>   in raw mode. Units: MMC sectors (1 sector = 512 bytes).
> >>>>>>>>>>>
> >>>>>>>>>>> The "default xxx if FOO" list is crazy!
> >>>>>>>>>>>
> >>>>>>>>>>> I think we need to discuss and come up with a unified way of doing this.
> >>>>>>>>>>>
> >>>>>>>>>>> I personally am in favor of the overriding mechanism, which is how
> >>>>>>>>>>> current x86 architecture Kconfig is organized. In the x86 arch
> >>>>>>>>>>> Kconfig, we have:
> >>>>>>>>>>>
> >>>>>>>>>>> # board-specific options below
> >>>>>>>>>>> source "board/advantech/Kconfig"
> >>>>>>>>>>> ...
> >>>>>>>>>>>
> >>>>>>>>>>> # platform-specific options below
> >>>>>>>>>>> source "arch/x86/cpu/apollolake/Kconfig"
> >>>>>>>>>>> ...
> >>>>>>>>>>>
> >>>>>>>>>>> # architecture-specific options below
> >>>>>>>>>>>
> >>>>>>>>>>> So that board behavior overrides platform/SoC behavior over
> >>>>>>>>>>> architecture behavior, and over the U-Boot common Kconfig options.
> >>>>>>>>>>> This to me is very clear.
> >>>>>>>>>>
> >>>>>>>>>> The problem I believe with overrides is that causes such churn to the
> >>>>>>>>>> defconfigs on re-sync as they're used.
> >>>>>>>>>>
> >>>>>>>>>> Personally I think this shows one of the problems with Kconfig as a
> >>>>>>>>>> language and the need for some tool to take these values from something
> >>>>>>>>>> else and spit out defines.  Perhaps now that u-boot, is a defined DT
> >>>>>>>>>> prefix we could something-something our way through storing these in
> >>>>>>>>>> -u-boot.dtsi files and use dtoc to get a header we use everywhere ala
> >>>>>>>>>> kconfig.h?
> >>>>>>>>>
> >>>>>>>>> I think for the case Bin mentions yes we could do that. Certainly not
> >>>>>>>>> nice to put this sort of thing in Kconfig. Perhaps a DT 'config/' node
> >>>>>>>>> a bit like the existing 'chosen', that dtoc emits?
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> Yes, for the example I gave, we can certainly put such information
> >>>>>>>> into DT config/ node. But this example may not be a good one because
> >>>>>>>> we cannot just put every Kconfig option into DT.
> >>>>>>
> >>>>>> Bin can you suggest an example with lots of variability, that is not
> >>>>>> suitable for DT?
> >>>>>
> >>>>> How about this one?
> >>>>>
> >>>>> config BUILD_TARGET
> >>>>>         string "Build target special images"
> >>>>>         default "u-boot-with-spl.sfp" if TARGET_SOCFPGA_ARRIA10
> >>>>>         default "u-boot-with-spl.sfp" if TARGET_SOCFPGA_GEN5
> >>>>>         default "u-boot-spl.kwb" if ARCH_MVEBU && SPL
> >>>>>         default "u-boot-elf.srec" if RCAR_GEN3
> >>>>>         default "u-boot.itb" if SPL_LOAD_FIT && (ARCH_ROCKCHIP || \
> >>>>>                                 ARCH_SUNXI || RISCV || ARCH_ZYNQMP)
> >>>>>         default "u-boot.kwb" if ARCH_KIRKWOOD
> >>>>>         default "u-boot-with-spl.bin" if ARCH_AT91 && SPL_NAND_SUPPORT
> >>>>>         default "u-boot-with-spl.imx" if ARCH_MX6 && SPL
> >>>>>         help
> >>>>>           Some SoCs need special image types (e.g. U-Boot binary
> >>>>>           with a special header) as build targets. By defining
> >>>>>           CONFIG_BUILD_TARGET in the SoC / board header, this
> >>>>>           special image will be automatically built upon calling
> >>>>>           make / buildman.
> >>>>>
> >>>>> It might not be a good example neither :)
> >>>>>
> >>>>> But what I really wanted to get some agreement among custodians about
> >>>>> the style. Which style do we want to go?
> >>>>
> >>>> I've been encouraging the "put defaults together" style as that I hope
> >>>> makes the problem scope for "now make handling this cleaner" easier to
> >>>> see.  When something is scattered in N files it's easier to miss what
> >>>> everyone looks like.  As you note, that too might fit well enough into a
> >>>> /config node or similar type thing instead.  It didn't really belong in
> >>>> include/config/${board}.h (or more often one of the common include files
> >>>> there) and doesn't quite fit in with Kconfig well either.
> >>>>
> >>>
> >>> Just keep in your mind that DT config {} node is something what u-boot
> >>> is using but never been reviewed by DT guys. We should normally use
> >>> chosen node instead (for example u-boot,spl-boot-order is IMHO one good
> >>> example).
> >>
> >> The chosen node is for passing things to linux, not (generally) for
> >> use within U-Boot.
> >
> > Well, it would be worth double checking what DT-the-spec says for
> > /chosen and also what other projects are using.  Maybe a patch to the DT
> > spec would be in order, maybe not.
>
>
> "The /chosen node does not represent a real device in the system but
> describes parameters chosen or specified by the
> system firmware at run time. It shall be a child of the root node."
>
> We can extend it to also mentioned that it can be parameters for
> firmware itself.
> Also there is a table where we can list u-boot, are optional parameters
> used for U-Boot bootloader.

I think it would be better to just have a new node, perhaps something
U-Boot-specific, or perhaps something intended for use by firmware and
not passed to the kernel, where these properties would just cause
confusion.

>
>
>
> >
> >>> I am trying to keep u-boot and linux dts in sync. I know we have
> >>> -uboot.dtsi where a lot of them are just adding u-boot,dm-XX; or binman
> >>> but if we want to use configuration via DT more then we should do it
> >>> properly in a compatible way.
> >>
> >> I think this is a worthy effort. I hope you can have some success. If
> >> you do I am very happy to help. But if the answer is that U-Boot has
> >> to only use Linux bindings, then it isn't going to work so well. I'd
> >> like to see more acceptance of the constraints of a boot loader with
> >> the bindings. The discussion over the 'clock' property in a debug UART
> >> is an example of that.
> >
> > Yes, the "clocks" thing is a good example of why we need to be a bit
> > cautious.  We've had a few cases of having to rework our code to deal
> > with DT changes / corrections.
>
> Any link?

Not sure - perhaps Tom is thinking of the UART clock problem. We don't
want to init the clock driver (or even driver model itself) to get the
UART running.

>
>
> >>> (Just a side node we should move u-boot,dm-pre-reloc = <&.. &.. &..> to
> >>> chosen node and iterate over instead of putting property to every node.
> >>
> >> (side node...it has a u-boot, prefix, so surely it is fine from a
> >> binding perspective...this is the sort of Linux-centric approach that
> >> drove me nuts when I tried to upstream things)
> >>
> >> This has been discussed before and it was the other option when I
> >> originally implemented it. I was worried about confusion efficiency at
> >> the time. I think it is possible to implement it reasonably
> >> efficiently by having a list of node offsets to take notice of, for
> >> the prior-to=relocation. For SPL/TPL it has already been dealt with by
> >> fdtgrep.
> >>
> >> But for confusion I am not so sure. The way it works at the moment we
> >> can put the tags in a .dtsi file that is shared among multiple boards.
> >> We really don't have a way to merge multiple properties into one,
> >> which is what I think would be needed here.
> >
> > I also remain unconvinced that it would be acceptable for the primary
> > authoritative DTS files for a platform to contain u-boot,dm-pre-reloc,
> > etc properties given other pushback over the years about changes that
> > result in growth of the DTB binary and in turn memory consumption in
> > Linux, etc.  I think it's good if we can inject what we need via
> > -u-boot.dtsi files and keep everything else in-sync as-is.
>
> We can try it and let's see. I expect the biggest problem will be that
> Rob is quite busy and it takes a lot of time to get things reviewed.
>
> >>> Take a look at
> >>> https://lists.denx.de/pipermail/u-boot/2020-April/405155.html
> >>> and follow up
> >>> https://lists.denx.de/pipermail/u-boot/2020-May/412102.html
> >>>
> >>
> >> Looks good, particularly the last one. Did your patch get applied?
> >
> > Yes, I believe that is the patch Rob applied.
>
> yes, it is in linux-next.

Great news!! That is real progress.

>
> >
> >>> I am definitely fine with moving stuff to DT but we should do it in
> >>> similar way how Linux is doing it today. Which means use the whole
> >>> infrastructure for checking these DTs.
> >>
> >> Are we missing something? I certainly get plenty of warnings when I
> >> build stuff :-) Perhaps we should start by syncing over the DTs to get
> >> rid of all the warnings?
> >
> > It would be good for various SoCs to re-sync with Linux and silence,
> > hopefully, a bunch of warnings.  I just worry about things breaking when
> > we resync so they need to be tested.
>
> I am considering this as platform maintainer responsibility. I expect we
> have platforms which are maintain regularly and amount of warnings just
> descrease over time. And then we have unmaintain platforms which none
> takes care about it. It should be custodian responsibility to take care
> about it. If even custodian doesn't care then we should consider to
> remove these platforms.

Regards,
Simon

More information about the U-Boot-Custodians mailing list