Skip to contentSkip to conversation

ARM: dts: aspeed: Add NVIDIA MGX Vera C2 BMC support

PATCH In review Jacky Huang 4 participants 8 messages Nov 3 – Nov 11

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v2 cover ARM: dts: aspeed: Add NVIDIA MGX Vera C2 BMC support

This series adds support for the NVIDIA MGX Vera C2 BMC, which is based on the ASPEED AST2600.

The board declares its boot-flash properties explicitly, allowing the same DTS to work before and after the flash-node cleanup [1].

The board DTS was checked with dt-check-style --mode=strict.

Changes in v2:
- Add Krzysztof Kozlowski's Acked-by to the board compatible patch.
- Use stdout-path for the console and baud rate.
- Use generic node names and fix gpio-line-names indentation.
- Remove duplicate GPIO line names.

v1: https://lore.kernel.org/all/[email protected]/

[1] https://lore.kernel.org/all/[email protected]/

Jacky Huang (2):
dt-bindings: arm: aspeed: Add NVIDIA MGX Vera C2 BMC
ARM: dts: aspeed: Add NVIDIA MGX Vera C2 BMC

 .../bindings/arm/aspeed/aspeed.yaml           |    1 +
 arch/arm/boot/dts/aspeed/Makefile             |    1 +
 .../aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts    | 1275 +++++++++++++++++
 3 files changed, 1277 insertions(+)
 create mode 100644 arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts

base-commit: ba3f2c447203166c640e40dc8e9b4b91fc613aa5

Signature · 1 line
2.43.0
#1 lore raw

PATCH v2 1/2 dt-bindings: arm: aspeed: Add NVIDIA MGX Vera C2 BMC

Add the board compatible for the NVIDIA MGX Vera C2 BMC, which is based on the ASPEED AST2600 SoC.

---
Changes in v2:
- Add Krzysztof Kozlowski's Acked-by.

 Documentation/devicetree/bindings/arm/aspeed/aspeed.yaml | 1 +
 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/arm/aspeed/aspeed.yaml +1 -0
index b7e70868e..92127d7c4 100644
@@ -115,6 +115,7 @@ properties:
               - jabil,rbp-bmc
               - nvidia,gb200nvl-bmc
               - nvidia,msx4-bmc
+              - nvidia,vr-c2-bmc
               - qcom,dc-scm-v1-bmc
               - quanta,s6q-bmc
               - ufispace,ncplite-bmc
Signature · 1 line
2.43.0
#2 lore raw

PATCH v2 2/2 ARM: dts: aspeed: Add NVIDIA MGX Vera C2 BMC

In reply to Jacky Huang

Add support for the NVIDIA MGX Vera C2 BMC based on the ASPEED AST2600 SoC.

---
Changes in v2:
- Specify the console and baud rate in stdout-path and drop bootargs.
- Use generic GPIO expander, fan controller and regulator node names.
- Fix gpio-line-names indentation and GPIO bank comment formatting.
- Remove duplicate GPIO line names.

 arch/arm/boot/dts/aspeed/Makefile             |    1 +
 .../aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts    | 1275 +++++++++++++++++
 2 files changed, 1276 insertions(+)
 create mode 100644 arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts
arch/arm/boot/dts/aspeed/Makefile +1 -0
index 6874b621d..e5c44d423 100644
@@ -66,6 +66,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
 	aspeed-bmc-microsoft-olympus.dtb \
 	aspeed-bmc-nvidia-gb200nvl-bmc.dtb \
 	aspeed-bmc-nvidia-msx4-bmc.dtb \
+	aspeed-bmc-nvidia-vr-c2-bmc.dtb \
 	aspeed-bmc-opp-lanyang.dtb \
 	aspeed-bmc-opp-mowgli.dtb \
 	aspeed-bmc-opp-nicole.dtb \
arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts new file +1275 -0

1,275 lines not on this page · raw message

Signature · 1 line
2.43.0
#3 lore raw

Re: [PATCH v2 2/2] ARM: dts: aspeed: Add NVIDIA MGX Vera C2 BMC

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] ARM: dts: aspeed: vr-c2-bmc mac2 pinctrl conflicts with gpio0 mux pins - [Low] ARM: dts: aspeed…

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] ARM: dts: aspeed: vr-c2-bmc mac2 pinctrl conflicts with gpio0 mux pins
- [Low] ARM: dts: aspeed: vr-c2: unreferenced framebuffer shared-dma-pool node --

commit 07cf82ff2465e47eff9ccc7235e61fcff4d5b27a
Author: Jacky Huang <[email protected]>

ARM: dts: aspeed: Add NVIDIA MGX Vera C2 BMC

This commit adds the device tree for the NVIDIA MGX Vera C2 BMC based on the ASPEED AST2600 SoC.

