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