Re: [PATCH] arm: Add Kconfig entry to select CONFIG_DTB_FILE

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.03.2021 14:11, Michal Orzel wrote:
> 
> 
> On 08.03.2021 12:28, Jan Beulich wrote:
>> On 08.03.2021 12:02, Michal Orzel wrote:
>>> On 08.03.2021 11:00, Jan Beulich wrote:
>>>> On 08.03.2021 10:52, Michal Orzel wrote:
>>>>> +config DTB_FILE
>>>>> + string "Absolute path to device tree blob"
>>>>> + default ""
>>>>> + depends on LINK_DTB
>>>>> + ---help---
>>>>> +   When using a bootloader that has no device tree support or when there
>>>>> +   is no bootloader at all, use this option to specify the absolute path
>>>>> +   to a device tree that will be linked directly inside Xen binary.
>>>>
>>>> How is selecting LINK_DTB but leaving DTB_FILE at an empty string
>>>> different from not having a LINK_DTB setting at all?
>>>>
>>> LINK_DTB acts as a switch to allow setting the dtb path. Not having 
>>> LINK_DTB option will result in
>>> a build failure each time the user does not want to embed dtb into 
>>> Xen(DTB_FILE is empty).
>>
>> Which isn't any different from having LINK_DTB and leaving the
>> string empty, is it? I.e. imo no improved user experience.
>>
>>> I do not see why someone would want to select LINK_DTB leaving DTB_FILE as 
>>> an empty string.
>>
>> People may not "want" to, but simply think accepting the default
>> is fine, considering they've already said to link in some DTB.
>> It may be obvious to you that there's no good default here, but
>> it may not be to the person configuring their Xen. I'm guessing
>> here, but did you try leaving out the default line? Would this
>> make kconfig insist on the person to type in something? (Likely
>> an empty string would still be accepted. As would be a relative
>> path, despite what the help text says; I guess some forms of
>> relative paths may even work.)
>>
> There is no option here to make kconfig insist on person to type something.
> There is one solution. If I change in here:
> https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/Makefile;h=16e6523e2cc6072b7d4cbcdeaf4726b7a9b1d381;hb=HEAD#l71
> from:
> ifdef CONFIG_DTB_FILE
> to:
> ifneq ($(CONFIG_DTB_FILE),"")
> then if user selects LINK_DTB but does not provide dtb path, the dtb will not 
> be embedded into Xen.

I think this would be preferable plus eliminate the need for the
separate LINK_DTB option.

Jan