Skip to contentSkip to conversation

powercap: arm_scmi: Add SCMI powercap based driver

bug report Dan Carpenter 2 participants 2 messages Jul 6, 2022

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw
Quoted 2 lines · Dan Carpenter

Hello Cristian Marussi,

Hi Dan Carpenter,

Quoted 21 lines

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);
Quoted 5 lines

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