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

[Michal Orzel <michal.orzel@xxxxxxx>]

On 18.03.2021 10:20, Jan Beulich wrote:
> 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
> 
Then if both of you agree that _sdtb should be defined in dtb.S, I will make it 
in v8

Michal