Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Zev Weiss <zweiss@equinix.com>
To:     Logananth Sundararaj <logananth13.hcl@gmail.com>
CC:     Arnd Bergmann <arnd@arndb.de>, Olof Johansson <olof@lixom.net>,
        "soc@kernel.org" <soc@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Joel Stanley <joel@jms.id.au>,
        Andrew Jeffery <andrew@aj.id.au>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-aspeed@lists.ozlabs.org" <linux-aspeed@lists.ozlabs.org>,
        "openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>,
        "thangavel.k@hcl.com" <thangavel.k@hcl.com>,
        "garnermic@gmail.com" <garnermic@gmail.com>,
        "velumanit@hcl.com" <velumanit@hcl.com>,
        "naveen.mosess@hcl.com" <naveen.mosess@hcl.com>
Subject: Re: [PATCH v4] ARM: dts: aspeed: Adding Facebook Yosemite V3.5 BMC
Thread-Topic: [PATCH v4] ARM: dts: aspeed: Adding Facebook Yosemite V3.5 BMC
Thread-Index: AQHYnI6+U8uG9DI9MU63PhVdLCGfIg==
Date:   Wed, 20 Jul 2022 23:16:31 +0000
Message-ID: <20220720231605.GA30806@packtop>
References: <20220720105331.GA23472@logan-ThinkPad-T14-Gen-1>
In-Reply-To: <20220720105331.GA23472@logan-ThinkPad-T14-Gen-1>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?ZURFkXNLFZp/RCW72qJbcBKMipP/bNmKRBebNQ7cjobCV+BjcGiYiXZ3jSfS?=
 =?us-ascii?Q?VMkbR8kJ14bmdvE7sDy+ebIsT51RouGA8ZikQ0qD2PymQ9VYVcHuvznk6BJQ?=
 =?us-ascii?Q?4A6KMNgBNzx6GBS6wiosxQBofbd5jv/vEEPzMIuGvFxwfCKKK2xtfrxGQ4sS?=
 =?us-ascii?Q?LxqLezNlJTquqJy8rYxxqqufdq8mtwRm7Z3yIGT3ni4z1YOO68KQ3eESYRww?=
 =?us-ascii?Q?M+agFArJjRdgGPQTI78FQqQ4/+ZrDhX5Cgz2kf48M+LIl5sn6at/K9MNaVoa?=
 =?us-ascii?Q?JZb2caQjqHkPTQ+Ckxvm7kC2rVlQeuStp1+/QPSk0Tzgm9moJkvfzeNAmW6H?=
 =?us-ascii?Q?lwkl5bHetvPxl7F7k+atHBivl7d/nEbItmwxnnu1j7HjHmR9BiY/u0KxfbFh?=
 =?us-ascii?Q?IaHnYtD5shTWAfhldCoDEXuEzATduD6P4mUN8znkB7e/GB3xNWQmtPzwlvd0?=
 =?us-ascii?Q?yRqWcHn5Lw5xH/aGxN4KEjx/mYWZAw3xauoUkPcAT2KlmkGLaHNw6EQwX514?=
 =?us-ascii?Q?3zZgz9KtwZudMvweYjnCoosIJdWlnYxXjrAgE1kXjxFkorBkSHbxWS7li2CI?=
 =?us-ascii?Q?CWQTTR9e33l+7p/bmEeetQ9ns1tjKgY4L5tt+mLsCVGSRRGYdDhXwMym8Kia?=
 =?us-ascii?Q?bdGSf7kkXD9+IsASiDeZ+87MznX1UMjEc0zlcA/1zJch3Gv5ZBGLkVyOkbEh?=
 =?us-ascii?Q?1Z4P1xCingz52jsO89BS4TgFfKQJT3tu+R3BeOtGXBO1URIrmNT6C2iOVclS?=
 =?us-ascii?Q?DaZlYvp9ZLEePtIQN9O6fjNrToIMoLcH3cw6cjR3xG14smZn2ykNF2k1mj5s?=
 =?us-ascii?Q?MEAuxlajrPMWMdnQiTECMNJwT8D/jbRuu3HO9ctBdlIMw2C4hChC5Mt0OAyJ?=
 =?us-ascii?Q?5HhKuuslnOHEomS54MXwthpydIl02F+pC1UHBZIN6UFxmHhjPTihcdKkJha4?=
 =?us-ascii?Q?1lfp+wKcAZ/yTDNcwJ8zXqsYrH8i2YRBuFgmAHpH/7OEvwgz1b9AK15BF01a?=
 =?us-ascii?Q?8/+dk0hW0g6lZd+UngY3LKdQeMVTY2xl5hqiN7ZGbo6TgBqiy089rVaZiJkJ?=
 =?us-ascii?Q?eo02fiJYPI+D5Tb9Fs8gm1HdfTwt9KyqGOIaH+J7j8CtgSI3zxhJ5X1eSw/x?=
 =?us-ascii?Q?PPrpIXb6WB+AdTwg3BKlcJJqt0CvJNOHcZ0h/yp1aLHx74dsC+0Z/do5GqGi?=
 =?us-ascii?Q?sE9prrq9JEroxQCIImEdfKOfoA4dbsfbcapX6HjbdnmpJ0dJ3gUkz3mp+DB1?=
 =?us-ascii?Q?UIwl+5HraibzOS8+yP+E2WmHaunjY/W7RNM0uqVOuFSpBkO+z/pAlVojO+7x?=
 =?us-ascii?Q?v9H2Q/46+N2mFJYFpEITtzCmXlF3SXWkxeuflf8ugc5JFi67LhYBt1kFzM3X?=
 =?us-ascii?Q?UkC8fNKsTZuEYXA1s6zAOCq0FLD7cFzVeSiQJKtgDC29xZYjGX9SLPSYwrLD?=
 =?us-ascii?Q?G87Bmq2kVKwhcCad6Wnv7OVABcV5Fokf6JMTNDc+HWUOjssmzdtAArQMrkHF?=
 =?us-ascii?Q?RZEk4B219wQ2rlrRAsAba+ZypiuXlkYqWZZhwxQVmvOw3+W1MPpDWH+ToQ9O?=
 =?us-ascii?Q?6/5W7hzRlcPQCs6AKtpaWR2esEfaoz3vUzzLjG/S8tZZ20KtPizit8JAndcs?=
 =?us-ascii?Q?sQ=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <7D972B41ABEB0844A1D1F7A458235263@namprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: equinix.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM8PR04MB8007.namprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: bee171b3-19d0-44f9-0bd4-08da6aa5e1a4
