<pre>
Thanks Andrew and Joel,

Our Liwu2 project has stopped, so I will not try committing it any more.

Thank you very much !
-----Original Message-----
From: Andrew Jeffery [mailto:andrew@aj.id.au]
Sent: Thursday, April 1, 2021 8:32 AM
To: Willie Thai; Joel Stanley; openbmc@lists.ozlabs.org
Cc: Thai. Willie (TPE)
Subject: [External Mail] Re:_[[PATCH_linux_dev-5.8]_ARM:_dts:_Aspeed:_Add_Compal's_Liwu2_BMC_machine]__ARM:_dts:_aspeed:_Add_device_tree_for_Compal's_Liwu2_BMC



On Fri, 19 Feb 2021, at 17:41, Willie Thai wrote:
> The Liwu2 is a server platform with an ASPEED AST2500 based BMC.
>
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

I don't recall providing my r-b tag.

I have comments below.

> Reviewed-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: willie_thai@compal.com
> ---
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/aspeed-bmc-compal-liwu2.dts | 325
++++++++++++++++++++++++++
> 2 files changed, 326 insertions(+)
> create mode 100644 arch/arm/boot/dts/aspeed-bmc-compal-liwu2.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 6320124..a67576d 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1354,6 +1354,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
> aspeed-bmc-arm-centriq2400-rep.dtb \
> aspeed-bmc-arm-stardragon4800-rep2.dtb \
> aspeed-bmc-bytedance-g220a.dtb \
> +aspeed-bmc-compal-liwu2.dtb \
> aspeed-bmc-facebook-cmm.dtb \
> aspeed-bmc-facebook-minipack.dtb \
> aspeed-bmc-facebook-tiogapass.dtb \
> diff --git a/arch/arm/boot/dts/aspeed-bmc-compal-liwu2.dts
> b/arch/arm/boot/dts/aspeed-bmc-compal-liwu2.dts
> new file mode 100644
> index 0000000..68faf3d
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed-bmc-compal-liwu2.dts
> @@ -0,0 +1,325 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +#include "aspeed-g5.dtsi"
> +#include <dt-bindings/gpio/aspeed-gpio.h>
> +
> +/ {
> +model = "AST2500 liwu2";
> +compatible = "aspeed,ast2500";
> +
> +aliases {
> +serial4 = &uart5;
> +};
> +
> +chosen {
> +stdout-path = &uart5;
> +bootargs = "console=tty0 console=ttyS4,115200 earlyprintk";
> +};
> +
> +memory@80000000 {
> +reg = <0x80000000 0x20000000>;
> +};
> +
> +reserved-memory {
> +#address-cells = <1>;
> +#size-cells = <1>;
> +ranges;
> +
> +gfx_memory: framebuffer {
> +size = <0x01000000>;
> +alignment = <0x01000000>;
> +compatible = "shared-dma-pool";
> +reusable;
> +};
> +};
> +
> +iio-hwmon {
> +compatible = "iio-hwmon";
> +io-channels = <&adc 0>, <&adc 1>, <&adc 2>, <&adc 3>,
> +<&adc 4>, <&adc 5>, <&adc 6>, <&adc 7>,
> +<&adc 8>, <&adc 9>, <&adc 10>, <&adc 11>,
> +<&adc 12>, <&adc 13>, <&adc 14>, <&adc 15>;
> +};
> +
> +leds {
> +compatible = "gpio-leds";
> +
> +led_fan0_fault {
> +label = "LED_FAN0_FAULT";
> +gpios = <&gpio ASPEED_GPIO(F, 4) GPIO_ACTIVE_LOW>;
> +};
> +
> +led_fan1_fault {
> +label = "LED_FAN1_FAULT";
> +gpios = <&gpio ASPEED_GPIO(F, 5) GPIO_ACTIVE_LOW>;
> +};
> +
> +led_fan2_fault {
> +label = "LED_FAN2_FAULT";
> +gpios = <&gpio ASPEED_GPIO(H, 2) GPIO_ACTIVE_LOW>;
> +};
> +
> +led_fan3_fault {
> +label = "LED_FAN3_FAULT";
> +gpios = <&gpio ASPEED_GPIO(H, 5) GPIO_ACTIVE_LOW>;
> +};
> +
> +led_fan4_fault {
> +label = "LED_FAN4_FAULT";
> +gpios = <&gpio ASPEED_GPIO(H, 6) GPIO_ACTIVE_LOW>;
> +};
> +
> +led_fan5_fault {
> +label = "LED_FAN5_FAULT";
> +gpios = <&gpio ASPEED_GPIO(H, 7) GPIO_ACTIVE_LOW>;
> +};
> +
> +fp_led_status_amber_n {
> +label = "FP_LED_STATUS_AMBER_N";
> +gpios = <&gpio ASPEED_GPIO(S, 5) GPIO_ACTIVE_LOW>;
> +};
> +
> +rear_id_led_n {
> +label = "REAR_ID_LED_N";
> +gpios = <&gpio ASPEED_GPIO(S, 6) GPIO_ACTIVE_LOW>;
> +};
> +};
> +};
> +
> +&fmc {
> +status = "okay";
> +flash@0 {
> +status = "okay";
> +m25p,fast-read;
> +label = "bmc";
> +spi-max-frequency = <50000000>;
> +#include "openbmc-flash-layout.dtsi"
> +};
> +};
> +
> +&spi1 {
> +status = "okay";

Should have at least:

pinctrl-names = "default";
pinctrl-0 = <&pinctrl_spi1_default>;

And then if you need additional chip selects, you should add them too.

> +flash@0 {
> +status = "okay";
> +m25p,fast-read;
> +label = "pnor";
> +spi-max-frequency = <100000000>;
> +};
> +};
> +
> +&spi2 {
> +status = "okay";

Is this necessary given no flash device is defined?

If it is, you also need pinctrl properties.

> +};
> +
> +&uart5 {
> +status = "okay";
> +};
> +
> +&mac0 {
> +status = "okay";
> +
> +pinctrl-names = "default";
> +pinctrl-0 = <&pinctrl_rgmii1_default &pinctrl_mdio1_default>;
> +};
> +
> +&mac1 {
> +status = "okay";
> +
> +pinctrl-names = "default";
> +pinctrl-0 = <&pinctrl_rgmii2_default &pinctrl_mdio2_default>;
> +};
> +
> +&adc {
> +status = "okay";

Given you're using the ADC lines in the iio-hwmon bridge, you need to
request the ADC pins from pinmux here. ADC is the default mux
configuration for the pins, so this will happen to work as is, but
pinmux will not prevent something else from requesting the pins out
from underneath the ADC.

Andrew
</pre><!--type:html--><!--{-->===============================================================================================================<br>
This message may contain information which is private, privileged or confidential of Compal Electronics, Inc.<br>
If you are not the intended recipient of this message, please notify the sender and destroy/delete the message.<br>
Any review, retransmission, dissemination or other use of, or taking of any action in reliance upon this information,<br>
by persons or entities other than the intended recipient is prohibited.<br>
===============================================================================================================<br>
<!--}-->