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(-)
_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 dan.carpenter@oracle.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index 204c6c945168..4d6c4576f7ae 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -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);
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 dan.carpenter@oracle.com Suggested-by: Stephen Boyd sboyd@codeaurora.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index 4d6c4576f7ae..72dc59248876 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -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)
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 dan.carpenter@oracle.com Suggested-by: Stephen Boyd sboyd@codeaurora.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- 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(-)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index 4d6c4576f7ae..803d8b7ced89 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -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)
On Monday, August 17, 2015 07:20:20 PM Viresh Kumar wrote:
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 dan.carpenter@oracle.com Suggested-by: Stephen Boyd sboyd@codeaurora.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
I've applied this and the [1/2], thanks!
Rafael
linaro-kernel@lists.linaro.org
-
Rafael J. Wysocki -
Viresh Kumar