Re: [PATCH v10 2/9] OPP: Add support for parsing the 'opp-level' property

[Rajendra Nayak]

On 1/9/2019 2:42 PM, Viresh Kumar wrote:

On 09-01-19, 14:34, Rajendra Nayak wrote:

Now that the OPP bindings are updated to include an optional
'opp-level' property, add support to parse it from device tree
and store it as part of dev_pm_opp structure.
Also add and export an helper 'dev_pm_opp_get_level()' that can be
used to get the level value read from device tree when present.

Signed-off-by: Rajendra Nayak <rna...@codeaurora.org>
---
  drivers/opp/core.c     | 18 ++++++++++++++++++
  drivers/opp/of.c       |  5 ++++-
  drivers/opp/opp.h      |  2 ++
  include/linux/pm_opp.h |  7 +++++++
  4 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index e5507add8f04..738b1783aa65 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -130,6 +130,24 @@ unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp)
  }
  EXPORT_SYMBOL_GPL(dev_pm_opp_get_freq);

+/**

+ * dev_pm_opp_get_level() - Gets the level corresponding to an available opp
+ * @opp:       opp for which level value has to be returned for
+ *
+ * Return: level read from device tree corresponding to the opp, else
+ * return 0

All a full-stop here.

will do

+ */
+unsigned int dev_pm_opp_get_level(struct dev_pm_opp *opp)
+{
+       if (IS_ERR_OR_NULL(opp) || !opp->available) {
+               pr_err("%s: Invalid parameters\n", __func__);
+               return 0;
+       }
+
+       return opp->level;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_get_level);
+
  /**
   * dev_pm_opp_is_turbo() - Returns if opp is turbo OPP or not
   * @opp: opp for which turbo mode is being verified
diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 06f0f632ec47..8274d3ba2c5b 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -568,7 +568,7 @@ static struct dev_pm_opp *_opp_add_static_v2(struct 
opp_table *opp_table,
  {
        struct dev_pm_opp *new_opp;
        u64 rate = 0;
-       u32 val;
+       u32 val, level = 0;
        int ret;
        bool rate_not_available = false;

@@ -594,6 +594,9 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table,

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

+ if (!of_property_read_u32(np, "opp-level", &level))

+               new_opp->level = level;
+

What about

         of_property_read_u32(np, "opp-level", &new_opp->level);

??

sure, will update

        /* Check if the OPP supports hardware's hierarchy of versions or not */
        if (!_opp_is_supported(dev, opp_table, np)) {
                dev_dbg(dev, "OPP not supported by hardware: %llu\n", rate);
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index e24d81497375..5accdd96867d 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -60,6 +60,7 @@ extern struct list_head opp_tables;
   * @suspend:  true if suspend OPP
   * @pstate: Device's power domain's performance state.
   * @rate:     Frequency in hertz
+ * @level:     level value to be comminucated to remote power manager

Just say "performance level" here. Btw, communicated was also
misspelled here.

will fix and respin

thanks for the review.
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation