PM /Domain/OPP: Add support to get performance state from DT

The "operating-points-v2" property can contain a list of phandles now, specifically for the power domain providers that provide multiple domains.

Add support to parse that.

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/opp/of.c       | 50 +++++++++++++++++++++++++++++++++++++++++---------
 include/linux/pm_opp.h |  6 ++++++
 2 files changed, 47 insertions(+), 9 deletions(-)

drivers/opp/of.c +41 -9

index cb716aa2f44b..22c9bd191f62 100644
@@ -250,20 +250,17 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table);
 
 /* Returns opp descriptor node for a device node, caller must
  * do of_node_put() */
-static struct device_node *_opp_of_get_opp_desc_node(struct device_node *np)
+static struct device_node *_opp_of_get_opp_desc_node(struct device_node *np,
+                                                    int index)
 {
-       /*
-        * There should be only ONE phandle present in "operating-points-v2"
-        * property.
-        */
-
-       return of_parse_phandle(np, "operating-points-v2", 0);
+       /* "operating-points-v2" can be an array for power domain providers */
+       return of_parse_phandle(np, "operating-points-v2", index);
 }
 
 /* Returns opp descriptor node for a device, caller must do of_node_put() */
 struct device_node *dev_pm_opp_of_get_opp_desc_node(struct device *dev)
 {
-       return _opp_of_get_opp_desc_node(dev->of_node);
+       return _opp_of_get_opp_desc_node(dev->of_node, 0);
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_of_get_opp_desc_node);
 

@@ -509,6 +506,41 @@ int dev_pm_opp_of_add_table(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_of_add_table);
 
+/**
+ * dev_pm_opp_of_add_table_indexed() - Initialize indexed opp table from device tree
+ * @dev:       device pointer used to lookup OPP table.
+ * @index:     Index number.
+ *
+ * Register the initial OPP table with the OPP library for given device only
+ * using the "operating-points-v2" property.
+ *
+ * Return:
+ * 0           On success OR
+ *             Duplicate OPPs (both freq and volt are same) and opp->available
+ * -EEXIST     Freq are same and volt are different OR
+ *             Duplicate OPPs (both freq and volt are same) and !opp->available
+ * -ENOMEM     Memory allocation failure
+ * -ENODEV     when 'operating-points' property is not found or is invalid data
+ *             in device node.
+ * -ENODATA    when empty 'operating-points' property is found
+ * -EINVAL     when invalid entries are found in opp-v2 table
+ */
+int dev_pm_opp_of_add_table_indexed(struct device *dev, int index)
+{
+       struct device_node *opp_np;
+       int ret;
+
+       opp_np = _opp_of_get_opp_desc_node(dev->of_node, index);
+       if (!opp_np)
+               return -ENODEV;
+
+       ret = _of_add_opp_table_v2(dev, opp_np);
+       of_node_put(opp_np);
+
+       return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_of_add_table_indexed);
+
 /* CPU device specific helpers */
 
 /**

@@ -613,7 +645,7 @@ int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev,
                }
 
                /* Get OPP descriptor node */
-               tmp_np = _opp_of_get_opp_desc_node(cpu_np);
+               tmp_np = _opp_of_get_opp_desc_node(cpu_np, 0);
                of_node_put(cpu_np);
                if (!tmp_np) {
                        pr_err("%pOF: Couldn't find opp node\n", cpu_np);

include/linux/pm_opp.h +6 -1

index 6c2d2e88f066..f042fdeaaa3c 100644
@@ -303,6 +303,7 @@ static inline void dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask
 
 #if defined(CONFIG_PM_OPP) && defined(CONFIG_OF)
 int dev_pm_opp_of_add_table(struct device *dev);
+int dev_pm_opp_of_add_table_indexed(struct device *dev, int index);
 void dev_pm_opp_of_remove_table(struct device *dev);
 int dev_pm_opp_of_cpumask_add_table(const struct cpumask *cpumask);
 void dev_pm_opp_of_cpumask_remove_table(const struct cpumask *cpumask);

@@ -314,6 +315,11 @@ static inline int dev_pm_opp_of_add_table(struct device *dev)
        return -ENOTSUPP;
 }
 
+static inline int dev_pm_opp_of_add_table_indexed(struct device *dev, int index)
+{
+       return -ENOTSUPP;
+}
+
 static inline void dev_pm_opp_of_remove_table(struct device *dev)
 {
 }
