[PATCH 1/1] ARM:dts:aspeed: Initial device tree for AMD Onyx Platform

[PATCH 1/1] ARM:dts:aspeed: Initial device tree for AMD Onyx Platform

[Supreeth Venkatesh]

This patch adds initial device tree and makefile updates for
AMD Onyx platform.

AMD Onyx platform is an AMD customer reference board with an Aspeed
ast2600 BMC manufactured by AMD.
It describes I2c devices, Fans, Kcs devices, Uarts, Mac, LEDs, etc.
present on AMD Onyx platform.

Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@amd.com>
---
 arch/arm/boot/dts/aspeed/Makefile             |  1 +
 .../boot/dts/aspeed/aspeed-bmc-amd-onyx.dts   | 98 +++++++++++++++++++
 2 files changed, 99 insertions(+)
 create mode 100644 arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts

diff --git a/arch/arm/boot/dts/aspeed/Makefile b/arch/arm/boot/dts/aspeed/Makefile
index fb9cc95f1b60..2b27d377aae2 100644
--- a/arch/arm/boot/dts/aspeed/Makefile
+++ b/arch/arm/boot/dts/aspeed/Makefile
@@ -5,6 +5,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
 	aspeed-ast2600-evb.dtb \
 	aspeed-bmc-amd-daytonax.dtb \
 	aspeed-bmc-amd-ethanolx.dtb \
+	aspeed-bmc-amd-onyx.dtb \
 	aspeed-bmc-ampere-mtjade.dtb \
 	aspeed-bmc-ampere-mtmitchell.dtb \
 	aspeed-bmc-arm-stardragon4800-rep2.dtb \
diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
new file mode 100644
index 000000000000..a7056cd29553
--- /dev/null
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2021 - 2024 AMD Inc.
+// Author: Supreeth Venkatesh <supreeth.venkatesh@amd.com>
+
+/dts-v1/;
+
+#include "aspeed-g6.dtsi"
+#include <dt-bindings/gpio/aspeed-gpio.h>
+
+/ {
+       model = "AMD Onyx BMC";
+       compatible = "amd,onyx-bmc", "aspeed,ast2600";
+
+       aliases {
+               serial0 = &uart1;
+               serial4 = &uart5;
+      };
+
+       chosen {
+               stdout-path = &uart5;
+               bootargs = "console=ttyS4,115200 earlyprintk vmalloc=512MB";
+       };
+
+       memory@80000000 {
+               device_type = "memory";
+               reg = <0x80000000 0x80000000>;
+       };
+
+};
+
+&mdio0 {
+       status = "okay";
+
+       ethphy0: ethernet-phy@0 {
+               compatible = "ethernet-phy-ieee802.3-c22";
+               reg = <0>;
+       };
+};
+
+&mac3 {
+       status = "okay";
+       phy-mode = "rgmii";
+       phy-handle = <&ethphy0>;
+
+       pinctrl-names = "default";
+       pinctrl-0 = <&pinctrl_rgmii4_default>;
+};
+
+
+
+&fmc {
+       status = "okay";
+       flash@0 {
+               compatible = "jedec,spi-nor";
+               status = "okay";
+               #include "openbmc-flash-layout-128.dtsi"
+       };
+};
+
+//Host Console
+&uart1 {
+       status = "okay";
+};
+
+//BMC Console
+&uart5 {
+       status = "okay";
+};
+
+&gpio0 {
+        gpio-line-names =
+        /*A0-A7*/       "","","","","","","","",
+        /*B0-B7*/       "","","","","MON_POST_COMPLETE","P0_PRESENT_L","","",
+        /*C0-C7*/       "","","","","","","","",
+        /*D0-D7*/       "","","","","","","","",
+        /*E0-E7*/       "","","","","","","","",
+        /*F0-F7*/       "","","","","","","","",
+        /*G0-G7*/       "","","","","","","","",
+        /*H0-H7*/       "","ASSERT_WARM_RST_BTN_L","ASSERT_SOC_RST_BTN_L","","","","","",
+        /*I0-I7*/       "","","","","","","","P0_I3C_APML_ALERT_L",
+        /*J0-J7*/       "","","","","","","","",
+        /*K0-K7*/       "","","","","","","","",
+        /*L0-L7*/       "","","","","","","","",
+        /*M0-M7*/       "","","","","","","","",
+        /*N0-N7*/       "","","","","","","PSP_SOFT_FUSE_NOTIFY","ASSERT_BMC_READY",
+        /*O0-O7*/       "","","HDT_SEL","HDT_XTRIG5","HDT_XTRIG6","JTAG_TRST_N","","",
+        /*P0-P7*/       "MON_RST_BTN_L","ASSERT_RST_BTN_L","MON_PWR_BTN_L","ASSERT_PWR_BTN_L","HPM_FPGA_LOCKOUT","ASSERT_NMI_BTN_L","MON_PWR_GOOD","",
+        /*Q0-Q7*/       "","","HDT_DBREQ_L","","BIOS_SPD_MUX_CTRL_RELEASED_L","","","",
+        /*R0-R7*/       "","","","","","","","",
+        /*S0-S7*/       "","","","","","","P0_DIMM_AF_ERROR","P0_DIMM_GL_ERROR",
+        /*T0-T7*/       "","","","","","","","",
+        /*U0-U7*/       "","","","","","","","",
+        /*V0-V7*/       "","","","","","","","",
+        /*W0-W7*/       "","","","","","","","",
+        /*X0-X7*/       "","","","","","","","",
+        /*Y0-Y7*/       "","","","","","","","",
+        /*Z0-Z7*/       "","","","","","","","";
+};
-- 
2.34.1

