PM / OPP: Fixes for 4.3

PATCH Superseded Viresh Kumar 2 participants 5 messages Aug 12, 2015 – Aug 28, 2015

Subsystems (4)

Driver core Suspend to RAM Hibernation Power management core

lore

Share & tools

lore

Viresh Kumar

author

2015-08-12 11:00 UTC #0 lore raw

v3 cover PM / OPP: Fixes for 4.3

Hi Rafael,

Dan's static checker reported few issues on the latest opp merges. And this fixes them.

The first patch here was part of the bigger 'debugfs support for opp' series earlier, but now separated as 4.3 material.

Rebased over: pm/bleeding-edge

Viresh Kumar (2):
PM / OPP: Free resources and properly return error on failure PM / OPP: Fix static checker warning (broken 64bit big endian systems)

 drivers/base/power/opp.c | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

Signature · 1 line

2.4.0

Viresh Kumar

author

2015-08-12 11:00 UTC #1 lore raw

PATCH v3 1/2 PM / OPP: Free resources and properly return error on failure

_of_init_opp_table_v2() isn't freeing up resources on some errors and the error values returned are also not correct always.

This fixes following problems:
- Return -ENOENT, if no entries are found in the table.
- Use IS_ERR() to properly check return value of _find_device_opp().
- Return error value with PTR_ERR() in above case.
- Free table if _find_device_opp() fails.

Reported-by Dan Carpenter <[email protected]>

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

---
 drivers/base/power/opp.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

drivers/base/power/opp.c +16 ~~-14~~

index 204c6c945168..4d6c4576f7ae 100644
@@ -1323,28 +1323,30 @@ static int _of_init_opp_table_v2(struct device *dev,
 		if (ret) {
 			dev_err(dev, "%s: Failed to add OPP, %d\n", __func__,
 				ret);
-			break;
+			goto free_table;
 		}
 	}
 
 	/* There should be one of more OPP defined */
-	if (WARN_ON(!count))
+	if (WARN_ON(!count)) {
+		ret = -ENOENT;
 		goto put_opp_np;
+	}
 
-	if (!ret) {
-		if (!dev_opp) {
-			dev_opp = _find_device_opp(dev);
-			if (WARN_ON(!dev_opp))
-				goto put_opp_np;
-		}
-
-		dev_opp->np = opp_np;
-		dev_opp->shared_opp = of_property_read_bool(opp_np,
-							    "opp-shared");
-	} else {
-		of_free_opp_table(dev);
+	dev_opp = _find_device_opp(dev);
+	if (WARN_ON(IS_ERR(dev_opp))) {
+		ret = PTR_ERR(dev_opp);
+		goto free_table;
 	}
 
+	dev_opp->np = opp_np;
+	dev_opp->shared_opp = of_property_read_bool(opp_np, "opp-shared");
+
+	of_node_put(opp_np);
+	return 0;
+
+free_table:
+	of_free_opp_table(dev);
 put_opp_np:
 	of_node_put(opp_np);

Signature · 1 line

2.4.0

Viresh Kumar

author

2015-08-12 11:00 UTC #2 lore raw

PATCH v3 2/2 PM / OPP: Fix static checker warning (broken 64bit big endian systems)

In reply to Viresh Kumar

Dan Carpenter reported (generated with static checker):

drivers/base/power/opp.c:949 _opp_add_static_v2()
warn: passing casted pointer '&new_opp->clock_latency_ns' to 'of_property_read_u32()' 64 vs 32.

This code will break on 64 bit, big endian machines.

Fix this by reading the value in a u32 type variable first and then assigning it to the unsigned long variable.

Reported-by Dan Carpenter <[email protected]>

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

---
 drivers/base/power/opp.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

drivers/base/power/opp.c +5 -3

index 4d6c4576f7ae..72dc59248876 100644
@@ -918,6 +918,7 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
 	struct device_opp *dev_opp;
 	struct dev_pm_opp *new_opp;
 	u64 rate;
+	u32 val;
 	int ret;
 
 	/* Hold our list modification lock here */

@@ -946,14 +947,15 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
 	new_opp->np = np;
 	new_opp->dynamic = false;
 	new_opp->available = true;
-	of_property_read_u32(np, "clock-latency-ns",
-			     (u32 *)&new_opp->clock_latency_ns);
+	of_property_read_u32(np, "clock-latency-ns", &val);
+	new_opp->clock_latency_ns = val;
 
 	ret = opp_get_microvolt(new_opp, dev);
 	if (ret)
 		goto free_opp;
 
-	of_property_read_u32(np, "opp-microamp", (u32 *)&new_opp->u_amp);
+	of_property_read_u32(np, "opp-microamp", &val);
+	new_opp->u_amp = val;
 
 	ret = _opp_add(dev, new_opp, dev_opp);
 	if (ret)

Signature · 1 line

2.4.0

Viresh Kumar

author

2015-08-17 13:50 UTC #3 lore raw

PATCH v4 2/2 PM / OPP: Fix static checker warning (broken 64bit big endian systems)

Dan Carpenter reported (generated with static checker):

drivers/base/power/opp.c:949 _opp_add_static_v2()
warn: passing casted pointer '&new_opp->clock_latency_ns' to 'of_property_read_u32()' 64 vs 32.

This code will break on 64 bit, big endian machines.

Fix this by reading the value in a u32 type variable first and then assigning it to the unsigned long variable.

Reported-by Dan Carpenter <[email protected]>

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

---
V3->V4:
- Assign values only if of_property_read_u32() is successful.

 drivers/base/power/opp.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

drivers/base/power/opp.c +6 -3

index 4d6c4576f7ae..803d8b7ced89 100644
@@ -918,6 +918,7 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
 	struct device_opp *dev_opp;
 	struct dev_pm_opp *new_opp;
 	u64 rate;
+	u32 val;
 	int ret;
 
 	/* Hold our list modification lock here */

@@ -946,14 +947,16 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
 	new_opp->np = np;
 	new_opp->dynamic = false;
 	new_opp->available = true;
-	of_property_read_u32(np, "clock-latency-ns",
-			     (u32 *)&new_opp->clock_latency_ns);
+
+	if (!of_property_read_u32(np, "clock-latency-ns", &val))
+		new_opp->clock_latency_ns = val;
 
 	ret = opp_get_microvolt(new_opp, dev);
 	if (ret)
 		goto free_opp;
 
-	of_property_read_u32(np, "opp-microamp", (u32 *)&new_opp->u_amp);
+	if (!of_property_read_u32(new_opp->np, "opp-microamp", &val))
+		new_opp->u_amp = val;
 
 	ret = _opp_add(dev, new_opp, dev_opp);
 	if (ret)

Signature · 1 line

2.4.0

Rafael J. Wysocki

Applied

2015-08-28 13:46 UTC #4 lore raw

Re: [PATCH V4 2/2] PM / OPP: Fix static checker warning (broken 64bit big endian systems)

Quoted 6 lines · Viresh Kumar

Dan Carpenter reported (generated with static checker):
drivers/base/power/opp.c:949 _opp_add_static_v2()
warn: passing casted pointer '&new_opp->clock_latency_ns' to 'of_property_read_u32()' 64 vs 32.

Quoted 8 lines

This code will break on 64 bit, big endian machines.
Fix this by reading the value in a u32 type variable first and then assigning it to the unsigned long variable.
Reported-by Dan Carpenter <[email protected]>
Suggested-by Stephen Boyd <[email protected]>
Signed-off-by Viresh Kumar <[email protected]>

I've applied this and the [1/2], thanks!

Rafael