RE: [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

[<Willie_Thai@compal.com>]

[-- Attachment #1.1: Type: text/html, Size: 9450 bytes --]

[-- Attachment #1.2: Type: text/plain, Size: 5347 bytes --]

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