Re: [PATCH 1/1] ARM:dts:aspeed: Initial device tree for AMD Onyx Platform

[Supreeth Venkatesh]

This patch is pending for a month now.

Can DT maintainers please help review this and provide feedback?

On 1/9/24 21:35, Supreeth Venkatesh wrote:
> This patch adds initial device tree and makefile updates for
> AMD Onyx platform.
>
> AMD Onyx platform is an AMD customer reference board with an Aspeed
> ast2600 BMC manufactured by AMD.
> It describes I2c devices, Fans, Kcs devices, Uarts, Mac, LEDs, etc.
> present on AMD Onyx platform.
>
> Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@amd.com>
> ---
>   arch/arm/boot/dts/aspeed/Makefile             |  1 +
>   .../boot/dts/aspeed/aspeed-bmc-amd-onyx.dts   | 98 +++++++++++++++++++
>   2 files changed, 99 insertions(+)
>   create mode 100644 arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
>
> diff --git a/arch/arm/boot/dts/aspeed/Makefile b/arch/arm/boot/dts/aspeed/Makefile
> index fb9cc95f1b60..2b27d377aae2 100644
> --- a/arch/arm/boot/dts/aspeed/Makefile
> +++ b/arch/arm/boot/dts/aspeed/Makefile
> @@ -5,6 +5,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
>   	aspeed-ast2600-evb.dtb \
>   	aspeed-bmc-amd-daytonax.dtb \
>   	aspeed-bmc-amd-ethanolx.dtb \
> +	aspeed-bmc-amd-onyx.dtb \
>   	aspeed-bmc-ampere-mtjade.dtb \
>   	aspeed-bmc-ampere-mtmitchell.dtb \
>   	aspeed-bmc-arm-stardragon4800-rep2.dtb \
> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
> new file mode 100644
> index 000000000000..a7056cd29553
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
> @@ -0,0 +1,98 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright (c) 2021 - 2024 AMD Inc.
> +// Author: Supreeth Venkatesh <supreeth.venkatesh@amd.com>
> +
> +/dts-v1/;
> +
> +#include "aspeed-g6.dtsi"
> +#include <dt-bindings/gpio/aspeed-gpio.h>
> +
> +/ {
> +       model = "AMD Onyx BMC";
> +       compatible = "amd,onyx-bmc", "aspeed,ast2600";
> +
> +       aliases {
> +               serial0 = &uart1;
> +               serial4 = &uart5;
> +      };
> +
> +       chosen {
> +               stdout-path = &uart5;
> +               bootargs = "console=ttyS4,115200 earlyprintk vmalloc=512MB";
> +       };
> +
> +       memory@80000000 {
> +               device_type = "memory";
> +               reg = <0x80000000 0x80000000>;
> +       };
> +
> +};
> +
> +&mdio0 {
> +       status = "okay";
> +
> +       ethphy0: ethernet-phy@0 {
> +               compatible = "ethernet-phy-ieee802.3-c22";
> +               reg = <0>;
> +       };
> +};
> +
> +&mac3 {
> +       status = "okay";
> +       phy-mode = "rgmii";
> +       phy-handle = <&ethphy0>;
> +
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&pinctrl_rgmii4_default>;
> +};
> +
> +
> +
> +&fmc {
> +       status = "okay";
> +       flash@0 {
> +               compatible = "jedec,spi-nor";
> +               status = "okay";
> +               #include "openbmc-flash-layout-128.dtsi"
> +       };
> +};
> +
> +//Host Console
> +&uart1 {
> +       status = "okay";
> +};
> +
> +//BMC Console
> +&uart5 {
> +       status = "okay";
> +};
> +
> +&gpio0 {
> +        gpio-line-names =
> +        /*A0-A7*/       "","","","","","","","",
> +        /*B0-B7*/       "","","","","MON_POST_COMPLETE","P0_PRESENT_L","","",
> +        /*C0-C7*/       "","","","","","","","",
> +        /*D0-D7*/       "","","","","","","","",
> +        /*E0-E7*/       "","","","","","","","",
> +        /*F0-F7*/       "","","","","","","","",
> +        /*G0-G7*/       "","","","","","","","",
> +        /*H0-H7*/       "","ASSERT_WARM_RST_BTN_L","ASSERT_SOC_RST_BTN_L","","","","","",
> +        /*I0-I7*/       "","","","","","","","P0_I3C_APML_ALERT_L",
> +        /*J0-J7*/       "","","","","","","","",
> +        /*K0-K7*/       "","","","","","","","",
> +        /*L0-L7*/       "","","","","","","","",
> +        /*M0-M7*/       "","","","","","","","",
> +        /*N0-N7*/       "","","","","","","PSP_SOFT_FUSE_NOTIFY","ASSERT_BMC_READY",
> +        /*O0-O7*/       "","","HDT_SEL","HDT_XTRIG5","HDT_XTRIG6","JTAG_TRST_N","","",
> +        /*P0-P7*/       "MON_RST_BTN_L","ASSERT_RST_BTN_L","MON_PWR_BTN_L","ASSERT_PWR_BTN_L","HPM_FPGA_LOCKOUT","ASSERT_NMI_BTN_L","MON_PWR_GOOD","",
> +        /*Q0-Q7*/       "","","HDT_DBREQ_L","","BIOS_SPD_MUX_CTRL_RELEASED_L","","","",
> +        /*R0-R7*/       "","","","","","","","",
> +        /*S0-S7*/       "","","","","","","P0_DIMM_AF_ERROR","P0_DIMM_GL_ERROR",
> +        /*T0-T7*/       "","","","","","","","",
> +        /*U0-U7*/       "","","","","","","","",
> +        /*V0-V7*/       "","","","","","","","",
> +        /*W0-W7*/       "","","","","","","","",
> +        /*X0-X7*/       "","","","","","","","",
> +        /*Y0-Y7*/       "","","","","","","","",
> +        /*Z0-Z7*/       "","","","","","","","";
> +};

