* [PATCH V3 0/2] PM / OPP: Fixes for 4.3 @ 2015-08-12 11:00 Viresh Kumar 2015-08-12 11:00 ` [PATCH V3 1/2] PM / OPP: Free resources and properly return error on failure Viresh Kumar 2015-08-12 11:00 ` [PATCH V3 2/2] PM / OPP: Fix static checker warning (broken 64bit big endian systems) Viresh Kumar 0 siblings, 2 replies; 5+ messages in thread From: Viresh Kumar @ 2015-08-12 11:00 UTC (permalink / raw) To: Rafael Wysocki Cc: linaro-kernel, linux-pm, dan.carpenter, nm, sboyd, Viresh Kumar 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(-) -- 2.4.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH V3 1/2] PM / OPP: Free resources and properly return error on failure 2015-08-12 11:00 [PATCH V3 0/2] PM / OPP: Fixes for 4.3 Viresh Kumar @ 2015-08-12 11:00 ` Viresh Kumar 2015-08-12 11:00 ` [PATCH V3 2/2] PM / OPP: Fix static checker warning (broken 64bit big endian systems) Viresh Kumar 1 sibling, 0 replies; 5+ messages in thread From: Viresh Kumar @ 2015-08-12 11:00 UTC (permalink / raw) To: Rafael Wysocki Cc: linaro-kernel, linux-pm, dan.carpenter, nm, sboyd, Viresh Kumar, Greg Kroah-Hartman, Len Brown, open list, Pavel Machek _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); -- 2.4.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH V3 2/2] PM / OPP: Fix static checker warning (broken 64bit big endian systems) 2015-08-12 11:00 [PATCH V3 0/2] PM / OPP: Fixes for 4.3 Viresh Kumar 2015-08-12 11:00 ` [PATCH V3 1/2] PM / OPP: Free resources and properly return error on failure Viresh Kumar @ 2015-08-12 11:00 ` Viresh Kumar 2015-08-17 13:50 ` [PATCH V4 " Viresh Kumar 1 sibling, 1 reply; 5+ messages in thread From: Viresh Kumar @ 2015-08-12 11:00 UTC (permalink / raw) To: Rafael Wysocki Cc: linaro-kernel, linux-pm, dan.carpenter, nm, sboyd, Viresh Kumar, Greg Kroah-Hartman, Len Brown, open list, Pavel Machek 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) -- 2.4.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH V4 2/2] PM / OPP: Fix static checker warning (broken 64bit big endian systems) 2015-08-12 11:00 ` [PATCH V3 2/2] PM / OPP: Fix static checker warning (broken 64bit big endian systems) Viresh Kumar @ 2015-08-17 13:50 ` Viresh Kumar 2015-08-28 14:14 ` Rafael J. Wysocki 0 siblings, 1 reply; 5+ messages in thread From: Viresh Kumar @ 2015-08-17 13:50 UTC (permalink / raw) To: Rafael Wysocki Cc: linaro-kernel, linux-pm, dan.carpenter, nm, sboyd, Viresh Kumar, Greg Kroah-Hartman, Len Brown, open list, Pavel Machek 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) -- 2.4.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V4 2/2] PM / OPP: Fix static checker warning (broken 64bit big endian systems) 2015-08-17 13:50 ` [PATCH V4 " Viresh Kumar @ 2015-08-28 14:14 ` Rafael J. Wysocki 0 siblings, 0 replies; 5+ messages in thread From: Rafael J. Wysocki @ 2015-08-28 14:14 UTC (permalink / raw) To: Viresh Kumar Cc: linaro-kernel, linux-pm, dan.carpenter, nm, sboyd, Greg Kroah-Hartman, Len Brown, open list, Pavel Machek 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-08-28 13:46 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-08-12 11:00 [PATCH V3 0/2] PM / OPP: Fixes for 4.3 Viresh Kumar 2015-08-12 11:00 ` [PATCH V3 1/2] PM / OPP: Free resources and properly return error on failure Viresh Kumar 2015-08-12 11:00 ` [PATCH V3 2/2] PM / OPP: Fix static checker warning (broken 64bit big endian systems) Viresh Kumar 2015-08-17 13:50 ` [PATCH V4 " Viresh Kumar 2015-08-28 14:14 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).