X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Jul 2022 23:16:31.3962
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 72adb271-2fc7-4afe-a5ee-9de6a59f6bfb
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: uttH3aYhQfGBsTpkJ2WAHYPiJdXCpc0qcpueEJVdxX1kl1+eb7txRwfiyqgPmM8qDRlqKfdJZB1yUHgGT2onJA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR04MB5230
X-Proofpoint-GUID: hSxpeyguU42IL41FAutE6R35yADKV5-c
X-Proofpoint-ORIG-GUID: hSxpeyguU42IL41FAutE6R35yADKV5-c
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-07-20_12,2022-07-20_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1011 impostorscore=0
 mlxlogscore=999 lowpriorityscore=0 malwarescore=0 bulkscore=0 phishscore=0
 spamscore=0 priorityscore=1501 adultscore=0 suspectscore=0 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2206140000
 definitions=main-2207200094
X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Jul 20, 2022 at 03:53:31AM PDT, Logananth Sundararaj wrote:
>The Yosemite V3.5 is a facebook multi-node server
>platform that host four OCP server. The BMC
>in the Yosemite V3.5 platform based on AST2600 SoC.
>
>This patch adds linux device tree entry related to
>Yosemite V3.5 specific devices connected to BMC SoC.
>
>Signed-off-by: Logananth Sundararaj <logananth_s@hcl.com>
>
>---
>--- v4 - Bootagrs and memory removed.
>--- v3 - Addressed v2 patch comments.
>--- v2 - Enabled i2c drivers.
>--- v1 - Initial draft.
>---
>---
> arch/arm/boot/dts/Makefile                    |   1 +
> .../boot/dts/aspeed-bmc-facebook-fby35.dts    | 260 ++++++++++++++++++
> 2 files changed, 261 insertions(+)
> create mode 100644 arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts
>
>diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>index 7e0934180724..58add093e5fb 100644
>--- a/arch/arm/boot/dts/Makefile
>+++ b/arch/arm/boot/dts/Makefile
>@@ -1465,6 +1465,7 @@ dtb-$(CONFIG_ARCH_ASPEED) +=3D \
> 	aspeed-bmc-facebook-cloudripper.dtb \
> 	aspeed-bmc-facebook-cmm.dtb \
> 	aspeed-bmc-facebook-elbert.dtb \
>+	aspeed-bmc-facebook-fby35.dtb \
> 	aspeed-bmc-facebook-fuji.dtb \
> 	aspeed-bmc-facebook-galaxy100.dtb \
> 	aspeed-bmc-facebook-minipack.dtb \
>diff --git a/arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts b/arch/arm/bo=
ot/dts/aspeed-bmc-facebook-fby35.dts
>new file mode 100644
>index 000000000000..05efba7117a8
>--- /dev/null
>+++ b/arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts
>@@ -0,0 +1,260 @@
>+// SPDX-License-Identifier: GPL-2.0-or-later
>+// Copyright (c) 2020 Facebook Inc.
>+
>+/dts-v1/;
>+
>+#include "aspeed-g6.dtsi"
>+#include <dt-bindings/gpio/aspeed-gpio.h>
>+#include <dt-bindings/i2c/i2c.h>
>+
>+/ {
>+	model =3D "Facebook fby35";
>+	compatible =3D "facebook,fby35", "aspeed,ast2600";
>+
>+	aliases {
>+		serial4 =3D &uart5;
>+	};
>+
>+	chosen {
>+		stdout-path =3D &uart5;
>+	};
>+
>+	iio-hwmon {
>+		compatible =3D "iio-hwmon";
>+		io-channels =3D <&adc0 0>, <&adc0 1>, <&adc0 2>, <&adc0 3>,
>+			<&adc0 4>, <&adc0 5>, <&adc0 6>, <&adc0 7>,
>+			<&adc1 0>, <&adc1 1>, <&adc1 2>, <&adc1 3>,
>+			<&adc1 4>, <&adc1 5>, <&adc1 6>;
>+	};

A blank line here would be nice for formatting consistency...

>+	spi_gpio: spi-gpio {
>+		status =3D "okay";
>+		compatible =3D "spi-gpio";
>+		#address-cells =3D <1>;
>+		#size-cells =3D <0>;
>+
>+		gpio-sck =3D <&gpio0 ASPEED_GPIO(X, 3) GPIO_ACTIVE_HIGH>;
>+		gpio-mosi =3D <&gpio0 ASPEED_GPIO(X, 4) GPIO_ACTIVE_HIGH>;
>+		gpio-miso =3D <&gpio0 ASPEED_GPIO(X, 5) GPIO_ACTIVE_HIGH>;
>+		num-chipselects =3D <1>;
>+		cs-gpios =3D <&gpio0 ASPEED_GPIO(X, 0) GPIO_ACTIVE_LOW>;
>+
>+		tpmdev@0 {
>+			compatible =3D "tcg,tpm_tis-spi";
>+			spi-max-frequency =3D <33000000>;
>+			reg =3D <0>;
>+		};
>+	};
>+

...and this one could be removed.

>+};
>+
>+&mac3 {
>+	status =3D "okay";
>+
>+	pinctrl-names =3D "default";
>+	pinctrl-0 =3D <&pinctrl_rmii4_default>;
>+	no-hw-checksum;
>+	use-ncsi;
>+	mlx,multi-host;
>+	ncsi-ctrl,start-redo-probe;
>+	ncsi-ctrl,no-channel-monitor;
>+	ncsi-package =3D <1>;
>+	ncsi-channel =3D <1>;
>+	ncsi-rexmit =3D <1>;
>+	ncsi-timeout =3D <2>;
>+};
>+
>+&uart1 {
>+	status =3D "okay";
>+};
>+
>+&uart2 {
>+	status =3D "okay";
>+};
>+
>+&uart3 {
>+	status =3D "okay";
>+};
>+
>+&uart4 {
>+	status =3D "okay";
>+};
>+
>+&uart5 {
>+	status =3D "okay";
>+	compatible =3D "snps,dw-apb-uart";
>+};
>+
>+&wdt1 {
>+	status =3D "okay";
>+	pinctrl-names =3D "default";
>+	pinctrl-0 =3D <&pinctrl_wdtrst1_default>;
>+	aspeed,reset-type =3D "soc";
>+	aspeed,external-signal;
>+	aspeed,ext-push-pull;
>+	aspeed,ext-active-high;
>+	aspeed,ext-pulse-duration =3D <256>;
>+};
>+
>+&rtc {
>+	status =3D "okay";
>+};
>+
>+&fmc {
>+	status =3D "okay";
>+	flash@0 {
>+		status =3D "okay";
>+		m25p,fast-read;
>+		label =3D "spi0.1";
>+		spi-max-frequency =3D <50000000>;
>+		#include "openbmc-flash-layout-128.dtsi"
>+	};
>+	flash@1 {
>+		status =3D "okay";
>+		m25p,fast-read;
>+		label =3D "spi0.0";
>+		spi-max-frequency =3D <50000000>;
>+		#include "openbmc-flash-layout.dtsi"
>+	};

The labels on these look backwards from what I'd guess was probably
intended (flash@0 is spi0.1 and flash@1 is spi0.0).

>+};
>+
>+&i2c0 {
>+	//Host1 IPMB bus

While it's not currently 100% consistent, I think the generally
preferred style is to put a space after '//' (here and elsewhere).

>+	status =3D "okay";
>+	multi-master;
>+	ipmb0@10 {
>+		compatible =3D "ipmb-dev";
>+		reg =3D <(0x10 | I2C_OWN_SLAVE_ADDRESS)>;
>+		i2c-protocol;
>+	};
>+};
>+
>+&i2c1 {
>+	//Host2 IPMB bus
>+	status =3D "okay";
>+	multi-master;
>+	ipmb1@10 {
>+		compatible =3D "ipmb-dev";
>+		reg =3D <(0x10 | I2C_OWN_SLAVE_ADDRESS)>;
>+		i2c-protocol;
>+	};
>+};
>+
>+&i2c2 {
>+	//Host3 IPMB bus
>+	status =3D "okay";
>+	multi-master;
>+	ipmb2@10 {
>+		compatible =3D "ipmb-dev";
>+		reg =3D <(0x10 | I2C_OWN_SLAVE_ADDRESS)>;
>+		i2c-protocol;
>+	};
>+};
>+
>+&i2c3 {
>+	//Host1 IPMB bus
>+	status =3D "okay";
>+	multi-master;
>+	ipmb3@10 {
>+		compatible =3D "ipmb-dev";
>+		reg =3D <(0x10 | I2C_OWN_SLAVE_ADDRESS)>;
>+		i2c-protocol;
>+	};
>+};
>+
>+&i2c4 {
>+	status =3D "okay";
>+};
>+
>+&i2c5 {
>+	status =3D "okay";
>+};
>+
>+&i2c6 {
>+	status =3D "okay";
>+};
>+
>+&i2c7 {
>+	status =3D "okay";
>+};
>+
>+&i2c8 {
>+	//NIC SENSOR TEMP
>+	status =3D "okay";
>+	tmp421@1f {
>+		compatible =3D "ti,tmp421";
>+		reg =3D <0x1f>;
>+	};
>+};
>+
>+&i2c9 {
>+	// Debug-Card IPMB bus
>+	status =3D "okay";
>+	multi-master;
>+	ipmb9@30 {
>+		compatible =3D "ipmb-dev";
>+		reg =3D <(0x30 | I2C_OWN_SLAVE_ADDRESS)>;
>+		i2c-protocol;
>+	};
>+};
>+
>+&i2c10 {
>+	status =3D "okay";
>+};
>+
>+&i2c11 {
>+	status =3D "okay";
>+	//FRU EEPROM
>+	eeprom@51 {
>+		compatible =3D "atmel,24c64";
>+		reg =3D <0x51>;
>+		pagesize =3D <32>;
>+	};
>+};
>+
>+&i2c12 {
>+	status =3D "okay";
>+	//INLET TEMP
>+	tmp75@4e {
>+		compatible =3D "ti,tmp75";
>+		reg =3D <0x4e>;
>+	};
>+	//OUTLET TEMP
>+	tmp75@4f {
>+		compatible =3D "ti,tmp75";
>+		reg =3D <0x4f>;
>+	};
>+};
>+
>+&i2c13 {
>+	status =3D "okay";
>+};
>+
>+&adc0 {
>+	ref_voltage =3D <2500>;
>+	status =3D "okay";
>+
>+	pinctrl-0 =3D <&pinctrl_adc0_default &pinctrl_adc1_default
>+		&pinctrl_adc2_default &pinctrl_adc3_default
>+		&pinctrl_adc4_default &pinctrl_adc5_default
>+		&pinctrl_adc6_default &pinctrl_adc7_default>;
>+};
>+
>+&adc1 {
>+	ref_voltage =3D <2500>;
>+	status =3D "okay";
>+
>+	pinctrl-0 =3D <&pinctrl_adc8_default &pinctrl_adc9_default
>+		&pinctrl_adc10_default &pinctrl_adc11_default
>+		&pinctrl_adc12_default &pinctrl_adc13_default>;
>+};

Missing blank line.

>+&ehci0 {
>+	status =3D "okay";
>+};
>+
>+&ehci1 {
>+	status =3D "okay";
>+};
>+
>+&uhci {
>+	status =3D "okay";
>+};
>--=20
>2.17.1
>=