Re: [PATCH v6 02/12] ARM: dts: aspeed: anacapa: Add EVT1 devicetree and point wrapper to it
flat view
From: sashiko-bot@kernel.org
Date: 2026-08-25 10:58:02
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [Critical] Devicetree ABI breakage due to removal of generic compatible string - [Medium] Incorrect removal of `p3v3_bmc_aux` fixed-regulator node - [Low] Included devicetrees use `.dts` extension instead of `.dtsi` --
--- Patch [2]: [PATCH v6 02/12] ARM: dts: aspeed: anacapa: Add EVT1 devicetree and point wrapper to it ---commit 9a026f3bc21757ceb40a47713b367e173351589e Author: Colin Huang [off-list ref] 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 hardware revision file, removes redundant pinctrl names, and fixes a dt-check-style strict violation.
Remove p3v3_bmc_aux node to fix '[unused-lable]' warning.
[Severity: Medium] Is it correct to remove the entire hardware node here? If the intention is just to fix an unused label warning, usually only the label itself (p3v3_bmc_aux:) is removed, preserving the regulator node to accurately describe the EVT1 hardware. While follow-up patches in the series (e.g., commit "ARM: dts: aspeed: anacapa: Add SPI flash VCC supply") add the regulator back into the EVT2 devicetree, the EVT1 devicetree baseline remains permanently stripped of this regulator node.
quoted hunk ↗ jump to hunk
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 --- /dev/null +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa-evt1.dts@@ -0,0 +1,1120 @@ +// 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> + +/ { + compatible = "facebook,anacapa-bmc-evt1", "aspeed,ast2600";
[Severity: Critical] Does removing the generic "facebook,anacapa-bmc" string from the compatible property break existing ABI? Bootloaders, kernel quirks, or userspace utilities (like systemd or OpenBMC) that rely on matching the generic string will fail to recognize the board during an upgrade. Even at the end of the patch series, the generic string is never restored. Should the wrapper file override this to retain the generic string, or should it be appended here? [ ... ]
quoted hunk ↗ jump to hunk
diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa.dts --- 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,3 @@ // SPDX-License-Identifier: GPL-2.0-or-later -/dts-v1/; -#include "aspeed-g6.dtsi"
[ ... ]
-/ {
- model = "Facebook Anacapa BMC";
- compatible = "facebook,anacapa-bmc", "aspeed,ast2600";[ ... ]
+#include "aspeed-bmc-facebook-anacapa-evt1.dts"
[Severity: Low] Is it intended for this included file to use a .dts extension? In kernel devicetree conventions, files intended to be included by other files typically use the .dtsi (Device Tree Source Include) extension. Since this evt1 file is strictly used as an include base and is not built as a separate target in the Makefile, would it be better named with a .dtsi extension? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260825-anacapa-devlop-phase-devicetree-v6-0-9059a2a279ce@gmail.com?part=2