Re: [PULL 36/58] pc-bios: Move device tree files in their own subdir

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 2/6/25 13:56, BALATON Zoltan wrote:
> On Mon, 2 Jun 2025, Philippe Mathieu-Daudé wrote:
>> On 30/5/25 02:54, BALATON Zoltan wrote:
>>> On Thu, 29 May 2025, Bernhard Beschow wrote:
>>>> Am 25. April 2025 15:28:20 UTC schrieb "Philippe Mathieu-Daudé" 
>>>> <philmd@linaro.org>:
>>>>> From: BALATON Zoltan <balaton@eik.bme.hu>
>>>>>
>>>>> We have several device tree files already and may have more in the
>>>>> future so add a new dtb subdirectory and move device tree files there
>>>>> so they are not mixed with ROM binaries.
>>>>>
>>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>> Message-ID: 
>>>>> <57f179bd3904c1f2ca062ca4d4ff9592bb4f4daa.1745402140.git.balaton@eik.bme.hu>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>> ---
>>>>> MAINTAINERS                                |   2 +-
>>>>> system/datadir.c                           |   4 +++-
>>>>> pc-bios/{ => dtb}/bamboo.dtb               | Bin
>>>>> pc-bios/{ => dtb}/bamboo.dts               |   0
>>>>> pc-bios/{ => dtb}/canyonlands.dtb          | Bin
>>>>> pc-bios/{ => dtb}/canyonlands.dts          |   0
>>>>> pc-bios/dtb/meson.build                    |  23 +++++++++++++++++++++
>>>>> pc-bios/{ => dtb}/petalogix-ml605.dtb      | Bin
>>>>> pc-bios/{ => dtb}/petalogix-ml605.dts      |   0
>>>>> pc-bios/{ => dtb}/petalogix-s3adsp1800.dtb | Bin
>>>>> pc-bios/{ => dtb}/petalogix-s3adsp1800.dts |   0
>>>>> pc-bios/meson.build                        |  23 +--------------------
>>>>> qemu.nsi                                   |   2 +-
>>>>> 13 files changed, 29 insertions(+), 25 deletions(-)
>>>>> rename pc-bios/{ => dtb}/bamboo.dtb (100%)
>>>>> rename pc-bios/{ => dtb}/bamboo.dts (100%)
>>>>> rename pc-bios/{ => dtb}/canyonlands.dtb (100%)
>>>>> rename pc-bios/{ => dtb}/canyonlands.dts (100%)
>>>>> create mode 100644 pc-bios/dtb/meson.build
>>>>> rename pc-bios/{ => dtb}/petalogix-ml605.dtb (100%)
>>>>> rename pc-bios/{ => dtb}/petalogix-ml605.dts (100%)
>>>>> rename pc-bios/{ => dtb}/petalogix-s3adsp1800.dtb (100%)
>>>>> rename pc-bios/{ => dtb}/petalogix-s3adsp1800.dts (100%)
>>>>>
>>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>>> index 661a47db5ac..d82d962f1a4 100644
>>>>> --- a/MAINTAINERS
>>>>> +++ b/MAINTAINERS
>>>>> @@ -1581,7 +1581,7 @@ F: hw/pci-host/ppc440_pcix.c
>>>>> F: hw/display/sm501*
>>>>> F: hw/ide/sii3112.c
>>>>> F: hw/rtc/m41t80.c
>>>>> -F: pc-bios/canyonlands.dt[sb]
>>>>> +F: pc-bios/dtb/canyonlands.dt[sb]
>>>>> F: pc-bios/u-boot-sam460ex-20100605.bin
>>>>> F: roms/u-boot-sam460ex
>>>>> F: docs/system/ppc/amigang.rst
>>>>> diff --git a/system/datadir.c b/system/datadir.c
>>>>> index e450b84ce91..f96f8fc2646 100644
>>>>> --- a/system/datadir.c
>>>>> +++ b/system/datadir.c
>>>>> @@ -44,9 +44,11 @@ char *qemu_find_file(QemuFileType type, const 
>>>>> char *name)
>>>>>
>>>>>     switch (type) {
>>>>>     case QEMU_FILE_TYPE_BIOS:
>>>>> -    case QEMU_FILE_TYPE_DTB:
>>>>>         subdir = "";
>>>>>         break;
>>>>> +    case QEMU_FILE_TYPE_DTB:
>>>>> +        subdir = "dtb/";
>>>>> +        break;
>>>>>     case QEMU_FILE_TYPE_KEYMAP:
>>>>>         subdir = "keymaps/";
>>>>>         break;
>>>>> diff --git a/pc-bios/bamboo.dtb b/pc-bios/dtb/bamboo.dtb
>>>>> similarity index 100%
>>>>> rename from pc-bios/bamboo.dtb
>>>>> rename to pc-bios/dtb/bamboo.dtb
>>>>> diff --git a/pc-bios/bamboo.dts b/pc-bios/dtb/bamboo.dts
>>>>> similarity index 100%
>>>>> rename from pc-bios/bamboo.dts
>>>>> rename to pc-bios/dtb/bamboo.dts
>>>>> diff --git a/pc-bios/canyonlands.dtb b/pc-bios/dtb/canyonlands.dtb
>>>>> similarity index 100%
>>>>> rename from pc-bios/canyonlands.dtb
>>>>> rename to pc-bios/dtb/canyonlands.dtb
>>>>> diff --git a/pc-bios/canyonlands.dts b/pc-bios/dtb/canyonlands.dts
>>>>> similarity index 100%
>>>>> rename from pc-bios/canyonlands.dts
>>>>> rename to pc-bios/dtb/canyonlands.dts
>>>>> diff --git a/pc-bios/dtb/meson.build b/pc-bios/dtb/meson.build
>>>>> new file mode 100644
>>>>> index 00000000000..7a71835bca7
>>>>> --- /dev/null
>>>>> +++ b/pc-bios/dtb/meson.build
>>>>> @@ -0,0 +1,23 @@
>>>>> +dtbs = [
>>>>> +  'bamboo.dtb',
>>>>> +  'canyonlands.dtb',
>>>>> +  'petalogix-ml605.dtb',
>>>>> +  'petalogix-s3adsp1800.dtb',
>>>>
>>>> Was it intended that the suffix changed from .dts to .dtb? This 
>>>> change isn't motivated in the commit message and usually source 
>>>> files rather than generated artifacts are listed in build files.
>>>
>>> I think I either found similar python code somewhere and followed 
>>> that because meson and python is not something I understand well. Or 
>>> changed it because we always need the list of dtbs to install but 
>>> only need the dts when compiling so this way we don't need another 
>>> list or replacing suffix twice.
>>
>> Should we revert?
> 
> Why? It works just lists the build targets instead of source files to 
> avoid converting between the two twice. So what it does was not 
> contested only the way it was implemented had a nit. I don't think this 
> is worth changing because of that.

Well, it is a hidden change that wasn't noticed during review.