--

2.15.0.194.g9af6a3dea062

A device's DT node or its OPP nodes can contain a phandle to other device's OPP node, in the "required-opp" property.

This patch implements a routine to find that required OPP from the node that contains the "required-opp" property.

Signed-off-by Viresh Kumar <[email protected]>

The power-domain core would be using the OPP core going forward and the OPP core has the basic requirement of a device structure for its working.

Add a struct device to the genpd structure and also add a genpd bus type for the devices.

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/base/power/domain.c | 37 +++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h   |  1 +
 2 files changed, 38 insertions(+)

drivers/base/power/domain.c +0 -0

index 0c80bea05bcb..013c1e206167 100644

drivers/base/power/domain.c +28 -0

+       if (ret) {
+               dev_err(&genpd->dev, "failed to add device: %d\n", ret);
+               put_device(&genpd->dev);
+               kfree(genpd->free);
+               return ret;
+       }
+
        mutex_lock(&gpd_list_lock);
        list_add(&genpd->gpd_list_node, &gpd_list);
        mutex_unlock(&gpd_list_lock);

@@ -1724,6 +1740,7 @@ static int genpd_remove(struct generic_pm_domain *genpd)
 
        list_del(&genpd->gpd_list_node);
        genpd_unlock(genpd);
+       device_del(&genpd->dev);
        cancel_work_sync(&genpd->power_off_work);
        kfree(genpd->free);
        pr_debug("%s: removed %s\n", __func__, genpd->name);

@@ -1888,6 +1905,7 @@ int of_genpd_add_provider_simple(struct device_node *np,
                if (!ret) {
                        genpd->provider = &np->fwnode;
                        genpd->has_provider = true;
+                       genpd->dev.of_node = np;
                }
        }
 

@@ -1924,6 +1942,7 @@ int of_genpd_add_provider_onecell(struct device_node *np,
 
                data->domains[i]->provider = &np->fwnode;
                data->domains[i]->has_provider = true;
+               data->domains[i]->dev.of_node = np;
        }
 
        ret = genpd_add_provider(np, data->xlate, data);

@@ -2691,3 +2710,21 @@ static void __exit genpd_debug_exit(void)
 }
 __exitcall(genpd_debug_exit);
 #endif /* CONFIG_DEBUG_FS */
+
+static int __init pm_genpd_core_init(void)
+{
+       int ret;
+
+       ret = bus_register(&genpd_bus_type);
+       if (ret)
+               pr_err("bus_register failed (%d)\n", ret);
+
+       return ret;
+}
+pure_initcall(pm_genpd_core_init);
+
+static void __exit pm_genpd_core_exit(void)
+{
+       bus_unregister(&genpd_bus_type);
+}
+__exitcall(pm_genpd_core_exit);

include/linux/pm_domain.h +1 -1

