Re: [PATCH v2 2/2] ARM: dts: aspeed: ventura2: Add Meta ventura2 BMC

From: Andrew Jeffery

Date: Wed Jan 07 2026 - 23:38:02 EST

On Wed, 2025-12-24 at 14:44 +0800, Kyle Hsieh wrote:
> Add linux device tree entry related to the Meta(Facebook) rmc-node.
> The system use an AT2600 BMC.
> This node is named "ventura2".
>
> Signed-off-by: Kyle Hsieh <kylehsieh1995@xxxxxxxxx>
> ---
> arch/arm/boot/dts/aspeed/Makefile | 1 +
> .../dts/aspeed/aspeed-bmc-facebook-ventura2.dts | 2945 ++++++++++++++++++++
> 2 files changed, 2946 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed/Makefile b/arch/arm/boot/dts/aspeed/Makefile
> index 9adf9278dc94..6b96997629d4 100644
> --- a/arch/arm/boot/dts/aspeed/Makefile
> +++ b/arch/arm/boot/dts/aspeed/Makefile
> @@ -32,6 +32,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
> aspeed-bmc-facebook-minipack.dtb \
> aspeed-bmc-facebook-santabarbara.dtb \
> aspeed-bmc-facebook-tiogapass.dtb \
> + aspeed-bmc-facebook-ventura2.dtb \
> aspeed-bmc-facebook-wedge40.dtb \
> aspeed-bmc-facebook-wedge100.dtb \
> aspeed-bmc-facebook-wedge400-data64.dtb \
> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-ventura2.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-ventura2.dts
> new file mode 100644
> index 000000000000..e22bbaf519ea
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-ventura2.dts
> @@ -0,0 +1,2945 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2023 Facebook Inc.
> +/dts-v1/;
> +
> +#include "aspeed-g6.dtsi"
> +#include <dt-bindings/i2c/i2c.h>
> +#include <dt-bindings/gpio/aspeed-gpio.h>
> +
> +/ {
> + model = "Facebook ventura2 RMC";
> + compatible = "facebook,ventura2-rmc", "aspeed,ast2600";
> + aliases {
> + serial4 = &uart5;
> +
> +

*snip*

> + };
> +
> + chosen {
> + stdout-path = "serial4:57600n8";
> + };
> +
> + iio-hwmon {
> + compatible = "iio-hwmon";
> + io-channels = <&adc0 0>, <&adc0 1>, <&adc0 2>, <&adc0 3>,
> + <&adc0 4>, <&adc0 5>, <&adc0 6>, <&adc0 7>,
> + <&adc1 2>;
> + };
> +
> + leds {
> + compatible = "gpio-leds";
> +
> + led-0 {
> + label = "bmc_heartbeat_amber";
> + gpios = <&gpio0 ASPEED_GPIO(P, 7) GPIO_ACTIVE_LOW>;
> + linux,default-trigger = "heartbeat";
> + };
> +
> + led-1 {
> + label = "fp_id_amber";
> + default-state = "off";
> + gpios = <&gpio0 ASPEED_GPIO(B, 5) GPIO_ACTIVE_HIGH>;
> + };
> +
> + led-2 {
> + label = "bmc_ready_noled";
> + default-state = "on";
> + gpios = <&gpio0 ASPEED_GPIO(B, 3) (GPIO_ACTIVE_HIGH|GPIO_TRANSITORY)>;
> + };
> +
> + led-3 {
> + label = "power_blue";
> + default-state = "off";
> + gpios = <&gpio0 ASPEED_GPIO(P, 4) GPIO_ACTIVE_HIGH>;
> + };
> + };
> +
> + fan_leds {

Can you please order these unit-address-less nodes (iio-hwmon, leds,
fan_leds, etc) alphabetically?

> + compatible = "gpio-leds";
> +
> + led-0 {
> + label = "fcb0fan0_ledd1_blue";
> + default-state = "off";
> + gpios = <&fan_io_expander0 0 GPIO_ACTIVE_LOW>;
> + };
> +
> + led-1 {
> + label = "fcb0fan1_ledd2_blue";
> + default-state = "off";
> + gpios = <&fan_io_expander0 1 GPIO_ACTIVE_LOW>;
> + };
> +
> + led-2 {
> + label = "fcb0fan2_ledd3_blue";
> + default-state = "off";
> + gpios = <&fan_io_expander1 0 GPIO_ACTIVE_LOW>;
> + };
> +
> + led-3 {
> + label = "fcb0fan3_ledd4_blue";
> + default-state = "off";
> + gpios = <&fan_io_expander1 1 GPIO_ACTIVE_LOW>;
> + };
> +
> + led-4 {
> + label = "fcb0fan0_ledd1_amber";
> + default-state = "off";
> + gpios = <&fan_io_expander0 4 GPIO_ACTIVE_LOW>;
> + };
> +
> + led-5 {
> + label = "fcb0fan1_ledd2_amber";
> + default-state = "off";
> + gpios = <&fan_io_expander0 5 GPIO_ACTIVE_LOW>;
> + };
> +
> + led-6 {
> + label = "fcb0fan2_ledd3_amber";
> + default-state = "off";
> + gpios = <&fan_io_expander1 4 GPIO_ACTIVE_LOW>;
> + };
> +
> + led-7 {
> + label = "fcb0fan3_ledd4_amber";
> + default-state = "off";
> + gpios = <&fan_io_expander1 5 GPIO_ACTIVE_LOW>;
> + };
> + };
> +
> + memory@80000000 {
> + device_type = "memory";
> + reg = <0x80000000 0x80000000>;
> + };
> +
> + p1v8_bmc_aux: regulator-p1v8-bmc-aux {
> + compatible = "regulator-fixed";
> + regulator-name = "p1v8_bmc_aux";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-always-on;
> + };
> +
> + p2v5_bmc_aux: regulator-p2v5-bmc-aux {
> + compatible = "regulator-fixed";
> + regulator-name = "p2v5_bmc_aux";
> + regulator-min-microvolt = <2500000>;
> + regulator-max-microvolt = <2500000>;
> + regulator-always-on;
> + };
> +
> + p5v_dac_aux: regulator-p5v-bmc-aux {
> + compatible = "regulator-fixed";
> + regulator-name = "p5v_dac_aux";
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + regulator-always-on;
> + };
> +
> + spi1_gpio: spi {
> + compatible = "spi-gpio";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + sck-gpios = <&gpio0 ASPEED_GPIO(Z, 3) GPIO_ACTIVE_HIGH>;
> + mosi-gpios = <&gpio0 ASPEED_GPIO(Z, 4) GPIO_ACTIVE_HIGH>;
> + miso-gpios = <&gpio0 ASPEED_GPIO(Z, 5) GPIO_ACTIVE_HIGH>;
> + cs-gpios = <&gpio0 ASPEED_GPIO(Z, 0) GPIO_ACTIVE_LOW>;
> + num-chipselects = <1>;
> +
> + tpm@0 {
> + compatible = "infineon,slb9670", "tcg,tpm_tis-spi";
> + spi-max-frequency = <33000000>;
> + reg = <0>;
> + };
> + };
> +};
> +
> +&fmc {
> + status = "okay";
> + flash@0 {
> + status = "okay";
> + m25p,fast-read;
> + label = "bmc";
> + spi-max-frequency = <50000000>;
> + #include "openbmc-flash-layout-128.dtsi"
> + };
> + flash@1 {
> + status = "okay";
> + m25p,fast-read;
> + label = "alt-bmc";
> + spi-max-frequency = <50000000>;
> + };
> +};
> +
> +&peci0 {
> + status = "okay";
> +};
> +
> +&mac2 {

Same for all the phandle-referenced nodes throughout: please order them
alphabetically. Another possible ordering is in unit address order, but
I prefer alphabetical in the dts because there's no unit address in
sight (it's in the corresponding dtsi).

> + status = "okay";
> + phy-mode = "rmii";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_rmii3_default>;
> + fixed-link {
> + speed = <100>;
> + full-duplex;
> + };

Andrew Lunn asked for a comment justifying the fixed-link node in [1]
and you said you'd add it in [2], but there's no comment? Can you
please add it?

[1]: https://lore.kernel.org/all/32ff7ca8-9cb0-4889-adb0-a6dae735630b@xxxxxxx/
[2]: https://lore.kernel.org/all/CAF7HswMRrs9hwKo_uHCLMtx7+h46-DPEJRcEqu0-zEG4CVvvjg@xxxxxxxxxxxxxx/

> +};
> +
> +&mac3 {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_rmii4_default>;
> + use-ncsi;
> +};
>

*snip*

> +
> +&sgpiom0 {
> + status = "okay";
> + ngpios = <128>;
> + bus-frequency = <2000000>;
> + gpio-line-names =
> + /*"input pin","output pin"*/
> + /*A0 - A7*/
> + "power-chassis-good","FM_PLD_HEARTBEAT_LVC3_R",
> + "host0-ready","",
> + "CONTROL_VT2_SUPPLY1_CLOSE","FM_MDIO_SW_SEL_PLD",
> + "CONTROL_VT2_SUPPLY2_CLOSE","FM_88E6393X_BIN_UPDATE_EN_N",
> + "CONTROL_VT2_SUPPLY3_CLOSE","Sequence_TransFR_Alert",
> + "RETURN_CNTL1_FB","WATER_VALVE1_OPEN",
> + "RETURN_CNTL2_FB","WATER_VALVE2_OPEN",
> + "RETURN_CNTL3_FB","WATER_VALVE3_OPEN",
> + /*B0 - B7*/
> + "IT_STOP_PUMP_R_CPLD","WATER_VALVE1_CLOSE",
> + "IT_STOP_PUMP_SPARE_R_CPLD","WATER_VALVE2_CLOSE",
> + "IT_STOP_PUMP_2_R_CPLD","WATER_VALVE3_CLOSE",
> + "IT_STOP_PUMP_SPARE_2_R_CPLD","REPORT_VT2_SUPPLY1_CLOSE",
> + "RPU_2_READY_SPARE_PLD_R","REPORT_VT2_SUPPLY2_CLOSE",
> + "RPU_2_READY_PLD_R","REPORT_VT2_SUPPLY3_CLOSE",
> + "RPU_READY_SPARE_PLD_R","PCIE_SSD1_PRSNT_N",
> + "RPU_READY_PLD_R","",
> + /*C0 - C7*/
> + "IOEXP8_INT_N","",
> + "SUPPLY_CNTL1_FB","",
> + "SUPPLY_CNTL2_FB","",
> + "SUPPLY_CNTL3_FB","",
> + "PRSNT_TRAY1_TO_40_R_BUF_N","",
> + "PWRGD_TRAY1_TO_40_R_BUF","",
> + "SMALL_LEAK_TRAY1_TO_40_R_BUF_N","",
> + "LEAK_DETECT_TRAY1_TO_40_R_BUF_N","",
> + /*D0 - D7*/
> + "PRSNT_CANBUSP1_TRAY1_TO_32_N","",
> + "PWRGD_CANBUSP1_TRAY1_TO_32_PWROK","",
> + "SMALL_LEAK_CANBUSP1_TRAY1_TO_32_N","",
> + "LEAK_DETECT_CANBUSP1_TRAY1_TO_32_N","",
> + "PRSNT_CANBUSP2_TRAY1_TO_32_N","",
> + "PWRGD_CANBUSP2_TRAY1_TO_32_PWROK","",
> + "SMALL_LEAK_CANBUSP2_TRAY1_TO_32_N","",
> + "LEAK_DETECT_CANBUSP2_TRAY1_TO_32_N","",
> + /*E0 - E7*/
> + "PRSNT_CANBUSP3_TRAY1_TO_32_N","",
> + "PWRGD_CANBUSP3_TRAY1_TO_32_PWROK","",
> + "SMALL_LEAK_CANBUSP3_TRAY1_TO_32_N","",
> + "LEAK_DETECT_CANBUSP3_TRAY1_TO_32_N","",
> + "PRSNT_CANBUSP4_TRAY1_TO_32_N","",
> + "PWRGD_CANBUSP4_TRAY1_TO_32_PWROK","",
> + "SMALL_LEAK_CANBUSP4_TRAY1_TO_32_N","",
> + "LEAK_DETECT_CANBUSP4_TRAY1_TO_32_N","",
> + /*F0 - F7*/
> + "PRSNT_CANBUSP5_TRAY1_TO_32_N","",
> + "PWRGD_CANBUSP5_TRAY1_TO_32_PWROK","",
> + "SMALL_LEAK_CANBUSP5_TRAY1_TO_32_N","",
> + "LEAK_DETECT_CANBUSP5_TRAY1_TO_32_N","",
> + "PRSNT_CANBUSP6_TRAY1_TO_32_N","",
> + "PWRGD_CANBUSP6_TRAY1_TO_32_PWROK","",
> + "SMALL_LEAK_CANBUSP6_TRAY1_TO_32_N","",
> + "LEAK_DETECT_CANBUSP6_TRAY1_TO_32_N","",
> + /*G0 - G7*/
> + "PRSNT_CANBUSP7_TRAY1_TO_32_N","",
> + "PWRGD_CANBUSP7_TRAY1_TO_32_PWROK","",
> + "SMALL_LEAK_CANBUSP7_TRAY1_TO_32_N","",
> + "LEAK_DETECT_CANBUSP7_TRAY1_TO_32_N","",
> + "PRSNT_CANBUSP8_TRAY1_TO_32_N","",
> + "PWRGD_CANBUSP8_TRAY1_TO_32_PWROK","",
> + "SMALL_LEAK_CANBUSP8_TRAY1_TO_32_N","",
> + "LEAK_DETECT_CANBUSP8_TRAY1_TO_32_N","",
> + /*H0 - H7*/
> + "wCHASSIS0_LEAK_Q_N_R","",
> + "wCHASSIS1_LEAK_Q_N_R","",
> + "wCHASSIS2_LEAK_Q_N_R","",
> + "wCHASSIS3_LEAK_Q_N_R","",
> + "wCHASSIS4_LEAK_Q_N_R","",
> + "wCHASSIS5_LEAK_Q_N_R","",
> + "wCHASSIS6_LEAK_Q_N_R","",
> + "wCHASSIS7_LEAK_Q_N_R","",
> + /*I0 - I7*/
> + "wCHASSIS8_LEAK_Q_N_R","",
> + "wCHASSIS9_LEAK_Q_N_R","",
> + "wCHASSIS10_LEAK_Q_N_R","",
> + "wCHASSIS11_LEAK_Q_N_R","",
> + "wAALC_RPU_READY","",

Out of curiosity, what's going on with the lower-case 'w' prefix here
(and again below)?

> + "","",
> + "","",
> + "","",
> + /*J0 - J7*/
> + "","",
> + "","",
> + "","",
> + "","",
> + "","",
> + "","",
> + "","",
> + "","",
> + /*K0 - K7*/
> + "","",
> + "","",
> + "","",
> + "","",
> + "","",
> + "","",
> + "","",
> + "","",
> + /*L0 - L7*/
> + "wIT_GEAR_RPU_2_LINK_PRSNT_SPARE_N_R","",
> + "wIT_GEAR_RPU_2_LINK_PRSNT_N_R","",
> + "wIT_GEAR_RPU_LINK_PRSNT_SPARE_N_R","",
> + "wIT_GEAR_RPU_LINK_PRSNT_N_R","",
> + "","",
> + "","",
> + "","",
> + "","",
> + /*M0 - M7*/
> + "","",
> + "","",
> + "wPRSNT_SENSOR_N","",
> + "wPRSNT3_VT2_PLD_N","",
> + "wPRSNT2_VT2_PLD_N","",
> + "wPRSNT1_VT2_PLD_N","",
> + "wPRSNT3_RETURN_PLD_N","",
> + "wPRSNT2_RETURN_PLD_N","",
> + /*N0 - N7*/
> + "wPRSNT1_RETURN_PLD_N","",
> + "wPRSNT3_SUPPLY_PLD_N","",
> + "wPRSNT2_SUPPLY_PLD_N","",
> + "wPRSNT1_SUPPLY_PLD_N","",
> + "wPRSNT_LEAK11_SENSOR_R_PLD_N","",
> + "wPRSNT_LEAK10_SENSOR_R_PLD_N","",
> + "wPRSNT_LEAK9_SENSOR_R_PLD_N","",
> + "wPRSNT_LEAK8_SENSOR_R_PLD_N","",
> + /*O0 - O7*/
> + "wPRSNT_LEAK7_SENSOR_R_PLD_N","",
> + "wPRSNT_LEAK6_SENSOR_R_PLD_N","",
> + "wPRSNT_LEAK5_SENSOR_R_PLD_N","",
> + "wPRSNT_LEAK4_SENSOR_R_PLD_N","",
> + "wPRSNT_LEAK3_SENSOR_R_PLD_N","",
> + "wPRSNT_LEAK2_SENSOR_R_PLD_N","",
> + "wPRSNT_LEAK1_SENSOR_R_PLD_N","",
> + "wPRSNT_LEAK0_SENSOR_R_PLD_N","",
> + /*P0 - P7*/
> + "","",
> + "","",
> + "","",
> + "","",
> + "","",
> + "","",
> + "","",
> + "","";
> +};
>
>

*snip*

> +
> +&i2c6 {
> + status = "okay";
> +
> + eeprom@50 {
> + compatible = "atmel,24c64";
> + reg = <0x50>;
> + };
> +
> + io_expander0: gpio@20 {
> + compatible = "nxp,pca9555";
> + reg = <0x20>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> +
> + io_expander1: gpio@21 {
> + compatible = "nxp,pca9555";
> + reg = <0x21>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> +
> + io_expander2: gpio@22 {
> + compatible = "nxp,pca9555";
> + reg = <0x22>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> +
> + io_expander7: gpio@23 {
> + compatible = "nxp,pca9555";
> + reg = <0x23>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> +
> + rtc@51 {
> + compatible = "nxp,pcf8563";
> + reg = <0x51>;
> + };
> +};
> +

*snip*

> +
> +&io_expander0 {

io_expander0 is a label you've defined in this same dts, just above.
Please just include the properties in the original node directly, don't
separate them like this.

Same applies to all other cases of the same pattern.

Andrew

> + gpio-line-names =
> + "RST_POE_BMC_N", "POE_DISABLE_BMC_N_R",
> + "RST_FT4232_1_BMC_N_R", "RST_FT4232_2_BMC_N_R",
> + "RST_FT4232_3_BMC_N_R", "PRSNT_FANBP_0_PWR_N",
> + "PRSNT_FANBP_0_SIG_N", "PRSNT_POE_PWR_N",
> + "PRSNT_POE_SIG_N", "IRQ_POE_BMC_N_R",
> + "PWRGD_P3V3_ISO_POE_BMC_R", "88E6393X_INT_N_R",
> + "P48V_HSC_ALERT_N", "SMB_TMC75_TEMP_ALERT_N_R",
> + "DEV_DIS_N", "PCI_DIS_N";
> +};
> +