diff mbox series

[V2,03/11] PM / OPP: "opp-hz" is optional for power domains

Message ID f34e16962f950af16b4602d272db96fd8dd27907.1523273291.git.viresh.kumar@linaro.org
State Accepted
Commit a1e8c13600bfd96c51580732ccf31f69bc6de4d1
Headers show
Series PM / genpd & OPP: Parse performance state from DT | expand

Commit Message

Viresh Kumar April 9, 2018, 11:43 a.m. UTC
"opp-hz" property is optional for power domains, don't error out if it
is missing.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

---
 drivers/opp/core.c    | 92 +++++++++++++++++++++++++++++++--------------------
 drivers/opp/debugfs.c | 15 +++++++--
 drivers/opp/of.c      | 26 ++++++++++-----
 drivers/opp/opp.h     |  3 +-
 4 files changed, 89 insertions(+), 47 deletions(-)

-- 
2.15.0.194.g9af6a3dea062

Comments

Ulf Hansson April 9, 2018, 2:55 p.m. UTC | #1
On 9 April 2018 at 13:43, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> "opp-hz" property is optional for power domains, don't error out if it

> is missing.


According to the changelog this seems like a very trivial patch, but
reviewing it told me a different.

I don't question the re-factoring to makes sense, but perhaps you can
make these kind of changes being standalone, as it would simplify the
review.

Kind regards
Uffe

>

> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

> ---

>  drivers/opp/core.c    | 92 +++++++++++++++++++++++++++++++--------------------

>  drivers/opp/debugfs.c | 15 +++++++--

>  drivers/opp/of.c      | 26 ++++++++++-----

>  drivers/opp/opp.h     |  3 +-

>  4 files changed, 89 insertions(+), 47 deletions(-)

>

> diff --git a/drivers/opp/core.c b/drivers/opp/core.c

> index 92fa94a6dcc1..a0f72c732718 100644

> --- a/drivers/opp/core.c

> +++ b/drivers/opp/core.c

> @@ -281,6 +281,23 @@ unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev)

>  }

>  EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_freq);

>

> +int _get_opp_count(struct opp_table *opp_table)

> +{

> +       struct dev_pm_opp *opp;

> +       int count = 0;

> +

> +       mutex_lock(&opp_table->lock);

> +

> +       list_for_each_entry(opp, &opp_table->opp_list, node) {

> +               if (opp->available)

> +                       count++;

> +       }

> +

> +       mutex_unlock(&opp_table->lock);

> +

> +       return count;

> +}

> +

>  /**

>   * dev_pm_opp_get_opp_count() - Get number of opps available in the opp table

>   * @dev:       device for which we do this operation

> @@ -291,25 +308,17 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_freq);

>  int dev_pm_opp_get_opp_count(struct device *dev)

>  {

>         struct opp_table *opp_table;

> -       struct dev_pm_opp *temp_opp;

> -       int count = 0;

> +       int count;

>

>         opp_table = _find_opp_table(dev);

>         if (IS_ERR(opp_table)) {

>                 count = PTR_ERR(opp_table);

>                 dev_dbg(dev, "%s: OPP table not found (%d)\n",

>                         __func__, count);

> -               return count;

> -       }

> -

> -       mutex_lock(&opp_table->lock);

> -

> -       list_for_each_entry(temp_opp, &opp_table->opp_list, node) {

> -               if (temp_opp->available)

> -                       count++;

> +               return 0;

>         }

>

> -       mutex_unlock(&opp_table->lock);

> +       count = _get_opp_count(opp_table);

>         dev_pm_opp_put_opp_table(opp_table);

>

>         return count;

> @@ -985,22 +994,11 @@ static bool _opp_supported_by_regulators(struct dev_pm_opp *opp,

>         return true;

>  }

>

> -/*

> - * Returns:

> - * 0: On success. And appropriate error message for duplicate OPPs.

> - * -EBUSY: For OPP with same freq/volt and is available. The callers of

> - *  _opp_add() must return 0 if they receive -EBUSY from it. This is to make

> - *  sure we don't print error messages unnecessarily if different parts of

> - *  kernel try to initialize the OPP table.

> - * -EEXIST: For OPP with same freq but different volt or is unavailable. This

> - *  should be considered an error by the callers of _opp_add().

> - */

> -int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,

> -            struct opp_table *opp_table)

> +static int _opp_is_duplicate(struct device *dev, struct dev_pm_opp *new_opp,

> +                            struct opp_table *opp_table,

> +                            struct list_head **head)

