Re: [PATCH] ARM: dts: aspeed-g6: add pcie-lpc and pcie-kcs4

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The newly added `pcie_lpc` node's memory region overlaps completely with the upper half of the existing `lpc` node's memory region.
- [Low] The nodes `pcie_lpc` and `pcie_kcs4` use specific, non-generic node names (`pcie-lpc` and `pcie-kcs`), violating Devicetree specification rules.
--

--- Patch [1]: [PATCH] ARM: dts: aspeed-g6: add pcie-lpc and pcie-kcs4 ---
commit 2d0eb7dfc73068ee32b00e22cfaac3679c6e6dbd
Author: Grégoire Layet <gregoire.layet@9elements.com>

ARM: dts: aspeed-g6: add pcie-lpc and pcie-kcs4

This commit adds a pcie_lpc node and a pcie_kcs4 child node to the ast2600
common dtsi. It allows exposing a fully KCS-compatible interface over PCIe,
which can be used by the host for IPMI.

> diff --git a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> index 56bb3b0444f7e..ac351f01048fe 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> +++ b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi

[ ... ]

> @@ -658,6 +658,21 @@ ibt: ibt@140 {
>  				};
>  			};
>  
> +			pcie_lpc: pcie-lpc@1e789800 {

[Severity: Low]
Is this node name aligned with Devicetree naming specifications?

Using "pcie-lpc" describes the specific bus connection rather than a generic
functional name like "lpc".

> +				compatible = "aspeed,ast2600-lpc-v2", "simple-mfd", "syscon";
> +				reg = <0x1e789800 0x800>;

[Severity: Medium]
Will this reg value cause a resource reservation conflict?

The existing lpc@1e789000 node is defined earlier in this file with a size of
0x1000:

			lpc: lpc@1e789000 {
				compatible = "aspeed,ast2600-lpc-v2", "simple-mfd", "syscon";
				reg = <0x1e789000 0x1000>;

This means the existing lpc node spans up to 0x1e78a000, which completely
overlaps the 0x1e789800 through 0x1e789fff memory range claimed by this
newly added node.

Could the original node's size be reduced to 0x800 to prevent the overlap?

> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +				ranges = <0x0 0x1e789800 0x800>;
> +
> +				pcie_kcs4: pcie-kcs@114 {

[Severity: Low]
Does this child node also need a more generic node name like "kcs@114"
instead of "pcie-kcs@114"?

> +					compatible = "aspeed,ast2600-kcs-bmc";
> +					reg = <0x114 0x1>, <0x118 0x1>, <0x11c 0x1>;
> +					interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>;
> +					status = "disabled";
> +				};
> +			};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/de49f2032f5b1f170161dcd6191c0daa48f0b0c2.1784117715.git.gregoire.layet@9elements.com?part=1