Re: [PATCH v3 2/9] ARM: dts: aspeed: anacapa: add EVT1 devicetree and point wrapper to it

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Removal of the generic 'facebook,anacapa-bmc' compatible string from the root node breaks backward compatibility.
- [Low] Including a `.dts` file directly from another `.dts` file instead of using a `.dtsi`, and failing to register the new `.dts` in the Makefile.
--

commit 07c91fec99a871ad9f2589f35f8c19205df00ae9
Author: Colin Huang <u8813345@gmail.com>

ARM: dts: aspeed: anacapa: add EVT1 devicetree and point wrapper to it

This change introduces a development-phase devicetree for the Facebook Anacapa
BMC EVT1 hardware revision and updates the Anacapa wrapper DTS to reference it.

> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa-evt1.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa-evt1.dts
> new file mode 100644
> index 0000000000000..9314ee493c617
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa-evt1.dts
> @@ -0,0 +1,1103 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +/dts-v1/;

[Severity: Low]
Is this intended to be compiled independently as a top-level board?

Files that are included by other device trees usually use the .dtsi extension
and omit the /dts-v1/; tag. This file is also not added to the Makefile.

> +#include "aspeed-g6.dtsi"
> +#include <dt-bindings/gpio/aspeed-gpio.h>
> +#include <dt-bindings/i2c/i2c.h>
> +
> +/ {
> +	model = "Facebook Anacapa BMC";
> +	compatible = "facebook,anacapa-bmc-evt1", "aspeed,ast2600";

[Severity: Critical]
Does the removal of the generic "facebook,anacapa-bmc" compatible string
break backward compatibility with existing bootloaders or management tools?

Replacing the wrapper file with an include to this one completely removes the
original compatible string from the output DTB. 

Could this include a fallback like the following?
compatible = "facebook,anacapa-bmc-evt1", "facebook,anacapa-bmc", "aspeed,ast2600";

[ ... ]

> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa.dts
> index 1fddf3a5d1383..209eef65cedbc 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa.dts
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa.dts
> @@ -1,1079 +1,4 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  
> -/dts-v1/;
> -#include "aspeed-g6.dtsi"
> -#include <dt-bindings/gpio/aspeed-gpio.h>
> -#include <dt-bindings/i2c/i2c.h>
> +#include "aspeed-bmc-facebook-anacapa-evt1.dts"

[Severity: Low]
Is there a reason to include a .dts file directly instead of using a .dtsi?

Using a .dts as an include bypasses standalone syntax checks and compilation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602-anacapa-devlop-phase-devicetree-v3-0-7c93c5df8d9b@gmail.com?part=2