Re: [PATCH v3 8/9] ARM: dts: aspeed: anacapa: Enable MCTP and FRU for NIC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Incomplete implementation: missing MCTP and FRU enablement for the Right Frontend NIC.
- [Medium] Undocumented removal of the `i2c-mux-idle-disconnect` property.
--

commit e274c5d6bfd9557e3fa26fe21930d08b6e1b3ec0
Author: Andy Chung <Andy.Chung@amd.com>

ARM: dts: aspeed: anacapa: Enable MCTP and FRU for NIC

Add the mctp-controller property to enable frontend NIC management
via PLDM over MCTP. Also add EEPROM device for NIC FRU and reorder
the I2C virtual bus index accroding to the system silkscreen index.

> 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
> index 29df10697613a..5b6ce3c556fe2 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa-evt1.dts
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-anacapa-evt1.dts

[ ... ]

>  // R Bridge Board
>  &i2c10 {
>  	status = "okay";
> +	multi-master;
> +	mctp@10 {
> +		compatible = "mctp-i2c-controller";
> +		reg = <(0x10 | I2C_OWN_SLAVE_ADDRESS)>;
> +	};
>  
>  	i2c-mux@71 {
>  		compatible = "nxp,pca9548";
>  		reg = <0x71>;
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> -		i2c-mux-idle-disconnect;

[Severity: Medium]
The commit message doesn't mention removing the i2c-mux-idle-disconnect
property.

While this is likely needed to allow asynchronous multi-master MCTP traffic
from endpoints, could the commit message document this topology change so
future maintainers don't inadvertently revert it?

>  
>  		i2c10mux0ch0: i2c@0 {
>  			reg = <0>;
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  		};

[Severity: Medium]
The commit message mentions enabling MCTP and FRU for frontend NICs, and the
symmetrically equivalent left frontend NIC (i2c11mux0ch0) was updated, but
this right frontend NIC node was left unmodified.

Did we miss adding the mctp-controller property and FRU eeprom for this
channel?

[ ... ]

>  // L Bridge Board
>  &i2c11 {
>  	status = "okay";
> +	multi-master;
> +	mctp@10 {
> +		compatible = "mctp-i2c-controller";
> +		reg = <(0x10 | I2C_OWN_SLAVE_ADDRESS)>;
> +	};
>  
>  	i2c-mux@71 {
>  		compatible = "nxp,pca9548";
>  		reg = <0x71>;
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> -		i2c-mux-idle-disconnect;

[Severity: Medium]
Similarly here on the left bridge board, could we mention the removal of
the i2c-mux-idle-disconnect property in the commit message?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602-anacapa-devlop-phase-devicetree-v3-0-7c93c5df8d9b@gmail.com?part=8