Quoted 5 lines
arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts new file +0 -0
new file mode 100644
index 0000000000000..a044dfa004fc1

[ ... ]

Quoted 13 lines
arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts new file +13 -0
+	reserved-memory {
+		ranges;
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		framebuffer@9f000000 {
+			reg = <0x9f000000 0x01000000>;
+			no-map;
+		};
+
+		ramoops@a0000000 {
+			compatible = "ramoops";
+			reg = <0xa0000000 0x100000>;

arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts line 107

Quoted 10 lines
arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts new file +10 -0
+			max-reason = <2>;
+			record-size = <0x10000>;
+		};
+
+		framebuffer {
+			compatible = "shared-dma-pool";
+			alignment = <0x01000000>;
+			reusable;
+			size = <0x01000000>;
+		};

[Severity: Low]
Is this framebuffer shared-dma-pool node intentionally left unreferenced?

During boot, fdt_scan_reserved_mem() -> __reserved_mem_alloc_size() -> rmem_cma_setup() allocates 16 MiB from memblock for this pool. Because linux,cma-default is not set, rmem_cma_setup() only stores the struct cma pointer in rmem->priv for per-device attachment via a memory-region phandle in of_reserved_mem_device_init_by_idx().

However, this node has no phandle label, no device node references it via memory-region, and &gfx is not enabled in this device tree, so no device can allocate DMA buffers from this 16 MiB pool. Should this node be removed, or labeled and referenced by an enabled &gfx node?

[ ... ]

Quoted 15 lines

+&gpio0 {

arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts new file +14 -0
+	gpio-line-names =
+		/* A0-A7 */
+		"", "", "", "", "", "", "", "",
+
+		/* B0-B7 */
+		"", "", "", "", "", "", "", "",
+
+		/* C0-C7 */
+		"SGPIO_I2C_MUX_SEL-O",
+		"", "", "", "", "", "", "",
+
+		/* D0-D7 */
+		"", "", "",
+		"UART1_MUX_SEL-O",

[ ... ]

arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts line 1234

Quoted 9 lines
arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts new file +9 -0
+/* NCSI MAC */
+&mac2 {
+	phy-mode = "rmii";
+	pinctrl-0 = <&pinctrl_rmii3_default>;
+	pinctrl-names = "default";
+	use-ncsi;
+
+	status = "okay";
+};

[Severity: Medium]
Does pinctrl_rmii3_default on &mac2 conflict with the gpio0 lines SGPIO_I2C_MUX_SEL-O (GPIOC0) and UART1_MUX_SEL-O (GPIOD3)?

In drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c, the RMII3 group includes balls H24 (GPIOC0 / RMII3RCLKO) and E26 (GPIOD3 / RMII3RXER):

    GROUP_DECL(RMII3, H24, J22, H22, H23, G23, F23, F26, F25, E26);
    GROUP_DECL(NCSI3, J22, H22, H23, G23, F23, F26, F25, E26);

When &mac2 probes with pinctrl_rmii3_default, pinmux_enable_setting() claims both H24 and E26 for 1e670000.ethernet and muxes H24 as the 50 MHz RMII3RCLKO output. Because aspeed_g6_pinmux_ops sets .strict = true, any subsequent GPIO request for GPIOC0 or GPIOD3 via aspeed_gpio_request() -> pinctrl_gpio_request() -> pin_request() will fail with -EINVAL.

Since &mac2 uses NCSI, should it use pinctrl_ncsi3_default so H24 (GPIOC0) is not claimed as RMII3RCLKO, and is GPIOD3 (E26) expected to be used as a GPIO while NCSI3 is active?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
#4 lore raw

Re: [PATCH v2 2/2] ARM: dts: aspeed: Add NVIDIA MGX Vera C2 BMC

Quoted 1 line

Is this framebuffer shared-dma-pool node intentionally left unreferenced?

This pool is reserved for future use. It is marked reusable, so unused pages remain available for movable allocations.

Quoted 2 lines

Does pinctrl_rmii3_default on &mac2 conflict with the gpio0 lines SGPIO_I2C_MUX_SEL-O (GPIOC0) and UART1_MUX_SEL-O (GPIOD3)?

H24 and E26 are not required for NC-SI on this platform. Their GPIO line names are retained for potential future use.

Jacky

maintainer
#5 lore raw

Re: [PATCH v2 2/2] ARM: dts: aspeed: Add NVIDIA MGX Vera C2 BMC

In reply to Jacky Huang
Quoted 2 lines · Jacky Huang

Add support for the NVIDIA MGX Vera C2 BMC based on the ASPEED AST2600 SoC.

Can you please expand the description of the platform? Why was it designed and built? What's its purpose?

Quoted 212 lines

---
Changes in v2:
- Specify the console and baud rate in stdout-path and drop bootargs.
- Use generic GPIO expander, fan controller and regulator node names.
- Fix gpio-line-names indentation and GPIO bank comment formatting.
- Remove duplicate GPIO line names.

 arch/arm/boot/dts/aspeed/Makefile             |    1 + .../aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts    | 1275 +++++++++++++++++

 2 files changed, 1276 insertions(+)

 create mode 100644 arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts

arch/arm/boot/dts/aspeed/Makefile +1 -0
index 6874b621d..e5c44d423 100644
@@ -66,6 +66,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
  	aspeed-bmc-microsoft-olympus.dtb \
  	aspeed-bmc-nvidia-gb200nvl-bmc.dtb \
  	aspeed-bmc-nvidia-msx4-bmc.dtb \
+	aspeed-bmc-nvidia-vr-c2-bmc.dtb \
  	aspeed-bmc-opp-lanyang.dtb \
  	aspeed-bmc-opp-mowgli.dtb \
  	aspeed-bmc-opp-nicole.dtb \
arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts new file +180 -0
new file mode 100644
index 000000000..a044dfa00
@@ -0,0 +1,1275 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+
+#include "aspeed-g6.dtsi"
+#include <dt-bindings/gpio/aspeed-gpio.h>
+#include <dt-bindings/i2c/i2c.h>
+
+/ {
+	model = "AST2600 MGX Vera C2 BMC";
+	compatible = "nvidia,vr-c2-bmc", "aspeed,ast2600";
+
+	aliases {
+		i2c16 = &i2c_usbhub;
+		i2c17 = &i2c_tpm;
+		i2c18 = &i2c_dp;
+		i2c19 = &i2c_rtc;
+		i2c20 = &i2c_fan_ctrl0;
+		i2c21 = &i2c_fan_ctrl1;
+		i2c22 = &i2c_fan_iox;
+		i2c23 = &i2c_fan_hsc;
+		i2c30 = &i2c_mcio_c0c1_north0;
+		i2c31 = &i2c_mcio_c0c1_north1;
+		i2c32 = &i2c_mcio_c0c1_north2;
+		i2c33 = &i2c_mcio_c0c1_north3;
+		i2c34 = &i2c_mcio_c0c1_north4;
+		i2c35 = &i2c_mcio_c0c1_north5;
+		i2c36 = &i2c_mcio_c0c1_north6;
+		i2c37 = &i2c_mcio_c0c1_north7;
+		i2c40 = &i2c_mcio_c0_south0;
+		i2c41 = &i2c_mcio_c0_south1;
+		i2c42 = &i2c_mcio_c0_south2;
+		i2c43 = &i2c_mcio_c0_south3;
+		i2c44 = &i2c_mcio_c0_south4;
+		i2c45 = &i2c_mcio_c0_south5;
+		i2c46 = &i2c_mcio_c0_south6;
+		i2c47 = &i2c_mcio_c0_south7;
+		i2c50 = &i2c_mcio_c1_south0;
+		i2c51 = &i2c_mcio_c1_south1;
+		i2c52 = &i2c_mcio_c1_south2;
+		i2c53 = &i2c_mcio_c1_south3;
+		i2c54 = &i2c_mcio_c1_south4;
+		i2c55 = &i2c_mcio_c1_south5;
+		i2c56 = &i2c_mcio_c1_south6;
+		i2c57 = &i2c_mcio_c1_south7;
+		serial2 = &uart3;
+		serial4 = &uart5;
+	};
+
+	chosen {
+		stdout-path = "serial4:115200n8";
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		fault-led {
+			gpios = <&exp7 12 GPIO_ACTIVE_LOW>;
+		};
+
+		power-led {
+			gpios = <&exp7 8 GPIO_ACTIVE_LOW>;
+		};
+
+		uid-led {
+			gpios = <&exp7 10 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	memory@80000000 {
+		device_type = "memory";
+		reg = <0x80000000 0x80000000>;
+	};
+
+	standby_power_regulator: regulator-standby-power {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio0 ASPEED_GPIO(M, 3) GPIO_ACTIVE_HIGH>;
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-max-microvolt = <1800000>;
+		regulator-min-microvolt = <1800000>;
+		regulator-name = "standby_power";
+	};
+
+	reserved-memory {
+		ranges;
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		framebuffer@9f000000 {
+			reg = <0x9f000000 0x01000000>;
+			no-map;
+		};
+
+		ramoops@a0000000 {
+			compatible = "ramoops";
+			reg = <0xa0000000 0x100000>;
+			max-reason = <2>;
+			record-size = <0x10000>;
+		};
+
+		framebuffer {
+			compatible = "shared-dma-pool";
+			alignment = <0x01000000>;
+			reusable;
+			size = <0x01000000>;
+		};
+
+		video_engine_memory: jpegbuffer {
+			compatible = "shared-dma-pool";
+			alignment = <0x01000000>;
+			reusable;
+			size = <0x02000000>;
+		};
+	};
+};
+
+/*
+ * NVIDIA multi-interface MCU enumerates as hid-generic by default
+ * and binds to hid-cp2112 at runtime
+ */
+&ehci1 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	status = "okay";
+
+	hub@1 {
+		compatible = "usb424,2514";
+		reg = <1>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		hub@2 {
+			compatible = "usb424,2422";
+			reg = <2>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			hub@1 {
+				compatible = "usb424,2514";
+				reg = <1>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				device@1 {
+					compatible = "usb955,cf11";
+					reg = <1>;
+					#address-cells = <2>;
+					#size-cells = <0>;
+
+					cp2112: interface@1 {
+						compatible = "usbif955,cf11.config1.1";
+						reg = <1 1>;
+						#gpio-cells = <2>;
+						gpio-controller;
+						#interrupt-cells = <2>;
+						interrupt-controller;
+
+						i2c {
+							#address-cells = <1>;
+							#size-cells = <0>;
+
+							/* CPU0 IROT/thermal/SOCAMM/JTAG */
+							gpio@50 {
+								compatible = "nxp,pca9555";
+								reg = <0x50>;
+								gpio-controller;
+								#gpio-cells = <2>;
+								interrupt-controller;
+								#interrupt-cells = <2>;
+								interrupt-parent = <&cp2112>;
+								interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+								gpio-line-names =
+									"CPU0_IROT_RECOVERY_L_3V3-I",
+									"CPU0_IROT_FATAL_ERR_L_3V3-I",
+									"CPU0_OVERT_L-I",
+									"C0_THERM_WARN_N-I",
+									"C0_S0_THERM_OVERT_N-I",
+									"CPU0_CHIPTHROT_L-I",

arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts line 190

Quoted 10 lines
arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts new file +10 -0
+									"C0_SOCAMM_0_ID_PRSNT_N_1V8-I",
+									"C0_SOCAMM_1_ID_PRSNT_N-I",
+									"C0_SOCAMM_2_ID_PRSNT_N-I",
+									"C0_SOCAMM_3_ID_PRSNT_N-I",
+									"C0_SOCAMM_4_ID_PRSNT_N-I",
+									"C0_SOCAMM_5_ID_PRSNT_N-I",
+									"C0_SOCAMM_6_ID_PRSNT_N-I",
+									"C0_SOCAMM_7_ID_PRSNT_N-I",
+									"MCU_JTAG_BYPASS_CPU0_S-O",
+									"MCU_JTAG_BYPASS_CPU0-O";

I assume the -{I,O} suffixes refer to input or output. Are they necessary?

Andrew

#6 lore raw

Re: [PATCH v2 2/2] ARM: dts: aspeed: Add NVIDIA MGX Vera C2 BMC

Hi Andrew,

Thanks for taking the time to review this.

Quoted 2 lines

Can you please expand the description of the platform? Why was it designed and built? What's its purpose?

The NVIDIA MGX Vera C2 is a Vera CPU-based server platform. I will expand the description in v3 to clarify its purpose.

Quoted 1 line

I assume the -{I,O} suffixes refer to input or output. Are they necessary?

Yes, the -I and -O suffixes indicate the intended GPIO direction and help distinguish inputs from outputs at a glance. This follows the same naming convention used on Facebook SanMiguel. The suffixes are not essential, and I would be happy to adjust the naming if it's preferred.

Thanks,
Jacky

#7 lore raw

Re: [PATCH v2 2/2] ARM: dts: aspeed: Add NVIDIA MGX Vera C2 BMC

In reply to Jacky Huang
Quoted 4 lines

+/*
+ * On-board PHY uses RX and TX RGMII internal delays
+ * so phy-mode is rgmii-id
+ */

This comment is not really correct.

https://elixir.bootlin.com/linux/v6.15/source/Documentation/devicetree/bindings/net/ethernet-controller.yaml#L287

Quoted 17 lines

+&mac0 {

arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts new file +16 -0
+	phy-handle = <&ethphy0>;
+	phy-mode = "rgmii-id";
+	pinctrl-0 = <&pinctrl_rgmii1_default>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
+
+/* NCSI MAC */
+&mac2 {
+	phy-mode = "rmii";
+	pinctrl-0 = <&pinctrl_rmii3_default>;
+	pinctrl-names = "default";
+	use-ncsi;
+
+	status = "okay";

arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts line 1243

Quoted 10 lines
arch/arm/boot/dts/aspeed/aspeed-bmc-nvidia-vr-c2-bmc.dts new file +10 -0
+};
+
+/* MDIO for schematic MAC1 PHY */
+&mdio0 {
+	status = "okay";
+
+	ethphy0: ethernet-phy@0 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <0>;
+	};

But these nodes themselves are O.K.

Andrew