* [PATCH] arm64: tegra: add topology data for Tegra194 cpu
@ 2019-01-31 18:35 Bo Yan
2019-01-31 22:25 ` Thierry Reding
0 siblings, 1 reply; 9+ messages in thread
From: Bo Yan @ 2019-01-31 18:35 UTC (permalink / raw)
To: thierry.reding, jonathanh, linux-tegra
Cc: mark.rutland, robh+dt, linux-kernel, Bo Yan
The xavier CPU architecture includes 8 CPU cores organized in
4 clusters. Add cpu-map data for topology initialization, add
cache data for cache node creation in sysfs.
Signed-off-by: Bo Yan <byan@nvidia.com>
---
arch/arm64/boot/dts/nvidia/tegra194.dtsi | 148 +++++++++++++++++++++++++++++--
1 file changed, 140 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index 6dfa1ca..7c2a1fb 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -870,63 +870,195 @@
#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";
+ i-cache-size = <131072>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <512>;
+ d-cache-size = <65536>;
+ d-cache-line-size = <64>;
+ d-cache_sets = <256>;
+ l2-cache = <&l2_0>;
};
- cpu@1 {
+ cl0_1: cpu@1 {
compatible = "nvidia,tegra194-carmel", "arm,armv8";
device_type = "cpu";
reg = <0x10001>;
enable-method = "psci";
+ i-cache-size = <131072>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <512>;
+ d-cache-size = <65536>;
+ d-cache-line-size = <64>;
+ d-cache_sets = <256>;
+ l2-cache = <&l2_0>;
};
- cpu@2 {
+ cl1_0: cpu@2 {
compatible = "nvidia,tegra194-carmel", "arm,armv8";
device_type = "cpu";
reg = <0x100>;
enable-method = "psci";
+ i-cache-size = <131072>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <512>;
+ d-cache-size = <65536>;
+ d-cache-line-size = <64>;
+ d-cache_sets = <256>;
+ l2-cache = <&l2_1>;
};
- cpu@3 {
+ cl1_1: cpu@3 {
compatible = "nvidia,tegra194-carmel", "arm,armv8";
device_type = "cpu";
reg = <0x101>;
enable-method = "psci";
+ i-cache-size = <131072>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <512>;
+ d-cache-size = <65536>;
+ d-cache-line-size = <64>;
+ d-cache_sets = <256>;
+ l2-cache = <&l2_1>;
};
- cpu@4 {
+ cl2_0: cpu@4 {
compatible = "nvidia,tegra194-carmel", "arm,armv8";
device_type = "cpu";
reg = <0x200>;
enable-method = "psci";
+ i-cache-size = <131072>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <512>;
+ d-cache-size = <65536>;
+ d-cache-line-size = <64>;
+ d-cache_sets = <256>;
+ l2-cache = <&l2_2>;
};
- cpu@5 {
+ cl2_1: cpu@5 {
compatible = "nvidia,tegra194-carmel", "arm,armv8";
device_type = "cpu";
reg = <0x201>;
enable-method = "psci";
+ i-cache-size = <131072>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <512>;
+ d-cache-size = <65536>;
+ d-cache-line-size = <64>;
+ d-cache_sets = <256>;
+ l2-cache = <&l2_2>;
};
- cpu@6 {
+ cl3_0: cpu@6 {
compatible = "nvidia,tegra194-carmel", "arm,armv8";
device_type = "cpu";
reg = <0x10300>;
enable-method = "psci";
+ i-cache-size = <131072>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <512>;
+ d-cache-size = <65536>;
+ d-cache-line-size = <64>;
+ d-cache_sets = <256>;
+ l2-cache = <&l2_3>;
};
- cpu@7 {
+ cl3_1: cpu@7 {
compatible = "nvidia,tegra194-carmel", "arm,armv8";
device_type = "cpu";
reg = <0x10301>;
enable-method = "psci";
+ i-cache-size = <131072>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <512>;
+ d-cache-size = <65536>;
+ d-cache-line-size = <64>;
+ d-cache_sets = <256>;
+ l2-cache = <&l2_3>;
};
};
+ l2_0: l2-cache0 {
+ cache-size = <2097152>;
+ cache-line-size = <64>;
+ cache-sets = <2048>;
+ next-level-cache = <&l3>;
+ };
+
+ l2_1: l2-cache1 {
+ cache-size = <2097152>;
+ cache-line-size = <64>;
+ cache-sets = <2048>;
+ next-level-cache = <&l3>;
+ };
+
+ l2_2: l2-cache2 {
+ cache-size = <2097152>;
+ cache-line-size = <64>;
+ cache-sets = <2048>;
+ next-level-cache = <&l3>;
+ };
+
+ l2_3: l2-cache3 {
+ cache-size = <2097152>;
+ cache-line-size = <64>;
+ cache-sets = <2048>;
+ next-level-cache = <&l3>;
+ };
+
+ l3: l3-cache {
+ cache-size = <4194304>;
+ cache-line-size = <64>;
+ cache-sets = <4096>;
+ };
+
psci {
compatible = "arm,psci-1.0";
status = "okay";
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] arm64: tegra: add topology data for Tegra194 cpu
2019-01-31 18:35 [PATCH] arm64: tegra: add topology data for Tegra194 cpu Bo Yan
@ 2019-01-31 22:25 ` Thierry Reding
2019-01-31 23:29 ` Bo Yan
2019-02-11 23:47 ` [PATCH V2] " Bo Yan
0 siblings, 2 replies; 9+ messages in thread
From: Thierry Reding @ 2019-01-31 22:25 UTC (permalink / raw)
To: Bo Yan; +Cc: jonathanh, linux-tegra, mark.rutland, robh+dt, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 6584 bytes --]
On Thu, Jan 31, 2019 at 10:35:54AM -0800, Bo Yan wrote:
> The xavier CPU architecture includes 8 CPU cores organized in
> 4 clusters. Add cpu-map data for topology initialization, add
> cache data for cache node creation in sysfs.
>
> Signed-off-by: Bo Yan <byan@nvidia.com>
> ---
> arch/arm64/boot/dts/nvidia/tegra194.dtsi | 148 +++++++++++++++++++++++++++++--
> 1 file changed, 140 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> index 6dfa1ca..7c2a1fb 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> @@ -870,63 +870,195 @@
> #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";
> + i-cache-size = <131072>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <512>;
> + d-cache-size = <65536>;
> + d-cache-line-size = <64>;
> + d-cache_sets = <256>;
These don't seem to be well-defined. They are mentioned in a very weird
locations (Documentation/devicetree/booting-without-of.txt) but there
seem to be examples and other device tree files that use them so maybe
those are all valid. It might be worth mentioning these in other places
where people can more easily find them.
According to the above document, {i,d}-cache-line-size are deprecated in
favour of {i,d}-cache-block-size.
I also don't see any mention of {i,d}-cache_sets in the device tree
bindings, though riscv/cpus.txt mentions {i,d}-cache-sets (note the
hyphen instead of underscore) in the examples. arm/l2c2x0.txt and
arm/uniphier/cache-unifier.txt describe cache-sets, though that's
slightly different.
Might make sense to document all these in more standard places. Maybe
adding them to arm/cpus.txt. For consistency with other properties, I
think there should be called {i,d}-cache-sets like for RISC-V.
> + l2-cache = <&l2_0>;
This seems to be called next-level-cache everywhere else, though it's
only formally described in arm/uniphier/cache-uniphier.txt. So might
also make sense to add this to arm/cpus.txt.
> };
>
> - cpu@1 {
> + cl0_1: cpu@1 {
> compatible = "nvidia,tegra194-carmel", "arm,armv8";
> device_type = "cpu";
> reg = <0x10001>;
> enable-method = "psci";
> + i-cache-size = <131072>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <512>;
> + d-cache-size = <65536>;
> + d-cache-line-size = <64>;
> + d-cache_sets = <256>;
> + l2-cache = <&l2_0>;
> };
>
> - cpu@2 {
> + cl1_0: cpu@2 {
> compatible = "nvidia,tegra194-carmel", "arm,armv8";
> device_type = "cpu";
> reg = <0x100>;
> enable-method = "psci";
> + i-cache-size = <131072>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <512>;
> + d-cache-size = <65536>;
> + d-cache-line-size = <64>;
> + d-cache_sets = <256>;
> + l2-cache = <&l2_1>;
> };
>
> - cpu@3 {
> + cl1_1: cpu@3 {
> compatible = "nvidia,tegra194-carmel", "arm,armv8";
> device_type = "cpu";
> reg = <0x101>;
> enable-method = "psci";
> + i-cache-size = <131072>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <512>;
> + d-cache-size = <65536>;
> + d-cache-line-size = <64>;
> + d-cache_sets = <256>;
> + l2-cache = <&l2_1>;
> };
>
> - cpu@4 {
> + cl2_0: cpu@4 {
> compatible = "nvidia,tegra194-carmel", "arm,armv8";
> device_type = "cpu";
> reg = <0x200>;
> enable-method = "psci";
> + i-cache-size = <131072>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <512>;
> + d-cache-size = <65536>;
> + d-cache-line-size = <64>;
> + d-cache_sets = <256>;
> + l2-cache = <&l2_2>;
> };
>
> - cpu@5 {
> + cl2_1: cpu@5 {
> compatible = "nvidia,tegra194-carmel", "arm,armv8";
> device_type = "cpu";
> reg = <0x201>;
> enable-method = "psci";
> + i-cache-size = <131072>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <512>;
> + d-cache-size = <65536>;
> + d-cache-line-size = <64>;
> + d-cache_sets = <256>;
> + l2-cache = <&l2_2>;
> };
>
> - cpu@6 {
> + cl3_0: cpu@6 {
> compatible = "nvidia,tegra194-carmel", "arm,armv8";
> device_type = "cpu";
> reg = <0x10300>;
> enable-method = "psci";
> + i-cache-size = <131072>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <512>;
> + d-cache-size = <65536>;
> + d-cache-line-size = <64>;
> + d-cache_sets = <256>;
> + l2-cache = <&l2_3>;
> };
>
> - cpu@7 {
> + cl3_1: cpu@7 {
> compatible = "nvidia,tegra194-carmel", "arm,armv8";
> device_type = "cpu";
> reg = <0x10301>;
> enable-method = "psci";
> + i-cache-size = <131072>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <512>;
> + d-cache-size = <65536>;
> + d-cache-line-size = <64>;
> + d-cache_sets = <256>;
> + l2-cache = <&l2_3>;
> };
> };
>
> + l2_0: l2-cache0 {
> + cache-size = <2097152>;
> + cache-line-size = <64>;
> + cache-sets = <2048>;
> + next-level-cache = <&l3>;
> + };
Does this need a compatible string? Also, are there controllers behind
these caches? I'm just wondering if these also need reg properties and
unit-addresses.
arm/l2c2x0.txt and arm/uniphier/cache-uniphier.txt describe an
additional property that you don't specify here: cache-level. This
sounds useful to have so that we don't have to guess the cache level
from the name, which may or may not work depending on what people name
the nodes.
Also, similar to the L1 cache, cache-block-size is preferred over
cache-line-size.
> + l3: l3-cache {
> + cache-size = <4194304>;
> + cache-line-size = <64>;
> + cache-sets = <4096>;
> + };
The same comments apply as for the L2 caches.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] arm64: tegra: add topology data for Tegra194 cpu
2019-01-31 22:25 ` Thierry Reding
@ 2019-01-31 23:29 ` Bo Yan
2019-02-11 23:34 ` Bo Yan
2019-02-11 23:47 ` [PATCH V2] " Bo Yan
1 sibling, 1 reply; 9+ messages in thread
From: Bo Yan @ 2019-01-31 23:29 UTC (permalink / raw)
To: Thierry Reding
Cc: jonathanh, linux-tegra, mark.rutland, robh+dt, linux-kernel
On 1/31/19 2:25 PM, Thierry Reding wrote:
> On Thu, Jan 31, 2019 at 10:35:54AM -0800, Bo Yan wrote:
>> The xavier CPU architecture includes 8 CPU cores organized in
>> 4 clusters. Add cpu-map data for topology initialization, add
>> cache data for cache node creation in sysfs.
>>
>> Signed-off-by: Bo Yan <byan@nvidia.com>
>> ---
>> arch/arm64/boot/dts/nvidia/tegra194.dtsi | 148 +++++++++++++++++++++++++++++--
>> 1 file changed, 140 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> index 6dfa1ca..7c2a1fb 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> @@ -870,63 +870,195 @@
>> #address-cells = <1>;
>> #size-cells = <0>;
> These don't seem to be well-defined. They are mentioned in a very weird
> locations (Documentation/devicetree/booting-without-of.txt) but there
> seem to be examples and other device tree files that use them so maybe
> those are all valid. It might be worth mentioning these in other places
> where people can more easily find them.
It might be logical to place a reference to this document
(booting-without-of.txt) in architecture specific documents, for
example, arm/cpus.txt. I see the need for improved documentation, but
this probably should be best done in a separate change.
>
> According to the above document, {i,d}-cache-line-size are deprecated in
> favour of {i,d}-cache-block-size.
Mostly, this seems to be derived from the oddity of PowerPC, which might
have different cache-line-size and cache-block-size. I don't know if
there are other examples? It looks like the {i,d}-cache-line-size are
being used in dts files for almost all architectures, the only exception
is arch/sh/boot/dts/j2_mimas_v2.dts. On ARM and ARM64, cache-line-size
is the same as cache-block-size. So I am wondering whether the
booting-without-of.txt should be fixed instead? just to keep it
consistent among dts files, especially in arm64.
>
> I also don't see any mention of {i,d}-cache_sets in the device tree
> bindings, though riscv/cpus.txt mentions {i,d}-cache-sets (note the
> hyphen instead of underscore) in the examples. arm/l2c2x0.txt and
> arm/uniphier/cache-unifier.txt describe cache-sets, though that's
> slightly different.
>
> Might make sense to document all these in more standard places. Maybe
> adding them to arm/cpus.txt. For consistency with other properties, I
> think there should be called {i,d}-cache-sets like for RISC-V.
>
>> + l2-cache = <&l2_0>;
>
> This seems to be called next-level-cache everywhere else, though it's
> only formally described in arm/uniphier/cache-uniphier.txt. So might
> also make sense to add this to arm/cpus.txt.
the improved documentation is certainly desired, I agree.
>
>> };
>>
>> - cpu@1 {
>> + cl0_1: cpu@1 {
>> compatible = "nvidia,tegra194-carmel", "arm,armv8";
>> device_type = "cpu";
>> reg = <0x10001>;
>> enable-method = "psci";
>> + i-cache-size = <131072>;
>> + i-cache-line-size = <64>;
>> + i-cache-sets = <512>;
>> + d-cache-size = <65536>;
>> + d-cache-line-size = <64>;
>> + d-cache_sets = <256>;
>> + l2-cache = <&l2_0>;
>> };
>>
>> - cpu@2 {
>> + cl1_0: cpu@2 {
>> compatible = "nvidia,tegra194-carmel", "arm,armv8";
>> device_type = "cpu";
>> reg = <0x100>;
>> enable-method = "psci";
>> + i-cache-size = <131072>;
>> + i-cache-line-size = <64>;
>> + i-cache-sets = <512>;
>> + d-cache-size = <65536>;
>> + d-cache-line-size = <64>;
>> + d-cache_sets = <256>;
>> + l2-cache = <&l2_1>;
>> };
>>
>> - cpu@3 {
>> + cl1_1: cpu@3 {
>> compatible = "nvidia,tegra194-carmel", "arm,armv8";
>> device_type = "cpu";
>> reg = <0x101>;
>> enable-method = "psci";
>> + i-cache-size = <131072>;
>> + i-cache-line-size = <64>;
>> + i-cache-sets = <512>;
>> + d-cache-size = <65536>;
>> + d-cache-line-size = <64>;
>> + d-cache_sets = <256>;
>> + l2-cache = <&l2_1>;
>> };
>>
>> - cpu@4 {
>> + cl2_0: cpu@4 {
>> compatible = "nvidia,tegra194-carmel", "arm,armv8";
>> device_type = "cpu";
>> reg = <0x200>;
>> enable-method = "psci";
>> + i-cache-size = <131072>;
>> + i-cache-line-size = <64>;
>> + i-cache-sets = <512>;
>> + d-cache-size = <65536>;
>> + d-cache-line-size = <64>;
>> + d-cache_sets = <256>;
>> + l2-cache = <&l2_2>;
>> };
>>
>> - cpu@5 {
>> + cl2_1: cpu@5 {
>> compatible = "nvidia,tegra194-carmel", "arm,armv8";
>> device_type = "cpu";
>> reg = <0x201>;
>> enable-method = "psci";
>> + i-cache-size = <131072>;
>> + i-cache-line-size = <64>;
>> + i-cache-sets = <512>;
>> + d-cache-size = <65536>;
>> + d-cache-line-size = <64>;
>> + d-cache_sets = <256>;
>> + l2-cache = <&l2_2>;
>> };
>>
>> - cpu@6 {
>> + cl3_0: cpu@6 {
>> compatible = "nvidia,tegra194-carmel", "arm,armv8";
>> device_type = "cpu";
>> reg = <0x10300>;
>> enable-method = "psci";
>> + i-cache-size = <131072>;
>> + i-cache-line-size = <64>;
>> + i-cache-sets = <512>;
>> + d-cache-size = <65536>;
>> + d-cache-line-size = <64>;
>> + d-cache_sets = <256>;
>> + l2-cache = <&l2_3>;
>> };
>>
>> - cpu@7 {
>> + cl3_1: cpu@7 {
>> compatible = "nvidia,tegra194-carmel", "arm,armv8";
>> device_type = "cpu";
>> reg = <0x10301>;
>> enable-method = "psci";
>> + i-cache-size = <131072>;
>> + i-cache-line-size = <64>;
>> + i-cache-sets = <512>;
>> + d-cache-size = <65536>;
>> + d-cache-line-size = <64>;
>> + d-cache_sets = <256>;
>> + l2-cache = <&l2_3>;
>> };
>> };
>>
>> + l2_0: l2-cache0 {
>> + cache-size = <2097152>;
>> + cache-line-size = <64>;
>> + cache-sets = <2048>;
>> + next-level-cache = <&l3>;
>> + };
>
> Does this need a compatible string? Also, are there controllers behind
> these caches? I'm just wondering if these also need reg properties and
> unit-addresses.
No need for compatible string. No reg properties and addresses. These
will be parsed by drivers/of/base.c and drivers/base/cacheinfo.c, they
are generic.
>
> arm/l2c2x0.txt and arm/uniphier/cache-uniphier.txt describe an
> additional property that you don't specify here: cache-level. This
> sounds useful to have so that we don't have to guess the cache level
> from the name, which may or may not work depending on what people name
> the nodes.
the cache level property is implied in device tree hierarchy, so after
system boots up, I can find cache level in related sysfs nodes:
[root@alarm cache]# cat index*/level
1
1
2
3
>
> Also, similar to the L1 cache, cache-block-size is preferred over
> cache-line-size.
>
>> + l3: l3-cache {
>> + cache-size = <4194304>;
>> + cache-line-size = <64>;
>> + cache-sets = <4096>;
>> + };
>
> The same comments apply as for the L2 caches.
>
> Thierry
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] arm64: tegra: add topology data for Tegra194 cpu
2019-01-31 23:29 ` Bo Yan
@ 2019-02-11 23:34 ` Bo Yan
0 siblings, 0 replies; 9+ messages in thread
From: Bo Yan @ 2019-02-11 23:34 UTC (permalink / raw)
To: Thierry Reding
Cc: jonathanh, linux-tegra, mark.rutland, robh+dt, linux-kernel
To make this simpler, I think it's best to isolate the cache information
in its own patch. So I will amend this patch to include topology
information only.
On 1/31/19 3:29 PM, Bo Yan wrote:
>
> On 1/31/19 2:25 PM, Thierry Reding wrote:
>> On Thu, Jan 31, 2019 at 10:35:54AM -0800, Bo Yan wrote:
>>> The xavier CPU architecture includes 8 CPU cores organized in
>>> 4 clusters. Add cpu-map data for topology initialization, add
>>> cache data for cache node creation in sysfs.
>>>
>>> Signed-off-by: Bo Yan <byan@nvidia.com>
>>> ---
>>> arch/arm64/boot/dts/nvidia/tegra194.dtsi | 148
>>> +++++++++++++++++++++++++++++--
>>> 1 file changed, 140 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>>> b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>>> index 6dfa1ca..7c2a1fb 100644
>>> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>>> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>>> @@ -870,63 +870,195 @@
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>
>> These don't seem to be well-defined. They are mentioned in a very weird
>> locations (Documentation/devicetree/booting-without-of.txt) but there
>> seem to be examples and other device tree files that use them so maybe
>> those are all valid. It might be worth mentioning these in other places
>> where people can more easily find them.
>
> It might be logical to place a reference to this document
> (booting-without-of.txt) in architecture specific documents, for
> example, arm/cpus.txt. I see the need for improved documentation, but
> this probably should be best done in a separate change.
>>
>> According to the above document, {i,d}-cache-line-size are deprecated in
>> favour of {i,d}-cache-block-size.
>
> Mostly, this seems to be derived from the oddity of PowerPC, which might
> have different cache-line-size and cache-block-size. I don't know if
> there are other examples? It looks like the {i,d}-cache-line-size are
> being used in dts files for almost all architectures, the only exception
> is arch/sh/boot/dts/j2_mimas_v2.dts. On ARM and ARM64, cache-line-size
> is the same as cache-block-size. So I am wondering whether the
> booting-without-of.txt should be fixed instead? just to keep it
> consistent among dts files, especially in arm64.
>
>>
>> I also don't see any mention of {i,d}-cache_sets in the device tree
>> bindings, though riscv/cpus.txt mentions {i,d}-cache-sets (note the
>> hyphen instead of underscore) in the examples. arm/l2c2x0.txt and
>> arm/uniphier/cache-unifier.txt describe cache-sets, though that's
>> slightly different.
>>
>> Might make sense to document all these in more standard places. Maybe
>> adding them to arm/cpus.txt. For consistency with other properties, I
>> think there should be called {i,d}-cache-sets like for RISC-V.
>>
>>> + l2-cache = <&l2_0>;
>>
>> This seems to be called next-level-cache everywhere else, though it's
>> only formally described in arm/uniphier/cache-uniphier.txt. So might
>> also make sense to add this to arm/cpus.txt.
>
> the improved documentation is certainly desired, I agree.
>>
>>> };
>>> - cpu@1 {
>>> + cl0_1: cpu@1 {
>>> compatible = "nvidia,tegra194-carmel", "arm,armv8";
>>> device_type = "cpu";
>>> reg = <0x10001>;
>>> enable-method = "psci";
>>> + i-cache-size = <131072>;
>>> + i-cache-line-size = <64>;
>>> + i-cache-sets = <512>;
>>> + d-cache-size = <65536>;
>>> + d-cache-line-size = <64>;
>>> + d-cache_sets = <256>;
>>> + l2-cache = <&l2_0>;
>>> };
>>> - cpu@2 {
>>> + cl1_0: cpu@2 {
>>> compatible = "nvidia,tegra194-carmel", "arm,armv8";
>>> device_type = "cpu";
>>> reg = <0x100>;
>>> enable-method = "psci";
>>> + i-cache-size = <131072>;
>>> + i-cache-line-size = <64>;
>>> + i-cache-sets = <512>;
>>> + d-cache-size = <65536>;
>>> + d-cache-line-size = <64>;
>>> + d-cache_sets = <256>;
>>> + l2-cache = <&l2_1>;
>>> };
>>> - cpu@3 {
>>> + cl1_1: cpu@3 {
>>> compatible = "nvidia,tegra194-carmel", "arm,armv8";
>>> device_type = "cpu";
>>> reg = <0x101>;
>>> enable-method = "psci";
>>> + i-cache-size = <131072>;
>>> + i-cache-line-size = <64>;
>>> + i-cache-sets = <512>;
>>> + d-cache-size = <65536>;
>>> + d-cache-line-size = <64>;
>>> + d-cache_sets = <256>;
>>> + l2-cache = <&l2_1>;
>>> };
>>> - cpu@4 {
>>> + cl2_0: cpu@4 {
>>> compatible = "nvidia,tegra194-carmel", "arm,armv8";
>>> device_type = "cpu";
>>> reg = <0x200>;
>>> enable-method = "psci";
>>> + i-cache-size = <131072>;
>>> + i-cache-line-size = <64>;
>>> + i-cache-sets = <512>;
>>> + d-cache-size = <65536>;
>>> + d-cache-line-size = <64>;
>>> + d-cache_sets = <256>;
>>> + l2-cache = <&l2_2>;
>>> };
>>> - cpu@5 {
>>> + cl2_1: cpu@5 {
>>> compatible = "nvidia,tegra194-carmel", "arm,armv8";
>>> device_type = "cpu";
>>> reg = <0x201>;
>>> enable-method = "psci";
>>> + i-cache-size = <131072>;
>>> + i-cache-line-size = <64>;
>>> + i-cache-sets = <512>;
>>> + d-cache-size = <65536>;
>>> + d-cache-line-size = <64>;
>>> + d-cache_sets = <256>;
>>> + l2-cache = <&l2_2>;
>>> };
>>> - cpu@6 {
>>> + cl3_0: cpu@6 {
>>> compatible = "nvidia,tegra194-carmel", "arm,armv8";
>>> device_type = "cpu";
>>> reg = <0x10300>;
>>> enable-method = "psci";
>>> + i-cache-size = <131072>;
>>> + i-cache-line-size = <64>;
>>> + i-cache-sets = <512>;
>>> + d-cache-size = <65536>;
>>> + d-cache-line-size = <64>;
>>> + d-cache_sets = <256>;
>>> + l2-cache = <&l2_3>;
>>> };
>>> - cpu@7 {
>>> + cl3_1: cpu@7 {
>>> compatible = "nvidia,tegra194-carmel", "arm,armv8";
>>> device_type = "cpu";
>>> reg = <0x10301>;
>>> enable-method = "psci";
>>> + i-cache-size = <131072>;
>>> + i-cache-line-size = <64>;
>>> + i-cache-sets = <512>;
>>> + d-cache-size = <65536>;
>>> + d-cache-line-size = <64>;
>>> + d-cache_sets = <256>;
>>> + l2-cache = <&l2_3>;
>>> };
>>> };
>>> + l2_0: l2-cache0 {
>>> + cache-size = <2097152>;
>>> + cache-line-size = <64>;
>>> + cache-sets = <2048>;
>>> + next-level-cache = <&l3>;
>>> + };
>>
>> Does this need a compatible string? Also, are there controllers behind
>> these caches? I'm just wondering if these also need reg properties and
>> unit-addresses.
>
> No need for compatible string. No reg properties and addresses. These
> will be parsed by drivers/of/base.c and drivers/base/cacheinfo.c, they
> are generic.
>>
>> arm/l2c2x0.txt and arm/uniphier/cache-uniphier.txt describe an
>> additional property that you don't specify here: cache-level. This
>> sounds useful to have so that we don't have to guess the cache level
>> from the name, which may or may not work depending on what people name
>> the nodes.
>
> the cache level property is implied in device tree hierarchy, so after
> system boots up, I can find cache level in related sysfs nodes:
>
> [root@alarm cache]# cat index*/level
> 1
> 1
> 2
> 3
>
>
>>
>> Also, similar to the L1 cache, cache-block-size is preferred over
>> cache-line-size.
>>
>>> + l3: l3-cache {
>>> + cache-size = <4194304>;
>>> + cache-line-size = <64>;
>>> + cache-sets = <4096>;
>>> + };
>>
>> The same comments apply as for the L2 caches.
>>
>> Thierry
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V2] arm64: tegra: add topology data for Tegra194 cpu
2019-01-31 22:25 ` Thierry Reding
2019-01-31 23:29 ` Bo Yan
@ 2019-02-11 23:47 ` Bo Yan
2019-02-13 8:12 ` Thierry Reding
1 sibling, 1 reply; 9+ messages in thread
From: Bo Yan @ 2019-02-11 23:47 UTC (permalink / raw)
To: thierry.reding, jonathanh
Cc: robh+dt, mark.rutland, linux-tegra, linux-kernel, Bo Yan
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(-)
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>;
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V2] arm64: tegra: add topology data for Tegra194 cpu
2019-02-11 23:47 ` [PATCH V2] " Bo Yan
@ 2019-02-13 8:12 ` Thierry Reding
2019-02-13 16:24 ` Bo Yan
2019-02-13 16:33 ` [PATCH V3] " Bo Yan
0 siblings, 2 replies; 9+ messages in thread
From: Thierry Reding @ 2019-02-13 8:12 UTC (permalink / raw)
To: Bo Yan; +Cc: jonathanh, robh+dt, mark.rutland, linux-tegra, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1780 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V2] arm64: tegra: add topology data for Tegra194 cpu
2019-02-13 8:12 ` Thierry Reding
@ 2019-02-13 16:24 ` Bo Yan
2019-02-13 16:33 ` [PATCH V3] " Bo Yan
1 sibling, 0 replies; 9+ messages in thread
From: Bo Yan @ 2019-02-13 16:24 UTC (permalink / raw)
To: Thierry Reding
Cc: jonathanh, robh+dt, mark.rutland, linux-tegra, linux-kernel
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V3] arm64: tegra: add topology data for Tegra194 cpu
2019-02-13 8:12 ` Thierry Reding
2019-02-13 16:24 ` Bo Yan
@ 2019-02-13 16:33 ` Bo Yan
2019-02-22 0:52 ` Bo Yan
1 sibling, 1 reply; 9+ messages in thread
From: Bo Yan @ 2019-02-13 16:33 UTC (permalink / raw)
To: thierry.reding, jonathanh
Cc: robh+dt, mark.rutland, linux-tegra, linux-kernel, Bo Yan
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>
---
V3: Replaced phandles with full path to CPU node
V2: remove cache nodes, add topology data only
arch/arm64/boot/dts/nvidia/tegra194.dtsi | 42 ++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index 6dfa1ca..708d20c 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -870,6 +870,48 @@
#address-cells = <1>;
#size-cells = <0>;
+ cpu-map {
+ cluster0 {
+ core0 {
+ cpu = <&{/cpus/cpu@0}>;
+ };
+
+ core1 {
+ cpu = <&{/cpus/cpu@1}>;
+ };
+ };
+
+ cluster1 {
+ core0 {
+ cpu = <&{/cpus/cpu@2}>;
+ };
+
+ core1 {
+ cpu = <&{/cpus/cpu@3}>;
+ };
+ };
+
+ cluster2 {
+ core0 {
+ cpu = <&{/cpus/cpu@4}>;
+ };
+
+ core1 {
+ cpu = <&{/cpus/cpu@5}>;
+ };
+ };
+
+ cluster3 {
+ core0 {
+ cpu = <&{/cpus/cpu@6}>;
+ };
+
+ core1 {
+ cpu = <&{/cpus/cpu@7}>;
+ };
+ };
+ };
+
cpu@0 {
compatible = "nvidia,tegra194-carmel", "arm,armv8";
device_type = "cpu";
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V3] arm64: tegra: add topology data for Tegra194 cpu
2019-02-13 16:33 ` [PATCH V3] " Bo Yan
@ 2019-02-22 0:52 ` Bo Yan
0 siblings, 0 replies; 9+ messages in thread
From: Bo Yan @ 2019-02-22 0:52 UTC (permalink / raw)
To: thierry.reding, jonathanh
Cc: robh+dt, mark.rutland, linux-tegra, linux-kernel
The patch V3 adopted changes suggested by Thierry.
On 2/13/19 8:33 AM, 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>
> ---
> V3: Replaced phandles with full path to CPU node
> V2: remove cache nodes, add topology data only
>
> arch/arm64/boot/dts/nvidia/tegra194.dtsi | 42 ++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> index 6dfa1ca..708d20c 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> @@ -870,6 +870,48 @@
> #address-cells = <1>;
> #size-cells = <0>;
>
> + cpu-map {
> + cluster0 {
> + core0 {
> + cpu = <&{/cpus/cpu@0}>;
> + };
> +
> + core1 {
> + cpu = <&{/cpus/cpu@1}>;
> + };
> + };
> +
> + cluster1 {
> + core0 {
> + cpu = <&{/cpus/cpu@2}>;
> + };
> +
> + core1 {
> + cpu = <&{/cpus/cpu@3}>;
> + };
> + };
> +
> + cluster2 {
> + core0 {
> + cpu = <&{/cpus/cpu@4}>;
> + };
> +
> + core1 {
> + cpu = <&{/cpus/cpu@5}>;
> + };
> + };
> +
> + cluster3 {
> + core0 {
> + cpu = <&{/cpus/cpu@6}>;
> + };
> +
> + core1 {
> + cpu = <&{/cpus/cpu@7}>;
> + };
> + };
> + };
> +
> cpu@0 {
> compatible = "nvidia,tegra194-carmel", "arm,armv8";
> device_type = "cpu";
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-02-22 0:52 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-31 18:35 [PATCH] arm64: tegra: add topology data for Tegra194 cpu Bo Yan
2019-01-31 22:25 ` Thierry Reding
2019-01-31 23:29 ` Bo Yan
2019-02-11 23:34 ` Bo Yan
2019-02-11 23:47 ` [PATCH V2] " Bo Yan
2019-02-13 8:12 ` Thierry Reding
2019-02-13 16:24 ` Bo Yan
2019-02-13 16:33 ` [PATCH V3] " Bo Yan
2019-02-22 0:52 ` Bo Yan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).