Re: [PATCH v3 12/14] ARM: dts: aspeed: Add IBM P11 FSI devices

[Krzysztof Kozlowski <krzk@xxxxxxxxxx>]

On 25/04/2024 23:36, Eddie James wrote:
> Add the P11 FSI device tree for use in upcoming BMC systems.
> Unlike P10, there is no system with only two processors, so
> only the quad processor FSI layout is necessary.
> 
> Signed-off-by: Eddie James <eajames@xxxxxxxxxxxxx>
> ---
>  .../arm/boot/dts/aspeed/ibm-power11-quad.dtsi | 1696 +++++++++++++++++
>  1 file changed, 1696 insertions(+)
>  create mode 100644 arch/arm/boot/dts/aspeed/ibm-power11-quad.dtsi
> 
> diff --git a/arch/arm/boot/dts/aspeed/ibm-power11-quad.dtsi b/arch/arm/boot/dts/aspeed/ibm-power11-quad.dtsi
> new file mode 100644
> index 000000000000..c3a0ecf12aa0
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed/ibm-power11-quad.dtsi
> @@ -0,0 +1,1696 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// Copyright 2024 IBM Corp.
> +
> +&fsim0 {

This does not make sense. You do not include any file here, so what do
you want to override?

How can you even test this file?

> +	status = "okay";
> +
> +	#address-cells = <2>;
> +	#size-cells = <0>;
> +
> +	cfam-reset-gpios = <&gpio0 ASPEED_GPIO(Q, 0) GPIO_ACTIVE_HIGH>;
> +


> +
> +&cfam3_i2c16 {
> +	fsi-i2cr@20 {
> +		compatible = "ibm,i2cr-fsi-master";
> +		reg = <0x20>;
> +		#address-cells = <2>;
> +		#size-cells = <0>;
> +
> +		cfam@0,0 {
> +			reg = <0 0>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			chip-id = <0>;
> +
> +			scom416: scom@1000 {
> +				compatible = "ibm,i2cr-scom";
> +				reg = <0x1000 0x400>;
> +			};
> +
> +			sbefifo416: sbefifo@2400 {
> +				compatible = "ibm,odyssey-sbefifo";
> +				reg = <0x2400 0x400>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +			};
> +		};
> +	};
> +};
> +
> +&cfam3_i2c17 {

This looks randomly ordered.


> +&fsi_occ1 {
> +	reg = <2>;
> +};
> +
> +&fsi_occ2 {
> +	reg = <3>;
> +};
> +
> +&fsi_occ3 {
> +	reg = <4>;
> +};
> +
> +/ {

Nope. Root node never goes to end of file. Look at all modern DTS.

> +	aliases {
> +		i2c100 = &cfam0_i2c0;
> +		i2c101 = &cfam0_i2c1;
> +		i2c110 = &cfam0_i2c10;
> +		i2c111 = &cfam0_i2c11;
> +		i2c112 = &cfam0_i2c12;
> +		i2c113 = &cfam0_i2c13;
> +		i2c114 = &cfam0_i2c14;
> +		i2c115 = &cfam0_i2c15;
> +		i2c202 = &cfam1_i2c2;
> +		i2c203 = &cfam1_i2c3;
> +		i2c210 = &cfam1_i2c10;



Best regards,
Krzysztof