diff mbox series

[v2] pc-bios/meson.build: Silent unuseful DTC warnings

Message ID 20231006064750.33852-1-philmd@linaro.org
State New
Headers show
Series [v2] pc-bios/meson.build: Silent unuseful DTC warnings | expand

Commit Message

Philippe Mathieu-Daudé Oct. 6, 2023, 6:47 a.m. UTC
QEMU consumes some device tree blobs, so these have been committed
to the tree in as firmware, along with the device tree source used
to generate them. We know the blobs are "good enough" to have QEMU
boot a system, so we don't really maintain and rebuild the sources.

These blobs were generated with older 'dtc' binaries. We use the
v1.6.1 version since 2021 (commit 962fde57b7 "dtc: Update to version
1.6.1").

Since commit 6e0dc9d2a8 ("meson: compile bundled device trees"),
if dtc binary is available, it is directly used to compile the
device tree sources. New versions of 'dtc' add checks which display
warnings or errors. Our sources are a bit old, so dtc v1.6.1 now
emit the following warnings on a fresh build:

  [163/3414] Generating pc-bios/canyonlands.dts with a custom command
  pc-bios/canyonlands.dts:47.9-50.4: Warning (unit_address_vs_reg): /memory: node has a reg or ranges property, but no unit name
  pc-bios/canyonlands.dts:210.13-429.5: Warning (unit_address_vs_reg): /plb/opb: node has a reg or ranges property, but no unit name
  pc-bios/canyonlands.dts:464.26-504.5: Warning (pci_bridge): /plb/pciex@d00000000: node name is not "pci" or "pcie"
  pc-bios/canyonlands.dts:506.26-546.5: Warning (pci_bridge): /plb/pciex@d20000000: node name is not "pci" or "pcie"
  pc-bios/canyonlands.dtb: Warning (unit_address_format): Failed prerequisite 'pci_bridge'
  pc-bios/canyonlands.dtb: Warning (pci_device_reg): Failed prerequisite 'pci_bridge'
  pc-bios/canyonlands.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
  pc-bios/canyonlands.dts:268.14-289.7: Warning (avoid_unnecessary_addr_size): /plb/opb/ebc/ndfc@3,0: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
  [164/3414] Generating pc-bios/petalogix-s3adsp1800.dts with a custom command
  pc-bios/petalogix-s3adsp1800.dts:258.33-266.5: Warning (interrupt_provider): /plb/interrupt-controller@81800000: Missing #address-cells in interrupt provider
  [165/3414] Generating pc-bios/petalogix-ml605.dts with a custom command
  pc-bios/petalogix-ml605.dts:234.39-241.5: Warning (interrupt_provider): /axi/interrupt-controller@81800000: Missing #address-cells in interrupt provider
  [177/3414] Generating pc-bios/bamboo.dts with a custom command
  pc-bios/bamboo.dts:45.9-48.4: Warning (unit_address_vs_reg): /memory: node has a reg or ranges property, but no unit name
  pc-bios/bamboo.dts:87.13-154.5: Warning (unit_address_vs_reg): /plb/opb: node has a reg or ranges property, but no unit name
  pc-bios/bamboo.dts:198.3-50: Warning (chosen_node_stdout_path): /chosen:linux,stdout-path: Use 'stdout-path' instead
  pc-bios/bamboo.dts:87.13-154.5: Warning (interrupts_property): /plb/opb: Missing interrupt-parent
  pc-bios/bamboo.dts:100.14-108.6: Warning (interrupts_property): /plb/opb/ebc: Missing interrupt-parent

From QEMU perspective, these warnings are not really useful. It is
the responsibility of developers adding DT source/blob to QEMU
repository to check the source doesn't produce warnings, but as
long as the blob is useful enough, QEMU can consume it. So these
warnings don't add any value, instead they are noisy and might
distract us to focus on important warnings. Better disable them.

'dtc' provides the '--quiet' option for that:

  $ dtc --help
  Usage: dtc [options] <input file>

  Options: -[qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AThv]
    -q, --quiet
          Quiet: -q suppress warnings, -qq errors, -qqq all

Update meson to disable these unuseful DTC warnings.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
v2: Complete $Subject

Note, meson outputs "Generating dts" instead of "Generating dtb".
---
 pc-bios/meson.build | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Cédric Le Goater Oct. 6, 2023, 8:13 a.m. UTC | #1
On 10/6/23 08:47, Philippe Mathieu-Daudé wrote:
> QEMU consumes some device tree blobs, so these have been committed
> to the tree in as firmware, along with the device tree source used
> to generate them. We know the blobs are "good enough" to have QEMU
> boot a system, so we don't really maintain and rebuild the sources.
> 
> These blobs were generated with older 'dtc' binaries. We use the
> v1.6.1 version since 2021 (commit 962fde57b7 "dtc: Update to version
> 1.6.1").
> 
> Since commit 6e0dc9d2a8 ("meson: compile bundled device trees"),
> if dtc binary is available, it is directly used to compile the
> device tree sources. New versions of 'dtc' add checks which display
> warnings or errors. Our sources are a bit old, so dtc v1.6.1 now
> emit the following warnings on a fresh build:
> 
>    [163/3414] Generating pc-bios/canyonlands.dts with a custom command
>    pc-bios/canyonlands.dts:47.9-50.4: Warning (unit_address_vs_reg): /memory: node has a reg or ranges property, but no unit name
>    pc-bios/canyonlands.dts:210.13-429.5: Warning (unit_address_vs_reg): /plb/opb: node has a reg or ranges property, but no unit name
>    pc-bios/canyonlands.dts:464.26-504.5: Warning (pci_bridge): /plb/pciex@d00000000: node name is not "pci" or "pcie"
>    pc-bios/canyonlands.dts:506.26-546.5: Warning (pci_bridge): /plb/pciex@d20000000: node name is not "pci" or "pcie"
>    pc-bios/canyonlands.dtb: Warning (unit_address_format): Failed prerequisite 'pci_bridge'
>    pc-bios/canyonlands.dtb: Warning (pci_device_reg): Failed prerequisite 'pci_bridge'
>    pc-bios/canyonlands.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
>    pc-bios/canyonlands.dts:268.14-289.7: Warning (avoid_unnecessary_addr_size): /plb/opb/ebc/ndfc@3,0: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>    [164/3414] Generating pc-bios/petalogix-s3adsp1800.dts with a custom command
>    pc-bios/petalogix-s3adsp1800.dts:258.33-266.5: Warning (interrupt_provider): /plb/interrupt-controller@81800000: Missing #address-cells in interrupt provider
>    [165/3414] Generating pc-bios/petalogix-ml605.dts with a custom command
>    pc-bios/petalogix-ml605.dts:234.39-241.5: Warning (interrupt_provider): /axi/interrupt-controller@81800000: Missing #address-cells in interrupt provider
>    [177/3414] Generating pc-bios/bamboo.dts with a custom command
>    pc-bios/bamboo.dts:45.9-48.4: Warning (unit_address_vs_reg): /memory: node has a reg or ranges property, but no unit name
>    pc-bios/bamboo.dts:87.13-154.5: Warning (unit_address_vs_reg): /plb/opb: node has a reg or ranges property, but no unit name
>    pc-bios/bamboo.dts:198.3-50: Warning (chosen_node_stdout_path): /chosen:linux,stdout-path: Use 'stdout-path' instead
>    pc-bios/bamboo.dts:87.13-154.5: Warning (interrupts_property): /plb/opb: Missing interrupt-parent
>    pc-bios/bamboo.dts:100.14-108.6: Warning (interrupts_property): /plb/opb/ebc: Missing interrupt-parent
> 
>  From QEMU perspective, these warnings are not really useful. It is
> the responsibility of developers adding DT source/blob to QEMU
> repository to check the source doesn't produce warnings, but as
> long as the blob is useful enough, QEMU can consume it. So these
> warnings don't add any value, instead they are noisy and might
> distract us to focus on important warnings. Better disable them.
> 
> 'dtc' provides the '--quiet' option for that:
> 
>    $ dtc --help
>    Usage: dtc [options] <input file>
> 
>    Options: -[qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AThv]
>      -q, --quiet
>            Quiet: -q suppress warnings, -qq errors, -qqq all
> 
> Update meson to disable these unuseful DTC warnings.


Why not try fixing the .dts instead ? These still exist under Linux :

   ./arch/powerpc/boot/dts/canyonlands.dts
   ./arch/powerpc/boot/dts/bamboo.dts


C.




> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> v2: Complete $Subject
> 
> Note, meson outputs "Generating dts" instead of "Generating dtb".
> ---
>   pc-bios/meson.build | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/pc-bios/meson.build b/pc-bios/meson.build
> index e67fa433a1..162663fed6 100644
> --- a/pc-bios/meson.build
> +++ b/pc-bios/meson.build
> @@ -95,7 +95,8 @@ foreach f : [
>           output: out,
>           install: get_option('install_blobs'),
>           install_dir: qemu_datadir,
> -        command: [ dtc, '-I', 'dts', '-O', 'dtb', '-o', '@OUTPUT@', '@INPUT0@' ])
> +        command: [ dtc, '-q', '-I', 'dts', '-O', 'dtb',
> +                        '-o', '@OUTPUT@', '@INPUT0@' ])
>     else
>       blobs += out
>     endif
Philippe Mathieu-Daudé Oct. 6, 2023, 8:18 a.m. UTC | #2
On 6/10/23 10:13, Cédric Le Goater wrote:
> On 10/6/23 08:47, Philippe Mathieu-Daudé wrote:
>> QEMU consumes some device tree blobs, so these have been committed
>> to the tree in as firmware, along with the device tree source used
>> to generate them. We know the blobs are "good enough" to have QEMU
>> boot a system, so we don't really maintain and rebuild the sources.
>>
>> These blobs were generated with older 'dtc' binaries. We use the
>> v1.6.1 version since 2021 (commit 962fde57b7 "dtc: Update to version
>> 1.6.1").
>>
>> Since commit 6e0dc9d2a8 ("meson: compile bundled device trees"),
>> if dtc binary is available, it is directly used to compile the
>> device tree sources. New versions of 'dtc' add checks which display
>> warnings or errors. Our sources are a bit old, so dtc v1.6.1 now
>> emit the following warnings on a fresh build:
>>
>>    [163/3414] Generating pc-bios/canyonlands.dts with a custom command
>>    pc-bios/canyonlands.dts:47.9-50.4: Warning (unit_address_vs_reg): 
>> /memory: node has a reg or ranges property, but no unit name
...

>>  From QEMU perspective, these warnings are not really useful. It is
>> the responsibility of developers adding DT source/blob to QEMU
>> repository to check the source doesn't produce warnings, but as
>> long as the blob is useful enough, QEMU can consume it. So these
>> warnings don't add any value, instead they are noisy and might
>> distract us to focus on important warnings. Better disable them.
>>
>> 'dtc' provides the '--quiet' option for that:
>>
>>    $ dtc --help
>>    Usage: dtc [options] <input file>
>>
>>    Options: -[qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AThv]
>>      -q, --quiet
>>            Quiet: -q suppress warnings, -qq errors, -qqq all
>>
>> Update meson to disable these unuseful DTC warnings.
> 
> 
> Why not try fixing the .dts instead ? These still exist under Linux :
> 
>    ./arch/powerpc/boot/dts/canyonlands.dts
>    ./arch/powerpc/boot/dts/bamboo.dts

Because QEMU != Linux, and there isn't always overlap between
communities?

1/ I tried but there isn't much interest:
https://lore.kernel.org/qemu-devel/20230914204206.79351-1-philmd@linaro.org/

2/ Peter and Zoltan raised issue with old firmwares when changing
properties such 'stdout-path', see this thread:
https://lore.kernel.org/qemu-devel/CAFEAcA-WJ9J1YQunJ+bSG=wnpxh1By+Bf18j2CyV7G0vZ=8b7g@mail.gmail.com/
Philippe Mathieu-Daudé Oct. 6, 2023, 8:19 a.m. UTC | #3
On 6/10/23 08:47, Philippe Mathieu-Daudé wrote:
> QEMU consumes some device tree blobs, so these have been committed
> to the tree in as firmware, along with the device tree source used
> to generate them. We know the blobs are "good enough" to have QEMU
> boot a system, so we don't really maintain and rebuild the sources.
> 
> These blobs were generated with older 'dtc' binaries. We use the
> v1.6.1 version since 2021 (commit 962fde57b7 "dtc: Update to version
> 1.6.1").
> 
> Since commit 6e0dc9d2a8 ("meson: compile bundled device trees"),
> if dtc binary is available, it is directly used to compile the
> device tree sources. New versions of 'dtc' add checks which display
> warnings or errors. Our sources are a bit old, so dtc v1.6.1 now
> emit the following warnings on a fresh build:
> 
>    [163/3414] Generating pc-bios/canyonlands.dts with a custom command
>    pc-bios/canyonlands.dts:47.9-50.4: Warning (unit_address_vs_reg): /memory: node has a reg or ranges property, but no unit name
>    pc-bios/canyonlands.dts:210.13-429.5: Warning (unit_address_vs_reg): /plb/opb: node has a reg or ranges property, but no unit name
>    pc-bios/canyonlands.dts:464.26-504.5: Warning (pci_bridge): /plb/pciex@d00000000: node name is not "pci" or "pcie"
>    pc-bios/canyonlands.dts:506.26-546.5: Warning (pci_bridge): /plb/pciex@d20000000: node name is not "pci" or "pcie"
>    pc-bios/canyonlands.dtb: Warning (unit_address_format): Failed prerequisite 'pci_bridge'
>    pc-bios/canyonlands.dtb: Warning (pci_device_reg): Failed prerequisite 'pci_bridge'
>    pc-bios/canyonlands.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
>    pc-bios/canyonlands.dts:268.14-289.7: Warning (avoid_unnecessary_addr_size): /plb/opb/ebc/ndfc@3,0: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>    [164/3414] Generating pc-bios/petalogix-s3adsp1800.dts with a custom command
>    pc-bios/petalogix-s3adsp1800.dts:258.33-266.5: Warning (interrupt_provider): /plb/interrupt-controller@81800000: Missing #address-cells in interrupt provider
>    [165/3414] Generating pc-bios/petalogix-ml605.dts with a custom command
>    pc-bios/petalogix-ml605.dts:234.39-241.5: Warning (interrupt_provider): /axi/interrupt-controller@81800000: Missing #address-cells in interrupt provider
>    [177/3414] Generating pc-bios/bamboo.dts with a custom command
>    pc-bios/bamboo.dts:45.9-48.4: Warning (unit_address_vs_reg): /memory: node has a reg or ranges property, but no unit name
>    pc-bios/bamboo.dts:87.13-154.5: Warning (unit_address_vs_reg): /plb/opb: node has a reg or ranges property, but no unit name
>    pc-bios/bamboo.dts:198.3-50: Warning (chosen_node_stdout_path): /chosen:linux,stdout-path: Use 'stdout-path' instead
>    pc-bios/bamboo.dts:87.13-154.5: Warning (interrupts_property): /plb/opb: Missing interrupt-parent
>    pc-bios/bamboo.dts:100.14-108.6: Warning (interrupts_property): /plb/opb/ebc: Missing interrupt-parent
> 
>  From QEMU perspective, these warnings are not really useful. It is
> the responsibility of developers adding DT source/blob to QEMU
> repository to check the source doesn't produce warnings, but as
> long as the blob is useful enough, QEMU can consume it. So these
> warnings don't add any value, instead they are noisy and might
> distract us to focus on important warnings. Better disable them.
> 
> 'dtc' provides the '--quiet' option for that:
> 
>    $ dtc --help
>    Usage: dtc [options] <input file>
> 
>    Options: -[qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AThv]
>      -q, --quiet
>            Quiet: -q suppress warnings, -qq errors, -qqq all
> 
> Update meson to disable these unuseful DTC warnings.
> 

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
(see 
https://lore.kernel.org/qemu-devel/CAFEAcA-WJ9J1YQunJ+bSG=wnpxh1By+Bf18j2CyV7G0vZ=8b7g@mail.gmail.com/)

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> v2: Complete $Subject
> 
> Note, meson outputs "Generating dts" instead of "Generating dtb".
> ---
>   pc-bios/meson.build | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/pc-bios/meson.build b/pc-bios/meson.build
> index e67fa433a1..162663fed6 100644
> --- a/pc-bios/meson.build
> +++ b/pc-bios/meson.build
> @@ -95,7 +95,8 @@ foreach f : [
>           output: out,
>           install: get_option('install_blobs'),
>           install_dir: qemu_datadir,
> -        command: [ dtc, '-I', 'dts', '-O', 'dtb', '-o', '@OUTPUT@', '@INPUT0@' ])
> +        command: [ dtc, '-q', '-I', 'dts', '-O', 'dtb',
> +                        '-o', '@OUTPUT@', '@INPUT0@' ])
>     else
>       blobs += out
>     endif
Cédric Le Goater Oct. 6, 2023, 8:32 a.m. UTC | #4
On 10/6/23 10:18, Philippe Mathieu-Daudé wrote:
> On 6/10/23 10:13, Cédric Le Goater wrote:
>> On 10/6/23 08:47, Philippe Mathieu-Daudé wrote:
>>> QEMU consumes some device tree blobs, so these have been committed
>>> to the tree in as firmware, along with the device tree source used
>>> to generate them. We know the blobs are "good enough" to have QEMU
>>> boot a system, so we don't really maintain and rebuild the sources.
>>>
>>> These blobs were generated with older 'dtc' binaries. We use the
>>> v1.6.1 version since 2021 (commit 962fde57b7 "dtc: Update to version
>>> 1.6.1").
>>>
>>> Since commit 6e0dc9d2a8 ("meson: compile bundled device trees"),
>>> if dtc binary is available, it is directly used to compile the
>>> device tree sources. New versions of 'dtc' add checks which display
>>> warnings or errors. Our sources are a bit old, so dtc v1.6.1 now
>>> emit the following warnings on a fresh build:
>>>
>>>    [163/3414] Generating pc-bios/canyonlands.dts with a custom command
>>>    pc-bios/canyonlands.dts:47.9-50.4: Warning (unit_address_vs_reg): /memory: node has a reg or ranges property, but no unit name
> ...
> 
>>>  From QEMU perspective, these warnings are not really useful. It is
>>> the responsibility of developers adding DT source/blob to QEMU
>>> repository to check the source doesn't produce warnings, but as
>>> long as the blob is useful enough, QEMU can consume it. So these
>>> warnings don't add any value, instead they are noisy and might
>>> distract us to focus on important warnings. Better disable them.
>>>
>>> 'dtc' provides the '--quiet' option for that:
>>>
>>>    $ dtc --help
>>>    Usage: dtc [options] <input file>
>>>
>>>    Options: -[qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AThv]
>>>      -q, --quiet
>>>            Quiet: -q suppress warnings, -qq errors, -qqq all
>>>
>>> Update meson to disable these unuseful DTC warnings.
>>
>>
>> Why not try fixing the .dts instead ? These still exist under Linux :
>>
>>    ./arch/powerpc/boot/dts/canyonlands.dts
>>    ./arch/powerpc/boot/dts/bamboo.dts
> 
> Because QEMU != Linux, and there isn't always overlap between
> communities?

sure but bamboo.dts came from Linux. So this should be safe to update.
Alex Graf did 10 years ago.

I can not tell for the sam460ex. It is probably safer to keep it as it is.

Sweeping dtc warnings under the rug for all .dts doesn't seem a good idea.
Should we get rid of the .dts and only keep the .dtb then ?

Thanks,

C.

> 1/ I tried but there isn't much interest:
> https://lore.kernel.org/qemu-devel/20230914204206.79351-1-philmd@linaro.org/
> 
> 2/ Peter and Zoltan raised issue with old firmwares when changing
> properties such 'stdout-path', see this thread:
> https://lore.kernel.org/qemu-devel/CAFEAcA-WJ9J1YQunJ+bSG=wnpxh1By+Bf18j2CyV7G0vZ=8b7g@mail.gmail.com/
Philippe Mathieu-Daudé Oct. 6, 2023, 8:38 a.m. UTC | #5
On 6/10/23 10:32, Cédric Le Goater wrote:
> On 10/6/23 10:18, Philippe Mathieu-Daudé wrote:
>> On 6/10/23 10:13, Cédric Le Goater wrote:
>>> On 10/6/23 08:47, Philippe Mathieu-Daudé wrote:
>>>> QEMU consumes some device tree blobs, so these have been committed
>>>> to the tree in as firmware, along with the device tree source used
>>>> to generate them. We know the blobs are "good enough" to have QEMU
>>>> boot a system, so we don't really maintain and rebuild the sources.
>>>>
>>>> These blobs were generated with older 'dtc' binaries. We use the
>>>> v1.6.1 version since 2021 (commit 962fde57b7 "dtc: Update to version
>>>> 1.6.1").
>>>>
>>>> Since commit 6e0dc9d2a8 ("meson: compile bundled device trees"),
>>>> if dtc binary is available, it is directly used to compile the
>>>> device tree sources. New versions of 'dtc' add checks which display
>>>> warnings or errors. Our sources are a bit old, so dtc v1.6.1 now
>>>> emit the following warnings on a fresh build:
>>>>
>>>>    [163/3414] Generating pc-bios/canyonlands.dts with a custom command
>>>>    pc-bios/canyonlands.dts:47.9-50.4: Warning (unit_address_vs_reg): 
>>>> /memory: node has a reg or ranges property, but no unit name
>> ...
>>
>>>>  From QEMU perspective, these warnings are not really useful. It is
>>>> the responsibility of developers adding DT source/blob to QEMU
>>>> repository to check the source doesn't produce warnings, but as
>>>> long as the blob is useful enough, QEMU can consume it. So these
>>>> warnings don't add any value, instead they are noisy and might
>>>> distract us to focus on important warnings. Better disable them.
>>>>
>>>> 'dtc' provides the '--quiet' option for that:
>>>>
>>>>    $ dtc --help
>>>>    Usage: dtc [options] <input file>
>>>>
>>>>    Options: -[qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AThv]
>>>>      -q, --quiet
>>>>            Quiet: -q suppress warnings, -qq errors, -qqq all
>>>>
>>>> Update meson to disable these unuseful DTC warnings.
>>>
>>>
>>> Why not try fixing the .dts instead ? These still exist under Linux :
>>>
>>>    ./arch/powerpc/boot/dts/canyonlands.dts
>>>    ./arch/powerpc/boot/dts/bamboo.dts
>>
>> Because QEMU != Linux, and there isn't always overlap between
>> communities?
> 
> sure but bamboo.dts came from Linux. So this should be safe to update.
> Alex Graf did 10 years ago.
> 
> I can not tell for the sam460ex. It is probably safer to keep it as it is.
> 
> Sweeping dtc warnings under the rug for all .dts doesn't seem a good idea.
> Should we get rid of the .dts and only keep the .dtb then ?

I *think* QEMU should generate DT blob with the qemu_fdt API (see
"sysemu/device_tree.h") with all the devices emulated, not forward
a Linux one.
BALATON Zoltan Oct. 6, 2023, 11:37 a.m. UTC | #6
On Fri, 6 Oct 2023, Philippe Mathieu-Daudé wrote:
> On 6/10/23 10:32, Cédric Le Goater wrote:
>> On 10/6/23 10:18, Philippe Mathieu-Daudé wrote:
>>> On 6/10/23 10:13, Cédric Le Goater wrote:
>>>> On 10/6/23 08:47, Philippe Mathieu-Daudé wrote:
>>>>> QEMU consumes some device tree blobs, so these have been committed
>>>>> to the tree in as firmware, along with the device tree source used
>>>>> to generate them. We know the blobs are "good enough" to have QEMU
>>>>> boot a system, so we don't really maintain and rebuild the sources.
>>>>> 
>>>>> These blobs were generated with older 'dtc' binaries. We use the
>>>>> v1.6.1 version since 2021 (commit 962fde57b7 "dtc: Update to version
>>>>> 1.6.1").
>>>>> 
>>>>> Since commit 6e0dc9d2a8 ("meson: compile bundled device trees"),
>>>>> if dtc binary is available, it is directly used to compile the
>>>>> device tree sources. New versions of 'dtc' add checks which display
>>>>> warnings or errors. Our sources are a bit old, so dtc v1.6.1 now
>>>>> emit the following warnings on a fresh build:
>>>>> 
>>>>>    [163/3414] Generating pc-bios/canyonlands.dts with a custom command
>>>>>    pc-bios/canyonlands.dts:47.9-50.4: Warning (unit_address_vs_reg): 
>>>>> /memory: node has a reg or ranges property, but no unit name
>>> ...
>>> 
>>>>>  From QEMU perspective, these warnings are not really useful. It is
>>>>> the responsibility of developers adding DT source/blob to QEMU
>>>>> repository to check the source doesn't produce warnings, but as
>>>>> long as the blob is useful enough, QEMU can consume it. So these
>>>>> warnings don't add any value, instead they are noisy and might
>>>>> distract us to focus on important warnings. Better disable them.
>>>>> 
>>>>> 'dtc' provides the '--quiet' option for that:
>>>>> 
>>>>>    $ dtc --help
>>>>>    Usage: dtc [options] <input file>
>>>>> 
>>>>>    Options: -[qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AThv]
>>>>>      -q, --quiet
>>>>>            Quiet: -q suppress warnings, -qq errors, -qqq all
>>>>> 
>>>>> Update meson to disable these unuseful DTC warnings.
>>>> 
>>>> 
>>>> Why not try fixing the .dts instead ? These still exist under Linux :
>>>> 
>>>>    ./arch/powerpc/boot/dts/canyonlands.dts
>>>>    ./arch/powerpc/boot/dts/bamboo.dts
>>> 
>>> Because QEMU != Linux, and there isn't always overlap between
>>> communities?
>> 
>> sure but bamboo.dts came from Linux. So this should be safe to update.
>> Alex Graf did 10 years ago.
>> 
>> I can not tell for the sam460ex. It is probably safer to keep it as it is.

I also opted for leaving it alone and not updating it because the current 
one is known to work and apart from these warnings there's no reason to 
change it. Updating and changing the dtb potentially can break booting 
something with -kernel so ignoring the warning is the safer option that 
would cause less work in the future.

>> Sweeping dtc warnings under the rug for all .dts doesn't seem a good idea.
>> Should we get rid of the .dts and only keep the .dtb then ?
>
> I *think* QEMU should generate DT blob with the qemu_fdt API (see
> "sysemu/device_tree.h") with all the devices emulated, not forward
> a Linux one.

The pegasos2 machine does that for VOF but it's not nice to do it that 
way. If I did if for another machine I'd probably put the skeleton, static 
parts in a dtb and only edit the dynamic part (such as mem size, CPU infos 
and PCI devices) with qemu_fdt after loading it which seems to be easier 
(and what's the sam460ex is doing) than doing everything from C. The 
pegasos2 does not have much static info so it does not worth changing it 
now but half of pegasos2.c is the dtb creation so putting that somewhere 
else may have made it more readable but I've modeled it after spapr so did 
not consider using a dtb back then. If I ever try to add dtb creation to 
mac machines I'd try doing that with a dtb and adding to that instead.

Regards,
BALATON Zoltan
BALATON Zoltan Oct. 6, 2023, 11:40 a.m. UTC | #7
On Fri, 6 Oct 2023, Philippe Mathieu-Daudé wrote:
> QEMU consumes some device tree blobs, so these have been committed
> to the tree in as firmware, along with the device tree source used
> to generate them. We know the blobs are "good enough" to have QEMU
> boot a system, so we don't really maintain and rebuild the sources.
>
> These blobs were generated with older 'dtc' binaries. We use the
> v1.6.1 version since 2021 (commit 962fde57b7 "dtc: Update to version
> 1.6.1").
>
> Since commit 6e0dc9d2a8 ("meson: compile bundled device trees"),
> if dtc binary is available, it is directly used to compile the
> device tree sources. New versions of 'dtc' add checks which display
> warnings or errors. Our sources are a bit old, so dtc v1.6.1 now
> emit the following warnings on a fresh build:
>
>  [163/3414] Generating pc-bios/canyonlands.dts with a custom command
>  pc-bios/canyonlands.dts:47.9-50.4: Warning (unit_address_vs_reg): /memory: node has a reg or ranges property, but no unit name
>  pc-bios/canyonlands.dts:210.13-429.5: Warning (unit_address_vs_reg): /plb/opb: node has a reg or ranges property, but no unit name
>  pc-bios/canyonlands.dts:464.26-504.5: Warning (pci_bridge): /plb/pciex@d00000000: node name is not "pci" or "pcie"
>  pc-bios/canyonlands.dts:506.26-546.5: Warning (pci_bridge): /plb/pciex@d20000000: node name is not "pci" or "pcie"
>  pc-bios/canyonlands.dtb: Warning (unit_address_format): Failed prerequisite 'pci_bridge'
>  pc-bios/canyonlands.dtb: Warning (pci_device_reg): Failed prerequisite 'pci_bridge'
>  pc-bios/canyonlands.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
>  pc-bios/canyonlands.dts:268.14-289.7: Warning (avoid_unnecessary_addr_size): /plb/opb/ebc/ndfc@3,0: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>  [164/3414] Generating pc-bios/petalogix-s3adsp1800.dts with a custom command
>  pc-bios/petalogix-s3adsp1800.dts:258.33-266.5: Warning (interrupt_provider): /plb/interrupt-controller@81800000: Missing #address-cells in interrupt provider
>  [165/3414] Generating pc-bios/petalogix-ml605.dts with a custom command
>  pc-bios/petalogix-ml605.dts:234.39-241.5: Warning (interrupt_provider): /axi/interrupt-controller@81800000: Missing #address-cells in interrupt provider
>  [177/3414] Generating pc-bios/bamboo.dts with a custom command
>  pc-bios/bamboo.dts:45.9-48.4: Warning (unit_address_vs_reg): /memory: node has a reg or ranges property, but no unit name
>  pc-bios/bamboo.dts:87.13-154.5: Warning (unit_address_vs_reg): /plb/opb: node has a reg or ranges property, but no unit name
>  pc-bios/bamboo.dts:198.3-50: Warning (chosen_node_stdout_path): /chosen:linux,stdout-path: Use 'stdout-path' instead
>  pc-bios/bamboo.dts:87.13-154.5: Warning (interrupts_property): /plb/opb: Missing interrupt-parent
>  pc-bios/bamboo.dts:100.14-108.6: Warning (interrupts_property): /plb/opb/ebc: Missing interrupt-parent
>
> From QEMU perspective, these warnings are not really useful. It is
> the responsibility of developers adding DT source/blob to QEMU
> repository to check the source doesn't produce warnings, but as
> long as the blob is useful enough, QEMU can consume it. So these
> warnings don't add any value, instead they are noisy and might
> distract us to focus on important warnings. Better disable them.
>
> 'dtc' provides the '--quiet' option for that:
>
>  $ dtc --help
>  Usage: dtc [options] <input file>
>
>  Options: -[qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AThv]
>    -q, --quiet
>          Quiet: -q suppress warnings, -qq errors, -qqq all
>
> Update meson to disable these unuseful DTC warnings.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Acked-by: BALATON Zoltan <balaton@eik.bme.hu>

> ---
> v2: Complete $Subject
>
> Note, meson outputs "Generating dts" instead of "Generating dtb".
> ---
> pc-bios/meson.build | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/pc-bios/meson.build b/pc-bios/meson.build
> index e67fa433a1..162663fed6 100644
> --- a/pc-bios/meson.build
> +++ b/pc-bios/meson.build
> @@ -95,7 +95,8 @@ foreach f : [
>         output: out,
>         install: get_option('install_blobs'),
>         install_dir: qemu_datadir,
> -        command: [ dtc, '-I', 'dts', '-O', 'dtb', '-o', '@OUTPUT@', '@INPUT0@' ])
> +        command: [ dtc, '-q', '-I', 'dts', '-O', 'dtb',
> +                        '-o', '@OUTPUT@', '@INPUT0@' ])
>   else
>     blobs += out
>   endif
>
diff mbox series

Patch

diff --git a/pc-bios/meson.build b/pc-bios/meson.build
index e67fa433a1..162663fed6 100644
--- a/pc-bios/meson.build
+++ b/pc-bios/meson.build
@@ -95,7 +95,8 @@  foreach f : [
         output: out,
         install: get_option('install_blobs'),
         install_dir: qemu_datadir,
-        command: [ dtc, '-I', 'dts', '-O', 'dtb', '-o', '@OUTPUT@', '@INPUT0@' ])
+        command: [ dtc, '-q', '-I', 'dts', '-O', 'dtb',
+                        '-o', '@OUTPUT@', '@INPUT0@' ])
   else
     blobs += out
   endif