From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753679AbcGHGKh (ORCPT ); Fri, 8 Jul 2016 02:10:37 -0400 Received: from mx0b-0016f401.pphosted.com ([67.231.156.173]:33207 "EHLO mx0b-0016f401.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750985AbcGHGKa (ORCPT ); Fri, 8 Jul 2016 02:10:30 -0400 Date: Fri, 8 Jul 2016 14:05:56 +0800 From: Jisheng Zhang To: Sebastian Hesselbarth CC: , , , , , , Subject: Re: [PATCH] arm64: dts: berlin4ct: Add L2 cache topology Message-ID: <20160708140556.4ae8abaa@xhacker> In-Reply-To: <577E8D02.5010401@gmail.com> References: <1466066418-1141-1-git-send-email-jszhang@marvell.com> <577D448D.8030701@gmail.com> <20160707134807.08b70b38@xhacker> <577E8D02.5010401@gmail.com> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-07-08_04:,, signatures=0 X-Proofpoint-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1607080059 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Dear Sebastian, On Thu, 7 Jul 2016 19:10:26 +0200 Sebastian Hesselbarth wrote: > On 07.07.2016 07:48, Jisheng Zhang wrote: > > On Wed, 6 Jul 2016 19:49:01 +0200 Sebastian Hesselbarth wrote: > >> On 16.06.2016 10:40, Jisheng Zhang wrote: > >>> This patch adds the L2 cache topology for berlin4ct which has 1MB L2 > >>> cache. > >>> > >>> Signed-off-by: Jisheng Zhang > >>> --- > >>> arch/arm64/boot/dts/marvell/berlin4ct.dtsi | 8 ++++++++ > >>> 1 file changed, 8 insertions(+) > >>> > >>> diff --git a/arch/arm64/boot/dts/marvell/berlin4ct.dtsi b/arch/arm64/boot/dts/marvell/berlin4ct.dtsi > >>> index 099ad93..c9e3a98 100644 > >>> --- a/arch/arm64/boot/dts/marvell/berlin4ct.dtsi > >>> +++ b/arch/arm64/boot/dts/marvell/berlin4ct.dtsi > >> [...] > >>> @@ -92,9 +95,14 @@ > >>> device_type = "cpu"; > >>> reg = <0x3>; > >>> enable-method = "psci"; > >>> + next-level-cache = <&L2_0>; > >>> cpu-idle-states = <&CPU_SLEEP_0>; > >>> }; > >>> > >>> + L2_0: l2-cache0 { > >> > >> The node name should just have a generic name that reflects > >> the purpose of the unit it represents, i.e. > >> s/l2-cache0/cache/ > > > > IMHO, "cache" is too generic, this is L2 cache topology, so in v2, I use > > "l2-cache" instead. what do you think? > > > > PS: I found other arm64 SoCs also use "l2-cache" as the node name. > > Yeah, I realized that too. Anyway, the node name should be as generic > as possible. Moreover, the more specific compatible string below also > is "cache", too. So I see no reason why the node name should be more > specific than the compatible. make sense, I agree with you now. > > >>> + compatible = "cache"; > >>> + }; > > If you want to have the cache-level represented in the node, I guess > you can use cache-level property. However, I cannot find any cache > related binding documentation other than for arm(32) and powerpc that > mentions cache-level property. > > If you are fine with it, I can pick up the v2 you sent earlier, rename > the node to "cache" only, and add a cache-level = <2>; property while > applying. Per my understanding of the code, drivers/base/cacheinfo.c and related source code, the "cache-level" property isn't used at all, so could you please only rename the node to "cache"? Thanks in advance, Jisheng From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jisheng Zhang Subject: Re: [PATCH] arm64: dts: berlin4ct: Add L2 cache topology Date: Fri, 8 Jul 2016 14:05:56 +0800 Message-ID: <20160708140556.4ae8abaa@xhacker> References: <1466066418-1141-1-git-send-email-jszhang@marvell.com> <577D448D.8030701@gmail.com> <20160707134807.08b70b38@xhacker> <577E8D02.5010401@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <577E8D02.5010401@gmail.com> Sender: linux-kernel-owner@vger.kernel.org To: Sebastian Hesselbarth Cc: robh+dt@kernel.org, mark.rutland@arm.com, catalin.marinas@arm.com, will.deacon@arm.com, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org List-Id: devicetree@vger.kernel.org Dear Sebastian, On Thu, 7 Jul 2016 19:10:26 +0200 Sebastian Hesselbarth wrote: > On 07.07.2016 07:48, Jisheng Zhang wrote: > > On Wed, 6 Jul 2016 19:49:01 +0200 Sebastian Hesselbarth wrote: > >> On 16.06.2016 10:40, Jisheng Zhang wrote: > >>> This patch adds the L2 cache topology for berlin4ct which has 1MB L2 > >>> cache. > >>> > >>> Signed-off-by: Jisheng Zhang > >>> --- > >>> arch/arm64/boot/dts/marvell/berlin4ct.dtsi | 8 ++++++++ > >>> 1 file changed, 8 insertions(+) > >>> > >>> diff --git a/arch/arm64/boot/dts/marvell/berlin4ct.dtsi b/arch/arm64/boot/dts/marvell/berlin4ct.dtsi > >>> index 099ad93..c9e3a98 100644 > >>> --- a/arch/arm64/boot/dts/marvell/berlin4ct.dtsi > >>> +++ b/arch/arm64/boot/dts/marvell/berlin4ct.dtsi > >> [...] > >>> @@ -92,9 +95,14 @@ > >>> device_type = "cpu"; > >>> reg = <0x3>; > >>> enable-method = "psci"; > >>> + next-level-cache = <&L2_0>; > >>> cpu-idle-states = <&CPU_SLEEP_0>; > >>> }; > >>> > >>> + L2_0: l2-cache0 { > >> > >> The node name should just have a generic name that reflects > >> the purpose of the unit it represents, i.e. > >> s/l2-cache0/cache/ > > > > IMHO, "cache" is too generic, this is L2 cache topology, so in v2, I use > > "l2-cache" instead. what do you think? > > > > PS: I found other arm64 SoCs also use "l2-cache" as the node name. > > Yeah, I realized that too. Anyway, the node name should be as generic > as possible. Moreover, the more specific compatible string below also > is "cache", too. So I see no reason why the node name should be more > specific than the compatible. make sense, I agree with you now. > > >>> + compatible = "cache"; > >>> + }; > > If you want to have the cache-level represented in the node, I guess > you can use cache-level property. However, I cannot find any cache > related binding documentation other than for arm(32) and powerpc that > mentions cache-level property. > > If you are fine with it, I can pick up the v2 you sent earlier, rename > the node to "cache" only, and add a cache-level = <2>; property while > applying. Per my understanding of the code, drivers/base/cacheinfo.c and related source code, the "cache-level" property isn't used at all, so could you please only rename the node to "cache"? Thanks in advance, Jisheng From mboxrd@z Thu Jan 1 00:00:00 1970 From: jszhang@marvell.com (Jisheng Zhang) Date: Fri, 8 Jul 2016 14:05:56 +0800 Subject: [PATCH] arm64: dts: berlin4ct: Add L2 cache topology In-Reply-To: <577E8D02.5010401@gmail.com> References: <1466066418-1141-1-git-send-email-jszhang@marvell.com> <577D448D.8030701@gmail.com> <20160707134807.08b70b38@xhacker> <577E8D02.5010401@gmail.com> Message-ID: <20160708140556.4ae8abaa@xhacker> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Dear Sebastian, On Thu, 7 Jul 2016 19:10:26 +0200 Sebastian Hesselbarth wrote: > On 07.07.2016 07:48, Jisheng Zhang wrote: > > On Wed, 6 Jul 2016 19:49:01 +0200 Sebastian Hesselbarth wrote: > >> On 16.06.2016 10:40, Jisheng Zhang wrote: > >>> This patch adds the L2 cache topology for berlin4ct which has 1MB L2 > >>> cache. > >>> > >>> Signed-off-by: Jisheng Zhang > >>> --- > >>> arch/arm64/boot/dts/marvell/berlin4ct.dtsi | 8 ++++++++ > >>> 1 file changed, 8 insertions(+) > >>> > >>> diff --git a/arch/arm64/boot/dts/marvell/berlin4ct.dtsi b/arch/arm64/boot/dts/marvell/berlin4ct.dtsi > >>> index 099ad93..c9e3a98 100644 > >>> --- a/arch/arm64/boot/dts/marvell/berlin4ct.dtsi > >>> +++ b/arch/arm64/boot/dts/marvell/berlin4ct.dtsi > >> [...] > >>> @@ -92,9 +95,14 @@ > >>> device_type = "cpu"; > >>> reg = <0x3>; > >>> enable-method = "psci"; > >>> + next-level-cache = <&L2_0>; > >>> cpu-idle-states = <&CPU_SLEEP_0>; > >>> }; > >>> > >>> + L2_0: l2-cache0 { > >> > >> The node name should just have a generic name that reflects > >> the purpose of the unit it represents, i.e. > >> s/l2-cache0/cache/ > > > > IMHO, "cache" is too generic, this is L2 cache topology, so in v2, I use > > "l2-cache" instead. what do you think? > > > > PS: I found other arm64 SoCs also use "l2-cache" as the node name. > > Yeah, I realized that too. Anyway, the node name should be as generic > as possible. Moreover, the more specific compatible string below also > is "cache", too. So I see no reason why the node name should be more > specific than the compatible. make sense, I agree with you now. > > >>> + compatible = "cache"; > >>> + }; > > If you want to have the cache-level represented in the node, I guess > you can use cache-level property. However, I cannot find any cache > related binding documentation other than for arm(32) and powerpc that > mentions cache-level property. > > If you are fine with it, I can pick up the v2 you sent earlier, rename > the node to "cache" only, and add a cache-level = <2>; property while > applying. Per my understanding of the code, drivers/base/cacheinfo.c and related source code, the "cache-level" property isn't used at all, so could you please only rename the node to "cache"? Thanks in advance, Jisheng