Re: [PATCH 2/2] ARM: dts: aspeed: Add NVIDIA VR-NVL BMC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrect USB node hierarchy for `cp2112a` and `cp2112b` prevents GPIO controller registration.
- [Low] Incorrect `compatible` strings are used for I2C GPIO expanders `exp7` and `exp18` given their specified I2C hardware addresses.
--

commit a02cef965c4453f472b6eb79d68aef51b62a7f10
Author: Jacky Huang <jackyhuang@nvidia.com>

ARM: dts: aspeed: Add NVIDIA VR-NVL BMC

This commit adds the device tree for the Aspeed AST2600 BMC for NVIDIA's
Vera Rubin NVL compute platform. It configures the on-board components,
including network interfaces, USB hubs, and multiple I2C devices.

Link: https://lore.kernel.org/all/20260629105202.1417679-1-jackyhuang@nvidia.com/

> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-nvl-bmc.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-nvl-bmc.dts
> new file mode 100644
> index 0000000000000..f035e9b28f64b
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-nvl-bmc.dts
> @@ -0,0 +1,901 @@
[ ... ]
> +				device@1 {
> +					reg = <1>;
> +					#address-cells = <2>;
> +					#size-cells = <0>;
> +
> +					cp2112a: interface@1 {
> +						reg = <1 1>;
> +						gpio-controller;
> +						#gpio-cells = <2>;
> +						interrupt-controller;
> +						#interrupt-cells = <2>;
> +					};
> +				};

[Severity: High]
Will placing these GPIO controller properties inside the interface subnode
cause them to be ignored by the USB OF core?

Since cp2112 devices expose a single HID interface, the kernel's
usb_of_has_combined_node() assigns the parent device node to the interface
and skips subnodes. This setup might prevent the hid-cp2112 driver from
finding these properties and registering the GPIO controllers.

Should these properties be placed directly on the device node, similar to
how cp2112c is configured in this same file?

[ ... ]
> +				device@1 {
> +					reg = <1>;
> +					#address-cells = <2>;
> +					#size-cells = <0>;
> +
> +					cp2112b: interface@1 {
> +						reg = <1 1>;
> +						gpio-controller;
> +						#gpio-cells = <2>;
> +						interrupt-controller;
> +						#interrupt-cells = <2>;
> +					};
> +				};

[Severity: High]
Is this interface subnode for cp2112b susceptible to the same OF node
combining regression mentioned above?

[ ... ]
> +	exp7: gpio@75 {
> +		compatible = "nxp,pca9555";
> +		reg = <0x75>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <ASPEED_GPIO(B, 6) IRQ_TYPE_LEVEL_LOW>;

[Severity: Low]
Does the compatible string nxp,pca9555 match the hardware at I2C address
0x75?

The PCA9555 has a fixed internal slave address prefix that restricts it to
I2C addresses 0x20 through 0x27. An I2C device at address 0x75 is typically
a PCA9539.

While the shared gpio-pca953x driver probes this successfully either way,
should the compatible string be updated to accurately describe the hardware?

[ ... ]
> +	exp18: gpio@38 {
> +		compatible = "nxp,pca9554";
> +		reg = <0x38>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <ASPEED_GPIO(B, 6) IRQ_TYPE_LEVEL_LOW>;

[Severity: Low]
Is nxp,pca9554 the correct compatible string for a device at I2C address
0x38?

Similar to the pca9555 above, the base PCA9554 is restricted to addresses
0x20 through 0x27. An address of 0x38 typically corresponds to a PCA9554A.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702165524.2168091-1-jackyhuang@nvidia.com?part=2