public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: Andrew Jeffery <andrew@codeconstruct•com.au>
To: "P.K. Lee" <pkleequanta@gmail•com>,
	robh+dt@kernel•org,  krzysztof.kozlowski+dt@linaro•org,
	conor+dt@kernel•org, joel@jms•id.au,  devicetree@vger•kernel.org,
	linux-arm-kernel@lists•infradead.org,
	 linux-aspeed@lists•ozlabs.org, linux-kernel@vger•kernel.org
Cc: Jason-Hsu@quantatw•com, p.k.lee@quantatw•com
Subject: Re: [PATCH v13 2/2] arm: dts: aspeed: ventura: add Meta Ventura BMC
Date: Mon, 18 May 2026 21:52:05 +0930	[thread overview]
Message-ID: <e5d1cbdb34dddf17b4d474446fb9cebddc0894d9.camel@codeconstruct.com.au> (raw)
In-Reply-To: <20260407081700.2658011-3-pkleequanta@gmail.com>

Hi P.K.

On Tue, 2026-04-07 at 16:17 +0800, P.K. Lee wrote:
> Add Linux device tree related to Meta (Facebook) Ventura specific
> devices connected to the BMC (AST2600) SoC. The purpose of Ventura is to
> detect liquid leakage from all compute trays, switch trays and rack
> sensors within the rack, log the events, and take necessary actions
> accordingly.
> 
> Signed-off-by: P.K. Lee <pkleequanta@gmail•com>
> ---
>  arch/arm/boot/dts/aspeed/Makefile             |    1 +
>  .../aspeed/aspeed-bmc-facebook-ventura.dts    | 1636 +++++++++++++++++
>  2 files changed, 1637 insertions(+)
>  create mode 100644 arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-ventura.dts
> 
> diff --git a/arch/arm/boot/dts/aspeed/Makefile b/arch/arm/boot/dts/aspeed/Makefile
> index 0f0b5b707654..f5ac72d5933c 100644
> --- a/arch/arm/boot/dts/aspeed/Makefile
> +++ b/arch/arm/boot/dts/aspeed/Makefile
> @@ -32,6 +32,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
>  	aspeed-bmc-facebook-minipack.dtb \
>  	aspeed-bmc-facebook-santabarbara.dtb \
>  	aspeed-bmc-facebook-tiogapass.dtb \
> +	aspeed-bmc-facebook-ventura.dtb \
>  	aspeed-bmc-facebook-wedge40.dtb \
>  	aspeed-bmc-facebook-wedge100.dtb \
>  	aspeed-bmc-facebook-wedge400-data64.dtb \
> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-ventura.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-ventura.dts
> new file mode 100644
> index 000000000000..6ce6201f7755
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-ventura.dts
> @@ -0,0 +1,1636 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright (c) 2023 Facebook Inc.
> +/dts-v1/;
> +
> +#include "aspeed-g6.dtsi"
> +#include <dt-bindings/i2c/i2c.h>
> +#include <dt-bindings/gpio/aspeed-gpio.h>
> +
> +/ {
> +	model = "Facebook ventura RMC";

I suggest capitalising 'Ventura'.

> +	compatible = "facebook,ventura-rmc", "aspeed,ast2600";
> +
> +	aliases {
> +		serial4 = &uart5;
> +		i2c16 = &i2c3mux0ch3;
> +		i2c17 = &i2c3mux0ch4;
> +		i2c18 = &i2c3mux0ch5;
> +		i2c19 = &i2c3mux0ch6;
> +		i2c20 = &i2c3mux0ch0;
> +		i2c21 = &i2c3mux0ch1;
> +		i2c22 = &i2c3mux0ch2;
> +		i2c23 = &i2c3mux0ch7;
> +		i2c24 = &i2c0mux0ch0;
> +		i2c25 = &i2c0mux0ch1;
> +		i2c26 = &i2c0mux0ch2;
> +		i2c27 = &i2c0mux0ch3;
> +		i2c28 = &i2c0mux0ch4;
> +		i2c29 = &i2c0mux0ch5;
> +		i2c30 = &i2c0mux0ch6;
> +		i2c31 = &i2c0mux0ch7;
> +		i2c32 = &i2c1mux0ch0;
> +		i2c33 = &i2c1mux0ch1;
> +		i2c34 = &i2c1mux0ch2;
> +		i2c35 = &i2c1mux0ch3;
> +		i2c36 = &i2c1mux0ch4;
> +		i2c37 = &i2c1mux0ch5;
> +		i2c38 = &i2c1mux0ch6;
> +		i2c39 = &i2c1mux0ch7;
> +		i2c40 = &i2c2mux0ch0;
> +		i2c41 = &i2c2mux0ch1;
> +		i2c42 = &i2c2mux0ch2;
> +		i2c43 = &i2c2mux0ch3;
> +		i2c44 = &i2c2mux0ch4;
> +		i2c45 = &i2c2mux0ch5;
> +		i2c46 = &i2c2mux0ch6;
> +		i2c47 = &i2c2mux0ch7;

Many of the buses aliased here don't have any devices described below
them. Can you add some commentary about why it's necessary to enable
and alias each of these?

> +	};
> +
> +	chosen {
> +		stdout-path = "serial4:57600n8";
> +	};
> +
> +	iio-hwmon {
> +		compatible = "iio-hwmon";
> +		io-channels = <&adc0 0>, <&adc0 1>, <&adc0 2>, <&adc0 3>,
> +			<&adc0 4>, <&adc0 5>, <&adc0 6>, <&adc0 7>,
> +			<&adc1 2>;
> +	};
> +

...

> +		i2c3mux0ch4: i2c@4 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <4>;
> +
> +			adc@1f {
> +				compatible = "ti,adc128d818";
> +				reg = <0x1f>;
> +				ti,mode = /bits/ 8 <1>;
> +			};
> +
> +			fan_leds_g2_gpio: gpio@21 {
> +				compatible = "nxp,pca9555";
> +				reg = <0x21>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +
> +				gpio-line-names =
> +				"", "",
> +				"", "",
> +				"", "",
> +				"", "",
> +				"FAN2_PRSNT", "FAN3_PRSNT",
> +				"", "",
> +				"", "",
> +				"", "";
> +			};
> +
> +			adc@35 {
> +				compatible = "maxim,max11617";
> +				reg = <0x35>;
> +			};
> +
> +			// Fan Board 1 FRU

I'd rather we pick one commenting style (/* */). Can you please fix
that throughout?

> +			eeprom@56 {
> +				compatible = "atmel,24c128";
> +				reg = <0x56>;
> +			};
> +		};
> +
> +		i2c3mux0ch5: i2c@5 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <5>;
> +
> +			pwm@20 {
> +				compatible = "maxim,max31790";
> +				reg = <0x20>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				channel@2 {
> +					reg = <2>;
> +					sensor-type = "TACH";
> +				};
> +				channel@5 {
> +					reg = <5>;
> +					sensor-type = "TACH";
> +				};
> +			};
> +
> +			hwmon: hwmon@23 {
> +				compatible = "nuvoton,nct7363";
> +				reg = <0x23>;
> +				#pwm-cells = <2>;
> +
> +				//fan 0 IL

Can you please add a space between the comment marker and the comment
itself? This needs fixing throughout.

> +				fan-0 {
> +					pwms = <&hwmon 0 20000>;
> +					tach-ch = /bits/ 8 <0x09>;
> +				};
> +
> +				//fan 0 OL
> +				fan-1 {
> +					pwms = <&hwmon 0 20000>;
> +					tach-ch = /bits/ 8 <0x0B>;
> +				};
> +
> +				//fan 1 IL
> +				fan-2 {
> +					pwms = <&hwmon 4 20000>;
> +					tach-ch = /bits/ 8 <0x0A>;
> +				};
> +
> +				//fan 1 OL
> +				fan-3 {
> +					pwms = <&hwmon 4 20000>;
> +					tach-ch = /bits/ 8 <0x0D>;
> +				};
> +
> +				//fan 2 IL
> +				fan-4 {
> +					pwms = <&hwmon 6 20000>;
> +					tach-ch = /bits/ 8 <0x0F>;
> +				};
> +
> +				//fan 2 OL
> +				fan-5 {
> +					pwms = <&hwmon 6 20000>;
> +					tach-ch = /bits/ 8 <0x01>;
> +				};
> +
> +				//fan 3 IL
> +				fan-6 {
> +					pwms = <&hwmon 10 20000>;
> +					tach-ch = /bits/ 8 <0x00>;
> +				};
> +
> +				//fan 3 OL
> +				fan-7 {
> +					pwms = <&hwmon 10 20000>;
> +					tach-ch = /bits/ 8 <0x03>;
> +				};
> +			};
> +		};
> +
> 

