[1/2] OPP: Use opp_table->regulators to verify no regulator case

Message ID 90e3577b5feb42bac1269e16bb3d2bdd8f6df40f.1544527580.git.viresh.kumar@linaro.org
State New
Headers show
Series
  • [1/2] OPP: Use opp_table->regulators to verify no regulator case
Related show

Commit Message

Viresh Kumar Dec. 11, 2018, 11:26 a.m.
The value of opp_table->regulator_count is not very consistent right now
and it may end up being 0 while we do have a "opp-microvolt" property in
the OPP table. It was kept that way as we used to check if any
regulators are set with the OPP core for a device or not using value of
regulator_count.

Lets use opp_table->regulators for that purpose as the meaning of
regulator_count is going to change in the later patches.

Reported-by: Quentin Perret <quentin.perret@arm.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

---
 drivers/opp/core.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

-- 
2.19.1.568.g152ad8e3369a

Comments

Quentin Perret Dec. 11, 2018, 1:48 p.m. | #1
On Tuesday 11 Dec 2018 at 16:56:29 (+0530), Viresh Kumar wrote:
> There is one case where we may end up with no "supply" directory for the

> OPPs in debugfs. That happens when the OPP core isn't managing the

> regulators for the device and the device's OPP do have microvolt

> property. It happens because the opp_table->regulator_count remains set

> to 0 and the debugfs routines don't add any supply directory in such a

> case.

> 

> This commit fixes that by setting opp_table->regulator_count to 1 in

> that particular case. But to make everything work nicely and not break

> other parts of the core, regulator_count is defined as "int" now instead

> of "unsigned int" and it can have different special values now. It is

> set to -1 initially to mark it "uninitialized" and later only we set it

> to 0 or positive values after checking how many supplies are there.

> 

> This also helps in finding the bugs where only few of the OPPs have the

> "opp-microvolt" property set and not all.


Tested on Juno r0 and Hikey960 successfully. The 'supply' directory is
now correctly exposed.

Feel free to add Tested-by: Quentin Perret <quentin.perret@arm.com>

Thanks,
Quentin
Viresh Kumar Dec. 12, 2018, 4:47 a.m. | #2
On 11-12-18, 13:48, Quentin Perret wrote:
> On Tuesday 11 Dec 2018 at 16:56:29 (+0530), Viresh Kumar wrote:

> > There is one case where we may end up with no "supply" directory for the

> > OPPs in debugfs. That happens when the OPP core isn't managing the

> > regulators for the device and the device's OPP do have microvolt

> > property. It happens because the opp_table->regulator_count remains set

> > to 0 and the debugfs routines don't add any supply directory in such a

> > case.

> > 

> > This commit fixes that by setting opp_table->regulator_count to 1 in

> > that particular case. But to make everything work nicely and not break

> > other parts of the core, regulator_count is defined as "int" now instead

> > of "unsigned int" and it can have different special values now. It is

> > set to -1 initially to mark it "uninitialized" and later only we set it

> > to 0 or positive values after checking how many supplies are there.

> > 

> > This also helps in finding the bugs where only few of the OPPs have the

> > "opp-microvolt" property set and not all.

> 

> Tested on Juno r0 and Hikey960 successfully. The 'supply' directory is

> now correctly exposed.

> 

> Feel free to add Tested-by: Quentin Perret <quentin.perret@arm.com>


Thanks.

-- 
viresh

Patch

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 2c2df4e4fc14..2d3d0d1180ea 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -196,12 +196,12 @@  unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
 	if (IS_ERR(opp_table))
 		return 0;
 
-	count = opp_table->regulator_count;
-
 	/* Regulator may not be required for the device */
-	if (!count)
+	if (!opp_table->regulators)
 		goto put_opp_table;
 
+	count = opp_table->regulator_count;
+
 	uV = kmalloc_array(count, sizeof(*uV), GFP_KERNEL);
 	if (!uV)
 		goto put_opp_table;
@@ -1049,6 +1049,9 @@  static bool _opp_supported_by_regulators(struct dev_pm_opp *opp,
 	struct regulator *reg;
 	int i;
 
+	if (!opp_table->regulators)
+		return true;
+
 	for (i = 0; i < opp_table->regulator_count; i++) {
 		reg = opp_table->regulators[i];
 
@@ -1333,7 +1336,7 @@  static int _allocate_set_opp_data(struct opp_table *opp_table)
 	struct dev_pm_set_opp_data *data;
 	int len, count = opp_table->regulator_count;
 
-	if (WARN_ON(!count))
+	if (WARN_ON(!opp_table->regulators))
 		return -EINVAL;
 
 	/* space for set_opp_data */