Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Leo Yan <leo.yan@linaro.org>
To:     Sudeep Holla <sudeep.holla@arm.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
        linux-kernel@vger.kernel.org
Cc:     Leo Yan <leo.yan@linaro.org>
Subject: [PATCH v1 1/3] arch_topology: Correct semantics for 'cap_parsing_failed'
Date:   Sun, 13 Mar 2022 13:55:10 +0800
Message-Id: <20220313055512.248571-2-leo.yan@linaro.org>
In-Reply-To: <20220313055512.248571-1-leo.yan@linaro.org>
References: <20220313055512.248571-1-leo.yan@linaro.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

As described in the DT binding document [1]: "capacity-dmips-mhz
property is all-or-nothing: if it is specified for a cpu node, it has to
be specified for every other cpu nodes, or the system will fall back to
the default capacity value for every CPU".

In other words, we can accept that cases that "capacity-dmips-mhz"
property is specified or not specified for all CPU nodes.  The only
failure is the DT binding is inconsistent for all CPUs nodes, e.g only
part of CPUs have bound the "capacity-dmips-mhz" property.

Currently kernel only considers all CPU nodes having "capacity-dmips-mhz"
property as a parsing success; for the other two cases, one is all CPU
nodes without "capacity-dmips-mhz" property, and another is inconsistent
binding crossing CPU nodes, kernel considers them as parsing failure and
set 'cap_parsing_failed' flag as true.

This patch makes more clear for the semantics of 'cap_parsing_failed',
it only takes the inconsistent binding case as parsing failure.  So it
sets the flag 'cap_parsing_failed' to true only when the array
'raw_capacity' is not NULL and current CPU node doesn't contain the
property "capacity-dmips-mhz".  It marks 'cap_parsing_failed' as
a static global variable to allow it can be used in the source file.

[1] Documentation/devicetree/bindings/arm/cpu-capacity.txt

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 drivers/base/arch_topology.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 976154140f0b..b81777ae6425 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -234,6 +234,7 @@ static int register_cpu_capacity_sysctl(void)
 subsys_initcall(register_cpu_capacity_sysctl);
 
 static int update_topology;
+static bool cap_parsing_failed;
 
 int topology_update_cpu_topology(void)
 {
@@ -291,7 +292,6 @@ void topology_normalize_cpu_scale(void)
 bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
 {
 	struct clk *cpu_clk;
-	static bool cap_parsing_failed;
 	int ret;
 	u32 cpu_capacity;
 
@@ -331,9 +331,9 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
 			pr_err("cpu_capacity: missing %pOF raw capacity\n",
 				cpu_node);
 			pr_err("cpu_capacity: partial information: fallback to 1024 for all CPUs\n");
+			cap_parsing_failed = true;
+			free_raw_capacity();
 		}
-		cap_parsing_failed = true;
-		free_raw_capacity();
 	}
 
 	return !ret;
-- 
2.25.1