[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAF7HswM1nch2Z1JCQiCowWK7UwrV4zvg_uVJUM6bfnFEWVx9Gw@mail.gmail.com>
Date: Tue, 13 Jan 2026 09:10:37 +0800
From: Kyle Hsieh <kylehsieh1995@...il.com>
To: Andrew Jeffery <andrew@...econstruct.com.au>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Joel Stanley <joel@....id.au>, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-aspeed@...ts.ozlabs.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] ARM: dts: aspeed: ventura2: Add Meta ventura2 BMC
On Thu, Jan 8, 2026 at 12:38 PM Andrew Jeffery
<andrew@...econstruct.com.au> wrote:
>
> 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@...il.com>
> > ---
> > 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?
Yes, I will modify in the v3 patch.
>
> > + 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@...00000 {
> > + 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).
I will modify in v3 patch.
>
> > + 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?
Last time, I pushed the v2 patch too early, so I have not yet added
the annotation.
I will add the annotation next version
>
> [1]: https://lore.kernel.org/all/32ff7ca8-9cb0-4889-adb0-a6dae735630b@lunn.ch/
> [2]: https://lore.kernel.org/all/CAF7HswMRrs9hwKo_uHCLMtx7+h46-DPEJRcEqu0-zEG4CVvvjg@mail.gmail.com/
>
> > +};
> > +
> > +&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)?
The prefix 'w' is the sgpio name, after confirming it can remove this prefix.
I will remove it in the next version.
>
> > + "","",
> > + "","",
> > + "","",
> > + /*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.
Thanks for review, I will put the properties to the same node.
>
> 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";
> > +};
> > +
Powered by blists - more mailing lists