Re: [PATCH v8 2/2] ARM: dts: aspeed: ventura: add Meta Ventura BMC

[Andrew Jeffery <andrew@codeconstruct.com.au>]

Hi Jason,

On Wed, 2025-06-11 at 19:26 +0800, Jason Hsu wrote:
> Add Linux device tree related to Meta(Facebook) Ventura specific

We're writing prose, not a C function invocation. Can you please add a
space after 'Meta':

   Meta (Facebook)

> devices connected to BMC(AST2600) SoC.

Can you provide a bit of a description of the platform design? This
will help review the devicetree content.

> 
> Signed-off-by: Jason Hsu <jasonhell19@gmail.com>
> ---
>  arch/arm/boot/dts/aspeed/Makefile             |    1 +
>  .../aspeed/aspeed-bmc-facebook-ventura.dts    | 1481 +++++++++++++++++
>  2 files changed, 1482 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 2e5f4833a073..e07666d902fb 100644
> --- a/arch/arm/boot/dts/aspeed/Makefile
> +++ b/arch/arm/boot/dts/aspeed/Makefile
> @@ -28,6 +28,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
>         aspeed-bmc-facebook-minerva.dtb \
>         aspeed-bmc-facebook-minipack.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.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..34a3ef3a0efe
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-ventura.dts
> @@ -0,0 +1,1481 @@
> +// 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>
> +
> +

...

> +&i2c5 {
> +       status = "okay";
> +
> +       // RMC FRU
> +       eeprom@54 {
> +               compatible = "atmel,24c128";
> +               reg = <0x54>;
> +       };
> +       // VR TEMP U399
> +       temperature-sensor@4c {
> +               compatible = "ti,tmp75";
> +               reg = <0x4c>;
> +       };

Can you please make sure the node ordering adheres to the documentation
here:

https://docs.kernel.org/devicetree/bindings/dts-coding-style.html#order-of-nodes

Please check the rest of the devicetree.

> +       // VR TEMP U397
> +       temperature-sensor@4d {
> +               compatible = "ti,tmp75";
> +               reg = <0x4d>;
> +       };
> +       // BRICK TEMP U398
> +       temperature-sensor@4e {
> +               compatible = "ti,tmp75";
> +               reg = <0x4e>;
> +       };
> +
> +       temperature-sensor@4f {
> +               compatible = "ti,tmp75";
> +               reg = <0x4f>;
> +       };
> +};
> +
> +&i2c6 {
> +       status = "okay";
> +
> +       gpio@20 {
> +               compatible = "nxp,pca9555";
> +               reg = <0x20>;
> +               gpio-controller;
> +               #gpio-cells = <2>;
> +       };
> +
> +       gpio@21 {
> +               compatible = "nxp,pca9555";
> +               reg = <0x21>;
> +               gpio-controller;
> +               #gpio-cells = <2>;
> +       };
> +
> +       gpio@22 {
> +               compatible = "nxp,pca9555";
> +               reg = <0x22>;
> +               gpio-controller;
> +               #gpio-cells = <2>;
> +       };
> +
> +       rtc@51 {
> +               compatible = "nxp,pcf8563";
> +               reg = <0x51>;
> +       };
> +};
> +
> +&i2c7 {
> +       status = "okay";
> +       bus-frequency = <100000>;
> +       multi-master;
> +       aspeed,hw-timeout-ms = <1000>;

Did you test this with `make dtbs_check`? I expect not, as it causes a
warning identified by Rob's bot:

arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-ventura.dtb: i2c@400 (aspeed,ast2600-i2c-bus): Unevaluated properties are not allowed ('aspeed,hw-timeout-ms' was unexpected)
	from schema $id: http://devicetree.org/schemas/i2c/aspeed,i2c.yaml#

Can you please fix that?

Andrew