* re: PM / OPP: Add clock-latency-ns support
@ 2015-08-10 16:38 Dan Carpenter
2015-08-11 8:12 ` Viresh Kumar
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2015-08-10 16:38 UTC (permalink / raw)
To: viresh.kumar; +Cc: linux-pm
Hello Viresh Kumar,
The patch 3ca9bb33c627: "PM / OPP: Add clock-latency-ns support" from
Jul 29, 2015, leads to the following static checker warning:
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.
drivers/base/power/opp.c
946 new_opp->np = np;
947 new_opp->dynamic = false;
948 new_opp->available = true;
949 of_property_read_u32(np, "clock-latency-ns",
950 (u32 *)&new_opp->clock_latency_ns);
^^^^^^^^^^^^^^^^^^^^^^^^^^
This code will break on 64 bit, big endian machines. I doin't know if
that is an issue for this driver. I saw this was in the power/
directory and I spent a minute googling to see if PowerPC machines are
big endian... :P
951
952 ret = opp_get_microvolt(new_opp, dev);
953 if (ret)
954 goto free_opp;
955
Also:
drivers/base/power/opp.c:956 _opp_add_static_v2()
warn: passing casted pointer '&new_opp->u_amp' to 'of_property_read_u32()' 64 vs 32.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: PM / OPP: Add clock-latency-ns support 2015-08-10 16:38 PM / OPP: Add clock-latency-ns support Dan Carpenter @ 2015-08-11 8:12 ` Viresh Kumar 2015-08-11 13:53 ` Dan Carpenter 0 siblings, 1 reply; 6+ messages in thread From: Viresh Kumar @ 2015-08-11 8:12 UTC (permalink / raw) To: Dan Carpenter, Stephen Boyd; +Cc: linux-pm On 10-08-15, 19:38, Dan Carpenter wrote: > Hello Viresh Kumar, > > The patch 3ca9bb33c627: "PM / OPP: Add clock-latency-ns support" from > Jul 29, 2015, leads to the following static checker warning: > > 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. > > drivers/base/power/opp.c > 946 new_opp->np = np; > 947 new_opp->dynamic = false; > 948 new_opp->available = true; > 949 of_property_read_u32(np, "clock-latency-ns", > 950 (u32 *)&new_opp->clock_latency_ns); > ^^^^^^^^^^^^^^^^^^^^^^^^^^ > > This code will break on 64 bit, big endian machines. I doin't know if > that is an issue for this driver. I saw this was in the power/ > directory and I spent a minute googling to see if PowerPC machines are > big endian... :P > > 951 > 952 ret = opp_get_microvolt(new_opp, dev); > 953 if (ret) > 954 goto free_opp; > 955 > > Also: > > drivers/base/power/opp.c:956 _opp_add_static_v2() > warn: passing casted pointer '&new_opp->u_amp' to 'of_property_read_u32()' 64 vs 32. Hi Dan, The problem is that the value here is of type 'unsigned long' which is 32/64 bit on 32/64 bit machines. So, I looked at how other places in code has done it, and that's what I found. -- viresh ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: PM / OPP: Add clock-latency-ns support 2015-08-11 8:12 ` Viresh Kumar @ 2015-08-11 13:53 ` Dan Carpenter 2015-08-11 18:54 ` Stephen Boyd 0 siblings, 1 reply; 6+ messages in thread From: Dan Carpenter @ 2015-08-11 13:53 UTC (permalink / raw) To: Viresh Kumar; +Cc: Stephen Boyd, linux-pm On Tue, Aug 11, 2015 at 01:42:28PM +0530, Viresh Kumar wrote: > On 10-08-15, 19:38, Dan Carpenter wrote: > > Hello Viresh Kumar, > > > > The patch 3ca9bb33c627: "PM / OPP: Add clock-latency-ns support" from > > Jul 29, 2015, leads to the following static checker warning: > > > > 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. > > > > drivers/base/power/opp.c > > 946 new_opp->np = np; > > 947 new_opp->dynamic = false; > > 948 new_opp->available = true; > > 949 of_property_read_u32(np, "clock-latency-ns", > > 950 (u32 *)&new_opp->clock_latency_ns); > > ^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > This code will break on 64 bit, big endian machines. I doin't know if > > that is an issue for this driver. I saw this was in the power/ > > directory and I spent a minute googling to see if PowerPC machines are > > big endian... :P > > > > 951 > > 952 ret = opp_get_microvolt(new_opp, dev); > > 953 if (ret) > > 954 goto free_opp; > > 955 > > > > Also: > > > > drivers/base/power/opp.c:956 _opp_add_static_v2() > > warn: passing casted pointer '&new_opp->u_amp' to 'of_property_read_u32()' 64 vs 32. > > Hi Dan, > > The problem is that the value here is of type 'unsigned long' which is > 32/64 bit on 32/64 bit machines. Yep. It won't work on 64 bit big endian machines as described earlier. > > So, I looked at how other places in code has done it, and that's what > I found. It's not portable. Sometimes we don't care about that because we know we don't care about 64 bit big endian systems. Hence, my email. regards, dan carpenter ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: PM / OPP: Add clock-latency-ns support 2015-08-11 13:53 ` Dan Carpenter @ 2015-08-11 18:54 ` Stephen Boyd 2015-08-12 6:57 ` Viresh Kumar 0 siblings, 1 reply; 6+ messages in thread From: Stephen Boyd @ 2015-08-11 18:54 UTC (permalink / raw) To: Dan Carpenter; +Cc: Viresh Kumar, linux-pm On 08/11, Dan Carpenter wrote: > On Tue, Aug 11, 2015 at 01:42:28PM +0530, Viresh Kumar wrote: > > > > The problem is that the value here is of type 'unsigned long' which is > > 32/64 bit on 32/64 bit machines. > > Yep. It won't work on 64 bit big endian machines as described earlier. > > > > > So, I looked at how other places in code has done it, and that's what > > I found. > > It's not portable. Sometimes we don't care about that because we know > we don't care about 64 bit big endian systems. Hence, my email. > Making it portable should be simple enough by having a temporary variable of type u32 though. Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> ---8<--- diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index 51b220e615d3..022715d1894c 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -919,6 +919,7 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np) struct dev_pm_opp *new_opp; u64 rate; int ret; + u32 val; /* Hold our list modification lock here */ mutex_lock(&dev_opp_list_lock); @@ -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) -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: PM / OPP: Add clock-latency-ns support 2015-08-11 18:54 ` Stephen Boyd @ 2015-08-12 6:57 ` Viresh Kumar 2015-08-12 7:58 ` Dan Carpenter 0 siblings, 1 reply; 6+ messages in thread From: Viresh Kumar @ 2015-08-12 6:57 UTC (permalink / raw) To: Stephen Boyd; +Cc: Dan Carpenter, linux-pm On 11-08-15, 11:54, Stephen Boyd wrote: > Making it portable should be simple enough by having a temporary > variable of type u32 though. Right. @Dan: Does this look fine to you? Message-Id: <40b3ad3c99c5fe1c50d997ba5418dd602c673f13.1439362565.git.viresh.kumar@linaro.org> From: Viresh Kumar <viresh.kumar@linaro.org> Date: Wed, 12 Aug 2015 12:20:49 +0530 Subject: [PATCH] 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 <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 204c6c945168..a9e0af0dd9e5 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) ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: PM / OPP: Add clock-latency-ns support 2015-08-12 6:57 ` Viresh Kumar @ 2015-08-12 7:58 ` Dan Carpenter 0 siblings, 0 replies; 6+ messages in thread From: Dan Carpenter @ 2015-08-12 7:58 UTC (permalink / raw) To: Viresh Kumar; +Cc: Stephen Boyd, linux-pm On Wed, Aug 12, 2015 at 12:27:09PM +0530, Viresh Kumar wrote: > On 11-08-15, 11:54, Stephen Boyd wrote: > > Making it portable should be simple enough by having a temporary > > variable of type u32 though. > > Right. > > @Dan: Does this look fine to you? Looks good. regards, dan carpenter ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-08-12 7:59 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-08-10 16:38 PM / OPP: Add clock-latency-ns support Dan Carpenter 2015-08-11 8:12 ` Viresh Kumar 2015-08-11 13:53 ` Dan Carpenter 2015-08-11 18:54 ` Stephen Boyd 2015-08-12 6:57 ` Viresh Kumar 2015-08-12 7:58 ` Dan Carpenter
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).