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

From: Jeremy Linton
Date: Fri Jan 19 2018 - 18:27:38 EST


Hi,

On 01/18/2018 04:14 AM, Sudeep Holla wrote:


On 17/01/18 18:51, Jeremy Linton wrote:
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.


Looks like I was missing to understand 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.

Indeed. I am trying to avoid use of cache_of_set_props outside.
How about skipping setting up of fw_unique in ci_leaf_init instead ?


I've been thinking about this, but I'm hesitant because I don't have a good test platform for this code. Plus, I'm not 100% sure that the results are the same (is it possible that the platform setup node isn't the same as the one the the common code would find?).

I also think I'm getting a little off topic with these patches in relation to what the core goal is (adding PPTT parsing).