Re: [PATCH 4/4] iio: imu: add BNO055 serdev driver

From: kernel test robot
Date: Thu Jul 15 2021 - 14:15:28 EST

Hi Andrea,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on iio/togreg]
[also build test WARNING on robh/for-next linus/master v5.14-rc1 next-20210715]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Andrea-Merello/Add-support-for-Bosch-BNO055-IMU/20210715-222018
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: riscv-allyesconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/616d1b9a99ec2045cdf6cc827751660a48ccc5d2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andrea-Merello/Add-support-for-Bosch-BNO055-IMU/20210715-222018
git checkout 616d1b9a99ec2045cdf6cc827751660a48ccc5d2
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross ARCH=riscv

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@xxxxxxxxx>

All warnings (new ones prefixed by >>):

In file included from include/linux/device.h:15,
from drivers/iio/imu/bno055/bno055_sl.c:18:
drivers/iio/imu/bno055/bno055_sl.c: In function 'bno055_sl_write_reg':
>> drivers/iio/imu/bno055/bno055_sl.c:286:31: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t' {aka 'long unsigned int'} [-Wformat=]
286 | dev_err(&priv->serdev->dev, "Invalid write count %d", count);
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
19 | #define dev_fmt(fmt) fmt
| ^~~
drivers/iio/imu/bno055/bno055_sl.c:286:3: note: in expansion of macro 'dev_err'
286 | dev_err(&priv->serdev->dev, "Invalid write count %d", count);
| ^~~~~~~
drivers/iio/imu/bno055/bno055_sl.c:286:53: note: format string is defined here
286 | dev_err(&priv->serdev->dev, "Invalid write count %d", count);
| ~^
| |
| int
| %ld
In file included from include/linux/device.h:15,
from drivers/iio/imu/bno055/bno055_sl.c:18:
drivers/iio/imu/bno055/bno055_sl.c: In function 'bno055_sl_read_reg':
drivers/iio/imu/bno055/bno055_sl.c:306:31: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t' {aka 'long unsigned int'} [-Wformat=]
306 | dev_err(&priv->serdev->dev, "Invalid read regsize %d",
| ^~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
19 | #define dev_fmt(fmt) fmt
| ^~~
drivers/iio/imu/bno055/bno055_sl.c:306:3: note: in expansion of macro 'dev_err'
306 | dev_err(&priv->serdev->dev, "Invalid read regsize %d",
| ^~~~~~~
drivers/iio/imu/bno055/bno055_sl.c:306:54: note: format string is defined here
306 | dev_err(&priv->serdev->dev, "Invalid read regsize %d",
| ~^
| |
| int
| %ld
In file included from include/linux/device.h:15,
from drivers/iio/imu/bno055/bno055_sl.c:18:
drivers/iio/imu/bno055/bno055_sl.c:312:31: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t' {aka 'long unsigned int'} [-Wformat=]
312 | dev_err(&priv->serdev->dev, "Invalid read valsize %d",
| ^~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
19 | #define dev_fmt(fmt) fmt
| ^~~
drivers/iio/imu/bno055/bno055_sl.c:312:3: note: in expansion of macro 'dev_err'
312 | dev_err(&priv->serdev->dev, "Invalid read valsize %d",
| ^~~~~~~
drivers/iio/imu/bno055/bno055_sl.c:312:54: note: format string is defined here
312 | dev_err(&priv->serdev->dev, "Invalid read valsize %d",
| ~^
| |
| int
| %ld
In file included from include/linux/printk.h:456,
from include/linux/kernel.h:19,
from include/linux/list.h:9,
from include/linux/swait.h:5,
from include/linux/completion.h:12,
from drivers/iio/imu/bno055/bno055_sl.c:17:
drivers/iio/imu/bno055/bno055_sl.c:318:30: warning: format '%d' expects argument of type 'int', but argument 5 has type 'size_t' {aka 'long unsigned int'} [-Wformat=]
318 | dev_dbg(&priv->serdev->dev, "rd reg 0x%x (len %d)", reg_addr, val_size);
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:134:15: note: in definition of macro '__dynamic_func_call'
134 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:166:2: note: in expansion of macro '_dynamic_func_call'
166 | _dynamic_func_call(fmt,__dynamic_dev_dbg, \
| ^~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:123:2: note: in expansion of macro 'dynamic_dev_dbg'
123 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~
include/linux/dev_printk.h:123:23: note: in expansion of macro 'dev_fmt'
123 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/iio/imu/bno055/bno055_sl.c:318:2: note: in expansion of macro 'dev_dbg'
318 | dev_dbg(&priv->serdev->dev, "rd reg 0x%x (len %d)", reg_addr, val_size);
| ^~~~~~~
drivers/iio/imu/bno055/bno055_sl.c:318:49: note: format string is defined here
318 | dev_dbg(&priv->serdev->dev, "rd reg 0x%x (len %d)", reg_addr, val_size);
| ~^
| |
| int
| %ld
In file included from include/linux/printk.h:456,
from include/linux/kernel.h:19,
from include/linux/list.h:9,
from include/linux/swait.h:5,
from include/linux/completion.h:12,
from drivers/iio/imu/bno055/bno055_sl.c:17:
drivers/iio/imu/bno055/bno055_sl.c: In function 'bno055_sl_receive_buf':
drivers/iio/imu/bno055/bno055_sl.c:394:30: warning: format '%d' expects argument of type 'int', but argument 4 has type 'size_t' {aka 'long unsigned int'} [-Wformat=]
394 | dev_dbg(&priv->serdev->dev, "recv (len %d): %*ph ", size, size, buf);
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:134:15: note: in definition of macro '__dynamic_func_call'
134 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:166:2: note: in expansion of macro '_dynamic_func_call'
166 | _dynamic_func_call(fmt,__dynamic_dev_dbg, \
| ^~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:123:2: note: in expansion of macro 'dynamic_dev_dbg'
123 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~
include/linux/dev_printk.h:123:23: note: in expansion of macro 'dev_fmt'
123 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/iio/imu/bno055/bno055_sl.c:394:2: note: in expansion of macro 'dev_dbg'
394 | dev_dbg(&priv->serdev->dev, "recv (len %d): %*ph ", size, size, buf);
| ^~~~~~~
drivers/iio/imu/bno055/bno055_sl.c:394:42: note: format string is defined here
394 | dev_dbg(&priv->serdev->dev, "recv (len %d): %*ph ", size, size, buf);
| ~^
| |
| int
| %ld
In file included from include/linux/printk.h:456,
from include/linux/kernel.h:19,
from include/linux/list.h:9,
from include/linux/swait.h:5,
from include/linux/completion.h:12,
from drivers/iio/imu/bno055/bno055_sl.c:17:
>> drivers/iio/imu/bno055/bno055_sl.c:394:30: warning: field width specifier '*' expects argument of type 'int', but argument 5 has type 'size_t' {aka 'long unsigned int'} [-Wformat=]
394 | dev_dbg(&priv->serdev->dev, "recv (len %d): %*ph ", size, size, buf);
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:134:15: note: in definition of macro '__dynamic_func_call'
134 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:166:2: note: in expansion of macro '_dynamic_func_call'
166 | _dynamic_func_call(fmt,__dynamic_dev_dbg, \
| ^~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:123:2: note: in expansion of macro 'dynamic_dev_dbg'
123 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~
include/linux/dev_printk.h:123:23: note: in expansion of macro 'dev_fmt'
123 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/iio/imu/bno055/bno055_sl.c:394:2: note: in expansion of macro 'dev_dbg'
394 | dev_dbg(&priv->serdev->dev, "recv (len %d): %*ph ", size, size, buf);
| ^~~~~~~
drivers/iio/imu/bno055/bno055_sl.c:394:47: note: format string is defined here
394 | dev_dbg(&priv->serdev->dev, "recv (len %d): %*ph ", size, size, buf);
| ~^~
| |
| int


vim +286 drivers/iio/imu/bno055/bno055_sl.c

277
278 static int bno055_sl_write_reg(void *context, const void *data, size_t count)
279 {
280 int ret;
281 int reg;
282 u8 *write_data = (u8 *)data + 1;
283 struct bno055_sl_priv *priv = context;
284
285 if (count < 2) {
> 286 dev_err(&priv->serdev->dev, "Invalid write count %d", count);
287 return -EINVAL;
288 }
289
290 reg = ((u8 *)data)[0];
291 dev_dbg(&priv->serdev->dev, "wr reg 0x%x = 0x%x", reg, ((u8 *)data)[1]);
292 ret = bno_sl_send_cmd(priv, 0, reg, count - 1, write_data);
293
294 return ret;
295 }
296
297 static int bno055_sl_read_reg(void *context,
298 const void *reg, size_t reg_size,
299 void *val, size_t val_size)
300 {
301 int ret;
302 int reg_addr;
303 struct bno055_sl_priv *priv = context;
304
305 if (reg_size != 1) {
306 dev_err(&priv->serdev->dev, "Invalid read regsize %d",
307 reg_size);
308 return -EINVAL;
309 }
310
311 if (val_size > 128) {
312 dev_err(&priv->serdev->dev, "Invalid read valsize %d",
313 val_size);
314 return -EINVAL;
315 }
316
317 reg_addr = ((u8 *)reg)[0];
318 dev_dbg(&priv->serdev->dev, "rd reg 0x%x (len %d)", reg_addr, val_size);
319 mutex_lock(&priv->lock);
320 priv->expected_data_len = val_size;
321 priv->response_buf = val;
322 mutex_unlock(&priv->lock);
323
324 ret = bno_sl_send_cmd(priv, 1, reg_addr, val_size, NULL);
325
326 mutex_lock(&priv->lock);
327 priv->response_buf = NULL;
328 mutex_unlock(&priv->lock);
329
330 return ret;
331 }
332
333 /*
334 * Handler for received data; this is called from the reicever callback whenever
335 * it got some packet from the serial bus. The status tell us whether the
336 * packet is valid (i.e. header ok && received payload len consistent wrt the
337 * header). It's now our responsability to check whether this is what we
338 * expected, of whether we got some unexpected, yet valid, packet.
339 */
340 static void bno055_sl_handle_rx(struct bno055_sl_priv *priv, int status)
341 {
342 mutex_lock(&priv->lock);
343 switch (priv->expect_response) {
344 case CMD_NONE:
345 dev_warn(&priv->serdev->dev, "received unexpected, yet valid, data from sensor");
346 mutex_unlock(&priv->lock);
347 return;
348
349 case CMD_READ:
350 priv->cmd_status = status;
351 if (status == STATUS_OK &&
352 priv->rx.databuf_count != priv->expected_data_len) {
353 /*
354 * If we got here, then the lower layer serial protocol
355 * seems consistent with itself; if we got an unexpected
356 * amount of data then signal it as a non critical error
357 */
358 priv->cmd_status = STATUS_FAIL;
359 dev_warn(&priv->serdev->dev, "received an unexpected amount of, yet valid, data from sensor");
360 }
361 break;
362
363 case CMD_WRITE:
364 priv->cmd_status = status;
365 break;
366 }
367
368 priv->expect_response = CMD_NONE;
369 complete(&priv->cmd_complete);
370 mutex_unlock(&priv->lock);
371 }
372
373 /*
374 * Serdev receiver FSM. This tracks the serial communication and parse the
375 * header. It pushes packets to bno055_sl_handle_rx(), eventually communicating
376 * failures (i.e. malformed packets).
377 * Idellay it doesn't know anything about upper layer (i.e. if this is the
378 * packet we were really expecting), but since we copies the payload into the
379 * receiver buffer (that is not valid when i.e. we don't expect data), we
380 * snoop a bit in the upper layer..
381 * Also, we assume to RX one pkt per time (i.e. the HW doesn't send anything
382 * unless we require to AND we don't queue more than one request per time).
383 */
384 static int bno055_sl_receive_buf(struct serdev_device *serdev,
385 const unsigned char *buf, size_t size)
386 {
387 int status;
388 struct bno055_sl_priv *priv = serdev_device_get_drvdata(serdev);
389 int _size = size;
390
391 if (size == 0)
392 return 0;
393
> 394 dev_dbg(&priv->serdev->dev, "recv (len %d): %*ph ", size, size, buf);
395 switch (priv->rx.state) {
396 case RX_IDLE:
397 /*
398 * New packet.
399 * Check for its 1st byte, that identifies the pkt type.
400 */
401 if (buf[0] != 0xEE && buf[0] != 0xBB) {
402 dev_err(&priv->serdev->dev,
403 "Invalid packet start %x", buf[0]);
404 bno055_sl_handle_rx(priv, STATUS_CRIT);
405 break;
406 }
407 priv->rx.type = buf[0];
408 priv->rx.state = RX_START;
409 size--;
410 buf++;
411 priv->rx.databuf_count = 0;
412 fallthrough;
413
414 case RX_START:
415 /*
416 * Packet RX in progress, we expect either 1-byte len or 1-byte
417 * status depending by the packet type.
418 */
419 if (size == 0)
420 break;
421
422 if (priv->rx.type == 0xEE) {
423 if (size > 1) {
424 dev_err(&priv->serdev->dev, "EE pkt. Extra data received");
425 status = STATUS_CRIT;
426
427 } else {
428 status = (buf[0] == 1) ? STATUS_OK : STATUS_FAIL;
429 }
430 bno055_sl_handle_rx(priv, status);
431 priv->rx.state = RX_IDLE;
432 break;
433
434 } else {
435 /*priv->rx.type == 0xBB */
436 priv->rx.state = RX_DATA;
437 priv->rx.expected_len = buf[0];
438 size--;
439 buf++;
440 }
441 fallthrough;
442
443 case RX_DATA:
444 /* Header parsed; now receiving packet data payload */
445 if (size == 0)
446 break;
447
448 if (priv->rx.databuf_count + size > priv->rx.expected_len) {
449 /*
450 * This is a inconsistency in serial protocol, we lost
451 * sync and we don't know how to handle further data
452 */
453 dev_err(&priv->serdev->dev, "BB pkt. Extra data received");
454 bno055_sl_handle_rx(priv, STATUS_CRIT);
455 priv->rx.state = RX_IDLE;
456 break;
457 }
458
459 mutex_lock(&priv->lock);
460 /*
461 * NULL e.g. when read cmd is stale or when no read cmd is
462 * actually pending.
463 */
464 if (priv->response_buf &&
465 /*
466 * Snoop on the upper layer protocol stuff to make sure not
467 * to write to an invalid memory. Apart for this, let's the
468 * upper layer manage any inconsistency wrt expected data
469 * len (as long as the serial protocol is consistent wrt
470 * itself (i.e. response header is consistent with received
471 * response len.
472 */
473 (priv->rx.databuf_count + size <= priv->expected_data_len))
474 memcpy(priv->response_buf + priv->rx.databuf_count,
475 buf, size);
476 mutex_unlock(&priv->lock);
477
478 priv->rx.databuf_count += size;
479
480 /*
481 * Reached expected len advertised by the IMU for the current
482 * packet. Pass it to the upper layer (for us it is just valid).
483 */
484 if (priv->rx.databuf_count == priv->rx.expected_len) {
485 bno055_sl_handle_rx(priv, STATUS_OK);
486 priv->rx.state = RX_IDLE;
487 }
488 break;
489 }
490
491 return _size;
492 }
493

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx

Attachment: .config.gz
Description: application/gzip