...

> +
> +&mdio0 {
> +	status = "okay";
> +	/* * Intentionally left empty.

The comment is a bit busted here. Can you please fix it?

Andrew

> +	 * Enabled to allow user-space tools (e.g., mdio)
> +	 * to access the unmanaged Marvell switch registers.
> +	 */
> +};
> +


  reply	other threads:[~2026-05-18 12:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-07  8:16 [PATCH v13 0/2] Add Meta (Facebook) Ventura BMC (AST2600) P.K. Lee
2026-04-07  8:16 ` [PATCH v13 1/2] dt-bindings: arm: aspeed: add Meta Ventura board P.K. Lee
2026-04-07  8:17 ` [PATCH v13 2/2] arm: dts: aspeed: ventura: add Meta Ventura BMC P.K. Lee
2026-05-18 12:22   ` Andrew Jeffery [this message]
2026-05-19 12:16     ` P.K. Lee
2026-05-18 12:27 ` [PATCH v13 0/2] Add Meta (Facebook) Ventura BMC (AST2600) Andrew Jeffery

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e5d1cbdb34dddf17b4d474446fb9cebddc0894d9.camel@codeconstruct.com.au \
    --to=andrew@codeconstruct$(echo .)com.au \
    --cc=Jason-Hsu@quantatw$(echo .)com \
    --cc=conor+dt@kernel$(echo .)org \
    --cc=devicetree@vger$(echo .)kernel.org \
    --cc=joel@jms$(echo .)id.au \
    --cc=krzysztof.kozlowski+dt@linaro$(echo .)org \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    --cc=linux-aspeed@lists$(echo .)ozlabs.org \
    --cc=linux-kernel@vger$(echo .)kernel.org \
    --cc=p.k.lee@quantatw$(echo .)com \
    --cc=pkleequanta@gmail$(echo .)com \
    --cc=robh+dt@kernel$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox