Re: [PATCH 2/2] ARM: dts: aspeed: add device tree for ASRock Rack ALTRAD8 BMC

[Krzysztof Kozlowski <krzk@kernel.org>]

On 11/09/2025 07:10, Rebecca Cran wrote:
> The ALTRAD8 BMC is an Aspeed AST2500-based BMC for the ASRock Rack
> ALTRAD8UD-1L2T and ALTRAD8UD2-1L2Q boards.
> 
> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
> ---
>  arch/arm/boot/dts/aspeed/Makefile                      |   1 +
>  arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-altrad8.dts | 647 ++++++++++++++++++++
>  2 files changed, 648 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/aspeed/Makefile b/arch/arm/boot/dts/aspeed/Makefile
> index aba7451ab749..6bffb7130839 100644
> --- a/arch/arm/boot/dts/aspeed/Makefile
> +++ b/arch/arm/boot/dts/aspeed/Makefile
> @@ -9,6 +9,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
>  	aspeed-bmc-ampere-mtjefferson.dtb \
>  	aspeed-bmc-ampere-mtmitchell.dtb \
>  	aspeed-bmc-arm-stardragon4800-rep2.dtb \
> +	aspeed-bmc-asrock-altrad8.dtb \
>  	aspeed-bmc-asrock-e3c246d4i.dtb \
>  	aspeed-bmc-asrock-e3c256d4i.dtb \
>  	aspeed-bmc-asrock-romed8hm3.dtb \
> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-altrad8.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-altrad8.dts
> new file mode 100644
> index 000000000000..61f6cf8018c0
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-altrad8.dts
> @@ -0,0 +1,647 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +#include "aspeed-g5.dtsi"
> +#include <dt-bindings/gpio/aspeed-gpio.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/i2c/i2c.h>
> +
> +/ {
> +	model = "ASRock ALTRAD8 BMC";
> +	compatible = "asrock,altrad8-bmc", "aspeed,ast2500";
> +
> +	aliases {
> +		serial4 = &uart5;
> +		i2c50 = &m2_2;
> +		i2c51 = &pcie4;
> +		i2c52 = &pcie5;
> +		i2c53 = &pcie6;
> +		i2c54 = &pcie7;
> +		i2c55 = &ocu_2;
> +		i2c56 = &ocu_1;
> +		i2c57 = &m2_1;
> +		i2c58 = &slim1_1;
> +		i2c59 = &slim2_1;
> +		i2c60 = &slim3_1;
> +		i2c61 = &slim4_1;
> +		i2c62 = &slim1_2;
> +		i2c63 = &slim2_2;
> +		i2c64 = &slim3_2;
> +		i2c65 = &slim4_2;
> +	};
> +
> +	chosen {
> +		stdout-path = &uart5;
> +		bootargs = "console=ttyS4,115200 earlycon";


Please drop bootargs. Baud rate goes to stdout-path and earlycon is
debugging tool, not suitable for mainline.

> +	};
> +
> +	memory@80000000 {
> +		reg = <0x80000000 0x20000000>;
> +	};
> +
> +	reserved-memory {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		gfx_memory: framebuffer {
> +			size = <0x01000000>;
> +			alignment = <0x01000000>;
> +			compatible = "shared-dma-pool";
> +			reusable;
> +		};
> +
> +		vga_memory: framebuffer@9f000000 {
> +			no-map;
> +			reg = <0x9f000000 0x01000000>; /* 16M */
> +		};
> +
> +		video_engine_memory: jpegbuffer {
> +			size = <0x02000000>;	/* 32M */
> +			alignment = <0x01000000>;
> +			compatible = "shared-dma-pool";
> +			reusable;
> +		};
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		systemfault {

Never tested.

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
Maybe you need to update your dtschema and yamllint. Don't rely on
distro packages for dtschema and be sure you are using the latest
released dtschema.


> +			gpios = <&gpio ASPEED_GPIO(G,3) GPIO_ACTIVE_LOW>;
> +			label = "platform:red:fault";
> +			color = <LED_COLOR_ID_RED>;
> +			function = LED_FUNCTION_FAULT;
> +		};
> +
> +		enclosure_identify {
> +			gpios = <&gpio ASPEED_GPIO(G,0) GPIO_ACTIVE_LOW>;
> +			label = "platform:green:indicator";
> +			color = <LED_COLOR_ID_GREEN>;
> +			function = LED_FUNCTION_INDICATOR;
> +		};
> +	};
> +
> +	leds-fanfail {
> +		compatible = "gpio-leds";
> +
> +		fan1 {
> +			retain-state-shutdown;
> +			default-state = "off";
> +			gpios = <&pca0 0 GPIO_ACTIVE_LOW>;
> +			label = "fan1:red:fault";
> +			color = <LED_COLOR_ID_RED>;
> +			function = LED_FUNCTION_FAULT;
> +		};
> +
> +		fan2 {
> +			retain-state-shutdown;
> +			default-state = "off";
> +			gpios = <&pca0 1 GPIO_ACTIVE_LOW>;
> +			label = "fan2:red:fault";
> +			color = <LED_COLOR_ID_RED>;
> +			function = LED_FUNCTION_FAULT;
> +		};
> +
> +		fan3 {
> +			retain-state-shutdown;
> +			default-state = "off";
> +			gpios = <&pca0 2 GPIO_ACTIVE_LOW>;
> +			label = "fan3:red:fault";
> +			color = <LED_COLOR_ID_RED>;
> +			function = LED_FUNCTION_FAULT;
> +		};
> +
> +		fan4 {
> +			retain-state-shutdown;
> +			default-state = "off";
> +			gpios = <&pca0 3 GPIO_ACTIVE_LOW>;
> +			label = "fan4:red:fault";
> +			color = <LED_COLOR_ID_RED>;
> +			function = LED_FUNCTION_FAULT;
> +		};
> +
> +		fan5{
> +			retain-state-shutdown;
> +			default-state = "off";
> +			gpios = <&pca0 4 GPIO_ACTIVE_LOW>;
> +			label = "fan5:red:fault";
> +			color = <LED_COLOR_ID_RED>;
> +			function = LED_FUNCTION_FAULT;
> +		};
> +	};
> +
> +	iio-hwmon {
> +		compatible = "iio-hwmon";
> +		io-channels =	<&adc 0>, <&adc 1>, <&adc 2>, <&adc 3>,
> +				<&adc 4> ,<&adc 5>, <&adc 6>, <&adc 7>,
> +				<&adc 8>, <&adc 9>, <&adc 10>, <&adc 11>,
> +				<&adc 12>, <&adc 13>, <&adc 14>, <&adc 15>;
> +	};
> +};
> +
> +&fmc {
> +	status = "okay";
> +	flash@0 {
> +		status = "okay";
> +		label = "bmc";
> +		m25p,fast-read;
> +		spi-max-frequency = <50000000>;
> +#include "openbmc-flash-layout-64.dtsi"
> +	};
> +};
> +
> +&spi1 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_spi1_default>;
> +
> +	flash@0 {
> +		status = "okay";
> +		m25p,fast-read;
> +		label = "pnor";
> +		spi-max-frequency = <100000000>;
> +
> +		partitions {
> +			compatible = "fixed-partitions";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			code@400000 {
> +				reg = <0x400000 0x1C00000>;
> +				label = "pnor-code";
> +			};
> +			tfa@400000 {
> +				reg = <0x400000 0x200000>;
> +				label = "pnor-tfa";
> +			};
> +			uefi@600000 {
> +				reg = <0x600000 0x1A00000>;
> +				label = "pnor-uefi";
> +			};
> +		};
> +	};
> +};
> +
> +&uart1 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_txd1_default
> +			 &pinctrl_rxd1_default
> +			 &pinctrl_ncts1_default
> +			 &pinctrl_nrts1_default>;
> +};
> +
> +&uart2 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_txd2_default
> +			 &pinctrl_rxd2_default>;
> +};
> +
> +&uart3 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_txd3_default
> +			 &pinctrl_rxd3_default>;
> +};
> +
> +&uart4 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_txd4_default
> +			 &pinctrl_rxd4_default>;
> +};
> +
> +/* The BMC's uart */
> +&uart5 {
> +	status = "okay";
> +};
> +
> +&mac0 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_rmii1_default>;
> +	clocks = <&syscon ASPEED_CLK_GATE_MAC1CLK>,
> +		 <&syscon ASPEED_CLK_MAC1RCLK>;
> +	clock-names = "MACCLK", "RCLK";
> +	use-ncsi;
> +
> +	nvmem-cells = <&eth0_macaddress>;
> +	nvmem-cell-names = "mac-address";
> +};
> +
> +&mac1 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_rgmii2_default &pinctrl_mdio2_default>;
> +
> +	nvmem-cells = <&eth1_macaddress>;
> +	nvmem-cell-names = "mac-address";
> +};
> +
> +&i2c0 {
> +	status = "okay";
> +	bus-frequency = <100000>;
> +
> +	ipmb0@10 {

0 suffix is not generic.

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
If you cannot find a name matching your device, please check in kernel
sources for similar cases or you can grow the spec (via pull request to
DT spec repo).


> +		compatible = "ipmb-dev";
> +		reg = <(0x10 | I2C_OWN_SLAVE_ADDRESS)>;
> +		i2c-protocol;
> +	};
> +
> +};
> +
> +&i2c1 {
> +	status = "okay";
> +	bus-frequency = <100000>;
> +
> +	pca9548@73 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
If you cannot find a name matching your device, please check in kernel
sources for similar cases or you can grow the spec (via pull request to
DT spec repo).


> +		compatible = "nxp,pca9548";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		reg = <0x73>;
> +		i2c-mux-idle-disconnect;
> +
> +		m2_2: i2c@0 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <0>;
> +		};
> +
> +		pcie4: i2c@1 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <1>;
> +		};
> +
> +		pcie5: i2c@2 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <2>;
> +		};
> +
> +		pcie6: i2c@3 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <3>;
> +		};
> +
> +		pcie7: i2c@4 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <4>;
> +		};
> +
> +		ocu_2: i2c@5 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <5>;
> +		};
> +
> +		ocu_1: i2c@6 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <6>;
> +		};
> +
> +		m2_1: i2c@7 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <7>;
> +		};
> +	};
> +
> +	pca9548@75 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
If you cannot find a name matching your device, please check in kernel
sources for similar cases or you can grow the spec (via pull request to
DT spec repo).

... and same comments everywhere else.

Since this wasn't ever tested, I am not doing full review.


Best regards,
Krzysztof