Re: [PATCH u-boot v2 2/2] arch: arm: dts: Add dts for the nvidia msx4 board

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

On Tue, 2025-12-02 at 15:52 -0800, Marc Olberding wrote:
> Adds a dts for the nvidia msx4 board, a granite rapids based
> motherboard
> 
> Signed-off-by: Marc Olberding <molberding@nvidia.com>
> ---
>  arch/arm/dts/Makefile                    |   1 +
>  arch/arm/dts/ast2600-msx4-bmc-nvidia.dts | 112
> +++++++++++++++++++++++++++++++
>  2 files changed, 113 insertions(+)
> 
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index
> dbb2fafc4f4c13302a02875cd678477ab95334cb..aebc5b60abc6b267669650be098
> 315f5d5715074 100755
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -689,6 +689,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
>  	ast2600-facebook.dtb \
>  	ast2600-fpga.dtb \
>  	ast2600-gb200nvl-bmc-nvidia.dtb \
> +        ast2600-msx4-bmc-nvidia.dtb \

Whitespace seems funky here?

>  	ast2600-greatlakes.dtb \
>  	ast2600-intel.dtb \
>  	ast2600-intel.dtb \
> diff --git a/arch/arm/dts/ast2600-msx4-bmc-nvidia.dts
> b/arch/arm/dts/ast2600-msx4-bmc-nvidia.dts
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..3f5146c65dec3ec569cd46745a1
> 3be5466f7218c
> --- /dev/null
> +++ b/arch/arm/dts/ast2600-msx4-bmc-nvidia.dts
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +#include "ast2600-u-boot.dtsi"
> +
> +/ {
> +	model = "AST2600 MSX4 BMC";
> +	compatible = "aspeed,ast2600-msx4", "aspeed,ast2600";
> +
> +	memory {
> +		device_type = "memory";
> +		reg = <0x80000000 0x40000000>;
> +	};
> +
> +	chosen {
> +		stdout-path = &uart5;
> +	};
> +
> +	aliases {
> +		spi0 = &fmc;
> +		ethernet0 = &mac0;
> +	};
> +
> +	cpus {
> +		cpu@0 {
> +			clock-frequency = <800000000>;
> +		};
> +		cpu@1 {
> +			clock-frequency = <800000000>;
> +		};
> +	};
> +};
> +
> +&display_port {
> +	status = "okay";
> +};
> +
> +&fmc {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_fmcquad_default>;
> +	aspeed,watchdog-disable;
> +	status = "okay";
> +
> +	flash@0 {
> +		status = "okay";
> +		spi-max-frequency = <10000000>;
> +		spi-tx-bus-width = <2>;
> +		spi-rx-bus-width = <2>;
> +	};
> +
> +	flash@1 {
> +		status = "okay";
> +		spi-max-frequency = <10000000>;
> +		spi-tx-bus-width = <1>;
> +		spi-rx-bus-width = <1>;
> +	};
> +};
> +
> +&hace {
> +	status = "okay";
> +};
> +
> +&mac0 {
> +	phy-mode = "rgmii";
> +	phy-handle = <&ethphy3>;
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_rgmii1_default>;
> +	status = "okay";
> +};
> +
> +&mdio {
> +	status = "okay";

Nit: It'd be nice if the status property were in a consistent location
(wrt ordering in e.g. the fmc node above).

I like it at the top as you have here, makes it clear that the node has
relevant changes.

> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_mdio4_default>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	ethphy3: ethernet-phy@0 {
> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reg = <2>;
> +	};
> +};
> +
> +&scu {
> +	mac0-clk-delay = <0x1d 0x0a
> +			  0x1d 0x0a
> +			  0x1d 0x0a>;
> +};
> +
> +&sdrammc {
> +	clock-frequency = <400000000>;
> +	aspeed,ecc-enabled;
> +	aspeed,ecc-size-mb = <0>;
> +};
> +
> +&uart5 {
> +	u-boot,dm-pre-reloc;
> +	status = "okay";
> +};
> +
> +&wdt1 {
> +	status = "okay";
> +};
> +
> +&wdt2 {
> +	status = "okay";
> +};
> +
> +&wdt3 {
> +	status = "okay";
> +};