[PATCH RFC 03/16] OPP: Retry failed default single-clock transitions

[Konrad Dybcio]

From: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>

A failed OPP transition need not leave the clock at the last successful
OPP's frequency. If hardware changed before the error, the default
single-clock path can subsequently skip a necessary retry because its
cached OPP matches the request. It can also choose voltage and dependency
ordering relative to the wrong operating point.

Treat an incomplete transition as dirty and use the observed clock rate
to establish retry direction. Do not trust same-OPP or same-rate
shortcuts, or allow table disable, until the state has been reconciled.

Limit this to the default single-clock path: custom callbacks can have
additional state which cannot be recovered from a clock-rate readback.

Assisted-by: LLM
Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
---
 drivers/opp/core.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++-------
 drivers/opp/opp.h  |  2 ++
 2 files changed, 65 insertions(+), 9 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 4ccce7b6afcb..340911d2971e 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1073,11 +1073,11 @@ _opp_config_clk_single(struct device *dev, struct opp_table *opp_table,
 	}
 
 	ret = clk_set_rate(opp_table->clk, freq);
+	opp_table->current_rate_single_clk = clk_get_rate(opp_table->clk);
 	if (ret) {
+		opp_table->config_dirty_single_clk = true;
 		dev_err(dev, "%s: failed to set clock rate: %d\n", __func__,
 			ret);
-	} else {
-		opp_table->current_rate_single_clk = freq;
 	}
 
 	return ret;
@@ -1261,6 +1261,28 @@ static void _find_current_opp(struct device *dev, struct opp_table *opp_table)
 	opp_table->current_opp = opp;
 }
 
+static bool _opp_uses_default_single_clk(struct opp_table *opp_table)
+{
+	return opp_table->config_clks == _opp_config_clk_single &&
+	       opp_table->clk_count == 1;
+}
+
+static struct dev_pm_opp *_find_current_opp_strict(struct opp_table *opp_table)
+{
+	unsigned long freq;
+
+	if (IS_ERR(opp_table->clk))
+		return ERR_CAST(opp_table->clk);
+
+	freq = clk_get_rate(opp_table->clk);
+	if (!freq)
+		return ERR_PTR(-ERANGE);
+
+	opp_table->current_rate_single_clk = freq;
+
+	return _find_freq_ceil(opp_table, &freq);
+}
+
 static int _disable_opp_table(struct device *dev, struct opp_table *opp_table)
 {
 	int ret;
@@ -1297,20 +1319,40 @@ static int _disable_opp_table(struct device *dev, struct opp_table *opp_table)
 static int _set_opp(struct device *dev, struct opp_table *opp_table,
 		    struct dev_pm_opp *opp, void *clk_data, bool forced)
 {
+	bool default_single = _opp_uses_default_single_clk(opp_table);
+	struct dev_pm_opp *actual_opp __free(put_opp) = NULL;
 	struct dev_pm_opp *old_opp;
 	int scaling_down, ret;
 
-	if (unlikely(!opp))
-		return _disable_opp_table(dev, opp_table);
+	if (unlikely(!opp)) {
+		if (default_single && opp_table->config_dirty_single_clk)
+			return -EBUSY;
+
+		if (default_single)
+			opp_table->config_dirty_single_clk = true;
+
+		ret = _disable_opp_table(dev, opp_table);
+		if (!ret && default_single)
+			opp_table->config_dirty_single_clk = false;
+
+		return ret;
+	}
 
 	/* Find the currently set OPP if we don't know already */
 	if (unlikely(!opp_table->current_opp))
 		_find_current_opp(dev, opp_table);
 
 	old_opp = opp_table->current_opp;
+	if (default_single && opp_table->config_dirty_single_clk) {
+		actual_opp = _find_current_opp_strict(opp_table);
+		if (IS_ERR(actual_opp))
+			return PTR_ERR(actual_opp);
+		old_opp = actual_opp;
+	}
 
 	/* Return early if nothing to do */
-	if (!forced && old_opp == opp && opp_table->enabled) {
+	if (!forced && !opp_table->config_dirty_single_clk &&
+	    old_opp == opp && opp_table->enabled) {
 		dev_dbg_ratelimited(dev, "%s: OPPs are same, nothing to do\n", __func__);
 		return 0;
 	}
@@ -1324,6 +1366,9 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table,
 	if (scaling_down == -1)
 		scaling_down = 0;
 
+	if (default_single)
+		opp_table->config_dirty_single_clk = true;
+
 	/* Scaling up? Configure required OPPs before frequency */
 	if (!scaling_down) {
 		ret = _set_required_opps(dev, opp_table, opp, true);
@@ -1391,10 +1436,12 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table,
 	}
 
 	opp_table->enabled = true;
-	dev_pm_opp_put(old_opp);
+	dev_pm_opp_put(opp_table->current_opp);
 
 	/* Make sure current_opp doesn't get freed */
 	opp_table->current_opp = dev_pm_opp_get(opp);
+	if (default_single)
+		opp_table->config_dirty_single_clk = false;
 
 	return ret;
 }
@@ -1432,8 +1479,14 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 		 * equivalent to a clk_set_rate()
 		 */
 		if (!_get_opp_count(opp_table)) {
-			return opp_table->config_clks(dev, opp_table, NULL,
-						      &target_freq, false);
+			int ret;
+
+			ret = opp_table->config_clks(dev, opp_table, NULL,
+						     &target_freq, false);
+			if (!ret && _opp_uses_default_single_clk(opp_table))
+				opp_table->config_dirty_single_clk = false;
+
+			return ret;
 		}
 
 		freq = clk_round_rate(opp_table->clk, target_freq);
@@ -1460,7 +1513,8 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 		 * value of the frequency. In such a case, do not abort but
 		 * configure the hardware to the desired frequency forcefully.
 		 */
-		forced = opp_table->current_rate_single_clk != freq;
+		forced = opp_table->config_dirty_single_clk ||
+			 opp_table->current_rate_single_clk != freq;
 	}
 
 	return _set_opp(dev, opp_table, opp, &freq, forced);
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index 9eba63e01a9e..4f0ec888effa 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -172,6 +172,7 @@ enum opp_table_access {
  * @parsed_static_opps: Count of devices for which OPPs are initialized from DT.
  * @shared_opp: OPP is shared between multiple devices.
  * @current_rate_single_clk: Currently configured frequency for single clk.
+ * @config_dirty_single_clk: Whether the single clk cached state is untrustworthy
  * @current_opp: Currently configured OPP for the table.
  * @suspend_opp: Pointer to OPP to be used during device suspend.
  * @required_opp_tables: List of device OPP tables that are required by OPPs in
@@ -221,6 +222,7 @@ struct opp_table {
 	unsigned int parsed_static_opps;
 	enum opp_table_access shared_opp;
 	unsigned long current_rate_single_clk;
+	bool config_dirty_single_clk;
 	struct dev_pm_opp *current_opp;
 	struct dev_pm_opp *suspend_opp;
 

-- 
2.56.0