diff mbox series

[V2] arm64: tegra: add topology data for Tegra194 cpu

Message ID 1549928827-14006-1-git-send-email-byan@nvidia.com
State Deferred
Headers show
Series [V2] arm64: tegra: add topology data for Tegra194 cpu | expand

Commit Message

Bo Yan Feb. 11, 2019, 11:47 p.m. UTC
The xavier CPU architecture includes 8 CPU cores organized in
4 clusters. Add cpu-map data for topology initialization, this
fixes the topology information in
/sys/devices/system/cpu/cpu[n]/topology

Signed-off-by: Bo Yan <byan@nvidia.com>
---
V2: remove cache nodes, add topology data only

 arch/arm64/boot/dts/nvidia/tegra194.dtsi | 58 +++++++++++++++++++++++++++-----
 1 file changed, 50 insertions(+), 8 deletions(-)

Comments

Thierry Reding Feb. 13, 2019, 8:12 a.m. UTC | #1
On Mon, Feb 11, 2019 at 03:47:07PM -0800, Bo Yan wrote:
> The xavier CPU architecture includes 8 CPU cores organized in
> 4 clusters. Add cpu-map data for topology initialization, this
> fixes the topology information in
> /sys/devices/system/cpu/cpu[n]/topology
> 
> Signed-off-by: Bo Yan <byan@nvidia.com>
> ---
> V2: remove cache nodes, add topology data only
> 
>  arch/arm64/boot/dts/nvidia/tegra194.dtsi | 58 +++++++++++++++++++++++++++-----
>  1 file changed, 50 insertions(+), 8 deletions(-)

This mostly looks good to me. One minor comment below.

> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> index 6dfa1ca..35e6e76 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> @@ -870,56 +870,98 @@
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  
> -		cpu@0 {
> +		cpu-map {
> +			cluster0 {
> +				core0 {
> +					cpu = <&cl0_0>;

I wonder if perhaps in this case it would be better to use the full path
to refer to the phandle here. That way we can avoid the labels, which
are somewhat cumbersome to write and the hierarchy, in my opinion, is a
much more natural way to reference these.

What I'm suggesting would look roughly like this:

		cpu-map {
			cluster0 {
				core0 {
					cpu = <&{/cpus/cpu@0}>;
				};

				core1 {
					cpu = <&{/cpus/cpu@1}>;
				};
			};

			cluster1 {
				core0 {
					cpu = <&{/cpus/cpu@2}>;
				};

				core1 {
					cpu = <&{/cpus/cpu@3}>;
				};
			};

			...
		};

That's slightly more characters, but I think it's much easier to read
than the labels.

I don't feel very strongly about it, though, so feel free to keep this
as-is if you prefer.

Thierry
Bo Yan Feb. 13, 2019, 4:24 p.m. UTC | #2
I agree, will update this patch with full paths replacing phandles.

On 2/13/19 12:12 AM, Thierry Reding wrote:
> On Mon, Feb 11, 2019 at 03:47:07PM -0800, Bo Yan wrote:
>> The xavier CPU architecture includes 8 CPU cores organized in
>> 4 clusters. Add cpu-map data for topology initialization, this
>> fixes the topology information in
>> /sys/devices/system/cpu/cpu[n]/topology
>>
>> Signed-off-by: Bo Yan <byan@nvidia.com>
>> ---
>> V2: remove cache nodes, add topology data only
>>
>>   arch/arm64/boot/dts/nvidia/tegra194.dtsi | 58 +++++++++++++++++++++++++++-----
>>   1 file changed, 50 insertions(+), 8 deletions(-)
> 
> This mostly looks good to me. One minor comment below.
> 
>>
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> index 6dfa1ca..35e6e76 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> @@ -870,56 +870,98 @@
>>   		#address-cells = <1>;
>>   		#size-cells = <0>;
>>   
>> -		cpu@0 {
>> +		cpu-map {
>> +			cluster0 {
>> +				core0 {
>> +					cpu = <&cl0_0>;
> 
> I wonder if perhaps in this case it would be better to use the full path
> to refer to the phandle here. That way we can avoid the labels, which
> are somewhat cumbersome to write and the hierarchy, in my opinion, is a
> much more natural way to reference these.
> 
> What I'm suggesting would look roughly like this:
> 
> 		cpu-map {
> 			cluster0 {
> 				core0 {
> 					cpu = <&{/cpus/cpu@0}>;
> 				};
> 
> 				core1 {
> 					cpu = <&{/cpus/cpu@1}>;
> 				};
> 			};
> 
> 			cluster1 {
> 				core0 {
> 					cpu = <&{/cpus/cpu@2}>;
> 				};
> 
> 				core1 {
> 					cpu = <&{/cpus/cpu@3}>;
> 				};
> 			};
> 
> 			...
> 		};
> 
> That's slightly more characters, but I think it's much easier to read
> than the labels.
> 
> I don't feel very strongly about it, though, so feel free to keep this
> as-is if you prefer.
> 
> Thierry
>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index 6dfa1ca..35e6e76 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -870,56 +870,98 @@ 
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		cpu@0 {
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&cl0_0>;
+				};
+
+				core1 {
+					cpu = <&cl0_1>;
+				};
+			};
+
+			cluster1 {
+				core0 {
+					cpu = <&cl1_0>;
+				};
+
+				core1 {
+					cpu = <&cl1_1>;
+				};
+			};
+
+			cluster2 {
+				core0 {
+					cpu = <&cl2_0>;
+				};
+
+				core1 {
+					cpu = <&cl2_1>;
+				};
+			};
+
+			cluster3 {
+				core0 {
+					cpu = <&cl3_0>;
+				};
+
+				core1 {
+					cpu = <&cl3_1>;
+				};
+			};
+		};
+
+		cl0_0: cpu@0 {
 			compatible = "nvidia,tegra194-carmel", "arm,armv8";
 			device_type = "cpu";
 			reg = <0x10000>;
 			enable-method = "psci";
 		};
 
-		cpu@1 {
+		cl0_1: cpu@1 {
 			compatible = "nvidia,tegra194-carmel", "arm,armv8";
 			device_type = "cpu";
 			reg = <0x10001>;
 			enable-method = "psci";
 		};
 
-		cpu@2 {
+		cl1_0: cpu@2 {
 			compatible = "nvidia,tegra194-carmel", "arm,armv8";
 			device_type = "cpu";
 			reg = <0x100>;
 			enable-method = "psci";
 		};
 
-		cpu@3 {
+		cl1_1: cpu@3 {
 			compatible = "nvidia,tegra194-carmel", "arm,armv8";
 			device_type = "cpu";
 			reg = <0x101>;
 			enable-method = "psci";
 		};
 
-		cpu@4 {
+		cl2_0: cpu@4 {
 			compatible = "nvidia,tegra194-carmel", "arm,armv8";
 			device_type = "cpu";
 			reg = <0x200>;
 			enable-method = "psci";
 		};
 
-		cpu@5 {
+		cl2_1: cpu@5 {
 			compatible = "nvidia,tegra194-carmel", "arm,armv8";
 			device_type = "cpu";
 			reg = <0x201>;
 			enable-method = "psci";
 		};
 
-		cpu@6 {
+		cl3_0: cpu@6 {
 			compatible = "nvidia,tegra194-carmel", "arm,armv8";
 			device_type = "cpu";
 			reg = <0x10300>;
 			enable-method = "psci";
 		};
 
-		cpu@7 {
+		cl3_1: cpu@7 {
 			compatible = "nvidia,tegra194-carmel", "arm,armv8";
 			device_type = "cpu";
 			reg = <0x10301>;