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

[Frank Rowand]

On 3/15/21 5:12 PM, Laurent Pinchart wrote:
> Hi Yamada-san,
> 
> On Tue, Mar 16, 2021 at 02:43:45AM +0900, Masahiro Yamada wrote:
>> On Mon, Mar 15, 2021 at 3:40 PM Viresh Kumar wrote:
>>> On 14-03-21, 20:16, Frank Rowand wrote:
>>>> On 3/12/21 11:11 PM, Frank Rowand wrote:
>>>>> On 3/12/21 1:13 AM, Viresh Kumar wrote:
>>>>>> On 12-03-21, 01:09, Frank Rowand wrote:
>>>>>>> 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.
>>>>>>
>>>>>> What about creating links instead then ?
>>>>>>
>>>>>
>>>>> I don't really like the idea of using links here.
>>>>>
>>>>> Maybe it is best to make the changes needed to allow the unittest
>>>>> overlays to be .dtso instead of .dts.
>>>>>
>>>>> Off the top of my head:
>>>>>
>>>>>   scripts/Makefile.lib:
>>>>>      The rule for %.dtb.S invokes cmd_dt_S_dtb, which puts the
>>>>>      overlay data in section .dtb.init.rodata, with a label
>>>>>      pointing to the beginning of the overlay __dtb_XXX_begin and
>>>>>      a label pointing to the end of the overlay __dtb_XXX_end,
>>>>>      for the overlay named XXX.  I _think_ that you could simply
>>>>>      add a corresponding rule for %.dtbo.S using a new command
>>>>>      cmd_dt_S_dtbo (the same as cmd_dt_S_dtb, except use labels
>>>>>      __dtbo_XXX_begin and __dtbo_XXX_end).
>>>>
>>>> If you do the above, please put it in drivers/of/unittest-data/Makefile
>>>> instead of scripts/Makefile.lib because it is unittest.c specific and
>>>> not meant to be anywhere else in the kernel.
>>>
>>> What about doing this then in unittest's Makefile instead (which I
>>> already suggested earlier), that will make everything work just fine
>>> without any other changes ?
>>>
>>> +# Required for of unittest files as they can't be renamed to .dtso
>>> +$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE
>>> +       $(call if_changed_dep,dtc)
>>>
>>
>> If those rules are only needed by drivers/of/unittest-data/Makefile,
>> they should not be located in scripts/Makefile.lib.
>>
>> But how can we fix drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a779*.dts
>> if these are doing bad things.
>> They seem to be overlay files even though the file name suffix is .dts
> 
> That is correct, they are overlays. I have no issue with those files
> being renamed to .dtso if that can help (but I haven't checked if that
> would have any adverse effect on the R-Car DU driver).

As Laurent replied, yes these are overlays.  They were grandfathered in
as a deprecated use of overlays.

> 
> These files are there to ensure backward compatibility with older DT
> bindings. The change was made 3 years ago and I wouldn't object to
> dropping this completely, but I understand I may not be the most
> cautious person when it comes to ensuring DT backward compatibility :-)

My memory is that the goal was to eventually remove these overlays
at some point in the future.  If everyone agrees that today is the
proper time, it would be helpful to go ahead and remove these .dts
files and the code that uses them.

-Frank

> 
>> $ find drivers -name '*.dts'
>> drivers/staging/pi433/Documentation/devicetree/pi433-overlay.dts
>> drivers/staging/mt7621-dts/gbpc2.dts
>> drivers/staging/mt7621-dts/gbpc1.dts
>> drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts
>> drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts
>> drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts
>> drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts
>> drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts
>> drivers/of/unittest-data/overlay_1.dts
>> drivers/of/unittest-data/testcases.dts
>> drivers/of/unittest-data/overlay_bad_add_dup_node.dts
>> drivers/of/unittest-data/overlay_bad_symbol.dts
>> drivers/of/unittest-data/overlay_0.dts
>> drivers/of/unittest-data/overlay_11.dts
>> drivers/of/unittest-data/overlay_gpio_03.dts
>> drivers/of/unittest-data/overlay_gpio_04a.dts
>> drivers/of/unittest-data/overlay_gpio_04b.dts
>> drivers/of/unittest-data/overlay_5.dts
>> drivers/of/unittest-data/overlay_bad_add_dup_prop.dts
>> drivers/of/unittest-data/overlay_gpio_01.dts
>> drivers/of/unittest-data/overlay_10.dts
>> drivers/of/unittest-data/overlay_7.dts
>> drivers/of/unittest-data/overlay_bad_phandle.dts
>> drivers/of/unittest-data/overlay_3.dts
>> drivers/of/unittest-data/overlay_6.dts
>> drivers/of/unittest-data/overlay_8.dts
>> drivers/of/unittest-data/overlay_12.dts
>> drivers/of/unittest-data/overlay_gpio_02a.dts
>> drivers/of/unittest-data/overlay_gpio_02b.dts
>> drivers/of/unittest-data/overlay_4.dts
>> drivers/of/unittest-data/overlay.dts
>> drivers/of/unittest-data/overlay_9.dts
>> drivers/of/unittest-data/overlay_2.dts
>> drivers/of/unittest-data/overlay_15.dts
>> drivers/of/unittest-data/overlay_base.dts
>> drivers/of/unittest-data/overlay_13.dts
>