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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.03.2021 08:21, Michal Orzel wrote:
> Hi Julien,
> 
> On 16.03.2021 15:54, Julien Grall wrote:
>> Hi Michal,
>>
>> On 15/03/2021 09:23, Michal Orzel wrote:
>>> Currently in order to link existing DTB into Xen image
>>> we need to either specify option CONFIG_DTB_FILE on the
>>> command line or manually add it into .config.
>>> Add Kconfig entry: CONFIG_DTB_FILE
>>> to be able to provide the path to DTB we want to embed
>>> into Xen image. If no path provided - the dtb will not
>>> be embedded.
>>>
>>> Remove the line: AFLAGS-y += -DCONFIG_DTB_FILE=\"$(CONFIG_DTB_FILE)\"
>>> as it is not needed since Kconfig will define it in a header
>>> with all the other config options.
>>>
>>> Make a change in the linker script from:
>>> _sdtb = .;
>>> to:
>>> PROVIDE(_sdtb = .);
>>> to avoid creation of _sdtb if there is no reference to it.
>>
>> This means that if someone is using #ifdef CONFIG_DTB_FILE rather than 
>> .ifnes, _sdtb will get defined.
> 
> Do we really want to overengineer something that simple?
> Why would someone use #ifdef CONFIG_DTB_FILE?
> In my opinion the rule of thumb is that you don't use #ifdef on configs of 
> string type.
> Using #ifdef CONFIG_DTB_FILE means that someone modifying the code did not 
> even spend 1 minute checking the Kconfig.
> 
> If you do not agree, I can modify the code so _sdtb will be created in dtb.S.
> In that case I would like Jan Beulich to give his opinion before I will send 
> v8.

TBH I'd find it more natural in any event if the symbol came from
dtb.S. So far I was assuming there was some (hidden) reason why
this wouldn't work.

Jan