>  {

>         struct dev_pm_opp *opp;

> -       struct list_head *head;

> -       int ret;

>

>         /*

>          * Insert new OPP in order of increasing frequency and discard if

> @@ -1010,17 +1008,14 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,

>          * loop, don't replace it with head otherwise it will become an infinite

>          * loop.

>          */

> -       mutex_lock(&opp_table->lock);

> -       head = &opp_table->opp_list;

> -

>         list_for_each_entry(opp, &opp_table->opp_list, node) {

>                 if (new_opp->rate > opp->rate) {

> -                       head = &opp->node;

> +                       *head = &opp->node;

>                         continue;

>                 }

>

>                 if (new_opp->rate < opp->rate)

> -                       break;

> +                       return 0;

>

>                 /* Duplicate OPPs */

>                 dev_warn(dev, "%s: duplicate OPPs detected. Existing: freq: %lu, volt: %lu, enabled: %d. New: freq: %lu, volt: %lu, enabled: %d\n",

> @@ -1029,11 +1024,38 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,

>                          new_opp->supplies[0].u_volt, new_opp->available);

>

>                 /* Should we compare voltages for all regulators here ? */

> -               ret = opp->available &&

> -                     new_opp->supplies[0].u_volt == opp->supplies[0].u_volt ? -EBUSY : -EEXIST;

> +               return opp->available &&

> +                      new_opp->supplies[0].u_volt == opp->supplies[0].u_volt ? -EBUSY : -EEXIST;

> +       }

> +

> +       return 0;

> +}

> +

> +/*

> + * Returns:

> + * 0: On success. And appropriate error message for duplicate OPPs.

> + * -EBUSY: For OPP with same freq/volt and is available. The callers of

> + *  _opp_add() must return 0 if they receive -EBUSY from it. This is to make

> + *  sure we don't print error messages unnecessarily if different parts of

> + *  kernel try to initialize the OPP table.

> + * -EEXIST: For OPP with same freq but different volt or is unavailable. This

> + *  should be considered an error by the callers of _opp_add().

> + */

> +int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,

> +            struct opp_table *opp_table, bool rate_not_available)