Re: [PATCH 1/1] ARM:dts:aspeed: Initial device tree for AMD Onyx Platform

[Andrew Jeffery]

On Thu, 2024-02-08 at 08:57 -0600, Supreeth Venkatesh wrote:
> This patch is pending for a month now.
> 
> Can DT maintainers please help review this and provide feedback?

Joel's on leave and I'm having to manage some personal concerns as well
as other professional priorities.

I'm trying to find time to address the BMC patch backlog but it really
is a best-effort thing at the moment.

Andrew

Re: [PATCH 1/1] ARM:dts:aspeed: Initial device tree for AMD Onyx Platform

[Krzysztof Kozlowski]

On 08/02/2024 15:57, Supreeth Venkatesh wrote:
> This patch is pending for a month now.
> 
> Can DT maintainers please help review this and provide feedback?

I don't have the original patch in my inbox anymore, so probably you
should resend. Anyway quick look points to obvious issues (comments below).

> 
> On 1/9/24 21:35, Supreeth Venkatesh wrote:
>> This patch adds initial device tree and makefile updates for
>> AMD Onyx platform.
>>
>> AMD Onyx platform is an AMD customer reference board with an Aspeed
>> ast2600 BMC manufactured by AMD.
>> It describes I2c devices, Fans, Kcs devices, Uarts, Mac, LEDs, etc.
>> present on AMD Onyx platform.
>>
>> Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@amd.com>
>> ---
>>   arch/arm/boot/dts/aspeed/Makefile             |  1 +
>>   .../boot/dts/aspeed/aspeed-bmc-amd-onyx.dts   | 98 +++++++++++++++++++
>>   2 files changed, 99 insertions(+)
>>   create mode 100644 arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
>>
>> diff --git a/arch/arm/boot/dts/aspeed/Makefile b/arch/arm/boot/dts/aspeed/Makefile
>> index fb9cc95f1b60..2b27d377aae2 100644
>> --- a/arch/arm/boot/dts/aspeed/Makefile
>> +++ b/arch/arm/boot/dts/aspeed/Makefile
>> @@ -5,6 +5,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
>>   	aspeed-ast2600-evb.dtb \
>>   	aspeed-bmc-amd-daytonax.dtb \
>>   	aspeed-bmc-amd-ethanolx.dtb \
>> +	aspeed-bmc-amd-onyx.dtb \
>>   	aspeed-bmc-ampere-mtjade.dtb \
>>   	aspeed-bmc-ampere-mtmitchell.dtb \
>>   	aspeed-bmc-arm-stardragon4800-rep2.dtb \
>> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
>> new file mode 100644
>> index 000000000000..a7056cd29553
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
>> @@ -0,0 +1,98 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +// Copyright (c) 2021 - 2024 AMD Inc.
>> +// Author: Supreeth Venkatesh <supreeth.venkatesh@amd.com>
>> +
>> +/dts-v1/;
>> +
>> +#include "aspeed-g6.dtsi"
>> +#include <dt-bindings/gpio/aspeed-gpio.h>
>> +
>> +/ {
>> +       model = "AMD Onyx BMC";
>> +       compatible = "amd,onyx-bmc", "aspeed,ast2600";

Undocumented compatibles.

Please run scripts/checkpatch.pl and fix reported warnings. Some
warnings can be ignored, but the code here looks like it needs a fix.
Feel free to get in touch if the warning is not clear.

>> +
>> +       aliases {
>> +               serial0 = &uart1;
>> +               serial4 = &uart5;
>> +      };
>> +
>> +       chosen {
>> +               stdout-path = &uart5;
>> +               bootargs = "console=ttyS4,115200 earlyprintk vmalloc=512MB";

earlyprintk is debugging, not for mainline, so drop.

Console should be encoded in stdout-path.

vmalloc looks like OS tuning, so also not suitable for mainline DTS.


>> +       };
>> +
>> +       memory@80000000 {
>> +               device_type = "memory";
>> +               reg = <0x80000000 0x80000000>;
>> +       };
>> +

Drop redundant blank lines.

>> +};
>> +
>> +&mdio0 {
>> +       status = "okay";
>> +
>> +       ethphy0: ethernet-phy@0 {
>> +               compatible = "ethernet-phy-ieee802.3-c22";
>> +               reg = <0>;
>> +       };
>> +};
>> +
>> +&mac3 {
>> +       status = "okay";
>> +       phy-mode = "rgmii";
>> +       phy-handle = <&ethphy0>;
>> +
>> +       pinctrl-names = "default";
>> +       pinctrl-0 = <&pinctrl_rgmii4_default>;
>> +};
>> +
>> +>> +

