The way to use Kconfig in U-Boot
Michal Simek
michal.simek at xilinx.com
Tue May 26 08:34:59 CEST 2020
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 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?
>>> (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.
>
>>> 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.
Thanks,
Michal
More information about the U-Boot-Custodians
mailing list