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

[Michal Orzel <michal.orzel@xxxxxxx>]

On 09.03.2021 15:18, Jan Beulich wrote:
> On 09.03.2021 14:55, Michal Orzel wrote:
>>
>>
>> On 09.03.2021 14:32, Julien Grall wrote:
>>>
>>>
>>> On 09/03/2021 11:07, Jan Beulich wrote:
>>>> On 09.03.2021 11:20, Julien Grall wrote:
>>>>> On 09/03/2021 07:34, Michal Orzel wrote:
>>>>>> On 08.03.2021 15:31, Julien Grall wrote:
>>>>>>> On 08/03/2021 13:59, Michal Orzel wrote:
>>>>>>>> --- a/xen/arch/arm/Makefile
>>>>>>>> +++ b/xen/arch/arm/Makefile
>>>>>>>> @@ -68,7 +68,7 @@ extra-y += $(TARGET_SUBARCH)/head.o
>>>>>>>>       #obj-bin-y += ....o
>>>>>>>>     -ifdef CONFIG_DTB_FILE
>>>>>>>> +ifneq ($(CONFIG_DTB_FILE),"")
>>>>>>>>     obj-y += dtb.o
>>>>>>>>     AFLAGS-y += -DCONFIG_DTB_FILE=\"$(CONFIG_DTB_FILE)\"
>>>>>>>>     endif
>>>>>>>> @@ -137,8 +137,6 @@ asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c
>>>>>>>>     xen.lds: xen.lds.S
>>>>>>>>         $(CPP) -P $(a_flags) -MQ $@ -o $@ $<
>>>>>>>>     -dtb.o: $(CONFIG_DTB_FILE)
>>>>>>>> -
>>>>>>>
>>>>>>> Why is this dropped?
>>>>>> 1)This line is not needed as it has no impact on creating dtb.o
>>>>>> 2)It causes the build failure once CONFIG_DTB_FILE option is in the 
>>>>>> Kconfig as string within quotes.
>>>>>
>>>>> Because of 1), this should have ideally be part of a separate patch. But
>>>>> I am OK to keep it in this patch so long it is explained in the commit
>>>>> message.
>>>>
>>>> Wasn't the intention to have dtb.o re-compiled when the blob
>>>> has changed? This would be lost with the removal of this line.
>>>
>>> Ah yes. I was only thinking about a name change (this would be caught via 
>>> the update of the config header) and not a file update.
>>>
>> I already pushed v3 but I agree. Something like this would do the job:
>> dtb.o: $(subst $\",,$(CONFIG_DTB_FILE))
>> to remove quotes
> 
> Besides struggling with the $\", may I suggest
> $(patsubst "%",%,$(CONFIG_DTB_FILE))? If the double quote needs
> special treatment, I think it wants to be done via an abstraction
> similar to squote (near the top of ./Config.mk).
> 
The line $(patsubst "%",%,$(CONFIG_DTB_FILE)) is sufficient.
I checked and dtb.o is recompiled when the blob is changed.
I will fix it in v4
> Jan
> 
Michal