[PATCH linux dev-5.0] ARM: dts: aspeed: Add Microsoft Olympus BMC

Hongwei Zhang Hongweiz at ami.com
Wed May 15 00:18:32 AEST 2019


Thanks Andrew for your prompt response!
Sure I will use the proper options in git format-patch at the next time, thanks for the tips :-)
Our team will review your comments and submit updated version soon.

Best Regards,
--Hongwei

-----Original Message-----
From: Andrew Jeffery <andrew at aj.id.au> 
Sent: Monday, May 13, 2019 10:54 PM
To: Hongwei Zhang <Hongweiz at ami.com>; Joel Stanley <joel at jms.id.au>; openbmc at lists.ozlabs.org
Cc: Brad Bishop <bradleyb at fuzziesquirrel.com>
Subject: Re: [PATCH linux dev-5.0] ARM: dts: aspeed: Add Microsoft Olympus BMC

Hello Hongwei,

Thanks for the patch!

I think this is the second or third iteration? If you send an updated patch in the future please make sure to use the `-v` option as well as`--subject-prefix` to `git format-patch` - this changes the subject to e.g. `[PATCH linux dev-5.0 v2]` and allows us to identify why you've sent the patch from reading the subject.

On Tue, 14 May 2019, at 03:48, Hongwei Zhang wrote:
> Olympus is a Microsoft OCP platform equipped with Aspeed 2400 BMC Soc.
> 
> Signed-off-by: Hongwei Zhang <hongweiz at ami.com>
> ---
>  arch/arm/boot/dts/Makefile                         |   1 +
>  arch/arm/boot/dts/aspeed-bmc-microsoft-olympus.dts | 204 
> +++++++++++++++++++++
>  2 files changed, 205 insertions(+)
>  create mode 100644 arch/arm/boot/dts/aspeed-bmc-microsoft-olympus.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile 
> index c01a7b1..072725a 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1242,6 +1242,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
>  	aspeed-bmc-facebook-tiogapass.dtb \
>  	aspeed-bmc-intel-s2600wf.dtb \
>  	aspeed-bmc-lenovo-hr630.dtb \
> +	aspeed-bmc-microsoft-olympus.dtb \
>  	aspeed-bmc-opp-lanyang.dtb \
>  	aspeed-bmc-opp-palmetto.dtb \
>  	aspeed-bmc-opp-romulus.dtb \
> diff --git a/arch/arm/boot/dts/aspeed-bmc-microsoft-olympus.dts
> b/arch/arm/boot/dts/aspeed-bmc-microsoft-olympus.dts
> new file mode 100644
> index 0000000..ed25fac
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed-bmc-microsoft-olympus.dts
> @@ -0,0 +1,204 @@
> +//SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Device Tree file for Microsoft Olympus platform.
> + */
> +
> +/dts-v1/;
> +
> +#include "aspeed-g4.dtsi"
> +#include <dt-bindings/gpio/aspeed-gpio.h>
> +
> +/ {
> +	model = "Olympus BMC";
> +	compatible = "microsoft,olympus-bmc", "aspeed,ast2400";
> +
> +	chosen {
> +		stdout-path = &uart5;
> +		bootargs = "console=ttyS4,115200 earlyprintk";
> +	};
> +
> +	memory at 40000000 {
> +		reg = <0x40000000 0x20000000>;
> +	};
> +
> +	reserved-memory {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		vga_memory: framebuffer at 5f000000 {
> +			no-map;
> +			reg = <0x5f000000 0x01000000>; /* 16M */
> +		};
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		bmc_heartbeat {
> +			gpios = <&gpio ASPEED_GPIO(B, 0) GPIO_ACTIVE_LOW>;
> +		};
> +
> +		power_green {
> +			gpios = <&gpio ASPEED_GPIO(U, 2) GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		power_amber {
> +			gpios = <&gpio ASPEED_GPIO(U, 3) GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		identify {
> +			gpios = <&gpio ASPEED_GPIO(Q, 5) GPIO_ACTIVE_LOW>;
> +		};
> +
> +		fault {
> +			gpios = <&gpio ASPEED_GPIO(A, 1) GPIO_ACTIVE_LOW>;
> +		};
> +	};
> +
> +
> +	iio-hwmon {
> +		compatible = "iio-hwmon";
> +		io-channels = <&adc 0>, <&adc 1>, <&adc 2>, <&adc 3>,
> +		<&adc 4>, <&adc 5>, <&adc 6>, <&adc 7>;
> +	};
> +};
> +
> +&adc {
> +	status = "okay";

Can you please add the pinmux properties for the ADC lines that you're using?
The iio-hwmon node suggests you're using lines 0-7.

> +};
> +
> +&fmc {
> +	status = "okay";
> +
> +	flash at 0 {
> +		status = "okay";
> +		m25p,fast-read;
> +		label = "bmc";
> +#include "openbmc-flash-layout.dtsi"
> +	};
> +};
> +
> +&spi {
> +	status = "okay";
> +	pinctrl-names = "default";

This is missing the pinctrl-0 property.

I think in the last version of the patch you had this set to:

pinctrl-0 = <&pinctrl_spi1debug_default>;

and I queried how the board was strapped - you still need the pinctrl-0 node, but what I should have been clearer about was that you should probably be using the following instead:

pinctrl-0 = <&pinctrl_spi1_default>;

unless your board is strapped for SPI debug mode. This is something you'll need to check on the schematics, as pinctrl won't modify the strapping value to accommodate the devicetree in this case.

Otherwise, the patch looks okay to me!

Cheers,

Andrew

> +
> +	flash at 0 {
> +		status = "okay";
> +		m25p,fast-read;
> +		label = "pnor";
> +	};
> +};
> +
> +&uart5 {
> +	status = "okay";
> +};
> +
> +&mac0 {
> +	status = "okay";
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_rgmii1_default &pinctrl_mdio1_default>; };
> +
> +&i2c0 {
> +	status = "okay";
> +};
> +
> +&i2c1 {
> +	status = "okay";
> +
> +	tmp421 at 4c {
> +		compatible = "ti,tmp421";
> +		reg = <0x4c>;
> +	};
> +};
> +
> +&i2c2 {
> +	status = "okay";
> +};
> +
> +&i2c3 {
> +	status = "okay";
> +};
> +
> +&i2c4 {
> +	status = "okay";
> +	clock-frequency = <100000>;
> +};
> +
> +&i2c5 {
> +	status = "okay";
> +};
> +
> +&i2c6 {
> +	status = "okay";
> +
> +	tmp421 at 4c {
> +		compatible = "ti,tmp421";
> +		reg = <0x4c>;
> +	};
> +};
> +
> +&i2c7 {
> +	status = "okay";
> +};
> +
> +&vuart {
> +	status = "okay";
> +};
> +
> +&wdt2 {
> +	status = "okay";
> +};
> +
> +&lpc_ctrl {
> +	status = "okay";
> +};
> +
> +&mbox {
> +	status = "okay";
> +};
> +
> +&pwm_tacho {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 =    <&pinctrl_pwm0_default
> +			&pinctrl_pwm1_default
> +			&pinctrl_pwm2_default
> +			&pinctrl_pwm3_default
> +			&pinctrl_pwm4_default
> +			&pinctrl_pwm5_default
> +			&pinctrl_pwm6_default>;
> +
> +	fan at 0 {
> +		reg = <0x00>;
> +		aspeed,fan-tach-ch = /bits/ 8 <0x00>;
> +	};
> +
> +	fan at 1 {
> +		reg = <0x01>;
> +		aspeed,fan-tach-ch = /bits/ 8 <0x01>;
> +	};
> +
> +	fan at 2 {
> +		reg = <0x02>;
> +		aspeed,fan-tach-ch = /bits/ 8 <0x02>;
> +	};
> +
> +	fan at 3 {
> +		reg = <0x03>;
> +		aspeed,fan-tach-ch = /bits/ 8 <0x03>;
> +	};
> +
> +	fan at 4 {
> +		reg = <0x04>;
> +		aspeed,fan-tach-ch = /bits/ 8 <0x04>;
> +	};
> +
> +	fan at 5 {
> +		reg = <0x05>;
> +		aspeed,fan-tach-ch = /bits/ 8 <0x05>;
> +	};
> +
> +};
> --
> 2.7.4
> 
>


More information about the openbmc mailing list