Drop redundant blank lines. You can open existing, recent DTS from
maintained platforms like Qcom or TI and look there at coding style.



Best regards,
Krzysztof

Re: [PATCH 1/1] ARM:dts:aspeed: Initial device tree for AMD Onyx Platform

[Andrew Jeffery]

On Tue, 2024-01-09 at 21:35 -0600, Supreeth Venkatesh wrote:
> This patch adds initial device tree and makefile updates for
> AMD Onyx platform.
> 
> AMD Onyx platform is an AMD customer reference board with an Aspeed
> ast2600 BMC manufactured by AMD.
> It describes I2c devices, Fans, Kcs devices, Uarts, Mac, LEDs, etc.
> present on AMD Onyx platform.
> 
> Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@amd.com>
> ---
>  arch/arm/boot/dts/aspeed/Makefile             |  1 +
>  .../boot/dts/aspeed/aspeed-bmc-amd-onyx.dts   | 98 +++++++++++++++++++
>  2 files changed, 99 insertions(+)
>  create mode 100644 arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
> 
> diff --git a/arch/arm/boot/dts/aspeed/Makefile b/arch/arm/boot/dts/aspeed/Makefile
> index fb9cc95f1b60..2b27d377aae2 100644
> --- a/arch/arm/boot/dts/aspeed/Makefile
> +++ b/arch/arm/boot/dts/aspeed/Makefile
> @@ -5,6 +5,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
>  	aspeed-ast2600-evb.dtb \
>  	aspeed-bmc-amd-daytonax.dtb \
>  	aspeed-bmc-amd-ethanolx.dtb \
> +	aspeed-bmc-amd-onyx.dtb \
>  	aspeed-bmc-ampere-mtjade.dtb \
>  	aspeed-bmc-ampere-mtmitchell.dtb \
>  	aspeed-bmc-arm-stardragon4800-rep2.dtb \
> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
> new file mode 100644
> index 000000000000..a7056cd29553
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
> @@ -0,0 +1,98 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright (c) 2021 - 2024 AMD Inc.
> +// Author: Supreeth Venkatesh <supreeth.venkatesh@amd.com>
> +
> +/dts-v1/;
> +
> +#include "aspeed-g6.dtsi"
> +#include <dt-bindings/gpio/aspeed-gpio.h>
> +
> +/ {
> +       model = "AMD Onyx BMC";
> +       compatible = "amd,onyx-bmc", "aspeed,ast2600";
> +
> +       aliases {
> +               serial0 = &uart1;
> +               serial4 = &uart5;
> +      };
> +
> +       chosen {
> +               stdout-path = &uart5;
> +               bootargs = "console=ttyS4,115200 earlyprintk vmalloc=512MB";

Why `vmalloc=512MB`? Can you add a comment explaining the need for
that?

That said, setting bootargs in the devicetree is generally discouraged.

> +       };
> +
> +       memory@80000000 {
> +               device_type = "memory";
> +               reg = <0x80000000 0x80000000>;
> +       };
> +
> +};
> +
> +&mdio0 {
> +       status = "okay";
> +
> +       ethphy0: ethernet-phy@0 {
> +               compatible = "ethernet-phy-ieee802.3-c22";
> +               reg = <0>;
> +       };
> +};
> +
> +&mac3 {
> +       status = "okay";
> +       phy-mode = "rgmii";
> +       phy-handle = <&ethphy0>;
> +
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&pinctrl_rgmii4_default>;
> +};
> +
> +
> +
> +&fmc {
> +       status = "okay";
> +       flash@0 {
> +               compatible = "jedec,spi-nor";
> +               status = "okay";
> +               #include "openbmc-flash-layout-128.dtsi"
> +       };
> +};
> +
> +//Host Console
> +&uart1 {
> +       status = "okay";
> +};

