Re: [PATCH v5 2/2] ARM: dts: aspeed: Add ASRock Rack B650D4U BMC

[Tan Siewert]

> Add the device tree for the ASRock Rack B650D4U BMC, which is
> based on the ASPEED AST2600 SoC.
> 
> Describe the BMC memory, SPI NOR flash layout, UART console,
> GPIO line names, video framebuffer memory, I2C EEPROM, and MAC0
> connected to an RTL8211F Ethernet PHY.
> 
> Use the EEPROM on I2C7 as the NVMEM provider for the MAC0 address.
> 
> Configure MAC0 to use the RTL8211F in RGMII-ID mode.
> 
> Signed-off-by: Prasanth Kumar Padarthi <prasanth.padarthi10@xxxxxxxxx>

dt-check-style --mode strict reports issues:

    arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts:8: [property-order] property 'compatible' out of canonical order (should sort before 'model')
    arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts:26: [property-order] property 'ranges' out of canonical order (should sort before '#size-cells')
    arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts:30: [property-order] property 'alignment' out of canonical order (should sort before 'size')
    arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts:31: [property-order] property 'compatible' out of canonical order (should sort before 'alignment')
    arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts:39: [property-order] property 'memory-region' out of canonical order (should sort before 'status')
    arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts:70: [property-order] property 'spi-max-frequency' out of canonical order (should sort before 'm25p,fast-read')
    arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts:72: [required-blank-lines] "status" must be preceded by a blank line
    arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts:80: [property-order] property 'phy-mode' out of canonical order (should sort before 'status')
    arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts:81: [property-order] property 'phy-handle' out of canonical order (should sort before 'phy-mode')
    arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts:83: [property-order] property 'pinctrl-0' out of canonical order (should sort before 'pinctrl-names')
    arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts:85: [property-order] property 'nvmem-cells' out of canonical order (should sort before 'pinctrl-0')

>
> diff --git a/arch/arm/boot/dts/aspeed/Makefile b/arch/arm/boot/dts/aspeed/Makefile
> index 6b68671f9379..b109491d5c8f 100644
> --- a/arch/arm/boot/dts/aspeed/Makefile
> +++ b/arch/arm/boot/dts/aspeed/Makefile
> @@ -10,6 +10,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
>  	aspeed-bmc-ampere-mtmitchell.dtb \
>  	aspeed-bmc-arm-stardragon4800-rep2.dtb \
>  	aspeed-bmc-asrock-altrad8.dtb \
> +	aspeed-bmc-asrock-b650d4u.dtb \
>  	aspeed-bmc-asrock-e3c246d4i.dtb \
>  	aspeed-bmc-asrock-e3c256d4i.dtb \
>  	aspeed-bmc-asrock-paul-ipmi-card.dtb \
> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts
> new file mode 100644
> index 000000000000..6585bda56a4c
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-b650d4u.dts
> @@ -0,0 +1,120 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/dts-v1/;
> +
> +#include "aspeed-g6.dtsi"
> +
> +/ {
> +	model = "ASRock Rack B650D4U BMC";
> +	compatible = "asrock,b650d4u-bmc", "aspeed,ast2600";
> +
> +	aliases {
> +		serial0 = &uart5;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	memory@80000000 {
> +		device_type = "memory";
> +		reg = <0x80000000 0x20000000>;
> +	};
> +
> +	reserved-memory {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		gfx_memory: framebuffer {
> +			size = <0x01000000>;
> +			alignment = <0x01000000>;
> +			compatible = "shared-dma-pool";
> +			reusable;
> +		};
> +	};
> +};
> +
> +&gfx {
> +	status = "okay";
> +	memory-region = <&gfx_memory>;
> +};
> +
> +&gpio0 {
> +	gpio-line-names =
> +		/* A0-A7 */	"","","","","","","","",
> +		/* B0-B7 */	"","","","","","","","",
> +		/* C0-C7 */	"","","","","","","","",
> +		/* D0-D7 */	"","","","","","","","",
> +		/* E0-E7 */	"","","","","","","","",
> +		/* F0-F7 */	"","","","","","button-reset-n","","",

How did you confirm this? According to the schematic it should be P0 (P0-P5
have GPIO passthrough functionality).

> +		/* G0-G7 */	"","","","","","","","",
> +		/* H0-H7 */	"","","","","","","","",
> +		/* I0-I7 */	"","","","","","","","",
> +		/* J0-J7 */	"","","","","","","","",
> +		/* K0-K7 */	"","","","","","","","",
> +		/* L0-L7 */	"","","","","","","","",
> +		/* M0-M7 */	"","","","","","","","",
> +		/* N0-N7 */	"","","","","","","","",
> +		/* O0-O7 */	"","","","","","","","",
> +		/* P0-P3 */	"","","","control-power-n";

Personally, I'm not a fan when `gpio-line-names` doesn't match with the amount
of defined `ngpios`, but that doesn't mean it's wrong. Maybe Andrew Jeffrey has
a different opinion on that.

> +};
> +
> +&fmc {
> +	status = "okay";
> +
> +	flash@0 {
> +		compatible = "jedec,spi-nor";
> +		reg = <0>;
> +		label = "bmc";
> +		m25p,fast-read;
> +		spi-max-frequency = <50000000>;
> +		spi-rx-bus-width = <4>;
> +		status = "okay";
> +
> +#include "openbmc-flash-layout-64.dtsi"
> +	};
> +};
> +
> +&mac0 {
> +	status = "okay";
> +	phy-mode = "rgmii-id";
> +	phy-handle = <&ethphy0>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_rgmii1_default>;
> +
> +	nvmem-cells = <&eth0_macaddress>;
> +	nvmem-cell-names = "mac-address";
> +};
> +
> +&mdio0 {
> +	status = "okay";
> +
> +	ethphy0: ethernet-phy@0 {
> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reg = <0>;
> +	};
> +};
> +
> +&uart5 {
> +	status = "okay";
> +};
> +
> +&i2c7 {

Please sort your nodes, as per the device-tree style guideline [0].

[0] https://docs.kernel.org/devicetree/bindings/dts-coding-style.html

Tan

-- 
Tan Siewert <tan.siewert@xxxxxxxxxxxxx>