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