How are you managing the host console? Enabling UART1 for that on the
BMC piques my interest :)

Apologies for the delay in reviewing.

Andrew

Re: [PATCH 1/1] ARM:dts:aspeed: Initial device tree for AMD Onyx Platform

[Paul Menzel]

Dear Supreeth,


Thank you for your patch. Should you resent three more nits.

Am 26.02.24 um 07:55 schrieb Andrew Jeffery:
> On Tue, 2024-01-09 at 21:35 -0600, Supreeth Venkatesh wrote:
>> This patch adds initial device tree and makefile updates for
>> AMD Onyx platform.
>>
>> AMD Onyx platform is an AMD customer reference board with an Aspeed
>> ast2600 BMC manufactured by AMD.
>> It describes I2c devices, Fans, Kcs devices, Uarts, Mac, LEDs, etc.
>> present on AMD Onyx platform.

I’d capitalize I2Cs, UARTs, and MAC, but would write fans.

Also, could you please make the summary a statement by adding a verb in 
imperative mood. Maybe:

Add initial device tree for AMD Onyx Platform

I’d also add spaces after the colons in the commit message summary.

[…]


Kind regards,

Paul

Re: [PATCH 1/1] ARM:dts:aspeed: Initial device tree for AMD Onyx Platform

[Supreeth Venkatesh]

