* [bug report] powercap: arm_scmi: Add SCMI powercap based driver
@ 2022-07-06 14:09 Dan Carpenter
2022-07-06 14:49 ` Cristian Marussi
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2022-07-06 14:09 UTC (permalink / raw)
To: cristian.marussi; +Cc: linux-pm
Hello Cristian Marussi,
The patch 31afdd34f2b9: "powercap: arm_scmi: Add SCMI powercap based
driver" from Jul 4, 2022, leads to the following Smatch static
checker warning:
drivers/powercap/arm_scmi_powercap.c:214 scmi_powercap_get_max_power_uw()
warn: cast after binop
drivers/powercap/arm_scmi_powercap.c
203 static int scmi_powercap_get_max_power_uw(struct powercap_zone *pz, int cid,
204 u64 *max_power_uw)
205 {
206 struct scmi_powercap_zone *spz = to_scmi_powercap_zone(pz);
207
208 if (!spz->info)
209 return -ENODEV;
210
211 if (spz->info->powercap_scale_uw)
212 *max_power_uw = (u64)spz->info->max_power_cap;
213 else
214 *max_power_uw = (u64)(spz->info->max_power_cap * 1000);
What's the deal with this cast? It would make more sense to do the cast
before the * 1000 so that it doesn't overflow the 32 bits. Is the cast
even required though? The cast on the line before is decorative so
maybe it's not required.
215
216 return 0;
217 }
218
219 static int scmi_powercap_get_min_power_uw(struct powercap_zone *pz, int cid,
220 u64 *min_power_uw)
221 {
222 struct scmi_powercap_zone *spz = to_scmi_powercap_zone(pz);
223
224 if (!spz->info)
225 return -ENODEV;
226
227 if (spz->info->powercap_scale_uw)
228 *min_power_uw = (u64)spz->info->min_power_cap;
229 else
230 *min_power_uw = (u64)(spz->info->min_power_cap * 1000);
Same cast here.
231
232 return 0;
233 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] powercap: arm_scmi: Add SCMI powercap based driver
2022-07-06 14:09 [bug report] powercap: arm_scmi: Add SCMI powercap based driver Dan Carpenter
@ 2022-07-06 14:49 ` Cristian Marussi
0 siblings, 0 replies; 2+ messages in thread
From: Cristian Marussi @ 2022-07-06 14:49 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-pm
On Wed, Jul 06, 2022 at 05:09:13PM +0300, Dan Carpenter wrote:
> Hello Cristian Marussi,
>
Hi Dan Carpenter,
> The patch 31afdd34f2b9: "powercap: arm_scmi: Add SCMI powercap based
> driver" from Jul 4, 2022, leads to the following Smatch static
> checker warning:
>
> drivers/powercap/arm_scmi_powercap.c:214 scmi_powercap_get_max_power_uw()
> warn: cast after binop
>
> drivers/powercap/arm_scmi_powercap.c
> 203 static int scmi_powercap_get_max_power_uw(struct powercap_zone *pz, int cid,
> 204 u64 *max_power_uw)
> 205 {
> 206 struct scmi_powercap_zone *spz = to_scmi_powercap_zone(pz);
> 207
> 208 if (!spz->info)
> 209 return -ENODEV;
> 210
> 211 if (spz->info->powercap_scale_uw)
> 212 *max_power_uw = (u64)spz->info->max_power_cap;
> 213 else
> 214 *max_power_uw = (u64)(spz->info->max_power_cap * 1000);
>
> What's the deal with this cast? It would make more sense to do the cast
> before the * 1000 so that it doesn't overflow the 32 bits. Is the cast
> even required though? The cast on the line before is decorative so
> maybe it's not required.
>
Yes indeed, I suspect it's a leftover from a previous implementation I
did. There are also other unneeded u64 casts indeed.
In fact this driver was NOT pulled with the last SCMI v3.1 updates/fixes given
the very scarce review it got upstream (as you can see...)
I'll review all of these and integrate your other fixes in my next post
of the SCMI Arm poercap driver.
Thanks,
Cristian
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-07-06 14:49 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-06 14:09 [bug report] powercap: arm_scmi: Add SCMI powercap based driver Dan Carpenter
2022-07-06 14:49 ` Cristian Marussi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.