Re: [PATCH v6 02/12] drivers: base: cacheinfo: setup DT cache properties early

From: Jeremy Linton
Date: Wed Jan 17 2018 - 13:51:39 EST


Hi,

On 01/17/2018 12:20 PM, Sudeep Holla wrote:


On 16/01/18 21:07, Jeremy Linton wrote:
Hi,

On 01/15/2018 06:33 AM, Sudeep Holla wrote:
On Fri, Jan 12, 2018 at 06:59:10PM -0600, Jeremy Linton wrote:
The original intent in cacheinfo was that an architecture
specific populate_cache_leaves() would probe the hardware
and then cache_shared_cpu_map_setup() and
cache_override_properties() would provide firmware help to
extend/expand upon what was probed. Arm64 was really
the only architecture that was working this way, and
with the removal of most of the hardware probing logic it
became clear that it was possible to simplify the logic a bit.

This patch combines the walk of the DT nodes with the
code updating the cache size/line_size and nr_sets.
cache_override_properties() (which was DT specific) is
then removed. The result is that cacheinfo.of_node is
no longer used as a temporary place to hold DT references
for future calls that update cache properties. That change
helps to clarify its one remaining use (matching
cacheinfo nodes that represent shared caches) which
will be used by the ACPI/PPTT code in the following patches.

Cc: Palmer Dabbelt <palmer@xxxxxxxxxx>
Cc: Albert Ou <albert@xxxxxxxxxx>
Signed-off-by: Jeremy Linton <jeremy.linton@xxxxxxx>
---
 arch/riscv/kernel/cacheinfo.c | 1 +
 drivers/base/cacheinfo.c | 65
+++++++++++++++++++------------------------
 include/linux/cacheinfo.h | 1 +
 3 files changed, 31 insertions(+), 36 deletions(-)

diff --git a/arch/riscv/kernel/cacheinfo.c
b/arch/riscv/kernel/cacheinfo.c
index 10ed2749e246..6f4500233cf8 100644
--- a/arch/riscv/kernel/cacheinfo.c
+++ b/arch/riscv/kernel/cacheinfo.c
@@ -30,6 +30,7 @@ static void ci_leaf_init(struct cacheinfo *this_leaf,
ÂÂÂÂÂÂÂÂÂ CACHE_WRITE_BACK
ÂÂÂÂÂÂÂÂÂ | CACHE_READ_ALLOCATE
ÂÂÂÂÂÂÂÂÂ | CACHE_WRITE_ALLOCATE;
+ÂÂÂ cache_of_set_props(this_leaf, node);

This may be necessary but can it be done as later patch ? So far nothing
is added that may break riscv IIUC.

Well I think you have a bisection issue where the additional information
will disappear between this patch and wherever we put this code back in.


Hmm, I am sorry but I fail to see the issue. Before this change,
populate_cache_leaves just populated the info as per ci_leaf_init in
arch/riscv/kernel/cacheinfo.c and cache_override_properties used to fill
the remaining.

After this patch, the same is achieved in cache_shared_cpu_map_setup.

In both case, it was by the end of detect_cache_attributes, so I see no
issue.



Hi,

I must be misunderstanding something.

AFAIK, The code in cache_setup_of_node() won't call cache_of_set_props() because it returns when there is an existing of_node (fw_unique) created by the riscv populate_cache_leaves(). That's why I'm making the direct call here. If we fail to get that change in place before cache_override_properties() is removed then the fields not set by the riscv code (size/etc) will be missing.