On 2/26/24 01:10, Paul Menzel wrote:
> Caution: This message originated from an External Source. Use proper 
> caution when opening attachments, clicking links, or responding.
>
>
> Dear Supreeth,
>
>
> Thank you for your patch. Should you resent three more nits.
>
> Am 26.02.24 um 07:55 schrieb Andrew Jeffery:
>> On Tue, 2024-01-09 at 21:35 -0600, Supreeth Venkatesh wrote:
>>> This patch adds initial device tree and makefile updates for
>>> AMD Onyx platform.
>>>
>>> AMD Onyx platform is an AMD customer reference board with an Aspeed
>>> ast2600 BMC manufactured by AMD.
>>> It describes I2c devices, Fans, Kcs devices, Uarts, Mac, LEDs, etc.
>>> present on AMD Onyx platform.
>
> I’d capitalize I2Cs, UARTs, and MAC, but would write fans.
Thank you. will incorporate in v2.
>
> Also, could you please make the summary a statement by adding a verb in
> imperative mood. Maybe:
>
> Add initial device tree for AMD Onyx Platform
Thank you. will incorporate in v2.
>
> I’d also add spaces after the colons in the commit message summary.
>
> […]
>
>
> Kind regards,
>
> Paul

Re: [PATCH 1/1] ARM:dts:aspeed: Initial device tree for AMD Onyx Platform

[Supreeth Venkatesh]

On 2/26/24 00:55, Andrew Jeffery wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On Tue, 2024-01-09 at 21:35 -0600, Supreeth Venkatesh wrote:
>> This patch adds initial device tree and makefile updates for
>> AMD Onyx platform.
>>
>> AMD Onyx platform is an AMD customer reference board with an Aspeed
>> ast2600 BMC manufactured by AMD.
>> It describes I2c devices, Fans, Kcs devices, Uarts, Mac, LEDs, etc.
>> present on AMD Onyx platform.
>>
>> Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@amd.com>
>> ---
>>   arch/arm/boot/dts/aspeed/Makefile             |  1 +
>>   .../boot/dts/aspeed/aspeed-bmc-amd-onyx.dts   | 98 +++++++++++++++++++
>>   2 files changed, 99 insertions(+)
>>   create mode 100644 arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
>>
>> diff --git a/arch/arm/boot/dts/aspeed/Makefile b/arch/arm/boot/dts/aspeed/Makefile
>> index fb9cc95f1b60..2b27d377aae2 100644
>> --- a/arch/arm/boot/dts/aspeed/Makefile
>> +++ b/arch/arm/boot/dts/aspeed/Makefile
>> @@ -5,6 +5,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
>>        aspeed-ast2600-evb.dtb \
>>        aspeed-bmc-amd-daytonax.dtb \
>>        aspeed-bmc-amd-ethanolx.dtb \
>> +     aspeed-bmc-amd-onyx.dtb \
>>        aspeed-bmc-ampere-mtjade.dtb \
>>        aspeed-bmc-ampere-mtmitchell.dtb \
>>        aspeed-bmc-arm-stardragon4800-rep2.dtb \
>> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
>> new file mode 100644
>> index 000000000000..a7056cd29553
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
>> @@ -0,0 +1,98 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +// Copyright (c) 2021 - 2024 AMD Inc.
>> +// Author: Supreeth Venkatesh <supreeth.venkatesh@amd.com>
>> +
>> +/dts-v1/;
>> +
>> +#include "aspeed-g6.dtsi"
>> +#include <dt-bindings/gpio/aspeed-gpio.h>
>> +
>> +/ {
>> +       model = "AMD Onyx BMC";
>> +       compatible = "amd,onyx-bmc", "aspeed,ast2600";
>> +
>> +       aliases {
>> +               serial0 = &uart1;
>> +               serial4 = &uart5;
>> +      };
>> +
>> +       chosen {
>> +               stdout-path = &uart5;
>> +               bootargs = "console=ttyS4,115200 earlyprintk vmalloc=512MB";
> Why `vmalloc=512MB`? Can you add a comment explaining the need for
> that?
>
> That said, setting bootargs in the devicetree is generally discouraged.
>
Thank you. will remove in v2.
>> +       };
>> +
>> +       memory@80000000 {
>> +               device_type = "memory";
>> +               reg = <0x80000000 0x80000000>;
>> +       };
>> +
>> +};
>> +
>> +&mdio0 {
>> +       status = "okay";
>> +
>> +       ethphy0: ethernet-phy@0 {
>> +               compatible = "ethernet-phy-ieee802.3-c22";
>> +               reg = <0>;
>> +       };
>> +};
>> +
>> +&mac3 {
>> +       status = "okay";
>> +       phy-mode = "rgmii";
>> +       phy-handle = <&ethphy0>;
>> +
>> +       pinctrl-names = "default";
>> +       pinctrl-0 = <&pinctrl_rgmii4_default>;
>> +};
>> +
>> +
>> +
>> +&fmc {
>> +       status = "okay";
>> +       flash@0 {
>> +               compatible = "jedec,spi-nor";
>> +               status = "okay";
>> +               #include "openbmc-flash-layout-128.dtsi"
>> +       };
>> +};
>> +
>> +//Host Console
>> +&uart1 {
>> +       status = "okay";
>> +};
> How are you managing the host console? Enabling UART1 for that on the
> BMC piques my interest :)
UART1 [BMC side] <-> UART 4 [Host side].
>
> Apologies for the delay in reviewing.
No problem. Thanks for reviewing. Sending v2 shortly.
>
> Andrew
>

