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

[Andrew Jeffery <andrew@codeconstruct.com.au>]

On Wed, 2025-09-10 at 23:10 -0600, 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";
> +

*snip*

> +};
> +
> +&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 {

Recently I've decided that I'd like the "usual" node references here in
the DTS to be ordered alphabetically. The style guide gives us two
ordering options, either by ascending unit address or alphabetically:

https://docs.kernel.org/devicetree/bindings/dts-coding-style.html#order-of-nodes

Without the unit address being visible it's hard to verify the former,
hence the preference for the latter.

Can you please sort these accordingly?

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

*snip*

> +
> +&i2c3 {
> +	status = "okay";
> +
> +	power-supply@3c {
> +		compatible = "pmbus";

Devicetrees describe the hardware and not abstract protocols like
PMBus, so the compatible string must refer to a specific
manufacturer,model here.

Cheers,

Andrew