All of lore.kernel.org
 help / color / mirror / Atom feed
From: jszhang@marvell.com (Jisheng Zhang)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm64: dts: berlin4ct: Add L2 cache topology
Date: Fri, 8 Jul 2016 14:05:56 +0800	[thread overview]
Message-ID: <20160708140556.4ae8abaa@xhacker> (raw)
In-Reply-To: <577E8D02.5010401@gmail.com>

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@marvell.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

WARNING: multiple messages have this Message-ID (diff)
From: Jisheng Zhang <jszhang@marvell.com>
To: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
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
Subject: Re: [PATCH] arm64: dts: berlin4ct: Add L2 cache topology
Date: Fri, 8 Jul 2016 14:05:56 +0800	[thread overview]
Message-ID: <20160708140556.4ae8abaa@xhacker> (raw)
In-Reply-To: <577E8D02.5010401@gmail.com>

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@marvell.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

WARNING: multiple messages have this Message-ID (diff)
From: Jisheng Zhang <jszhang@marvell.com>
To: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
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>
Subject: Re: [PATCH] arm64: dts: berlin4ct: Add L2 cache topology
Date: Fri, 8 Jul 2016 14:05:56 +0800	[thread overview]
Message-ID: <20160708140556.4ae8abaa@xhacker> (raw)
In-Reply-To: <577E8D02.5010401@gmail.com>

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@marvell.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

  reply	other threads:[~2016-07-08  6:05 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-16  8:40 [PATCH] arm64: dts: berlin4ct: Add L2 cache topology Jisheng Zhang
2016-06-16  8:40 ` Jisheng Zhang
2016-06-16  8:40 ` Jisheng Zhang
2016-07-06 17:49 ` Sebastian Hesselbarth
2016-07-06 17:49   ` Sebastian Hesselbarth
2016-07-06 17:49   ` Sebastian Hesselbarth
2016-07-07  5:48   ` Jisheng Zhang
2016-07-07  5:48     ` Jisheng Zhang
2016-07-07  5:48     ` Jisheng Zhang
2016-07-07 17:10     ` Sebastian Hesselbarth
2016-07-07 17:10       ` Sebastian Hesselbarth
2016-07-07 17:10       ` Sebastian Hesselbarth
2016-07-08  6:05       ` Jisheng Zhang [this message]
2016-07-08  6:05         ` Jisheng Zhang
2016-07-08  6:05         ` Jisheng Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160708140556.4ae8abaa@xhacker \
    --to=jszhang@marvell.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.