* [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).