index 04dbef9847d3..aaacaa35005d 100644
@@ -49,6 +49,7 @@ struct genpd_power_state {
 struct genpd_lock_ops;
 
 struct generic_pm_domain {
+       struct device dev;
        struct dev_pm_domain domain;    /* PM domain operations */
        struct list_head gpd_list_node; /* Node in the global PM domains list */
        struct list_head master_links;  /* Links with PM domain as a master */
--

2.15.0.194.g9af6a3dea062

Quoted 2 lines · Viresh Kumar

The power-domain core would be using the OPP core going forward and the OPP core has the basic requirement of a device structure for its working.

According to the OPP core also seems to require the ->dev.of_node to be set. Actually, it seems like that part belongs in patch4 instead, while starting to make use of the opp OF APIs. Could you please move it?

drivers/base/power/domain.c static void genpd_lock_init(struct generic_pm_domain *genpd) · line 1635

Quoted 3 lines

drivers/base/power/domain.c +3 -0

+static struct bus_type genpd_bus_type = {
+       .name =         "genpd",
+};

This seems silly. Can't we just avoid having a bus type altogether, thus no need to call bus_register() as well?

Quoted 8 lines

drivers/base/power/domain.c +1 -0

+
 /**
  * pm_genpd_init - Initialize a generic I/O PM domain object.
  * @genpd: PM domain object to initialize.

@@ -1687,6 +1691,18 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
                        return ret;
        }
 

drivers/base/power/domain.c int pm_genpd_init(struct generic_pm_domain *genpd, · line 1698

Quoted 5 lines

drivers/base/power/domain.c +5 -0

+       genpd->dev.bus = &genpd_bus_type;
+       device_initialize(&genpd->dev);
+       dev_set_name(&genpd->dev, "%s", genpd->name);
+
+       ret = device_add(&genpd->dev);

What's the point of adding the device? Can we skip this step, as I guess the opp core is only using the device as cookie rather actual using it? No?

Quoted 4 lines · Ulf Hansson

Quoted 2 lines · Viresh Kumar

The power-domain core would be using the OPP core going forward and the OPP core has the basic requirement of a device structure for its working.

Quoted 4 lines

According to the OPP core also seems to require the ->dev.of_node to be set. Actually, it seems like that part belongs in patch4 instead, while starting to make use of the opp OF APIs. Could you please move it?

You meaning setting of the of_node to the next patch? I can do that if that;s what you want.

Quoted 7 lines

drivers/base/power/domain.c static void genpd_lock_init(struct generic_pm_domain *genpd) · line 1635

Quoted 4 lines

drivers/base/power/domain.c +3 -0

+static struct bus_type genpd_bus_type = {
+       .name =         "genpd",
+};

This seems silly. Can't we just avoid having a bus type altogether, thus no need to call bus_register() as well?

I thought we need a bus where we want to add the device, haven't tried with that pointer being empty. Will try that and let you know.

Quoted 9 lines

Quoted 9 lines

drivers/base/power/domain.c +1 -0

+
 /**
  * pm_genpd_init - Initialize a generic I/O PM domain object.
  * @genpd: PM domain object to initialize.

@@ -1687,6 +1691,18 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
                        return ret;
        }
 

Quoted 9 lines

drivers/base/power/domain.c int pm_genpd_init(struct generic_pm_domain *genpd, · line 1698

Quoted 5 lines

drivers/base/power/domain.c +5 -0

+       genpd->dev.bus = &genpd_bus_type;
+       device_initialize(&genpd->dev);
+       dev_set_name(&genpd->dev, "%s", genpd->name);
+
+       ret = device_add(&genpd->dev);

What's the point of adding the device? Can we skip this step, as I guess the opp core is only using the device as cookie rather actual using it? No?

We also use it for the OPP debugfs stuff, so that would be required I believe.

We also use it for playing with clk/regulator stuff as well, though we may not use it in genpd case for now.

--
viresh

Quoted 9 lines · Viresh Kumar

Quoted 7 lines · Ulf Hansson

Quoted 1 line · Viresh Kumar

+ ret = device_add(&genpd->dev);

What's the point of adding the device? Can we skip this step, as I guess the opp core is only using the device as cookie rather actual using it? No?

Quoted 1 line

We also use it for the OPP debugfs stuff, so that would be required I believe.

Right, however, isn't that only using the dev_name(dev), which you don't need to add the device to make use of.

Or maybe I missing something around this...

Quoted 15 lines · Ulf Hansson

Quoted 10 lines · Viresh Kumar

Quoted 7 lines · Ulf Hansson

Quoted 1 line · Viresh Kumar

+ ret = device_add(&genpd->dev);

What's the point of adding the device? Can we skip this step, as I guess the opp core is only using the device as cookie rather actual using it? No?

We also use it for the OPP debugfs stuff, so that would be required I believe.

Right, however, isn't that only using the dev_name(dev), which you don't need to add the device to make use of.

Quoted 1 line

Or maybe I missing something around this...

So I tested this bit. The code works fine even if the device isn't added (registered), but this looks a bit sloppy to attempt.

Please let me know what would you prefer in this case, add the device or not.

Parse the OPP table for power domains if they have their set_performance_state() callback set.

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/base/power/domain.c | 78 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 62 insertions(+), 16 deletions(-)

drivers/base/power/domain.c +53 -14

index 013c1e206167..1ad4ad0b0de0 100644
@@ -10,6 +10,7 @@
 #include <linux/kernel.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
+#include <linux/pm_opp.h>
 #include <linux/pm_runtime.h>
 #include <linux/pm_domain.h>
 #include <linux/pm_qos.h>

@@ -1900,15 +1901,33 @@ int of_genpd_add_provider_simple(struct device_node *np,
 
        mutex_lock(&gpd_list_lock);
 
-       if (genpd_present(genpd)) {
-               ret = genpd_add_provider(np, genpd_xlate_simple, genpd);
-               if (!ret) {
-                       genpd->provider = &np->fwnode;
-                       genpd->has_provider = true;
-                       genpd->dev.of_node = np;
+       if (!genpd_present(genpd))
+               goto unlock;
+
+       genpd->dev.of_node = np;
+
+       /* Parse genpd OPP table */
+       if (genpd->set_performance_state) {
+               ret = dev_pm_opp_of_add_table(&genpd->dev);
+               if (ret) {
+                       dev_err(&genpd->dev, "Failed to add OPP table: %d\n",
+                               ret);
+                       goto unlock;
                }
        }
 
+       ret = genpd_add_provider(np, genpd_xlate_simple, genpd);
+       if (ret) {
+               if (genpd->set_performance_state)
+                       dev_pm_opp_of_remove_table(&genpd->dev);
+
+               goto unlock;
+       }
+
+       genpd->provider = &np->fwnode;
+       genpd->has_provider = true;
+
+unlock:
        mutex_unlock(&gpd_list_lock);
 
        return ret;

@@ -1923,6 +1942,7 @@ EXPORT_SYMBOL_GPL(of_genpd_add_provider_simple);
 int of_genpd_add_provider_onecell(struct device_node *np,
                                  struct genpd_onecell_data *data)
 {
+       struct generic_pm_domain *genpd;
        unsigned int i;
        int ret = -EINVAL;
 

@@ -1935,14 +1955,27 @@ int of_genpd_add_provider_onecell(struct device_node *np,
                data->xlate = genpd_xlate_onecell;
 
        for (i = 0; i < data->num_domains; i++) {
-               if (!data->domains[i])
+               genpd = data->domains[i];
+
+               if (!genpd)
                        continue;
-               if (!genpd_present(data->domains[i]))
+               if (!genpd_present(genpd))
                        goto error;
 
-               data->domains[i]->provider = &np->fwnode;
-               data->domains[i]->has_provider = true;
-               data->domains[i]->dev.of_node = np;
+               genpd->dev.of_node = np;
+
+               /* Parse genpd OPP table */
+               if (genpd->set_performance_state) {
+                       ret = dev_pm_opp_of_add_table_indexed(&genpd->dev, i);
+                       if (ret) {
+                               dev_err(&genpd->dev, "Failed to add OPP table for index %d: %d\n",
+                                       i, ret);
+                               goto error;
+                       }
+               }
+
+               genpd->provider = &np->fwnode;
+               genpd->has_provider = true;
        }
 
        ret = genpd_add_provider(np, data->xlate, data);

@@ -1955,10 +1988,16 @@ int of_genpd_add_provider_onecell(struct device_node *np,
 
 error:
        while (i--) {
-               if (!data->domains[i])
+               genpd = data->domains[i];
+
+               if (!genpd)
                        continue;
-               data->domains[i]->provider = NULL;
-               data->domains[i]->has_provider = false;
+
+               genpd->provider = NULL;
+               genpd->has_provider = false;
+
+               if (genpd->set_performance_state)
+                       dev_pm_opp_of_remove_table(&genpd->dev);
        }
 
        mutex_unlock(&gpd_list_lock);

@@ -1985,10 +2024,17 @@ void of_genpd_del_provider(struct device_node *np)
                         * provider, set the 'has_provider' to false

drivers/base/power/domain.c +5 -1

+
+                                       dev_pm_opp_of_remove_table(&gpd->dev);
+                               }
+                       }
+
                        list_del(&cp->link);
                        of_node_put(cp->node);
                        kfree(cp);
--

2.15.0.194.g9af6a3dea062

Nitpick, perhaps change this to:

if (gpd->set_performance_state)

       dev_pm_opp_of_remove_table(&gpd->dev);

Quoted 4 lines · Ulf Hansson

Nitpick, perhaps change this to:

if (gpd->set_performance_state)

       dev_pm_opp_of_remove_table(&gpd->dev);

I probably did that to save the 80 column thing. So crossing 80 columns in this case is fine ?

This implements of_dev_pm_genpd_get_performance_state() which can be used from the device drivers or the OPP core to find the performance state encoded in the "required-opp" property of a node. Different

platforms may encode the performance state differently using the OPP table (they may simply return value of opp-hz or opp-microvolt, or apply some algorithm on top of those values) and so a new callback is implemented to allow platform specific drivers to convert the power domain OPP to a performance state.

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/base/power/domain.c | 48 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h   | 11 +++++++++++
 2 files changed, 59 insertions(+)

drivers/base/power/domain.c +8 -0

index 1ad4ad0b0de0..ef43b75982fa 100644
@@ -2415,6 +2415,54 @@ int of_genpd_parse_idle_states(struct device_node *dn,
 }
 EXPORT_SYMBOL_GPL(of_genpd_parse_idle_states);
 
+/**
+ * of_dev_pm_genpd_get_performance_state- Gets performance state of device's
+ * power domain corresponding to a DT node's "required-opp" property.
+ *
+ * @dev: Device for which the performance-state needs to be found.
+ * @np: DT node where the "required-opp" property is present. This can be
+ *     the device node itself (if it doesn't have an OPP table) or a node
+ *     within the OPP table of a device (if device has an OPP table).

drivers/base/power/domain.c +30 -0

+{
+       struct generic_pm_domain *genpd;
+       struct dev_pm_opp *opp;
+       int state = 0;
+
+       genpd = dev_to_genpd(dev);
+       if (IS_ERR(genpd))
+               return 0;
+
+       if (unlikely(!genpd->set_performance_state))
+               return 0;
+
+       genpd_lock(genpd);
+
+       opp = of_dev_pm_opp_find_required_opp(&genpd->dev, np);
+       if (IS_ERR(opp)) {
+               state = PTR_ERR(opp);
+               goto unlock;
+       }
+
+       state = genpd->get_performance_state(genpd, opp);
+       dev_pm_opp_put(opp);
+
+unlock:
+       genpd_unlock(genpd);
+
+       return state;
+}
+EXPORT_SYMBOL_GPL(of_dev_pm_genpd_get_performance_state);
+
 #endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
 
 

include/linux/pm_domain.h +1 -0

index aaacaa35005d..4edbdaa54cec 100644
@@ -47,6 +47,7 @@ struct genpd_power_state {
 };
 
 struct genpd_lock_ops;
+struct dev_pm_opp;
 
 struct generic_pm_domain {
        struct device dev;

include/linux/pm_domain.h +8 -1

        int (*set_performance_state)(struct generic_pm_domain *genpd,
                                     unsigned int state);
        struct gpd_dev_ops dev_ops;

@@ -244,6 +247,8 @@ extern int of_genpd_add_subdomain(struct of_phandle_args *parent,
 extern struct generic_pm_domain *of_genpd_remove_last(struct device_node *np);
 extern int of_genpd_parse_idle_states(struct device_node *dn,
                        struct genpd_power_state **states, int *n);
+extern unsigned int of_dev_pm_genpd_get_performance_state(struct device *dev,
+                               struct device_node *np);
 
 int genpd_dev_pm_attach(struct device *dev);
 #else /* !CONFIG_PM_GENERIC_DOMAINS_OF */

@@ -279,6 +284,12 @@ static inline int of_genpd_parse_idle_states(struct device_node *dn,
        return -ENODEV;
 }
 
+static inline unsigned int of_dev_pm_genpd_get_performance_state(struct device *dev,
+                                       struct device_node *np)
+{
+       return -ENODEV;
+}
+
 static inline int genpd_dev_pm_attach(struct device *dev)
 {
        return -ENODEV;
--

2.15.0.194.g9af6a3dea062

The genpd core provides an API now to retrieve the performance state from DT, use that instead of the ->get_pstate() callback.

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/opp/core.c |  3 ---
 drivers/opp/of.c   | 20 +++++++++++++++++++-
 2 files changed, 19 insertions(+), 4 deletions(-)

drivers/opp/core.c +0 -3

index bfcad8983fd1..6194219fb95f 100644
@@ -1034,9 +1034,6 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
                return ret;
        }
 
-       if (opp_table->get_pstate)
-               new_opp->pstate = opp_table->get_pstate(dev, new_opp->rate);
-
        list_add(&new_opp->node, head);
        mutex_unlock(&opp_table->lock);
 

drivers/opp/of.c +19 -2

index 2d1bb348ae73..21265af55117 100644
@@ -17,6 +17,7 @@
 #include <linux/errno.h>
 #include <linux/device.h>
 #include <linux/of_device.h>
+#include <linux/pm_domain.h>
 #include <linux/slab.h>
 #include <linux/export.h>
 

@@ -321,6 +322,8 @@ static int _opp_add_static_v2(struct opp_table *opp_table, struct device *dev,
        if (!of_property_read_u32(np, "clock-latency-ns", &val))
                new_opp->clock_latency_ns = val;
 
+       new_opp->pstate = of_dev_pm_genpd_get_performance_state(dev, np);
+
        ret = opp_parse_supplies(new_opp, dev, opp_table);
        if (ret)
                goto free_opp;

@@ -371,7 +374,8 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
 {
        struct device_node *np;
        struct opp_table *opp_table;
-       int ret = 0, count = 0;
+       int ret = 0, count = 0, pstate_count = 0;
+       struct dev_pm_opp *opp;
 
        opp_table = _managed_opp(opp_np);
        if (opp_table) {

@@ -405,6 +409,20 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
                goto put_opp_table;
        }
 
+       list_for_each_entry(opp, &opp_table->opp_list, node)
+               pstate_count += !!opp->pstate;
+
+       /* Either all or none of the nodes shall have performance state set */
+       if (pstate_count && pstate_count != count) {
+               dev_err(dev, "Not all nodes have performance state set (%d: %d)\n",
+                       count, pstate_count);
+               ret = -ENOENT;
+               goto put_opp_table;
+       }
+
+       if (pstate_count)
+               opp_table->genpd_performance_state = true;
+
        opp_table->np = opp_np;
        if (of_property_read_bool(opp_np, "opp-shared"))
                opp_table->shared_opp = OPP_TABLE_ACCESS_SHARED;
--

2.15.0.194.g9af6a3dea062

These helpers aren't used anymore, remove them.

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/opp/core.c     | 75 --------------------------------------------------
 drivers/opp/opp.h      |  2 --
 include/linux/pm_opp.h | 10 -------
 3 files changed, 87 deletions(-)

drivers/opp/core.c +0 -75

index 6194219fb95f..9e3c40437991 100644
@@ -1545,81 +1545,6 @@ void dev_pm_opp_unregister_set_opp_helper(struct opp_table *opp_table)
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_unregister_set_opp_helper);
 
-/**
- * dev_pm_opp_register_get_pstate_helper() - Register get_pstate() helper.
- * @dev: Device for which the helper is getting registered.
- * @get_pstate: Helper.
- *
- * TODO: Remove this callback after the same information is available via Device
- * Tree.
- *
- * This allows a platform to initialize the performance states of individual
- * OPPs for its devices, until we get similar information directly from DT.
- *
- * This must be called before the OPPs are initialized for the device.
- */
-struct opp_table *dev_pm_opp_register_get_pstate_helper(struct device *dev,
-               int (*get_pstate)(struct device *dev, unsigned long rate))
-{
-       struct opp_table *opp_table;
-       int ret;
-
-       if (!get_pstate)
-               return ERR_PTR(-EINVAL);
-
-       opp_table = dev_pm_opp_get_opp_table(dev);
-       if (!opp_table)
-               return ERR_PTR(-ENOMEM);
-
-       /* This should be called before OPPs are initialized */
-       if (WARN_ON(!list_empty(&opp_table->opp_list))) {
-               ret = -EBUSY;
-               goto err;
-       }
-
-       /* Already have genpd_performance_state set */
-       if (WARN_ON(opp_table->genpd_performance_state)) {
-               ret = -EBUSY;
-               goto err;
-       }
-
-       opp_table->genpd_performance_state = true;
-       opp_table->get_pstate = get_pstate;
-
-       return opp_table;
-
-err:
-       dev_pm_opp_put_opp_table(opp_table);
-
-       return ERR_PTR(ret);
-}
-EXPORT_SYMBOL_GPL(dev_pm_opp_register_get_pstate_helper);
-
-/**
- * dev_pm_opp_unregister_get_pstate_helper() - Releases resources blocked for
- *                                        get_pstate() helper
- * @opp_table: OPP table returned from dev_pm_opp_register_get_pstate_helper().
- *
- * Release resources blocked for platform specific get_pstate() helper.
- */
-void dev_pm_opp_unregister_get_pstate_helper(struct opp_table *opp_table)
-{
-       if (!opp_table->genpd_performance_state) {
-               pr_err("%s: Doesn't have performance states set\n",
-                      __func__);
-               return;
-       }
-
-       /* Make sure there are no concurrent readers while updating opp_table */
-       WARN_ON(!list_empty(&opp_table->opp_list));
-
-       opp_table->genpd_performance_state = false;
-       opp_table->get_pstate = NULL;
-
-       dev_pm_opp_put_opp_table(opp_table);
-}
-EXPORT_SYMBOL_GPL(dev_pm_opp_unregister_get_pstate_helper);
-
 /**
  * dev_pm_opp_add()  - Add an OPP table from a table definitions
  * @dev:       device for which we do this operation

drivers/opp/opp.h +0 -2

index b181032e6938..ae99295b5382 100644
@@ -140,7 +140,6 @@ enum opp_table_access {
  * @genpd_performance_state: Device's power domain support performance state.
  * @set_opp: Platform specific set_opp callback
  * @set_opp_data: Data to be passed to set_opp callback
- * @get_pstate: Platform specific get_pstate callback
  * @dentry:    debugfs dentry pointer of the real device directory (not links).
  * @dentry_name: Name of the real dentry.
  *

@@ -178,7 +177,6 @@ struct opp_table {
 
        int (*set_opp)(struct dev_pm_set_opp_data *data);
        struct dev_pm_set_opp_data *set_opp_data;
-       int (*get_pstate)(struct device *dev, unsigned long rate);
 
 #ifdef CONFIG_DEBUG_FS
        struct dentry *dentry;

include/linux/pm_opp.h +0 -11

index 70686f434c13..528a7d9cf2ef 100644
@@ -125,8 +125,6 @@ struct opp_table *dev_pm_opp_set_clkname(struct device *dev, const char * name);
 void dev_pm_opp_put_clkname(struct opp_table *opp_table);
 struct opp_table *dev_pm_opp_register_set_opp_helper(struct device *dev, int (*set_opp)(struct dev_pm_set_opp_data *data));
 void dev_pm_opp_unregister_set_opp_helper(struct opp_table *opp_table);
-struct opp_table *dev_pm_opp_register_get_pstate_helper(struct device *dev, int (*get_pstate)(struct device *dev, unsigned long rate));
-void dev_pm_opp_unregister_get_pstate_helper(struct opp_table *opp_table);
 int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
 int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask);
 int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask);

@@ -247,14 +245,6 @@ static inline struct opp_table *dev_pm_opp_register_set_opp_helper(struct device
 
 static inline void dev_pm_opp_unregister_set_opp_helper(struct opp_table *opp_table) {}
 
-static inline struct opp_table *dev_pm_opp_register_get_pstate_helper(struct device *dev,
-               int (*get_pstate)(struct device *dev, unsigned long rate))
-{
-       return ERR_PTR(-ENOTSUPP);
-}
-
-static inline void dev_pm_opp_unregister_get_pstate_helper(struct opp_table *opp_table) {}
-
 static inline struct opp_table *dev_pm_opp_set_prop_name(struct device *dev, const char *name)
 {
        return ERR_PTR(-ENOTSUPP);
--

2.15.0.194.g9af6a3dea062

Hi,

Now that the DT bindings [1] are already Reviewed/Acked by respective maintainers, here is the code to start using them.

The first two patches provide helpers in the OPP core, [3-5]/7 update the PM domain core to start supporting domain OPP tables, etc, 6/7 updates the OPP core to use the new callback provided by the PM domains to get performance state and the last one removes the unused helpers now.

This is tested on Hikey620 and works just fine.

Quoted 11 lines · Viresh Kumar

Hi,

Now that the DT bindings [1] are already Reviewed/Acked by respective maintainers, here is the code to start using them.

The first two patches provide helpers in the OPP core, [3-5]/7 update the PM domain core to start supporting domain OPP tables, etc, 6/7 updates the OPP core to use the new callback provided by the PM domains to get performance state and the last one removes the unused helpers now.

Quoted 1 line

This is tested on Hikey620 and works just fine.

Ping !

Quoted 12 lines · CET Viresh Kumar

Quoted 11 lines · Viresh Kumar

Hi,

Now that the DT bindings [1] are already Reviewed/Acked by respective maintainers, here is the code to start using them.

The first two patches provide helpers in the OPP core, [3-5]/7 update the PM domain core to start supporting domain OPP tables, etc, 6/7 updates the OPP core to use the new callback provided by the PM domains to get performance state and the last one removes the unused helpers now.

Quoted 3 lines

Quoted 1 line

This is tested on Hikey620 and works just fine.

Ping !

Well, whom are you pinging exactly and why?