linux-kernel - Re: [PATCH] arm64: dts: berlin4ct: Add L2 cache topology

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20160708140556.4ae8abaa@xhacker>
Date:	Fri, 8 Jul 2016 14:05:56 +0800
From:	Jisheng Zhang <jszhang@...vell.com>
To:	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
CC:	<robh+dt@...nel.org>, <mark.rutland@....com>,
	<catalin.marinas@....com>, <will.deacon@....com>,
	<linux-arm-kernel@...ts.infradead.org>,
	<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] arm64: dts: berlin4ct: Add L2 cache topology

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 <jszhang@...vell.com>
> >>> ---
> >>>  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