Re: [PATCH v5 02/15] ARM: dts: aspeed: anacapa: Add EVT1 devicetree and point wrapper to it

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] The wrapper devicetree aspeed-bmc-facebook-anacapa.dts drops the generic compatible string 'facebook,anacapa-bmc', breaking backward compatibility.
- [Medium] Including a top-level .dts file from another .dts file to hijack the build target name violates standard Device Tree architecture.
--

--- Patch [2]: [PATCH v5 02/15] ARM: dts: aspeed: anacapa: Add EVT1 devicetree and point wrapper to it ---
commit 35fc682b92e94ff2d9be35a5fb9d50d3d7e5a11c
Author: Colin Huang <[email protected]>

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

This commit introduces an EVT1 development-phase devicetree for the Facebook
Anacapa BMC. It updates the top-level wrapper DTS to include the EVT1
devicetree, aiming to allow future revisions to inherit from a common baseline.

> 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..a095be48a3fe6
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa-evt1.dts
> @@ -0,0 +1,1079 @@
> +// 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>
> +
> +/ {
> +	model = "Facebook Anacapa BMC";
> +	compatible = "facebook,anacapa-bmc-evt1", "aspeed,ast2600";

[Severity: Critical]
Does dropping the generic "facebook,anacapa-bmc" compatible string cause
a regression for backward compatibility?

Bootloaders, hardware management systems (like OpenBMC's entity-manager),
and userspace applications might rely on the generic identifier to match
this board. Should we preserve the generic compatible string as a fallback
in this list?

[ ... ]

> 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: Medium]
Is it standard practice to include a fully formed top-level .dts file
(which already contains /dts-v1/;) from another .dts file?

Typically, shared definitions are moved into a .dtsi file, and distinct
hardware variants (like EVT1, EVT2) have their own explicit .dtb targets
in the Makefile. Could masking hardware variants behind a single wrapper
cause issues for the build system or future extensibility?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813-anacapa-devlop-phase-devicetree-v5-0-c7058f63dc67@gmail.com?part=2