| Message ID | 20240130085652.198010-1-Kelly_Hung@asus.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ARM: dts: aspeed: asus: Add ASUS X4TF BMC | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 7 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 30/01/2024 09:56, Kelly Hung wrote: > This initial device-tree provides the necessary configuration for > basic BMC functionality and work on ASUS X4TF production. > > Signed-off-by: Kelly Hung <Kelly_Hung@asus.com> Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. Best regards, Krzysztof
On 30/01/2024 09:56, Kelly Hung wrote: > From: kelly1732000 <Kelly_Hung@asus.com> > > This initial device-tree provides the necessary configuration for > basic BMC functionality and work on ASUS X4TF production. > > Signed-off-by: kelly1732000 <Kelly_Hung@asus.com> > --- > .../boot/dts/aspeed/aspeed-bmc-asus-x4tf.dts | 828 ++++++++++++++++++ > 1 file changed, 828 insertions(+) > create mode 100644 arch/arm/boot/dts/aspeed/aspeed-bmc-asus-x4tf.dts It's impossible to test this file. You miss Makefile. > > diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asus-x4tf.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-asus-x4tf.dts > new file mode 100644 > index 000000000000..723bbb33137f > --- /dev/null > +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asus-x4tf.dts > @@ -0,0 +1,828 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +// Copyright 2023 ASUS Corp. > +/dts-v1/; > + > +#include "aspeed-g6.dtsi" > +#include <dt-bindings/gpio/aspeed-gpio.h> > +#include <dt-bindings/i2c/i2c.h> > + > + > +/ { > + model = "ASUS-X4TF"; > + compatible = "asus,x4tf", "aspeed,ast2600"; > + > + chosen { > + stdout-path = &uart5; > + bootargs = "console=ttyS4,115200n8 earlycon"; earlycon is a debugging feature, not mainline. Also, move console to stdout-path, so entire bootargs can be dropped. > + }; > + > + memory@80000000 { > + device_type = "memory"; > + reg = <0x80000000 0x40000000>; > + }; > + > + reserved-memory { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + gfx_memory: framebuffer { > + size = <0x01000000>; > + alignment = <0x01000000>; > + compatible = "shared-dma-pool"; > + reusable; > + }; > + > + video_engine_memory: video { > + size = <0x04000000>; > + alignment = <0x01000000>; > + compatible = "shared-dma-pool"; > + reusable; > + }; > + > + rvas_memory: rvas { > + size = <0x04000000>; > + alignment = <0x01000000>; > + compatible = "shared-dma-pool"; > + reusable; > + }; > + > + bmc_dev_memory: bmc_dev_memory { No underscores in node names. This applies everywhere. > + size = <0x00100000>; > + alignment = <0x00100000>; > + compatible = "shared-dma-pool"; > + no-map; > + }; > + > + pci_msi_memory: pci_msi_memory { > + no-map; > + reg = <0x9e770000 0x100>; > + compatible = "shared-dma-pool"; > + }; > + /* 1GB memory */ > + vga_memory: region@bf000000 { > + no-map; > + compatible = "shared-dma-pool"; > + reg = <0xbf000000 0x01000000>; /* 16M */ > + }; > + ssp_memory: ssp_memory { > + size = <0x02000000>; > + alignment = <0x01000000>; > + compatible = "shared-dma-pool"; > + no-map; > + }; > + }; > + > + vcc_sdhci0: regulator-vcc-sdhci0 { > + compatible = "regulator-fixed"; > + status = "disabled"; Why? It makes it entirely pointless. > + regulator-name = "SDHCI0 Vcc"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + gpios = <&gpio0 ASPEED_GPIO(V, 0) GPIO_ACTIVE_HIGH>; > + enable-active-high; > + }; > + > + vccq_sdhci0: regulator-vccq-sdhci0 { > + compatible = "regulator-gpio"; > + status = "disabled"; Pointless node. > + regulator-name = "SDHCI0 VccQ"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + gpios = <&gpio0 ASPEED_GPIO(V, 1) GPIO_ACTIVE_HIGH>; > + gpios-states = <1>; > + states = <3300000 1>, > + <1800000 0>; > + }; > + > + vcc_sdhci1: regulator-vcc-sdhci1 { > + compatible = "regulator-fixed"; > + status = "disabled"; Pointless node. > + regulator-name = "SDHCI1 Vcc"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + gpios = <&gpio0 ASPEED_GPIO(V, 2) GPIO_ACTIVE_HIGH>; > + enable-active-high; > + }; > + > + vccq_sdhci1: regulator-vccq-sdhci1 { > + compatible = "regulator-gpio"; > + status = "disabled"; Pointless node. > + regulator-name = "SDHCI1 VccQ"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + gpios = <&gpio0 ASPEED_GPIO(V, 3) GPIO_ACTIVE_HIGH>; > + gpios-states = <1>; > + states = <3300000 1>, > + <1800000 0>; > + }; Missing blank line. > + iio-hwmon { > + compatible = "iio-hwmon"; > + io-channels = <&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>, <&adc1 7>; > + }; > +> + video-engine@1e700000 { Wrong placement. Also tools would tell you that. It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). > + compatible = "aspeed,ast2600-video-engine"; > + reg = <0x1e700000 0x20000>; > + clocks = <&syscon ASPEED_CLK_GATE_VCLK>, <&syscon ASPEED_CLK_GATE_ECLK>; > + clock-names = "vclk", "eclk"; > + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>; > + memory-region = <&video_engine_memory>; > + //resets = <&syscon ASPEED_RESET_VIDEO>; Dead code. > + }; > + > + leds { > + compatible = "gpio-leds"; > + > + heartbeat { It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). > + gpios = <&gpio0 ASPEED_GPIO(P, 7) GPIO_ACTIVE_LOW>; > + linux,default-trigger = "heartbeat"; > + }; > + uid_led { It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). > + gpios = <&gpio0 ASPEED_GPIO(P, 1) (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>; > + default-state = "off"; > + }; Missing blank line, everywhere. > + status_Y { > + gpios = <&gpio1 ASPEED_GPIO(B, 1) GPIO_ACTIVE_LOW>; > + default-state = "off"; > + }; > + sys_boot_status { > + gpios = <&gpio1 ASPEED_GPIO(B, 0) GPIO_ACTIVE_LOW>; > + default-state = "off"; > + }; > + }; > + spigpio { It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). > + status = "okay"; Why? > + compatible = "spi-gpio"; > + #address-cells = <1>; > + #size-cells = <0>; > + > + sck-gpios = <&gpio0 ASPEED_GPIO(Z, 3) GPIO_ACTIVE_HIGH>; > + mosi-gpios = <&gpio0 ASPEED_GPIO(Z, 4) GPIO_ACTIVE_HIGH>; > + miso-gpios = <&gpio0 ASPEED_GPIO(Z, 5) GPIO_ACTIVE_HIGH>; > + num-chipselects = <1>; > + cs-gpios = <&gpio0 ASPEED_GPIO(Z, 0) GPIO_ACTIVE_HIGH>; > + }; > + Redundant blank line. > +}; > + > +&adc0 { > + vref = <2500>; > + status = "okay"; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&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 { > + vref = <2500>; > + status = "okay"; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_adc8_default &pinctrl_adc9_default > + &pinctrl_adc10_default &pinctrl_adc11_default > + &pinctrl_adc12_default &pinctrl_adc13_default > + &pinctrl_adc14_default &pinctrl_adc15_default>; > +}; > + > +&peci0 { > + status = "okay"; > +}; > + > +&lpc_snoop { > + snoop-ports = <0x80>; > + status = "okay"; > +}; > + > + Redundant blank line. Clean this code before sending upstream from such trivialities. > +&mdio0 { > + status = "disabled"; > +}; > + > + > +&mdio1 { > + status = "disabled"; > +}; > + > +&mdio2 { > + status = "okay"; > + > + ethphy2: ethernet-phy@0 { > + compatible = "ethernet-phy-ieee802.3-c22"; > + reg = <0>; > + }; > +}; > + > +&mdio3 { > + status = "okay"; > + > + ethphy3: ethernet-phy@0 { > + compatible = "ethernet-phy-ieee802.3-c22"; > + reg = <0>; > + }; > +}; > + > +&mac0 { > + status = "disabled"; > +}; > + > +&mac1 { > + status = "disabled"; > +}; > + > +//for X4TF > +&mac2 { > + status = "okay"; > + phy-mode = "rmii"; > + #phy-handle = <ðphy2>; > + use-ncsi; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_rmii3_default>; > +}; > + > + Ditto > +&mac3 { > + status = "okay"; > + phy-mode = "rmii"; > + #phy-handle = <ðphy3>; > + use-ncsi; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_rmii4_default>; > +}; > + > + Ditto > +&syscon { > + uart-clock-high-speed; That's a syscon property? It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). > + status = "okay"; > +}; > + > +&rtc { > + status = "disabled"; > +}; > + > +&fmc { > + status = "okay"; > + flash@0 { > + status = "okay"; Drop. Everywhere where it is not needed. > + m25p,fast-read; > + label = "bmc-spi"; > + spi-max-frequency = <50000000>; > + spi-bus-width = <1>; > + partitions { > + compatible = "fixed-partitions"; > + #address-cells = <1>; > + #size-cells = <1>; > + bmc@0 { > + label = "bmc"; > + reg = <0x0 0x4000000>; > + }; > + u-boot@0 { > + label = "u-boot"; > + reg = <0x0 0x200000>; > + }; > + u-boot-env@1f0000 { > + label = "u-boot-env"; > + reg = <0x1f0000 0x10000>; > + }; > + kernel@200000 { > + label = "kernel"; > + reg = <0x200000 0xc00000>; > + }; > + rofs@a00000 { > + label = "rofs"; > + reg = <0xa00000 0x2a00000>; > + }; > + rwfs@2a00000 { > + label = "rwfs"; > + reg = <0x2a00000 0x43f0000>; > + }; > + }; > + }; > +}; > + > +&spi1 { > + status = "okay"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_spi1_default>; > + > + flash@0 { > + status = "okay"; > + w25q256,fast-read; > + label = "bios-spi"; > + spi-max-frequency = <50000000>; > + partitions { > + compatible = "fixed-partitions"; > + #address-cells = <1>; > + #size-cells = <1>; > + biosfullimg@0 { > + reg = <0x0 0x2000000>; // 32768 *1024 = 0x2000000, 32 MB > + label = "biosfullimg"; > + }; > + }; > + }; > +}; > + > +&i2c0 { > + status = "okay"; > +}; > + > +&i2c1 { > + status = "okay"; > +}; > + > +&i2c2 { > + status = "okay"; > +}; > + > +&i2c3 { > + status = "okay"; > +}; > + > +&i2c4 { > + status = "okay"; > + tmp75@48 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "ti,tmp75"; > + reg = <0x48>; > + }; > + tmp75@49 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation I am going to skip the rest. Please fix all the trivialities and test your patches with tools. Best regards, Krzysztof
On 30/01/2024 09:56, Kelly Hung wrote: > From: kelly1732000 <Kelly_Hung@asus.com> > > This initial device-tree provides the necessary configuration for > basic BMC functionality and work on ASUS X4TF production. > > Signed-off-by: kelly1732000 <Kelly_Hung@asus.com> Please use full name. Best regards, Krzysztof
Hi, Krzysztof, Thanks for review, I notice there were a lot of errors. I've corrected them and done a schema and patch check, then resend the patch on 2/22. Best Regards Kelly -----Original Message----- From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Sent: Tuesday, January 30, 2024 6:39 PM To: Kelly Hung <ppighouse@gmail.com>; robh+dt@kernel.org Cc: krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; joel@jms.id.au; andrew@codeconstruct.com.au; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-aspeed@lists.ozlabs.org; linux-kernel@vger.kernel.org; Kelly Hung(洪嘉莉) <Kelly_Hung@asus.com> Subject: Re: [PATCH] ARM: dts: aspeed: asus: Add ASUS X4TF BMC External email : Ensure your email is secure before opening links and attachments. On 30/01/2024 09:56, Kelly Hung wrote: > From: kelly1732000 <Kelly_Hung@asus.com> > > This initial device-tree provides the necessary configuration for > basic BMC functionality and work on ASUS X4TF production. > > Signed-off-by: kelly1732000 <Kelly_Hung@asus.com> > --- > .../boot/dts/aspeed/aspeed-bmc-asus-x4tf.dts | 828 > ++++++++++++++++++ > 1 file changed, 828 insertions(+) > create mode 100644 arch/arm/boot/dts/aspeed/aspeed-bmc-asus-x4tf.dts It's impossible to test this file. You miss Makefile. > > diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asus-x4tf.dts > b/arch/arm/boot/dts/aspeed/aspeed-bmc-asus-x4tf.dts > new file mode 100644 > index 000000000000..723bbb33137f > --- /dev/null > +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asus-x4tf.dts > @@ -0,0 +1,828 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later // Copyright 2023 ASUS > +Corp. > +/dts-v1/; > + > +#include "aspeed-g6.dtsi" > +#include <dt-bindings/gpio/aspeed-gpio.h> #include > +<dt-bindings/i2c/i2c.h> > + > + > +/ { > + model = "ASUS-X4TF"; > + compatible = "asus,x4tf", "aspeed,ast2600"; > + > + chosen { > + stdout-path = &uart5; > + bootargs = "console=ttyS4,115200n8 earlycon"; earlycon is a debugging feature, not mainline. Also, move console to stdout-path, so entire bootargs can be dropped. > + }; > + > + memory@80000000 { > + device_type = "memory"; > + reg = <0x80000000 0x40000000>; > + }; > + > + reserved-memory { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + gfx_memory: framebuffer { > + size = <0x01000000>; > + alignment = <0x01000000>; > + compatible = "shared-dma-pool"; > + reusable; > + }; > + > + video_engine_memory: video { > + size = <0x04000000>; > + alignment = <0x01000000>; > + compatible = "shared-dma-pool"; > + reusable; > + }; > + > + rvas_memory: rvas { > + size = <0x04000000>; > + alignment = <0x01000000>; > + compatible = "shared-dma-pool"; > + reusable; > + }; > + > + bmc_dev_memory: bmc_dev_memory { No underscores in node names. This applies everywhere. > + size = <0x00100000>; > + alignment = <0x00100000>; > + compatible = "shared-dma-pool"; > + no-map; > + }; > + > + pci_msi_memory: pci_msi_memory { > + no-map; > + reg = <0x9e770000 0x100>; > + compatible = "shared-dma-pool"; > + }; > + /* 1GB memory */ > + vga_memory: region@bf000000 { > + no-map; > + compatible = "shared-dma-pool"; > + reg = <0xbf000000 0x01000000>; /* 16M */ > + }; > + ssp_memory: ssp_memory { > + size = <0x02000000>; > + alignment = <0x01000000>; > + compatible = "shared-dma-pool"; > + no-map; > + }; > + }; > + > + vcc_sdhci0: regulator-vcc-sdhci0 { > + compatible = "regulator-fixed"; > + status = "disabled"; Why? It makes it entirely pointless. > + regulator-name = "SDHCI0 Vcc"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + gpios = <&gpio0 ASPEED_GPIO(V, 0) GPIO_ACTIVE_HIGH>; > + enable-active-high; > + }; > + > + vccq_sdhci0: regulator-vccq-sdhci0 { > + compatible = "regulator-gpio"; > + status = "disabled"; Pointless node. > + regulator-name = "SDHCI0 VccQ"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + gpios = <&gpio0 ASPEED_GPIO(V, 1) GPIO_ACTIVE_HIGH>; > + gpios-states = <1>; > + states = <3300000 1>, > + <1800000 0>; > + }; > + > + vcc_sdhci1: regulator-vcc-sdhci1 { > + compatible = "regulator-fixed"; > + status = "disabled"; Pointless node. > + regulator-name = "SDHCI1 Vcc"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + gpios = <&gpio0 ASPEED_GPIO(V, 2) GPIO_ACTIVE_HIGH>; > + enable-active-high; > + }; > + > + vccq_sdhci1: regulator-vccq-sdhci1 { > + compatible = "regulator-gpio"; > + status = "disabled"; Pointless node. > + regulator-name = "SDHCI1 VccQ"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + gpios = <&gpio0 ASPEED_GPIO(V, 3) GPIO_ACTIVE_HIGH>; > + gpios-states = <1>; > + states = <3300000 1>, > + <1800000 0>; > + }; Missing blank line. > + iio-hwmon { > + compatible = "iio-hwmon"; > + io-channels = <&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>, <&adc1 7>; > + }; > +> + video-engine@1e700000 { Wrong placement. Also tools would tell you that. It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). > + compatible = "aspeed,ast2600-video-engine"; > + reg = <0x1e700000 0x20000>; > + clocks = <&syscon ASPEED_CLK_GATE_VCLK>, <&syscon ASPEED_CLK_GATE_ECLK>; > + clock-names = "vclk", "eclk"; > + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>; > + memory-region = <&video_engine_memory>; > + //resets = <&syscon ASPEED_RESET_VIDEO>; Dead code. > + }; > + > + leds { > + compatible = "gpio-leds"; > + > + heartbeat { It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). > + gpios = <&gpio0 ASPEED_GPIO(P, 7) GPIO_ACTIVE_LOW>; > + linux,default-trigger = "heartbeat"; > + }; > + uid_led { It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). > + gpios = <&gpio0 ASPEED_GPIO(P, 1) (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>; > + default-state = "off"; > + }; Missing blank line, everywhere. > + status_Y { > + gpios = <&gpio1 ASPEED_GPIO(B, 1) GPIO_ACTIVE_LOW>; > + default-state = "off"; > + }; > + sys_boot_status { > + gpios = <&gpio1 ASPEED_GPIO(B, 0) GPIO_ACTIVE_LOW>; > + default-state = "off"; > + }; > + }; > + spigpio { It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). > + status = "okay"; Why? > + compatible = "spi-gpio"; > + #address-cells = <1>; > + #size-cells = <0>; > + > + sck-gpios = <&gpio0 ASPEED_GPIO(Z, 3) GPIO_ACTIVE_HIGH>; > + mosi-gpios = <&gpio0 ASPEED_GPIO(Z, 4) GPIO_ACTIVE_HIGH>; > + miso-gpios = <&gpio0 ASPEED_GPIO(Z, 5) GPIO_ACTIVE_HIGH>; > + num-chipselects = <1>; > + cs-gpios = <&gpio0 ASPEED_GPIO(Z, 0) GPIO_ACTIVE_HIGH>; > + }; > + Redundant blank line. > +}; > + > +&adc0 { > + vref = <2500>; > + status = "okay"; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&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 { > + vref = <2500>; > + status = "okay"; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_adc8_default &pinctrl_adc9_default > + &pinctrl_adc10_default &pinctrl_adc11_default > + &pinctrl_adc12_default &pinctrl_adc13_default > + &pinctrl_adc14_default &pinctrl_adc15_default>; }; > + > +&peci0 { > + status = "okay"; > +}; > + > +&lpc_snoop { > + snoop-ports = <0x80>; > + status = "okay"; > +}; > + > + Redundant blank line. Clean this code before sending upstream from such trivialities. > +&mdio0 { > + status = "disabled"; > +}; > + > + > +&mdio1 { > + status = "disabled"; > +}; > + > +&mdio2 { > + status = "okay"; > + > + ethphy2: ethernet-phy@0 { > + compatible = "ethernet-phy-ieee802.3-c22"; > + reg = <0>; > + }; > +}; > + > +&mdio3 { > + status = "okay"; > + > + ethphy3: ethernet-phy@0 { > + compatible = "ethernet-phy-ieee802.3-c22"; > + reg = <0>; > + }; > +}; > + > +&mac0 { > + status = "disabled"; > +}; > + > +&mac1 { > + status = "disabled"; > +}; > + > +//for X4TF > +&mac2 { > + status = "okay"; > + phy-mode = "rmii"; > + #phy-handle = <ðphy2>; > + use-ncsi; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_rmii3_default>; }; > + > + Ditto > +&mac3 { > + status = "okay"; > + phy-mode = "rmii"; > + #phy-handle = <ðphy3>; > + use-ncsi; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_rmii4_default>; }; > + > + Ditto > +&syscon { > + uart-clock-high-speed; That's a syscon property? It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). > + status = "okay"; > +}; > + > +&rtc { > + status = "disabled"; > +}; > + > +&fmc { > + status = "okay"; > + flash@0 { > + status = "okay"; Drop. Everywhere where it is not needed. > + m25p,fast-read; > + label = "bmc-spi"; > + spi-max-frequency = <50000000>; > + spi-bus-width = <1>; > + partitions { > + compatible = "fixed-partitions"; > + #address-cells = <1>; > + #size-cells = <1>; > + bmc@0 { > + label = "bmc"; > + reg = <0x0 0x4000000>; > + }; > + u-boot@0 { > + label = "u-boot"; > + reg = <0x0 0x200000>; > + }; > + u-boot-env@1f0000 { > + label = "u-boot-env"; > + reg = <0x1f0000 0x10000>; > + }; > + kernel@200000 { > + label = "kernel"; > + reg = <0x200000 0xc00000>; > + }; > + rofs@a00000 { > + label = "rofs"; > + reg = <0xa00000 0x2a00000>; > + }; > + rwfs@2a00000 { > + label = "rwfs"; > + reg = <0x2a00000 0x43f0000>; > + }; > + }; > + }; > +}; > + > +&spi1 { > + status = "okay"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_spi1_default>; > + > + flash@0 { > + status = "okay"; > + w25q256,fast-read; > + label = "bios-spi"; > + spi-max-frequency = <50000000>; > + partitions { > + compatible = "fixed-partitions"; > + #address-cells = <1>; > + #size-cells = <1>; > + biosfullimg@0 { > + reg = <0x0 0x2000000>; // 32768 *1024 = 0x2000000, 32 MB > + label = "biosfullimg"; > + }; > + }; > + }; > +}; > + > +&i2c0 { > + status = "okay"; > +}; > + > +&i2c1 { > + status = "okay"; > +}; > + > +&i2c2 { > + status = "okay"; > +}; > + > +&i2c3 { > + status = "okay"; > +}; > + > +&i2c4 { > + status = "okay"; > + tmp75@48 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "ti,tmp75"; > + reg = <0x48>; > + }; > + tmp75@49 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation I am going to skip the rest. Please fix all the trivialities and test your patches with tools. Best regards, Krzysztof <p></p> =================================================================================================================================== This email and any attachments to it contain confidential information and are intended solely for the use of the individual to whom it is addressed. If you are not the intended recipient or receive it accidentally, please immediately notify the sender by e-mail and delete the message and any attachments from your computer system, and destroy all hard copies. Please be advised that any unauthorized disclosure, copying, distribution or any action taken or omitted in reliance on this, is illegal and prohibited. Any views or opinions expressed are solely those of the author and do not represent those of ASUSTeK. For pricing information, ASUS is only entitled to set a recommendation resale price. All customers are free to set their own price as they wish. ===================================================================================================================================
On 04/03/2024 11:45, Kelly Hung(洪嘉莉) wrote: > Hi, Krzysztof, > > Thanks for review, I notice there were a lot of errors. I've corrected them and done a schema and patch check, then resend the patch on 2/22. And what happened with my other messages? Why you were not replying to them? Why only some parts were implemented? Best regards, Krzysztof
Hi, Krzysztof, Please see my response below. Thanks a log. Best Regards Kelly -----Original Message----- From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Sent: Tuesday, January 30, 2024 6:39 PM To: Kelly Hung <ppighouse@gmail.com>; robh+dt@kernel.org Cc: krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; joel@jms.id.au; andrew@codeconstruct.com.au; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-aspeed@lists.ozlabs.org; linux-kernel@vger.kernel.org; Kelly Hung(洪嘉莉) <Kelly_Hung@asus.com> Subject: Re: [PATCH] ARM: dts: aspeed: asus: Add ASUS X4TF BMC External email : Ensure your email is secure before opening links and attachments. On 30/01/2024 09:56, Kelly Hung wrote: > From: kelly1732000 <Kelly_Hung@asus.com> > > This initial device-tree provides the necessary configuration for > basic BMC functionality and work on ASUS X4TF production. > > Signed-off-by: kelly1732000 <Kelly_Hung@asus.com> > --- > .../boot/dts/aspeed/aspeed-bmc-asus-x4tf.dts | 828 > ++++++++++++++++++ > 1 file changed, 828 insertions(+) > create mode 100644 arch/arm/boot/dts/aspeed/aspeed-bmc-asus-x4tf.dts It's impossible to test this file. You miss Makefile. Kelly: I've add aspeed-bmc-asus-x4tf.dtb in Makefle. > > diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asus-x4tf.dts > b/arch/arm/boot/dts/aspeed/aspeed-bmc-asus-x4tf.dts > new file mode 100644 > index 000000000000..723bbb33137f > --- /dev/null > +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asus-x4tf.dts > @@ -0,0 +1,828 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later // Copyright 2023 ASUS > +Corp. > +/dts-v1/; > + > +#include "aspeed-g6.dtsi" > +#include <dt-bindings/gpio/aspeed-gpio.h> #include > +<dt-bindings/i2c/i2c.h> > + > + > +/ { > + model = "ASUS-X4TF"; > + compatible = "asus,x4tf", "aspeed,ast2600"; > + > + chosen { > + stdout-path = &uart5; > + bootargs = "console=ttyS4,115200n8 earlycon"; earlycon is a debugging feature, not mainline. Also, move console to stdout-path, so entire bootargs can be dropped. Kelly: I've moved "bootargs" into "stdout-path" and resend my patch. > + }; > + > + memory@80000000 { > + device_type = "memory"; > + reg = <0x80000000 0x40000000>; > + }; > + > + reserved-memory { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + gfx_memory: framebuffer { > + size = <0x01000000>; > + alignment = <0x01000000>; > + compatible = "shared-dma-pool"; > + reusable; > + }; > + > + video_engine_memory: video { > + size = <0x04000000>; > + alignment = <0x01000000>; > + compatible = "shared-dma-pool"; > + reusable; > + }; > + > + rvas_memory: rvas { > + size = <0x04000000>; > + alignment = <0x01000000>; > + compatible = "shared-dma-pool"; > + reusable; > + }; > + > + bmc_dev_memory: bmc_dev_memory { No underscores in node names. This applies everywhere. Kelly: Yes, thanks for reminding, I've removed it and resent my patch. > + size = <0x00100000>; > + alignment = <0x00100000>; > + compatible = "shared-dma-pool"; > + no-map; > + }; > + > + pci_msi_memory: pci_msi_memory { > + no-map; > + reg = <0x9e770000 0x100>; > + compatible = "shared-dma-pool"; > + }; > + /* 1GB memory */ > + vga_memory: region@bf000000 { > + no-map; > + compatible = "shared-dma-pool"; > + reg = <0xbf000000 0x01000000>; /* 16M */ > + }; > + ssp_memory: ssp_memory { > + size = <0x02000000>; > + alignment = <0x01000000>; > + compatible = "shared-dma-pool"; > + no-map; > + }; > + }; > + > + vcc_sdhci0: regulator-vcc-sdhci0 { > + compatible = "regulator-fixed"; > + status = "disabled"; Why? It makes it entirely pointless. Kelly: I've removed it and loaded into our X4TF platform and it boot into BMC console. I have resent my patch. > + regulator-name = "SDHCI0 Vcc"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + gpios = <&gpio0 ASPEED_GPIO(V, 0) GPIO_ACTIVE_HIGH>; > + enable-active-high; > + }; > + > + vccq_sdhci0: regulator-vccq-sdhci0 { > + compatible = "regulator-gpio"; > + status = "disabled"; Pointless node. Kelly: I've removed it and loaded into our X4TF platform and it boot into BMC console. I have resent my patch. > + regulator-name = "SDHCI0 VccQ"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + gpios = <&gpio0 ASPEED_GPIO(V, 1) GPIO_ACTIVE_HIGH>; > + gpios-states = <1>; > + states = <3300000 1>, > + <1800000 0>; > + }; > + > + vcc_sdhci1: regulator-vcc-sdhci1 { > + compatible = "regulator-fixed"; > + status = "disabled"; Pointless node. Kelly: I've removed it and loaded into our X4TF platform and it boot into BMC console. I have resent my patch. > + regulator-name = "SDHCI1 Vcc"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + gpios = <&gpio0 ASPEED_GPIO(V, 2) GPIO_ACTIVE_HIGH>; > + enable-active-high; > + }; > + > + vccq_sdhci1: regulator-vccq-sdhci1 { > + compatible = "regulator-gpio"; > + status = "disabled"; Pointless node. Kelly: I've removed it and loaded into our X4TF platform and it boot into BMC console. I have resent my patch. > + regulator-name = "SDHCI1 VccQ"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + gpios = <&gpio0 ASPEED_GPIO(V, 3) GPIO_ACTIVE_HIGH>; > + gpios-states = <1>; > + states = <3300000 1>, > + <1800000 0>; > + }; Missing blank line. Kelly: I've review our dts and add blank line or remove redundant blank lines. > + iio-hwmon { > + compatible = "iio-hwmon"; > + io-channels = <&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>, <&adc1 7>; > + }; > +> + video-engine@1e700000 { Wrong placement. Also tools would tell you that. It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). Kelly: I've checked our dts, removed unnecessary parts and run the DTS check script. > + compatible = "aspeed,ast2600-video-engine"; > + reg = <0x1e700000 0x20000>; > + clocks = <&syscon ASPEED_CLK_GATE_VCLK>, <&syscon ASPEED_CLK_GATE_ECLK>; > + clock-names = "vclk", "eclk"; > + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>; > + memory-region = <&video_engine_memory>; > + //resets = <&syscon ASPEED_RESET_VIDEO>; Dead code. Kelly: I have removed this part and resent the patch. > + }; > + > + leds { > + compatible = "gpio-leds"; > + > + heartbeat { It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). Kelly: I added led- for all LED configurations. Heartbeat will be led-heartbeat. > + gpios = <&gpio0 ASPEED_GPIO(P, 7) GPIO_ACTIVE_LOW>; > + linux,default-trigger = "heartbeat"; > + }; > + uid_led { It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). Kelly: I added led- for all LED configurations. > + gpios = <&gpio0 ASPEED_GPIO(P, 1) (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>; > + default-state = "off"; > + }; Missing blank line, everywhere. Kelly: I've review our dts and add blank line or remove redundant blank lines. > + status_Y { > + gpios = <&gpio1 ASPEED_GPIO(B, 1) GPIO_ACTIVE_LOW>; > + default-state = "off"; > + }; > + sys_boot_status { > + gpios = <&gpio1 ASPEED_GPIO(B, 0) GPIO_ACTIVE_LOW>; > + default-state = "off"; > + }; > + }; > + spigpio { It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). > + status = "okay"; Why? Kelly: I've removed spigpio and tested on our X4TF platform and it boots to the BMC console. I've resent our new patch. > + compatible = "spi-gpio"; > + #address-cells = <1>; > + #size-cells = <0>; > + > + sck-gpios = <&gpio0 ASPEED_GPIO(Z, 3) GPIO_ACTIVE_HIGH>; > + mosi-gpios = <&gpio0 ASPEED_GPIO(Z, 4) GPIO_ACTIVE_HIGH>; > + miso-gpios = <&gpio0 ASPEED_GPIO(Z, 5) GPIO_ACTIVE_HIGH>; > + num-chipselects = <1>; > + cs-gpios = <&gpio0 ASPEED_GPIO(Z, 0) GPIO_ACTIVE_HIGH>; > + }; > + Redundant blank line. Kelly: I've review our dts and add blank line or remove redundant blank lines. > +}; > + > +&adc0 { > + vref = <2500>; > + status = "okay"; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&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 { > + vref = <2500>; > + status = "okay"; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_adc8_default &pinctrl_adc9_default > + &pinctrl_adc10_default &pinctrl_adc11_default > + &pinctrl_adc12_default &pinctrl_adc13_default > + &pinctrl_adc14_default &pinctrl_adc15_default>; }; > + > +&peci0 { > + status = "okay"; > +}; > + > +&lpc_snoop { > + snoop-ports = <0x80>; > + status = "okay"; > +}; > + > + Redundant blank line. Clean this code before sending upstream from such trivialities. Kelly: I've review our dts and add blank line or remove redundant blank lines. > +&mdio0 { > + status = "disabled"; > +}; > + > + > +&mdio1 { > + status = "disabled"; > +}; > + > +&mdio2 { > + status = "okay"; > + > + ethphy2: ethernet-phy@0 { > + compatible = "ethernet-phy-ieee802.3-c22"; > + reg = <0>; > + }; > +}; > + > +&mdio3 { > + status = "okay"; > + > + ethphy3: ethernet-phy@0 { > + compatible = "ethernet-phy-ieee802.3-c22"; > + reg = <0>; > + }; > +}; > + > +&mac0 { > + status = "disabled"; > +}; > + > +&mac1 { > + status = "disabled"; > +}; > + > +//for X4TF > +&mac2 { > + status = "okay"; > + phy-mode = "rmii"; > + #phy-handle = <ðphy2>; > + use-ncsi; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_rmii3_default>; }; > + > + Ditto Kelly: I've review our dts and add blank line or remove redundant blank lines. > +&mac3 { > + status = "okay"; > + phy-mode = "rmii"; > + #phy-handle = <ðphy3>; > + use-ncsi; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_rmii4_default>; }; > + > + Ditto Kelly: I've review our dts and add blank line or remove redundant blank lines. > +&syscon { > + uart-clock-high-speed; That's a syscon property? It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). Kelly: I've removed syscon and tested on our X4TF platform and it boots to the BMC console. I've resent our new patch. > + status = "okay"; > +}; > + > +&rtc { > + status = "disabled"; > +}; > + > +&fmc { > + status = "okay"; > + flash@0 { > + status = "okay"; Drop. Everywhere where it is not needed. Kelly: I've adjusted fmc config and resent the patch. > + m25p,fast-read; > + label = "bmc-spi"; > + spi-max-frequency = <50000000>; > + spi-bus-width = <1>; > + partitions { > + compatible = "fixed-partitions"; > + #address-cells = <1>; > + #size-cells = <1>; > + bmc@0 { > + label = "bmc"; > + reg = <0x0 0x4000000>; > + }; > + u-boot@0 { > + label = "u-boot"; > + reg = <0x0 0x200000>; > + }; > + u-boot-env@1f0000 { > + label = "u-boot-env"; > + reg = <0x1f0000 0x10000>; > + }; > + kernel@200000 { > + label = "kernel"; > + reg = <0x200000 0xc00000>; > + }; > + rofs@a00000 { > + label = "rofs"; > + reg = <0xa00000 0x2a00000>; > + }; > + rwfs@2a00000 { > + label = "rwfs"; > + reg = <0x2a00000 0x43f0000>; > + }; > + }; > + }; > +}; > + > +&spi1 { > + status = "okay"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_spi1_default>; > + > + flash@0 { > + status = "okay"; > + w25q256,fast-read; > + label = "bios-spi"; > + spi-max-frequency = <50000000>; > + partitions { > + compatible = "fixed-partitions"; > + #address-cells = <1>; > + #size-cells = <1>; > + biosfullimg@0 { > + reg = <0x0 0x2000000>; // 32768 *1024 = 0x2000000, 32 MB > + label = "biosfullimg"; > + }; > + }; > + }; > +}; > + > +&i2c0 { > + status = "okay"; > +}; > + > +&i2c1 { > + status = "okay"; > +}; > + > +&i2c2 { > + status = "okay"; > +}; > + > +&i2c3 { > + status = "okay"; > +}; > + > +&i2c4 { > + status = "okay"; > + tmp75@48 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "ti,tmp75"; > + reg = <0x48>; > + }; > + tmp75@49 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation I am going to skip the rest. Please fix all the trivialities and test your patches with tools. Kelly: I have renamed tmp75 as temperature-sensor. I've checked our dts, removed unnecessary parts and run the DTS check script. Best regards, Krzysztof <p></p> =================================================================================================================================== This email and any attachments to it contain confidential information and are intended solely for the use of the individual to whom it is addressed. If you are not the intended recipient or receive it accidentally, please immediately notify the sender by e-mail and delete the message and any attachments from your computer system, and destroy all hard copies. Please be advised that any unauthorized disclosure, copying, distribution or any action taken or omitted in reliance on this, is illegal and prohibited. Any views or opinions expressed are solely those of the author and do not represent those of ASUSTeK. For pricing information, ASUS is only entitled to set a recommendation resale price. All customers are free to set their own price as they wish. ===================================================================================================================================
Hi, Krzysztof, I'll use the full name in future patches. Thanks for reminding. Best Regards Kelly -----Original Message----- From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Sent: Tuesday, January 30, 2024 6:40 PM To: Kelly Hung <ppighouse@gmail.com>; robh+dt@kernel.org Cc: krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; joel@jms.id.au; andrew@codeconstruct.com.au; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-aspeed@lists.ozlabs.org; linux-kernel@vger.kernel.org; Kelly Hung(洪嘉莉) <Kelly_Hung@asus.com> Subject: Re: [PATCH] ARM: dts: aspeed: asus: Add ASUS X4TF BMC External email : Ensure your email is secure before opening links and attachments. On 30/01/2024 09:56, Kelly Hung wrote: > From: kelly1732000 <Kelly_Hung@asus.com> > > This initial device-tree provides the necessary configuration for > basic BMC functionality and work on ASUS X4TF production. > > Signed-off-by: kelly1732000 <Kelly_Hung@asus.com> Please use full name. Best regards, Krzysztof <p></p> =================================================================================================================================== This email and any attachments to it contain confidential information and are intended solely for the use of the individual to whom it is addressed. If you are not the intended recipient or receive it accidentally, please immediately notify the sender by e-mail and delete the message and any attachments from your computer system, and destroy all hard copies. Please be advised that any unauthorized disclosure, copying, distribution or any action taken or omitted in reliance on this, is illegal and prohibited. Any views or opinions expressed are solely those of the author and do not represent those of ASUSTeK. For pricing information, ASUS is only entitled to set a recommendation resale price. All customers are free to set their own price as they wish. ===================================================================================================================================
Hi, Krzysztof, Thanks for reminding. I've finished and corrected my theme prefix. Best Regards Kelly -----Original Message----- From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Sent: Tuesday, January 30, 2024 6:35 PM To: Kelly Hung <ppighouse@gmail.com>; robh+dt@kernel.org Cc: krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; joel@jms.id.au; andrew@codeconstruct.com.au; devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-aspeed@lists.ozlabs.org; linux-kernel@vger.kernel.org; Kelly Hung(洪嘉莉) <Kelly_Hung@asus.com> Subject: Re: [PATCH] ARM: dts: aspeed: asus: Add ASUS X4TF BMC External email : Ensure your email is secure before opening links and attachments. On 30/01/2024 09:56, Kelly Hung wrote: > This initial device-tree provides the necessary configuration for > basic BMC functionality and work on ASUS X4TF production. > > Signed-off-by: Kelly Hung <Kelly_Hung@asus.com> Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. Best regards, Krzysztof <p></p> =================================================================================================================================== This email and any attachments to it contain confidential information and are intended solely for the use of the individual to whom it is addressed. If you are not the intended recipient or receive it accidentally, please immediately notify the sender by e-mail and delete the message and any attachments from your computer system, and destroy all hard copies. Please be advised that any unauthorized disclosure, copying, distribution or any action taken or omitted in reliance on this, is illegal and prohibited. Any views or opinions expressed are solely those of the author and do not represent those of ASUSTeK. For pricing information, ASUS is only entitled to set a recommendation resale price. All customers are free to set their own price as they wish. ===================================================================================================================================
diff --git a/Documentation/devicetree/bindings/arm/aspeed/aspeed.yaml b/Documentation/devicetree/bindings/arm/aspeed/aspeed.yaml index 749ee54a3ff8..80009948e14a 100644 --- a/Documentation/devicetree/bindings/arm/aspeed/aspeed.yaml +++ b/Documentation/devicetree/bindings/arm/aspeed/aspeed.yaml @@ -74,6 +74,7 @@ properties: - ampere,mtmitchell-bmc - aspeed,ast2600-evb - aspeed,ast2600-evb-a1 + - asus,x4tf - facebook,bletchley-bmc - facebook,cloudripper-bmc - facebook,elbert-bmc
This initial device-tree provides the necessary configuration for basic BMC functionality and work on ASUS X4TF production. Signed-off-by: Kelly Hung <Kelly_Hung@asus.com> --- Documentation/devicetree/bindings/arm/aspeed/aspeed.yaml | 1 + 1 file changed, 1 insertion(+)