Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files

From: Frank Rowand <frowand.list@gmail.com>
To: Viresh Kumar <viresh.kumar@linaro.org>,
	Masahiro Yamada <masahiroy@kernel.org>
Cc: Michal Marek <michal.lkml@markovi.net>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	David Gibson <david@gibson.dropbear.id.au>,
	Michal Simek <michal.simek@xilinx.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Anmar Oueja <anmar.oueja@linaro.org>,
	Bill Mills <bill.mills@linaro.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH V11 3/5] kbuild: Allow .dtso format for overlay source files
Date: Fri, 12 Mar 2021 01:09:25 -0600	[thread overview]
Message-ID: <4d9bee7a-416e-50a1-65a5-0674ae83d42e@gmail.com> (raw)
In-Reply-To: <17c65559-865f-f742-660f-0ab30ed45d90@gmail.com>

On 3/12/21 1:03 AM, Frank Rowand wrote:
> Hi Viresh,
> 
> On 3/11/21 10:47 PM, Viresh Kumar wrote:
>> On 10-03-21, 20:24, Masahiro Yamada wrote:
>>> Even without "-I dts",
>>>
>>>    inform = guess_input_format(arg, "dts");
>>>
>>> seems to fall back to "dts" anyway,
>>> but I guess you wanted to make this explicit, correct?
>>
>>
>>>> +# Required for of unit-test files as they can't be renamed to .dtso
>>>
>>> If you go with *.dtso, I think you will rename
>>> all *.dts under the drivers/ directory.
>>>
>>> What is blocking you from making this consistent?
>>
>> What about this patch instead ? This localizes the dts->dtbo hack to
>> unitest's Makefile at least.
> 
> It is late here, so I am not going to take the time to actually try what
> I am going to suggest.  I apologize in advance if I send you off on a
> wild goose chase.
> 
> Would it work to create a .dtso file for each of the unittest overlay .dts
> files, where the .dtso would simply #include the .dts file.  Then the corresponding
> .dtbo files could be added to the obj-$(CONFIG_OF_OVERLAY) list.

I suggested having the .dtso files include the .dts file because that is a relatively
small and easy change to test.  What would probably make more sense is the rename
the existing overlay .dts files to be .dtso files and then for each overlay .dtso
file create a new .dts file that #includes the corresponding .dtso file.  This is
more work and churn, but easier to document that the .dts files are a hack that is
needed so that the corresponding .dtb.S files will be generated.

If it works, I am fine with either approach.

-Frank

> 
> I would like to avoid having the unitest-data/Makefile have different rules to
> build objects because then the normal build rule is not being tested.
> 
> -Frank
> 
>>
>> diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
>> index a5d2d9254b2c..9f3426ec3fab 100644
>> --- a/drivers/of/unittest-data/Makefile
>> +++ b/drivers/of/unittest-data/Makefile
>> @@ -86,3 +86,7 @@ static_test_1-dtbs := static_base_1.dtb $(apply_static_overlay_1)
>>  static_test_2-dtbs := static_base_2.dtb $(apply_static_overlay_2)
>>  
>>  dtb-$(CONFIG_OF_OVERLAY) += static_test_1.dtb static_test_2.dtb
>> +
>> +# Required for of unittest files as they can't be renamed to .dtso
>> +$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
>> +       $(call if_changed_dep,dtc)
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index bc045a54a34e..77a9be055e51 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -347,7 +347,7 @@ cmd_dtc = $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ;
>>  $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
>>         $(call if_changed_dep,dtc)
>>  
>> -$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
>> +$(obj)/%.dtbo: $(src)/%.dtso $(DTC) FORCE
>>         $(call if_changed_dep,dtc)
>>  
>>  overlay-y := $(addprefix $(obj)/, $(overlay-y))
>> @@ -375,6 +375,9 @@ endef
>>  $(obj)/%.dt.yaml: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE
>>         $(call if_changed_rule,dtc,yaml)
>>  
>> +$(obj)/%.dt.yaml: $(src)/%.dtso $(DTC) $(DT_TMP_SCHEMA) FORCE
>> +       $(call if_changed_rule,dtc,yaml)
>> +
>>  dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)
>>  
>>  # Bzip2
>>
> 