Re: [PATCH v3 1/5] ARM: dts: aspeed: santabarbara: Add system monitoring GPIOs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] The pre-existing gpio@27 node was not updated to route its interrupt to the new ioexp0 aggregator, breaking the intended design.
- [Medium] The newly added gpio@28 node uses an I2C address (0x28) that is physically impossible for a standard PCA9555 chip.
- [Medium] The gpio@23 downstream expander node is missing from the device tree.
- [Medium] Conflict between interrupt assignments and SGPIO pin names for gpio@74 and ioexp0.
--

--- Patch [1]: [PATCH v3 1/5] ARM: dts: aspeed: santabarbara: Add system monitoring GPIOs ---
commit 8d0ec3e4ec2df315ded9be783e4deeb197721ee9
Author: Fred Chen <fredchen.openbmc@gmail.com>

ARM: dts: aspeed: santabarbara: Add system monitoring GPIOs

This commit adds several GPIO expanders on the switch board for NIC, PEX,
and cable presence monitoring. Due to a design change, ioexp0 (0x20) now
aggregates interrupts from the downstream expanders (0x21-0x27) onto a
single sgpiom0 line.

> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-santabarbara.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-santabarbara.dts
> --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-santabarbara.dts
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-santabarbara.dts
> @@ -616,6 +616,8 @@ gpio@74 {
>  				reg = <0x74>;
>  				gpio-controller;
>  				#gpio-cells = <2>;
> +				interrupt-parent = <&sgpiom0>;
> +				interrupts = <146 IRQ_TYPE_LEVEL_LOW>;

[Severity: Medium]
Does this interrupt assignment conflict with the SGPIO pin names?

The interrupt 146 is assigned to gpio@74 here, but in sgpiom0's
gpio-line-names, line 146 is labeled 'FM_SMB_ALERT_MCIO_0A_N'.

Is this an intentional override or a hardware description mismatch that
could lead to incorrect interrupt handling?

[ ... ]

> +	ioexp0: gpio@20 {
> +		compatible = "nxp,pca9555";
> +		reg = <0x20>;
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +		interrupt-controller;
> +		#interrupt-cells = <2>;
> +		interrupt-parent = <&sgpiom0>;
> +		interrupts = <148 IRQ_TYPE_LEVEL_LOW>;

[Severity: Medium]
Similar to the issue above, interrupt 148 is assigned here to ioexp0,
but sgpiom0's line 148 is named 'I3C_MCIO_0B_ALERT_ISO_R_N'. Could this
conflict cause misinterpretation of alerts?

> +		gpio-line-names =
> +			"IOEXP_21h_INT_N","IOEXP_22h_INT_N",
> +			"IOEXP_23h_INT_N","IOEXP_24h_INT_N",

[Severity: Medium]
The commit message states that ioexp0 aggregates interrupts from the contiguous
downstream expanders range 0x21-0x27, and the line names here explicitly
reserve pin 2 for 'IOEXP_23h_INT_N'.

However, there doesn't seem to be a gpio@23 node provided in the device tree.
Will the hardware connected to the 0x23 expander be unmanaged by the kernel?

[ ... ]

>  	gpio@27 {
>  		compatible = "nxp,pca9555";
>  		reg = <0x27>;
> @@ -1349,6 +1457,21 @@ gpio@27 {
>  			"SPI_MUX_SEL","","","";
>  	};

[Severity: Medium]
Since the commit explicitly changes the design to route 0x21-0x27 expander
interrupts to ioexp0, should this existing gpio@27 node also be updated with
an 'interrupt-parent = <&ioexp0>' property?

Without it, will the physical interrupts on the gpio@27 INT_N pin assert
ioexp0's line 6 but remain unhandled, potentially leading to an interrupt
storm?

> +	gpio@28 {
> +		compatible = "nxp,pca9555";
> +		reg = <0x28>;

[Severity: Medium]
Can the PCA9555 physically respond to address 0x28?

The NXP PCA9555 hardware typically has 3 address pins (A0, A1, A2) and a fixed
base of 0x20, meaning its valid 7-bit I2C addresses are strictly 0x20 through
0x27. Will the kernel fail to probe the PCA9555 GPIO expander at this address?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807083415.672008-1-fredchen.openbmc@gmail.com?part=1