Re: [PATCH v7 01/11] 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 3 potential issue(s) to consider:

New issues:
- [Critical] The board compatible string was silently changed from "facebook,anacapa-bmc" to "facebook,anacapa", breaking dt-bindings and backward compatibility.
- [Low] The /dts-v1/ directive is improperly placed inside a .dtsi include file instead of the top-level .dts file.

Pre-existing issues:
- [High] Multiple consumer nodes request GPIO pins and interrupts from sgpiom0 that strictly exceed its defined maximum limit of 128.
--

commit aa5900d50b8d457379595fa4812e97d97a192959
Author: Colin Huang <u8813345@gmail.com>
Subject: 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 and pushes down several configurations.

> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa-evt1.dtsi b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa-evt1.dtsi
> new file mode 100644
> index 0000000000000..017700920cb1d
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa-evt1.dtsi
> @@ -0,0 +1,1128 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +/dts-v1/;
> +#include "aspeed-g6.dtsi"

[Severity: Low]
Is it standard practice to place the /dts-v1/ directive inside a .dtsi
include file instead of the top-level .dts file?

> +#include <dt-bindings/gpio/aspeed-gpio.h>
> +#include <dt-bindings/i2c/i2c.h>
> +
> +/ {
> +	compatible = "facebook,anacapa", "aspeed,ast2600";

[Severity: Critical]
Does changing the board compatible string from "facebook,anacapa-bmc" to
"facebook,anacapa" break dt-bindings and backward compatibility? 

Userspace applications and bootloaders often rely on the exact string in
/proc/device-tree/compatible to identify the board. Could this modification
cause them to fail unconditionally on boot?

[ ... ]
> +		i2c8mux0ch0: i2c@0 {
> +			reg = <0>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			adc@1f {
> +				compatible = "ti,adc128d818";
> +				reg = <0x1f>;
> +				ti,mode = /bits/ 8 <1>;
> +			};
> +
> +			gpio@22 {
> +				compatible = "nxp,pca9555";
> +				reg = <0x22>;
> +				#gpio-cells = <2>;
> +				gpio-controller;
[ ... ]
> +				interrupt-parent = <&sgpiom0>;
> +
> +				interrupts = <174 IRQ_TYPE_LEVEL_LOW>;
> +			};

[Severity: High]
This is a pre-existing issue, but does requesting IRQ line 174 from the
sgpiom0 controller exceed its defined maximum limit of 128?

The gpiolib and IRQ domain could reject this out-of-bounds request, causing
the driver to fail probing with -EINVAL.

[ ... ]
> +		i2c13mux0ch7: i2c@7 {
> +			reg = <7>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			nfc@28 {
> +				compatible = "nxp,nxp-nci-i2c";
> +				reg = <0x28>;
> +
> +				enable-gpios = <&sgpiom0 241 GPIO_ACTIVE_HIGH>;
> +				interrupt-parent = <&sgpiom0>;
> +
> +				interrupts = <156 IRQ_TYPE_LEVEL_HIGH>;
> +			};

[Severity: High]
This is a pre-existing issue, but does requesting GPIO 241 and IRQ 156 from
sgpiom0 completely overshoot the 128-pin cap?

Since the controller restricts its capacity, the out-of-bounds requests could
lead to probe failures unconditionally during driver initialization.

[ ... ]
> +&sgpiom0 {
> +	bus-frequency = <2000000>;
[ ... ]
> +	ngpios = <128>;
> +
> +	status = "okay";
> +};

[ ... ]
> 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..724f4d312de4d 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,3 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  
> -/dts-v1/;
> -#include "aspeed-g6.dtsi"

[Severity: Low]
Does stripping the /dts-v1/ directive from the top-level .dts file violate
style and architecture practices for Device Trees?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-anacapa-devlop-phase-devicetree-v7-0-0bd71d2e0105@gmail.com?part=1