2024-01-10 03:36:19

by Supreeth Venkatesh

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

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 <[email protected]>
---
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 <[email protected]>
+
+/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



2024-02-08 14:57:35

by Supreeth Venkatesh

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

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 <[email protected]>
> ---
> 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 <[email protected]>
> +
> +/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*/ "","","","","","","","";
> +};

2024-02-08 21:55:18

by Andrew Jeffery

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

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

2024-02-09 12:39:59

by Krzysztof Kozlowski

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

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 <[email protected]>
>> ---
>> 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 <[email protected]>
>> +
>> +/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


2024-02-26 06:55:20

by Andrew Jeffery

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

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 <[email protected]>
> ---
> 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 <[email protected]>
> +
> +/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


2024-02-26 07:11:45

by Paul Menzel

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

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

2024-03-01 21:01:36

by Supreeth Venkatesh

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


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

2024-03-01 21:02:55

by Supreeth Venkatesh

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


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 <[email protected]>
>>> ---
>>> 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 <[email protected]>
>>> +
>>> +/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
>

2024-03-01 21:11:44

by Supreeth Venkatesh

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


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 <[email protected]>
>> ---
>> 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 <[email protected]>
>> +
>> +/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
>

2024-03-02 16:26:03

by Krzysztof Kozlowski

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

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 <[email protected]>
>>>> +
>>>> +/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