[PATCH 2/2] OPP/pmdomain: Fix the assignment of the required-devs

[Ulf Hansson <ulf.hansson@linaro.org>]

The recent attempt to make genpd first lookup an OPP table for a device
that has been attached to it and then let the OPP core validate whether the
OPP table is correct, fails for some configurations.

More precisely, if a device has multiple power-domains, which all has an
OPP table associated, doesn't necessarily mean that the device's OPP table
needs multiple phandles specified in the required-opps. Instead it's
perfectly possible to use a single phandle in the required-opps, which is
typically where the current code fails to assign the correct required-dev.

To fix this problem, let's instead start by letting the OPP core find the
device node for the required OPP table and then let genpd search for a
corresponding OPP table, allowing us the find the correct required-dev to
assign for it.

Reported-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
Fixes: f0d2dcc9b087 ("OPP/pmdomain: Set the required_dev for a required OPP during genpd attach")
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/opp/core.c      | 15 +++++++-----
 drivers/pmdomain/core.c | 52 +++++++++++++++++++++++------------------
 include/linux/pm_opp.h  |  7 ++++--
 3 files changed, 43 insertions(+), 31 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 66cac7a1d9db..538612886446 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2363,7 +2363,7 @@ static void _opp_put_config_regulators_helper(struct opp_table *opp_table)
 static int _opp_set_required_dev(struct opp_table *opp_table,
 				 struct device *dev,
 				 struct device *required_dev,
-				 struct opp_table *required_opp_table)
+				 config_required_dev_t config_required_dev)
 {
 	int i;
 
@@ -2380,6 +2380,7 @@ static int _opp_set_required_dev(struct opp_table *opp_table,
 
 	for (i = 0; i < opp_table->required_opp_count; i++) {
 		struct opp_table *table = opp_table->required_opp_tables[i];
+		struct opp_table *required_opp_table;
 
 		/*
 		 * The OPP table should be available at this point. If not, it's
@@ -2396,7 +2397,9 @@ static int _opp_set_required_dev(struct opp_table *opp_table,
 		 * We need to compare the nodes for the OPP tables, rather than
 		 * the OPP tables themselves, as we may have separate instances.
 		 */
-		if (required_opp_table->np == table->np) {
+		required_opp_table = config_required_dev(required_dev,
+							 table->np);
+		if (required_opp_table) {
 			/*
 			 * The required_opp_tables parsing is not perfect, as
 			 * the OPP core does the parsing solely based on the DT
@@ -2422,8 +2425,8 @@ static int _opp_set_required_dev(struct opp_table *opp_table,
 		}
 	}
 
-	dev_err(dev, "Missing OPP table, unable to set the required dev\n");
-	return -ENODEV;
+	/* No matching OPP table found for the required_dev. */
+	return -ERANGE;
 }
 
 static void _opp_put_required_dev(struct opp_table *opp_table,
@@ -2547,10 +2550,10 @@ int dev_pm_opp_set_config(struct device *dev, struct dev_pm_opp_config *config)
 		data->flags |= OPP_CONFIG_REGULATOR;
 	}
 
-	if (config->required_dev && config->required_opp_table) {
+	if (config->required_dev && config->config_required_dev) {
 		ret = _opp_set_required_dev(opp_table, dev,
 					    config->required_dev,
-					    config->required_opp_table);
+					    config->config_required_dev);
 		if (ret < 0)
 			goto err;
 
diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index edef1a520110..0ff0b513b2a1 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -2874,20 +2874,21 @@ static void genpd_dev_pm_sync(struct device *dev)
 	genpd_queue_power_off_work(pd);
 }
 
-static struct opp_table *genpd_find_opp_table(struct generic_pm_domain *genpd,
-					      unsigned int depth)
+static struct opp_table *_genpd_find_opp_table(struct generic_pm_domain *genpd,
+					       struct device_node *np,
+					       unsigned int depth)
 {
-	struct opp_table *opp_table;
+	struct opp_table *opp_table = genpd->opp_table;
 	struct gpd_link *link;
 
-	if (genpd->opp_table)
-		return genpd->opp_table;
+	if (opp_table && (dev_pm_opp_table_to_of_node(opp_table) == np))
+		return opp_table;
 
 	list_for_each_entry(link, &genpd->child_links, child_node) {
 		struct generic_pm_domain *parent = link->parent;
 
 		genpd_lock_nested(parent, depth + 1);
-		opp_table = genpd_find_opp_table(parent, depth + 1);
+		opp_table = _genpd_find_opp_table(parent, np, depth + 1);
 		genpd_unlock(parent);
 
 		if (opp_table)
@@ -2897,12 +2898,27 @@ static struct opp_table *genpd_find_opp_table(struct generic_pm_domain *genpd,
 	return NULL;
 }
 
-static int genpd_set_required_opp_dev(struct device *dev,
-				      struct device *base_dev)
+static struct opp_table *genpd_find_opp_table(struct device *dev,
+					      struct device_node *np)
 {
 	struct generic_pm_domain *genpd = dev_to_genpd(dev);
 	struct opp_table *opp_table;
-	int ret = 0;
+
+	genpd_lock(genpd);
+	opp_table = _genpd_find_opp_table(genpd, np, 0);
+	genpd_unlock(genpd);
+
+	return opp_table;
+}
+
+static int genpd_set_required_opp_dev(struct device *dev,
+				      struct device *base_dev)
+{
+	struct dev_pm_opp_config config = {
+		.required_dev = dev,
+		.config_required_dev = genpd_find_opp_table,
+	};
+	int ret;
 
 	/* Limit support to non-providers for now. */
 	if (of_property_present(base_dev->of_node, "#power-domain-cells"))
@@ -2911,20 +2927,10 @@ static int genpd_set_required_opp_dev(struct device *dev,
 	if (!dev_pm_opp_of_has_required_opp(base_dev))
 		return 0;
 
-	genpd_lock(genpd);
-	opp_table = genpd_find_opp_table(genpd, 0);
-	genpd_unlock(genpd);
-
-	if (opp_table) {
-		struct dev_pm_opp_config config = {
-			.required_dev = dev,
-			.required_opp_table = opp_table,
-		};
-
-		ret = devm_pm_opp_set_config(base_dev, &config);
-		if (ret < 0)
-			dev_err(dev, "failed to set opp config %d\n", ret);
-	}
+	ret = devm_pm_opp_set_config(base_dev, &config);
+	ret = ret == -ERANGE ? 0 : ret;
+	if (ret < 0)
+		dev_err(dev, "failed to set opp config %d\n", ret);
 
 	return ret;
 }
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 7894e631cded..0ada4a5057c8 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -53,6 +53,9 @@ typedef int (*config_regulators_t)(struct device *dev,
 typedef int (*config_clks_t)(struct device *dev, struct opp_table *opp_table,
 			struct dev_pm_opp *opp, void *data, bool scaling_down);
 
+typedef struct opp_table *(*config_required_dev_t)(struct device *dev,
+			struct device_node *opp_table_np);
+
 /**
  * struct dev_pm_opp_config - Device OPP configuration values
  * @clk_names: Clk names, NULL terminated array.
@@ -63,7 +66,7 @@ typedef int (*config_clks_t)(struct device *dev, struct opp_table *opp_table,
  * @supported_hw_count: Number of elements in the array.
  * @regulator_names: Array of pointers to the names of the regulator, NULL terminated.
  * @required_dev: Required OPP device.
- * @required_opp_table: The corresponding required OPP table for @required_dev.
+ * @config_required_dev: Custom helper to find the required OPP table for $required_dev.
  *
  * This structure contains platform specific OPP configurations for the device.
  */
@@ -77,7 +80,7 @@ struct dev_pm_opp_config {
 	unsigned int supported_hw_count;
 	const char * const *regulator_names;
 	struct device *required_dev;
-	struct opp_table *required_opp_table;
+	config_required_dev_t config_required_dev;
 };
 
 #define OPP_LEVEL_UNSET			U32_MAX
-- 
2.34.1