Re: [PATCH 1/1] ARM:dts:aspeed: Initial device tree for AMD Onyx Platform

[Supreeth Venkatesh]

On 2/9/24 06:38, Krzysztof Kozlowski wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On 08/02/2024 15:57, Supreeth Venkatesh wrote:
>> This patch is pending for a month now.
>>
>> Can DT maintainers please help review this and provide feedback?
> I don't have the original patch in my inbox anymore, so probably you
> should resend. Anyway quick look points to obvious issues (comments below).
>
>> On 1/9/24 21:35, Supreeth Venkatesh wrote:
>>> This patch adds initial device tree and makefile updates for
>>> AMD Onyx platform.
>>>
>>> AMD Onyx platform is an AMD customer reference board with an Aspeed
>>> ast2600 BMC manufactured by AMD.
>>> It describes I2c devices, Fans, Kcs devices, Uarts, Mac, LEDs, etc.
>>> present on AMD Onyx platform.
>>>
>>> Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@amd.com>
>>> ---
>>>    arch/arm/boot/dts/aspeed/Makefile             |  1 +
>>>    .../boot/dts/aspeed/aspeed-bmc-amd-onyx.dts   | 98 +++++++++++++++++++
>>>    2 files changed, 99 insertions(+)
>>>    create mode 100644 arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
>>>
>>> diff --git a/arch/arm/boot/dts/aspeed/Makefile b/arch/arm/boot/dts/aspeed/Makefile
>>> index fb9cc95f1b60..2b27d377aae2 100644
>>> --- a/arch/arm/boot/dts/aspeed/Makefile
>>> +++ b/arch/arm/boot/dts/aspeed/Makefile
>>> @@ -5,6 +5,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
>>>       aspeed-ast2600-evb.dtb \
>>>       aspeed-bmc-amd-daytonax.dtb \
>>>       aspeed-bmc-amd-ethanolx.dtb \
>>> +    aspeed-bmc-amd-onyx.dtb \
>>>       aspeed-bmc-ampere-mtjade.dtb \
>>>       aspeed-bmc-ampere-mtmitchell.dtb \
>>>       aspeed-bmc-arm-stardragon4800-rep2.dtb \
>>> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
>>> new file mode 100644
>>> index 000000000000..a7056cd29553
>>> --- /dev/null
>>> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
>>> @@ -0,0 +1,98 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +// Copyright (c) 2021 - 2024 AMD Inc.
>>> +// Author: Supreeth Venkatesh <supreeth.venkatesh@amd.com>
>>> +
>>> +/dts-v1/;
>>> +
>>> +#include "aspeed-g6.dtsi"
>>> +#include <dt-bindings/gpio/aspeed-gpio.h>
>>> +
>>> +/ {
>>> +       model = "AMD Onyx BMC";
>>> +       compatible = "amd,onyx-bmc", "aspeed,ast2600";
> Undocumented compatibles.
This is a new device tree file.
>
> Please run scripts/checkpatch.pl and fix reported warnings. Some
> warnings can be ignored, but the code here looks like it needs a fix.
> Feel free to get in touch if the warning is not clear.
>
Will do.
>>> +
>>> +       aliases {
>>> +               serial0 = &uart1;
>>> +               serial4 = &uart5;
>>> +      };
>>> +
>>> +       chosen {
>>> +               stdout-path = &uart5;
>>> +               bootargs = "console=ttyS4,115200 earlyprintk vmalloc=512MB";
> earlyprintk is debugging, not for mainline, so drop.
>
> Console should be encoded in stdout-path.
>
> vmalloc looks like OS tuning, so also not suitable for mainline DTS.
>
Thanks. Will remove vmalloc. However, I will check whether existing 
device trees for BMCs to check

whether existing DTs have earlyprintk.

>>> +       };
>>> +
>>> +       memory@80000000 {
>>> +               device_type = "memory";
>>> +               reg = <0x80000000 0x80000000>;
>>> +       };
>>> +
> Drop redundant blank lines.
ACK.
>
>>> +};
>>> +
>>> +&mdio0 {
>>> +       status = "okay";
>>> +
>>> +       ethphy0: ethernet-phy@0 {
>>> +               compatible = "ethernet-phy-ieee802.3-c22";
>>> +               reg = <0>;
>>> +       };
>>> +};
>>> +
>>> +&mac3 {
>>> +       status = "okay";
>>> +       phy-mode = "rgmii";
>>> +       phy-handle = <&ethphy0>;
>>> +
>>> +       pinctrl-names = "default";
>>> +       pinctrl-0 = <&pinctrl_rgmii4_default>;
>>> +};
>>> +
>>> +>> +
> Drop redundant blank lines. You can open existing, recent DTS from
> maintained platforms like Qcom or TI and look there at coding style.
ACK.
>
>
>
> Best regards,
> Krzysztof
>

