* [bug report] soundwire: Add IO transfer
@ 2018-01-05 13:47 Dan Carpenter
2018-01-08 3:37 ` Vinod Koul
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2018-01-05 13:47 UTC (permalink / raw)
To: vinod.koul; +Cc: alsa-devel
Hello Vinod Koul,
The patch 9d715fa005eb: "soundwire: Add IO transfer" from Dec 14,
2017, leads to the following static checker warning:
drivers/soundwire/bus.c:309 sdw_nread()
info: return a literal instead of 'ret'
drivers/soundwire/bus.c
297 int sdw_nread(struct sdw_slave *slave, u32 addr, size_t count, u8 *val)
298 {
299 struct sdw_msg msg;
300 int ret;
301
302 ret = sdw_fill_msg(&msg, slave, addr, count,
303 slave->dev_num, SDW_MSG_FLAG_READ, val);
304 if (ret < 0)
305 return ret;
306
307 ret = pm_runtime_get_sync(slave->bus->dev);
308 if (!ret)
309 return ret;
Changing "return ret;" to "return 0;" is more readable, but I mostly
am not sure this is correct. rpm_resume() has crap comments. It
sometimes returns negatives, sometimes zero and sometimes one but I have
no idea what it means... Probably this check should be:
if (ret <= 0)
return ret;
(Bugs like this is why the static checker warning exists).
310
311 ret = sdw_transfer(slave->bus, &msg);
312 pm_runtime_put(slave->bus->dev);
313
314 return ret;
315 }
316 EXPORT_SYMBOL(sdw_nread);
317
318 /**
319 * sdw_nwrite() - Write "n" contiguous SDW Slave registers
320 * @slave: SDW Slave
321 * @addr: Register address
322 * @count: length
323 * @val: Buffer for values to be read
324 */
325 int sdw_nwrite(struct sdw_slave *slave, u32 addr, size_t count, u8 *val)
326 {
327 struct sdw_msg msg;
328 int ret;
329
330 ret = sdw_fill_msg(&msg, slave, addr, count,
331 slave->dev_num, SDW_MSG_FLAG_WRITE, val);
332 if (ret < 0)
333 return ret;
334
335 ret = pm_runtime_get_sync(slave->bus->dev);
336 if (!ret)
337 return ret;
Same static checker warning here.
338
339 ret = sdw_transfer(slave->bus, &msg);
340 pm_runtime_put(slave->bus->dev);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] soundwire: Add IO transfer
2018-01-05 13:47 [bug report] soundwire: Add IO transfer Dan Carpenter
@ 2018-01-08 3:37 ` Vinod Koul
0 siblings, 0 replies; 2+ messages in thread
From: Vinod Koul @ 2018-01-08 3:37 UTC (permalink / raw)
To: Dan Carpenter; +Cc: alsa-devel, Shreyas NC
On Fri, Jan 05, 2018 at 04:47:05PM +0300, Dan Carpenter wrote:
> Hello Vinod Koul,
>
> The patch 9d715fa005eb: "soundwire: Add IO transfer" from Dec 14,
> 2017, leads to the following static checker warning:
>
> drivers/soundwire/bus.c:309 sdw_nread()
> info: return a literal instead of 'ret'
>
> drivers/soundwire/bus.c
> 297 int sdw_nread(struct sdw_slave *slave, u32 addr, size_t count, u8 *val)
> 298 {
> 299 struct sdw_msg msg;
> 300 int ret;
> 301
> 302 ret = sdw_fill_msg(&msg, slave, addr, count,
> 303 slave->dev_num, SDW_MSG_FLAG_READ, val);
> 304 if (ret < 0)
> 305 return ret;
> 306
> 307 ret = pm_runtime_get_sync(slave->bus->dev);
> 308 if (!ret)
> 309 return ret;
>
> Changing "return ret;" to "return 0;" is more readable, but I mostly
> am not sure this is correct. rpm_resume() has crap comments. It
> sometimes returns negatives, sometimes zero and sometimes one but I have
> no idea what it means... Probably this check should be:
>
> if (ret <= 0)
> return ret;
That's right we already have a patch for this in our internal code, will send
that out. rpm_resume can return positive values and yes that should be
documented somewhere :)
>
> (Bugs like this is why the static checker warning exists).
>
> 310
> 311 ret = sdw_transfer(slave->bus, &msg);
> 312 pm_runtime_put(slave->bus->dev);
> 313
> 314 return ret;
> 315 }
> 316 EXPORT_SYMBOL(sdw_nread);
> 317
> 318 /**
> 319 * sdw_nwrite() - Write "n" contiguous SDW Slave registers
> 320 * @slave: SDW Slave
> 321 * @addr: Register address
> 322 * @count: length
> 323 * @val: Buffer for values to be read
> 324 */
> 325 int sdw_nwrite(struct sdw_slave *slave, u32 addr, size_t count, u8 *val)
> 326 {
> 327 struct sdw_msg msg;
> 328 int ret;
> 329
> 330 ret = sdw_fill_msg(&msg, slave, addr, count,
> 331 slave->dev_num, SDW_MSG_FLAG_WRITE, val);
> 332 if (ret < 0)
> 333 return ret;
> 334
> 335 ret = pm_runtime_get_sync(slave->bus->dev);
> 336 if (!ret)
> 337 return ret;
>
> Same static checker warning here.
>
> 338
> 339 ret = sdw_transfer(slave->bus, &msg);
> 340 pm_runtime_put(slave->bus->dev);
>
> regards,
> dan carpenter
--
~Vinod
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-01-08 3:33 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-05 13:47 [bug report] soundwire: Add IO transfer Dan Carpenter
2018-01-08 3:37 ` Vinod Koul
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.