> +{

> +       struct list_head *head;

> +       int ret;

> +

> +       mutex_lock(&opp_table->lock);

> +       head = &opp_table->opp_list;

>

> -               mutex_unlock(&opp_table->lock);

> -               return ret;

> +       if (likely(!rate_not_available)) {

> +               ret = _opp_is_duplicate(dev, new_opp, opp_table, &head);

> +               if (ret) {

> +                       mutex_unlock(&opp_table->lock);

> +                       return ret;

> +               }

>         }

>

>         if (opp_table->get_pstate)

> @@ -1104,7 +1126,7 @@ int _opp_add_v1(struct opp_table *opp_table, struct device *dev,

>         new_opp->available = true;

>         new_opp->dynamic = dynamic;

>

> -       ret = _opp_add(dev, new_opp, opp_table);

> +       ret = _opp_add(dev, new_opp, opp_table, false);

>         if (ret) {

>                 /* Don't return error for duplicate OPPs */

>                 if (ret == -EBUSY)

> diff --git a/drivers/opp/debugfs.c b/drivers/opp/debugfs.c

> index b03c03576a62..e6828e5f81b0 100644

> --- a/drivers/opp/debugfs.c

> +++ b/drivers/opp/debugfs.c

> @@ -77,10 +77,21 @@ int opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table)

>  {

>         struct dentry *pdentry = opp_table->dentry;

>         struct dentry *d;

> +       unsigned long id;

>         char name[25];  /* 20 chars for 64 bit value + 5 (opp:\0) */

>

> -       /* Rate is unique to each OPP, use it to give opp-name */

> -       snprintf(name, sizeof(name), "opp:%lu", opp->rate);

> +       /*

> +        * Get directory name for OPP.

> +        *

> +        * - Normally rate is unique to each OPP, use it to get unique opp-name.

> +        * - For some devices rate isn't available, use index instead.

> +        */

> +       if (likely(opp->rate))

> +               id = opp->rate;

> +       else

> +               id = _get_opp_count(opp_table);

> +

> +       snprintf(name, sizeof(name), "opp:%lu", id);

>

>         /* Create per-opp directory */

>         d = debugfs_create_dir(name, pdentry);

> diff --git a/drivers/opp/of.c b/drivers/opp/of.c

> index cb716aa2f44b..c5c5afcaa2e3 100644

> --- a/drivers/opp/of.c

> +++ b/drivers/opp/of.c

> @@ -292,6 +292,7 @@ static int _opp_add_static_v2(struct opp_table *opp_table, struct device *dev,

>         u64 rate;

>         u32 val;

>         int ret;

> +       bool rate_not_available = false;

>

>         new_opp = _opp_allocate(opp_table);

>         if (!new_opp)

> @@ -299,8 +300,21 @@ static int _opp_add_static_v2(struct opp_table *opp_table, struct device *dev,

>

>         ret = of_property_read_u64(np, "opp-hz", &rate);

>         if (ret < 0) {

> -               dev_err(dev, "%s: opp-hz not found\n", __func__);

> -               goto free_opp;

> +               /* "opp-hz" is optional for devices like power domains. */

> +               if (!of_find_property(dev->of_node, "#power-domain-cells",

> +                                     NULL)) {

> +                       dev_err(dev, "%s: opp-hz not found\n", __func__);

> +                       goto free_opp;

> +               }

> +

> +               rate_not_available = true;

> +       } else {

> +               /*

> +                * Rate is defined as an unsigned long in clk API, and so

> +                * casting explicitly to its type. Must be fixed once rate is 64

> +                * bit guaranteed in clk API.

> +                */

> +               new_opp->rate = (unsigned long)rate;

>         }

>

>         /* Check if the OPP supports hardware's hierarchy of versions or not */

> @@ -309,12 +323,6 @@ static int _opp_add_static_v2(struct opp_table *opp_table, struct device *dev,

>                 goto free_opp;

>         }

>

> -       /*

> -        * Rate is defined as an unsigned long in clk API, and so casting

> -        * explicitly to its type. Must be fixed once rate is 64 bit

> -        * guaranteed in clk API.

> -        */

> -       new_opp->rate = (unsigned long)rate;

>         new_opp->turbo = of_property_read_bool(np, "turbo-mode");

>

>         new_opp->np = np;

> @@ -328,7 +336,7 @@ static int _opp_add_static_v2(struct opp_table *opp_table, struct device *dev,

>         if (ret)

>                 goto free_opp;

>

> -       ret = _opp_add(dev, new_opp, opp_table);

> +       ret = _opp_add(dev, new_opp, opp_table, rate_not_available);

>         if (ret) {

>                 /* Don't return error for duplicate OPPs */

>                 if (ret == -EBUSY)

> diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h

> index 4d00061648a3..381a4fb15d5c 100644

> --- a/drivers/opp/opp.h

> +++ b/drivers/opp/opp.h

> @@ -188,13 +188,14 @@ struct opp_table {

>

>  /* Routines internal to opp core */

>  void _get_opp_table_kref(struct opp_table *opp_table);

> +int _get_opp_count(struct opp_table *opp_table);

>  struct opp_table *_find_opp_table(struct device *dev);

>  struct opp_device *_add_opp_dev(const struct device *dev, struct opp_table *opp_table);

>  void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev, bool remove_all);

>  void _dev_pm_opp_find_and_remove_table(struct device *dev, bool remove_all);

>  struct dev_pm_opp *_opp_allocate(struct opp_table *opp_table);

>  void _opp_free(struct dev_pm_opp *opp);

> -int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *opp_table);

> +int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *opp_table, bool rate_not_available);

>  int _opp_add_v1(struct opp_table *opp_table, struct device *dev, unsigned long freq, long u_volt, bool dynamic);

>  void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, bool of);

>  struct opp_table *_add_opp_table(struct device *dev);

> --

> 2.15.0.194.g9af6a3dea062

>
Viresh Kumar April 10, 2018, 5:14 a.m. UTC | #2
On 09-04-18, 16:55, Ulf Hansson wrote:
> On 9 April 2018 at 13:43, Viresh Kumar <viresh.kumar@linaro.org> wrote:

> > "opp-hz" property is optional for power domains, don't error out if it

> > is missing.

> 

> According to the changelog this seems like a very trivial patch, but

> reviewing it told me a different.

> 

> I don't question the re-factoring to makes sense, but perhaps you can

> make these kind of changes being standalone, as it would simplify the

> review.


By standalone you meant to post this patch separately from this
series? But this patch really belongs to this series. It is required
because we want to parse power domain OPP tables.

It may look a bit bigger as two new routines were created out of
existing code, which were required to support the optional opp-hz
thing. Now that can be done in a separate patch, but I am not sure if
we should as that is not really irrelevant from the $subject.

-- 
viresh
Ulf Hansson April 24, 2018, 11:57 a.m. UTC | #3
On 10 April 2018 at 07:14, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 09-04-18, 16:55, Ulf Hansson wrote:

>> On 9 April 2018 at 13:43, Viresh Kumar <viresh.kumar@linaro.org> wrote:

>> > "opp-hz" property is optional for power domains, don't error out if it

>> > is missing.

>>

>> According to the changelog this seems like a very trivial patch, but

>> reviewing it told me a different.

>>

>> I don't question the re-factoring to makes sense, but perhaps you can

>> make these kind of changes being standalone, as it would simplify the

>> review.

>

> By standalone you meant to post this patch separately from this

> series? But this patch really belongs to this series. It is required

> because we want to parse power domain OPP tables.


I just meant that a re-factoring patch can precede a patch that makes
the actual change needed, but of course being part of the series.

>

> It may look a bit bigger as two new routines were created out of

> existing code, which were required to support the optional opp-hz

> thing. Now that can be done in a separate patch, but I am not sure if

> we should as that is not really irrelevant from the $subject.


It's up to you, I have no strong opinions.

However, if you intend to keep it as is, perhaps some more information
in changelog could be useful.

Kind regards
Uffe
Viresh Kumar April 25, 2018, 4:37 a.m. UTC | #4
On 24-04-18, 13:57, Ulf Hansson wrote:
> However, if you intend to keep it as is, perhaps some more information

> in changelog could be useful.


Here is the new changelog:

commit cab6b82d8e5b98d15023a26a1b3ccc7a4dd2d6f0
Author: Viresh Kumar <viresh.kumar@linaro.org>
Date:   Fri Apr 6 14:35:45 2018 +0530

    PM / OPP: "opp-hz" is optional for power domains
    
    "opp-hz" property is optional for power domains now and we shouldn't
    error out if it is missing for power domains.
    
    This patch creates two new routines, _get_opp_count() and
    _opp_is_duplicate(), by separating existing code from their parent
    functions. Also skip duplicate OPP check for power domain OPPs as they
    may not have any the "opp-hz" field, but a platform specific performance
    state binding to uniquely identify OPP nodes.
    
    By default the debugfs OPP nodes are named using the "rate" value, but
    that isn't possible for the power domain OPP nodes and hence they use
    the index of the OPP node in the OPP node list instead.
    
    Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>



Looks fine ?

-- 
viresh
Ulf Hansson April 25, 2018, 6:45 a.m. UTC | #5
On 25 April 2018 at 06:37, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 24-04-18, 13:57, Ulf Hansson wrote:

>> However, if you intend to keep it as is, perhaps some more information

>> in changelog could be useful.

>

> Here is the new changelog:

>

> commit cab6b82d8e5b98d15023a26a1b3ccc7a4dd2d6f0

> Author: Viresh Kumar <viresh.kumar@linaro.org>

> Date:   Fri Apr 6 14:35:45 2018 +0530

>

>     PM / OPP: "opp-hz" is optional for power domains

>

>     "opp-hz" property is optional for power domains now and we shouldn't

>     error out if it is missing for power domains.

>

>     This patch creates two new routines, _get_opp_count() and

>     _opp_is_duplicate(), by separating existing code from their parent

>     functions. Also skip duplicate OPP check for power domain OPPs as they

>     may not have any the "opp-hz" field, but a platform specific performance

>     state binding to uniquely identify OPP nodes.

>

>     By default the debugfs OPP nodes are named using the "rate" value, but

>     that isn't possible for the power domain OPP nodes and hence they use

>     the index of the OPP node in the OPP node list instead.

>

>     Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

>

>

> Looks fine ?


Yep!

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>


Kind regards
Uffe
diff mbox series

Patch

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 92fa94a6dcc1..a0f72c732718 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -281,6 +281,23 @@  unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_freq);
 
+int _get_opp_count(struct opp_table *opp_table)
+{
+	struct dev_pm_opp *opp;
+	int count = 0;
+
+	mutex_lock(&opp_table->lock);
+
+	list_for_each_entry(opp, &opp_table->opp_list, node) {
+		if (opp->available)
+			count++;
+	}
+
+	mutex_unlock(&opp_table->lock);
+
+	return count;
+}
+
 /**
  * dev_pm_opp_get_opp_count() - Get number of opps available in the opp table
  * @dev:	device for which we do this operation
@@ -291,25 +308,17 @@  EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_freq);
 int dev_pm_opp_get_opp_count(struct device *dev)
 {
 	struct opp_table *opp_table;
-	struct dev_pm_opp *temp_opp;
-	int count = 0;
+	int count;
 
 	opp_table = _find_opp_table(dev);
 	if (IS_ERR(opp_table)) {
 		count = PTR_ERR(opp_table);
 		dev_dbg(dev, "%s: OPP table not found (%d)\n",
 			__func__, count);
-		return count;
-	}
-
-	mutex_lock(&opp_table->lock);
-
-	list_for_each_entry(temp_opp, &opp_table->opp_list, node) {
-		if (temp_opp->available)
-			count++;
+		return 0;
 	}
 
-	mutex_unlock(&opp_table->lock);
+	count = _get_opp_count(opp_table);
 	dev_pm_opp_put_opp_table(opp_table);
 
 	return count;
@@ -985,22 +994,11 @@  static bool _opp_supported_by_regulators(struct dev_pm_opp *opp,
 	return true;
 }
 
-/*
- * Returns:
- * 0: On success. And appropriate error message for duplicate OPPs.
- * -EBUSY: For OPP with same freq/volt and is available. The callers of
- *  _opp_add() must return 0 if they receive -EBUSY from it. This is to make
- *  sure we don't print error messages unnecessarily if different parts of
- *  kernel try to initialize the OPP table.
- * -EEXIST: For OPP with same freq but different volt or is unavailable. This
- *  should be considered an error by the callers of _opp_add().
- */
-int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
-	     struct opp_table *opp_table)
+static int _opp_is_duplicate(struct device *dev, struct dev_pm_opp *new_opp,
+			     struct opp_table *opp_table,
+			     struct list_head **head)
 {
 	struct dev_pm_opp *opp;
-	struct list_head *head;
-	int ret;
 
 	/*
 	 * Insert new OPP in order of increasing frequency and discard if
@@ -1010,17 +1008,14 @@  int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
 	 * loop, don't replace it with head otherwise it will become an infinite
 	 * loop.
 	 */
-	mutex_lock(&opp_table->lock);
-	head = &opp_table->opp_list;
-
 	list_for_each_entry(opp, &opp_table->opp_list, node) {
 		if (new_opp->rate > opp->rate) {
-			head = &opp->node;
+			*head = &opp->node;
 			continue;
 		}
 
 		if (new_opp->rate < opp->rate)
-			break;
+			return 0;
 
 		/* Duplicate OPPs */
 		dev_warn(dev, "%s: duplicate OPPs detected. Existing: freq: %lu, volt: %lu, enabled: %d. New: freq: %lu, volt: %lu, enabled: %d\n",
@@ -1029,11 +1024,38 @@  int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
 			 new_opp->supplies[0].u_volt, new_opp->available);
 
 		/* Should we compare voltages for all regulators here ? */
-		ret = opp->available &&
-		      new_opp->supplies[0].u_volt == opp->supplies[0].u_volt ? -EBUSY : -EEXIST;
+		return opp->available &&
+		       new_opp->supplies[0].u_volt == opp->supplies[0].u_volt ? -EBUSY : -EEXIST;
+	}
+
+	return 0;
+}
+
+/*
+ * Returns:
+ * 0: On success. And appropriate error message for duplicate OPPs.
+ * -EBUSY: For OPP with same freq/volt and is available. The callers of
+ *  _opp_add() must return 0 if they receive -EBUSY from it. This is to make
+ *  sure we don't print error messages unnecessarily if different parts of
+ *  kernel try to initialize the OPP table.
+ * -EEXIST: For OPP with same freq but different volt or is unavailable. This
+ *  should be considered an error by the callers of _opp_add().
+ */
+int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
+	     struct opp_table *opp_table, bool rate_not_available)
+{
+	struct list_head *head;
+	int ret;
+
+	mutex_lock(&opp_table->lock);
+	head = &opp_table->opp_list;
 
-		mutex_unlock(&opp_table->lock);
-		return ret;
+	if (likely(!rate_not_available)) {
+		ret = _opp_is_duplicate(dev, new_opp, opp_table, &head);
+		if (ret) {
+			mutex_unlock(&opp_table->lock);
+			return ret;
+		}
 	}
 
 	if (opp_table->get_pstate)
@@ -1104,7 +1126,7 @@  int _opp_add_v1(struct opp_table *opp_table, struct device *dev,
 	new_opp->available = true;
 	new_opp->dynamic = dynamic;
 
-	ret = _opp_add(dev, new_opp, opp_table);
+	ret = _opp_add(dev, new_opp, opp_table, false);
 	if (ret) {
 		/* Don't return error for duplicate OPPs */
 		if (ret == -EBUSY)
diff --git a/drivers/opp/debugfs.c b/drivers/opp/debugfs.c
index b03c03576a62..e6828e5f81b0 100644
--- a/drivers/opp/debugfs.c
+++ b/drivers/opp/debugfs.c
@@ -77,10 +77,21 @@  int opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table)
 {
 	struct dentry *pdentry = opp_table->dentry;
 	struct dentry *d;
+	unsigned long id;
 	char name[25];	/* 20 chars for 64 bit value + 5 (opp:\0) */
 
-	/* Rate is unique to each OPP, use it to give opp-name */
-	snprintf(name, sizeof(name), "opp:%lu", opp->rate);
+	/*
+	 * Get directory name for OPP.
+	 *
+	 * - Normally rate is unique to each OPP, use it to get unique opp-name.
+	 * - For some devices rate isn't available, use index instead.
+	 */
+	if (likely(opp->rate))
+		id = opp->rate;
+	else
+		id = _get_opp_count(opp_table);
+
+	snprintf(name, sizeof(name), "opp:%lu", id);
 
 	/* Create per-opp directory */
 	d = debugfs_create_dir(name, pdentry);
diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index cb716aa2f44b..c5c5afcaa2e3 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -292,6 +292,7 @@  static int _opp_add_static_v2(struct opp_table *opp_table, struct device *dev,
 	u64 rate;
 	u32 val;
 	int ret;
+	bool rate_not_available = false;
 
 	new_opp = _opp_allocate(opp_table);
 	if (!new_opp)
@@ -299,8 +300,21 @@  static int _opp_add_static_v2(struct opp_table *opp_table, struct device *dev,
 
 	ret = of_property_read_u64(np, "opp-hz", &rate);
 	if (ret < 0) {
-		dev_err(dev, "%s: opp-hz not found\n", __func__);
-		goto free_opp;
+		/* "opp-hz" is optional for devices like power domains. */
+		if (!of_find_property(dev->of_node, "#power-domain-cells",
+				      NULL)) {
+			dev_err(dev, "%s: opp-hz not found\n", __func__);
+			goto free_opp;
+		}
+
+		rate_not_available = true;
+	} else {
+		/*
+		 * Rate is defined as an unsigned long in clk API, and so
+		 * casting explicitly to its type. Must be fixed once rate is 64
+		 * bit guaranteed in clk API.
+		 */
+		new_opp->rate = (unsigned long)rate;
 	}
 
 	/* Check if the OPP supports hardware's hierarchy of versions or not */
@@ -309,12 +323,6 @@  static int _opp_add_static_v2(struct opp_table *opp_table, struct device *dev,
 		goto free_opp;
 	}
 
-	/*
-	 * Rate is defined as an unsigned long in clk API, and so casting
-	 * explicitly to its type. Must be fixed once rate is 64 bit
-	 * guaranteed in clk API.
-	 */
-	new_opp->rate = (unsigned long)rate;
 	new_opp->turbo = of_property_read_bool(np, "turbo-mode");
 
 	new_opp->np = np;
@@ -328,7 +336,7 @@  static int _opp_add_static_v2(struct opp_table *opp_table, struct device *dev,
 	if (ret)
 		goto free_opp;
 
-	ret = _opp_add(dev, new_opp, opp_table);
+	ret = _opp_add(dev, new_opp, opp_table, rate_not_available);
 	if (ret) {
 		/* Don't return error for duplicate OPPs */
 		if (ret == -EBUSY)
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index 4d00061648a3..381a4fb15d5c 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -188,13 +188,14 @@  struct opp_table {
 
 /* Routines internal to opp core */
 void _get_opp_table_kref(struct opp_table *opp_table);
+int _get_opp_count(struct opp_table *opp_table);
 struct opp_table *_find_opp_table(struct device *dev);
 struct opp_device *_add_opp_dev(const struct device *dev, struct opp_table *opp_table);
 void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev, bool remove_all);
 void _dev_pm_opp_find_and_remove_table(struct device *dev, bool remove_all);
 struct dev_pm_opp *_opp_allocate(struct opp_table *opp_table);
 void _opp_free(struct dev_pm_opp *opp);
-int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *opp_table);
+int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *opp_table, bool rate_not_available);
 int _opp_add_v1(struct opp_table *opp_table, struct device *dev, unsigned long freq, long u_volt, bool dynamic);
 void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, bool of);
 struct opp_table *_add_opp_table(struct device *dev);