Re: [PATCH 1/1] ARM:dts:aspeed: Initial device tree for AMD Onyx Platform

[Krzysztof Kozlowski]

On 01/03/2024 22:02, Supreeth Venkatesh wrote:
>>>> ---
>>>>    arch/arm/boot/dts/aspeed/Makefile             |  1 +
>>>>    .../boot/dts/aspeed/aspeed-bmc-amd-onyx.dts   | 98 +++++++++++++++++++
>>>>    2 files changed, 99 insertions(+)
>>>>    create mode 100644 arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
>>>>
>>>> diff --git a/arch/arm/boot/dts/aspeed/Makefile b/arch/arm/boot/dts/aspeed/Makefile
>>>> index fb9cc95f1b60..2b27d377aae2 100644
>>>> --- a/arch/arm/boot/dts/aspeed/Makefile
>>>> +++ b/arch/arm/boot/dts/aspeed/Makefile
>>>> @@ -5,6 +5,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
>>>>       aspeed-ast2600-evb.dtb \
>>>>       aspeed-bmc-amd-daytonax.dtb \
>>>>       aspeed-bmc-amd-ethanolx.dtb \
>>>> +    aspeed-bmc-amd-onyx.dtb \
>>>>       aspeed-bmc-ampere-mtjade.dtb \
>>>>       aspeed-bmc-ampere-mtmitchell.dtb \
>>>>       aspeed-bmc-arm-stardragon4800-rep2.dtb \
>>>> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
>>>> new file mode 100644
>>>> index 000000000000..a7056cd29553
>>>> --- /dev/null
>>>> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-amd-onyx.dts
>>>> @@ -0,0 +1,98 @@
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>> +// Copyright (c) 2021 - 2024 AMD Inc.
>>>> +// Author: Supreeth Venkatesh <supreeth.venkatesh@amd.com>
>>>> +
>>>> +/dts-v1/;
>>>> +
>>>> +#include "aspeed-g6.dtsi"
>>>> +#include <dt-bindings/gpio/aspeed-gpio.h>
>>>> +
>>>> +/ {
>>>> +       model = "AMD Onyx BMC";
>>>> +       compatible = "amd,onyx-bmc", "aspeed,ast2600";
>> Undocumented compatibles.
> This is a new device tree file.

It's obvious, so what does it even mean?

Anyway, fix warnings.


Best regards,
Krzysztof