mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andrew Jeffery <andrew@codeconstruct.com.au>
To: Kyle Hsieh <kylehsieh1995@gmail.com>,
	Rob Herring <robh@kernel.org>,
	 Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>, Joel Stanley <joel@jms.id.au>
Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	 linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] ARM: dts: aspeed: ventura2: Add Meta ventura2 BMC
Date: Thu, 08 Jan 2026 15:07:58 +1030	[thread overview]
Message-ID: <baa2e4e86b0b6851a56ef97501f6cad67d7970e6.camel@codeconstruct.com.au> (raw)
In-Reply-To: <20251224-ventura2_initial_dts-v2-2-f193ba5d4073@gmail.com>

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@gmail.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?

> +		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@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)?

> +	"","",
> +	"","",
> +	"","",
> +	/*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";
> +};
> +

  reply	other threads:[~2026-01-08  4:38 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-24  6:44 [PATCH v2 0/2] Add Meta(Facebook) ventura2 BMC(AST2600) Kyle Hsieh
2025-12-24  6:44 ` [PATCH v2 1/2] dt-bindings: arm: aspeed: add Meta ventura2 board Kyle Hsieh
2025-12-24  9:10   ` Krzysztof Kozlowski
2025-12-24  6:44 ` [PATCH v2 2/2] ARM: dts: aspeed: ventura2: Add Meta ventura2 BMC Kyle Hsieh
2026-01-08  4:37   ` Andrew Jeffery [this message]
2026-01-13  1:10     ` Kyle Hsieh
2025-12-29 20:39 ` [PATCH v2 0/2] Add Meta(Facebook) ventura2 BMC(AST2600) Rob Herring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=baa2e4e86b0b6851a56ef97501f6cad67d7970e6.camel@codeconstruct.com.au \
    --to=andrew@codeconstruct.com.au \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=joel@jms.id.au \
    --cc=krzk+dt@kernel.org \
    --cc=kylehsieh1995@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-aspeed@lists.ozlabs.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®