mbox series

[v5,00/14] Add support for Bosch BNO055 IMU

Message ID 20220426131102.23966-1-andrea.merello@gmail.com (mailing list archive)
Headers show
Series Add support for Bosch BNO055 IMU | expand

Message

Andrea Merello April 26, 2022, 1:10 p.m. UTC
From: Andrea Merello <andrea.merello@iit.it>

This series (tries to) add support for Bosch BNO055 IMU to Linux IIO
subsystem. It is made up several patches:

  1/14 to 6/14: add some IIO modifiers, and their documentation, to the IIO
                core layer, in order to being able to expose the linear
                acceleration and Euler angles among standard attributes.
                Also update the IIO event monitor tool

  7/14: fix binary attributes didn't work with IIO

  8/14 to 11/14: add the core IIO BNO055 driver and documentation for sysfs
                 attributes and DT bindings

  12/14: adds serdev BNO055 driver to actually use the IMU via serial line

  13/14: adds I2C BNO055 driver to actually use the IMU via I2C wiring

  14/14: add a documentation file that describe the bno055 driver and
         specifically the calibration

Differences wrt v4:
- be more tolerant wrt unrecognized chip IDs
- rename 'serial_number' attribute in 'serialnumber'
- fix missing NULL variable initialization
- use sign_extend32() instead of s16 casting where appropriate
- fix quaternion unit
- minor stuff (e.g. comments..)
- reduce (slightly) locking in serdev driver
- rework tracepoint support (e.g. remove dedicated config option)

Differences wrt other BNO055 drivers:

  Previously at least another driver for the very same chip has been posted
  to the Linux ML [0], but it has been never merged, and it seems no one
  cared of it since quite a long time.

  This driver differs from the above driver on the following aspects:

  - This driver supports also serial access (to be reliable, reset pin is
    required to be wired)

  - The above driver tried to support all IMU HW modes by allowing to
    choose one in the DT, and adapting IIO attributes accordingly. This
    driver does not rely on DT for this, instead settings are done via
    sysfs attributes.  All IIO attributes are always exposed; more on this
    later on. This driver however supports only a subset of the
    HW-supported modes.

  - This driver has some support for managing the IMU calibration

Supported operation modes:

  - AMG (accelerometer, magnetometer and gyroscope) mode, which provides
    raw (uncalibrated) measurements from the said sensors, and allows for
    setting some parameters about them (e.g. filter cut-off frequency, max
    sensor ranges, etc).

  - Fusion mode, which still provides AMG measures, while it also provides
    other data calculated by the IMU (e.g. rotation angles, linear
    acceleration, etc). In this mode user has no freedom to set any sensor
    parameter, since the HW locks them. Autocalibration and correction is
    performed by the IMU.

  IIO attributes exposing sensors parameters are always present, but in
  fusion modes the available values are constrained to just the one used by
  the HW. This is reflected in the '*_available' IIO attributes.

  Trying to set a not-supported value always falls back to the closest
  supported one, which in this case is just the one in use by the HW.

  IIO attributes for unavailable measurements (e.g. Euler angles in AMG
  mode) can't be read (return -EBUSY, or refuse to enable buffer).

IMU calibration:

  The IMU supports for two sets of calibration parameters:

  - SIC matrix. user-provided; this driver doesn't currently support it

  - Offset and radius parameters. The IMU automatically finds out them when
    it is running in fusion mode; supported by this driver.

  The driver provides access to autocalibration flags (i.e. you can known
  if the IMU has successfully autocalibrated) and to calibration data blob.
  The user can save this blob in a "firmware" file (i.e. in /lib/firmware)
  that the driver looks for at probe time. If found, then the IMU is
  initialized with this calibration data. This saves the user from
  performing the calibration procedure every time (which consist of moving
  the IMU in various way).

  The driver looks for calibration data file using two different names:
  first a file whose name is suffixed with the IMU unique ID is searched
  for; this is useful when there is more than one IMU instance. If this
  file is not found, then a "generic" calibration file is searched for
  (which can be used when only one IMU is present, without struggling with
  fancy names, that changes on each device).

  In AMG mode the IIO 'offset' attributes provide access to the offsets
  from calibration data (if any), so that the user can apply them to the
  accel, angvel and magn IIO attributes. In fusion mode they are not needed
  and read as zero.


Access protocols and serdev module:

  The serial protocol is quite simple, but there are tricks to make it
  really works. Those tricks and workarounds are documented in the driver
  source file.

  The core BNO055 driver tries to group readings in burst when appropriate,
  in order to optimize triggered buffer operation. The threshold for
  splitting a burst (i.e. max number of unused bytes in the middle of a
  burst that will be throw away) is provided to the core driver by the
  lowlevel access driver (either serdev or I2C) at probe time.

[0] https://www.spinics.net/lists/linux-iio/msg25508.html

Andrea Merello (14):
  iio: add modifiers for linear acceleration
  iio: document linear acceleration modifiers
  iio: event_monitor: add linear acceleration modifiers
  iio: add modifers for pitch, yaw, roll
  iio: document pitch, yaw, roll modifiers
  iio: event_monitor: add pitch, yaw and roll modifiers
  iio: add support for binary attributes
  iio: imu: add Bosch Sensortec BNO055 core driver
  iio: document bno055 private sysfs attributes
  iio: document "serialnumber" sysfs attribute
  dt-bindings: iio/imu: Add Bosch BNO055
  iio: imu: add BNO055 serdev driver
  iio: imu: add BNO055 I2C driver
  docs: iio: add documentation for BNO055 driver

 Documentation/ABI/testing/sysfs-bus-iio       |   25 +
 .../ABI/testing/sysfs-bus-iio-bno055          |   81 +
 .../bindings/iio/imu/bosch,bno055.yaml        |   59 +
 Documentation/iio/bno055.rst                  |   50 +
 Documentation/iio/index.rst                   |    2 +
 drivers/iio/imu/Kconfig                       |    1 +
 drivers/iio/imu/Makefile                      |    1 +
 drivers/iio/imu/bno055/Kconfig                |   25 +
 drivers/iio/imu/bno055/Makefile               |   10 +
 drivers/iio/imu/bno055/bno055.c               | 1710 +++++++++++++++++
 drivers/iio/imu/bno055/bno055.h               |   12 +
 drivers/iio/imu/bno055/bno055_i2c.c           |   57 +
 drivers/iio/imu/bno055/bno055_ser_core.c      |  560 ++++++
 drivers/iio/imu/bno055/bno055_ser_trace.h     |  104 +
 drivers/iio/industrialio-core.c               |   10 +-
 include/uapi/linux/iio/types.h                |    7 +-
 tools/iio/iio_event_monitor.c                 |    6 +
 17 files changed, 2718 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-bno055
 create mode 100644 Documentation/devicetree/bindings/iio/imu/bosch,bno055.yaml
 create mode 100644 Documentation/iio/bno055.rst
 create mode 100644 drivers/iio/imu/bno055/Kconfig
 create mode 100644 drivers/iio/imu/bno055/Makefile
 create mode 100644 drivers/iio/imu/bno055/bno055.c
 create mode 100644 drivers/iio/imu/bno055/bno055.h
 create mode 100644 drivers/iio/imu/bno055/bno055_i2c.c
 create mode 100644 drivers/iio/imu/bno055/bno055_ser_core.c
 create mode 100644 drivers/iio/imu/bno055/bno055_ser_trace.h

--
2.17.1

Comments

Andy Shevchenko April 27, 2022, 1:42 p.m. UTC | #1
On Tue, Apr 26, 2022 at 3:11 PM Andrea Merello <andrea.merello@gmail.com> wrote:
>
> From: Andrea Merello <andrea.merello@iit.it>
>
> This series (tries to) add support for Bosch BNO055 IMU to Linux IIO
> subsystem. It is made up several patches:
>
>   1/14 to 6/14: add some IIO modifiers, and their documentation, to the IIO
>                 core layer, in order to being able to expose the linear
>                 acceleration and Euler angles among standard attributes.
>                 Also update the IIO event monitor tool
>
>   7/14: fix binary attributes didn't work with IIO
>
>   8/14 to 11/14: add the core IIO BNO055 driver and documentation for sysfs
>                  attributes and DT bindings
>
>   12/14: adds serdev BNO055 driver to actually use the IMU via serial line
>
>   13/14: adds I2C BNO055 driver to actually use the IMU via I2C wiring
>
>   14/14: add a documentation file that describe the bno055 driver and
>          specifically the calibration

FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
for non-commented patches (12 out of 14 AFAICS).

> Differences wrt v4:
> - be more tolerant wrt unrecognized chip IDs
> - rename 'serial_number' attribute in 'serialnumber'
> - fix missing NULL variable initialization
> - use sign_extend32() instead of s16 casting where appropriate
> - fix quaternion unit
> - minor stuff (e.g. comments..)
> - reduce (slightly) locking in serdev driver
> - rework tracepoint support (e.g. remove dedicated config option)
>
> Differences wrt other BNO055 drivers:
>
>   Previously at least another driver for the very same chip has been posted
>   to the Linux ML [0], but it has been never merged, and it seems no one
>   cared of it since quite a long time.
>
>   This driver differs from the above driver on the following aspects:
>
>   - This driver supports also serial access (to be reliable, reset pin is
>     required to be wired)
>
>   - The above driver tried to support all IMU HW modes by allowing to
>     choose one in the DT, and adapting IIO attributes accordingly. This
>     driver does not rely on DT for this, instead settings are done via
>     sysfs attributes.  All IIO attributes are always exposed; more on this
>     later on. This driver however supports only a subset of the
>     HW-supported modes.
>
>   - This driver has some support for managing the IMU calibration
>
> Supported operation modes:
>
>   - AMG (accelerometer, magnetometer and gyroscope) mode, which provides
>     raw (uncalibrated) measurements from the said sensors, and allows for
>     setting some parameters about them (e.g. filter cut-off frequency, max
>     sensor ranges, etc).
>
>   - Fusion mode, which still provides AMG measures, while it also provides
>     other data calculated by the IMU (e.g. rotation angles, linear
>     acceleration, etc). In this mode user has no freedom to set any sensor
>     parameter, since the HW locks them. Autocalibration and correction is
>     performed by the IMU.
>
>   IIO attributes exposing sensors parameters are always present, but in
>   fusion modes the available values are constrained to just the one used by
>   the HW. This is reflected in the '*_available' IIO attributes.
>
>   Trying to set a not-supported value always falls back to the closest
>   supported one, which in this case is just the one in use by the HW.
>
>   IIO attributes for unavailable measurements (e.g. Euler angles in AMG
>   mode) can't be read (return -EBUSY, or refuse to enable buffer).
>
> IMU calibration:
>
>   The IMU supports for two sets of calibration parameters:
>
>   - SIC matrix. user-provided; this driver doesn't currently support it
>
>   - Offset and radius parameters. The IMU automatically finds out them when
>     it is running in fusion mode; supported by this driver.
>
>   The driver provides access to autocalibration flags (i.e. you can known
>   if the IMU has successfully autocalibrated) and to calibration data blob.
>   The user can save this blob in a "firmware" file (i.e. in /lib/firmware)
>   that the driver looks for at probe time. If found, then the IMU is
>   initialized with this calibration data. This saves the user from
>   performing the calibration procedure every time (which consist of moving
>   the IMU in various way).
>
>   The driver looks for calibration data file using two different names:
>   first a file whose name is suffixed with the IMU unique ID is searched
>   for; this is useful when there is more than one IMU instance. If this
>   file is not found, then a "generic" calibration file is searched for
>   (which can be used when only one IMU is present, without struggling with
>   fancy names, that changes on each device).
>
>   In AMG mode the IIO 'offset' attributes provide access to the offsets
>   from calibration data (if any), so that the user can apply them to the
>   accel, angvel and magn IIO attributes. In fusion mode they are not needed
>   and read as zero.
>
>
> Access protocols and serdev module:
>
>   The serial protocol is quite simple, but there are tricks to make it
>   really works. Those tricks and workarounds are documented in the driver
>   source file.
>
>   The core BNO055 driver tries to group readings in burst when appropriate,
>   in order to optimize triggered buffer operation. The threshold for
>   splitting a burst (i.e. max number of unused bytes in the middle of a
>   burst that will be throw away) is provided to the core driver by the
>   lowlevel access driver (either serdev or I2C) at probe time.
>
> [0] https://www.spinics.net/lists/linux-iio/msg25508.html
>
> Andrea Merello (14):
>   iio: add modifiers for linear acceleration
>   iio: document linear acceleration modifiers
>   iio: event_monitor: add linear acceleration modifiers
>   iio: add modifers for pitch, yaw, roll
>   iio: document pitch, yaw, roll modifiers
>   iio: event_monitor: add pitch, yaw and roll modifiers
>   iio: add support for binary attributes
>   iio: imu: add Bosch Sensortec BNO055 core driver
>   iio: document bno055 private sysfs attributes
>   iio: document "serialnumber" sysfs attribute
>   dt-bindings: iio/imu: Add Bosch BNO055
>   iio: imu: add BNO055 serdev driver
>   iio: imu: add BNO055 I2C driver
>   docs: iio: add documentation for BNO055 driver
>
>  Documentation/ABI/testing/sysfs-bus-iio       |   25 +
>  .../ABI/testing/sysfs-bus-iio-bno055          |   81 +
>  .../bindings/iio/imu/bosch,bno055.yaml        |   59 +
>  Documentation/iio/bno055.rst                  |   50 +
>  Documentation/iio/index.rst                   |    2 +
>  drivers/iio/imu/Kconfig                       |    1 +
>  drivers/iio/imu/Makefile                      |    1 +
>  drivers/iio/imu/bno055/Kconfig                |   25 +
>  drivers/iio/imu/bno055/Makefile               |   10 +
>  drivers/iio/imu/bno055/bno055.c               | 1710 +++++++++++++++++
>  drivers/iio/imu/bno055/bno055.h               |   12 +
>  drivers/iio/imu/bno055/bno055_i2c.c           |   57 +
>  drivers/iio/imu/bno055/bno055_ser_core.c      |  560 ++++++
>  drivers/iio/imu/bno055/bno055_ser_trace.h     |  104 +
>  drivers/iio/industrialio-core.c               |   10 +-
>  include/uapi/linux/iio/types.h                |    7 +-
>  tools/iio/iio_event_monitor.c                 |    6 +
>  17 files changed, 2718 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-bno055
>  create mode 100644 Documentation/devicetree/bindings/iio/imu/bosch,bno055.yaml
>  create mode 100644 Documentation/iio/bno055.rst
>  create mode 100644 drivers/iio/imu/bno055/Kconfig
>  create mode 100644 drivers/iio/imu/bno055/Makefile
>  create mode 100644 drivers/iio/imu/bno055/bno055.c
>  create mode 100644 drivers/iio/imu/bno055/bno055.h
>  create mode 100644 drivers/iio/imu/bno055/bno055_i2c.c
>  create mode 100644 drivers/iio/imu/bno055/bno055_ser_core.c
>  create mode 100644 drivers/iio/imu/bno055/bno055_ser_trace.h
>
> --
> 2.17.1
Jonathan Cameron May 1, 2022, 5:03 p.m. UTC | #2
On Wed, 27 Apr 2022 15:42:49 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Tue, Apr 26, 2022 at 3:11 PM Andrea Merello <andrea.merello@gmail.com> wrote:
> >
> > From: Andrea Merello <andrea.merello@iit.it>
> >
> > This series (tries to) add support for Bosch BNO055 IMU to Linux IIO
> > subsystem. It is made up several patches:
> >
> >   1/14 to 6/14: add some IIO modifiers, and their documentation, to the IIO
> >                 core layer, in order to being able to expose the linear
> >                 acceleration and Euler angles among standard attributes.
> >                 Also update the IIO event monitor tool
> >
> >   7/14: fix binary attributes didn't work with IIO
> >
> >   8/14 to 11/14: add the core IIO BNO055 driver and documentation for sysfs
> >                  attributes and DT bindings
> >
> >   12/14: adds serdev BNO055 driver to actually use the IMU via serial line
> >
> >   13/14: adds I2C BNO055 driver to actually use the IMU via I2C wiring
> >
> >   14/14: add a documentation file that describe the bno055 driver and
> >          specifically the calibration  
> 
> FWIW,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> for non-commented patches (12 out of 14 AFAICS).
> 
FWIW I'm fine with the series once you've tidied up the stuff Andy picked up
on.

Thanks Andy for the detailed reviewing btw.

Jonathan

> > Differences wrt v4:
> > - be more tolerant wrt unrecognized chip IDs
> > - rename 'serial_number' attribute in 'serialnumber'
> > - fix missing NULL variable initialization
> > - use sign_extend32() instead of s16 casting where appropriate
> > - fix quaternion unit
> > - minor stuff (e.g. comments..)
> > - reduce (slightly) locking in serdev driver
> > - rework tracepoint support (e.g. remove dedicated config option)
> >
> > Differences wrt other BNO055 drivers:
> >
> >   Previously at least another driver for the very same chip has been posted
> >   to the Linux ML [0], but it has been never merged, and it seems no one
> >   cared of it since quite a long time.
> >
> >   This driver differs from the above driver on the following aspects:
> >
> >   - This driver supports also serial access (to be reliable, reset pin is
> >     required to be wired)
> >
> >   - The above driver tried to support all IMU HW modes by allowing to
> >     choose one in the DT, and adapting IIO attributes accordingly. This
> >     driver does not rely on DT for this, instead settings are done via
> >     sysfs attributes.  All IIO attributes are always exposed; more on this
> >     later on. This driver however supports only a subset of the
> >     HW-supported modes.
> >
> >   - This driver has some support for managing the IMU calibration
> >
> > Supported operation modes:
> >
> >   - AMG (accelerometer, magnetometer and gyroscope) mode, which provides
> >     raw (uncalibrated) measurements from the said sensors, and allows for
> >     setting some parameters about them (e.g. filter cut-off frequency, max
> >     sensor ranges, etc).
> >
> >   - Fusion mode, which still provides AMG measures, while it also provides
> >     other data calculated by the IMU (e.g. rotation angles, linear
> >     acceleration, etc). In this mode user has no freedom to set any sensor
> >     parameter, since the HW locks them. Autocalibration and correction is
> >     performed by the IMU.
> >
> >   IIO attributes exposing sensors parameters are always present, but in
> >   fusion modes the available values are constrained to just the one used by
> >   the HW. This is reflected in the '*_available' IIO attributes.
> >
> >   Trying to set a not-supported value always falls back to the closest
> >   supported one, which in this case is just the one in use by the HW.
> >
> >   IIO attributes for unavailable measurements (e.g. Euler angles in AMG
> >   mode) can't be read (return -EBUSY, or refuse to enable buffer).
> >
> > IMU calibration:
> >
> >   The IMU supports for two sets of calibration parameters:
> >
> >   - SIC matrix. user-provided; this driver doesn't currently support it
> >
> >   - Offset and radius parameters. The IMU automatically finds out them when
> >     it is running in fusion mode; supported by this driver.
> >
> >   The driver provides access to autocalibration flags (i.e. you can known
> >   if the IMU has successfully autocalibrated) and to calibration data blob.
> >   The user can save this blob in a "firmware" file (i.e. in /lib/firmware)
> >   that the driver looks for at probe time. If found, then the IMU is
> >   initialized with this calibration data. This saves the user from
> >   performing the calibration procedure every time (which consist of moving
> >   the IMU in various way).
> >
> >   The driver looks for calibration data file using two different names:
> >   first a file whose name is suffixed with the IMU unique ID is searched
> >   for; this is useful when there is more than one IMU instance. If this
> >   file is not found, then a "generic" calibration file is searched for
> >   (which can be used when only one IMU is present, without struggling with
> >   fancy names, that changes on each device).
> >
> >   In AMG mode the IIO 'offset' attributes provide access to the offsets
> >   from calibration data (if any), so that the user can apply them to the
> >   accel, angvel and magn IIO attributes. In fusion mode they are not needed
> >   and read as zero.
> >
> >
> > Access protocols and serdev module:
> >
> >   The serial protocol is quite simple, but there are tricks to make it
> >   really works. Those tricks and workarounds are documented in the driver
> >   source file.
> >
> >   The core BNO055 driver tries to group readings in burst when appropriate,
> >   in order to optimize triggered buffer operation. The threshold for
> >   splitting a burst (i.e. max number of unused bytes in the middle of a
> >   burst that will be throw away) is provided to the core driver by the
> >   lowlevel access driver (either serdev or I2C) at probe time.
> >
> > [0] https://www.spinics.net/lists/linux-iio/msg25508.html
> >
> > Andrea Merello (14):
> >   iio: add modifiers for linear acceleration
> >   iio: document linear acceleration modifiers
> >   iio: event_monitor: add linear acceleration modifiers
> >   iio: add modifers for pitch, yaw, roll
> >   iio: document pitch, yaw, roll modifiers
> >   iio: event_monitor: add pitch, yaw and roll modifiers
> >   iio: add support for binary attributes
> >   iio: imu: add Bosch Sensortec BNO055 core driver
> >   iio: document bno055 private sysfs attributes
> >   iio: document "serialnumber" sysfs attribute
> >   dt-bindings: iio/imu: Add Bosch BNO055
> >   iio: imu: add BNO055 serdev driver
> >   iio: imu: add BNO055 I2C driver
> >   docs: iio: add documentation for BNO055 driver
> >
> >  Documentation/ABI/testing/sysfs-bus-iio       |   25 +
> >  .../ABI/testing/sysfs-bus-iio-bno055          |   81 +
> >  .../bindings/iio/imu/bosch,bno055.yaml        |   59 +
> >  Documentation/iio/bno055.rst                  |   50 +
> >  Documentation/iio/index.rst                   |    2 +
> >  drivers/iio/imu/Kconfig                       |    1 +
> >  drivers/iio/imu/Makefile                      |    1 +
> >  drivers/iio/imu/bno055/Kconfig                |   25 +
> >  drivers/iio/imu/bno055/Makefile               |   10 +
> >  drivers/iio/imu/bno055/bno055.c               | 1710 +++++++++++++++++
> >  drivers/iio/imu/bno055/bno055.h               |   12 +
> >  drivers/iio/imu/bno055/bno055_i2c.c           |   57 +
> >  drivers/iio/imu/bno055/bno055_ser_core.c      |  560 ++++++
> >  drivers/iio/imu/bno055/bno055_ser_trace.h     |  104 +
> >  drivers/iio/industrialio-core.c               |   10 +-
> >  include/uapi/linux/iio/types.h                |    7 +-
> >  tools/iio/iio_event_monitor.c                 |    6 +
> >  17 files changed, 2718 insertions(+), 2 deletions(-)
> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-bno055
> >  create mode 100644 Documentation/devicetree/bindings/iio/imu/bosch,bno055.yaml
> >  create mode 100644 Documentation/iio/bno055.rst
> >  create mode 100644 drivers/iio/imu/bno055/Kconfig
> >  create mode 100644 drivers/iio/imu/bno055/Makefile
> >  create mode 100644 drivers/iio/imu/bno055/bno055.c
> >  create mode 100644 drivers/iio/imu/bno055/bno055.h
> >  create mode 100644 drivers/iio/imu/bno055/bno055_i2c.c
> >  create mode 100644 drivers/iio/imu/bno055/bno055_ser_core.c
> >  create mode 100644 drivers/iio/imu/bno055/bno055_ser_trace.h
> >
> > --
> > 2.17.1  
> 
> 
>
Andrea Merello May 2, 2022, 6:33 a.m. UTC | #3
Il giorno dom 1 mag 2022 alle ore 18:54 Jonathan Cameron
<jic23@kernel.org> ha scritto:
>
> On Wed, 27 Apr 2022 15:42:49 +0200
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
> > On Tue, Apr 26, 2022 at 3:11 PM Andrea Merello <andrea.merello@gmail.com> wrote:
> > >
> > > From: Andrea Merello <andrea.merello@iit.it>
> > >
> > > This series (tries to) add support for Bosch BNO055 IMU to Linux IIO
> > > subsystem. It is made up several patches:
> > >
> > >   1/14 to 6/14: add some IIO modifiers, and their documentation, to the IIO
> > >                 core layer, in order to being able to expose the linear
> > >                 acceleration and Euler angles among standard attributes.
> > >                 Also update the IIO event monitor tool
> > >
> > >   7/14: fix binary attributes didn't work with IIO
> > >
> > >   8/14 to 11/14: add the core IIO BNO055 driver and documentation for sysfs
> > >                  attributes and DT bindings
> > >
> > >   12/14: adds serdev BNO055 driver to actually use the IMU via serial line
> > >
> > >   13/14: adds I2C BNO055 driver to actually use the IMU via I2C wiring
> > >
> > >   14/14: add a documentation file that describe the bno055 driver and
> > >          specifically the calibration
> >
> > FWIW,
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > for non-commented patches (12 out of 14 AFAICS).
> >
> FWIW I'm fine with the series once you've tidied up the stuff Andy picked up
> on.
>
> Thanks Andy for the detailed reviewing btw.
>
> Jonathan

I'm very grateful to both of you and to everyone who commented on
those patches. Thanks :). Beside the "Reviewed-by" tags where
appropriate, is it usual/appropriate to put some tag like "Thanks-to
.. [for comments]" ?

BTW I have also gone through some kernel-robot reports; they also
state "If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>". I'd say that it would
be OK to add this tag to a patch that just fixes what is reported, but
I'm unsure whether it is appropriate to add this tag to the patches in
my series, because they add the code and the fix at once. Any advice
here?

Andrea
Andy Shevchenko May 2, 2022, 7:47 a.m. UTC | #4
On Mon, May 2, 2022 at 8:33 AM Andrea Merello <andrea.merello@gmail.com> wrote:
> Il giorno dom 1 mag 2022 alle ore 18:54 Jonathan Cameron
> <jic23@kernel.org> ha scritto:
> > On Wed, 27 Apr 2022 15:42:49 +0200
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > On Tue, Apr 26, 2022 at 3:11 PM Andrea Merello <andrea.merello@gmail.com> wrote:

...

> > > FWIW,
> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > for non-commented patches (12 out of 14 AFAICS).
> > >
> > FWIW I'm fine with the series once you've tidied up the stuff Andy picked up
> > on.
> >
> > Thanks Andy for the detailed reviewing btw.

You/re welcome!

> I'm very grateful to both of you and to everyone who commented on
> those patches. Thanks :). Beside the "Reviewed-by" tags where
> appropriate, is it usual/appropriate to put some tag like "Thanks-to
> .. [for comments]" ?

Nope, just mention that in the cover letter.

> BTW I have also gone through some kernel-robot reports; they also
> state "If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>". I'd say that it would
> be OK to add this tag to a patch that just fixes what is reported, but
> I'm unsure whether it is appropriate to add this tag to the patches in
> my series, because they add the code and the fix at once. Any advice
> here?

For this we specifically amended the kernel documentation recently.
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes

"The tag is intended for bugs; please do not use it to credit feature requests."
Andrea Merello May 2, 2022, 8:31 a.m. UTC | #5
Il giorno lun 2 mag 2022 alle ore 09:48 Andy Shevchenko
<andy.shevchenko@gmail.com> ha scritto:
>

[ .. ]

> > BTW I have also gone through some kernel-robot reports; they also
> > state "If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>". I'd say that it would
> > be OK to add this tag to a patch that just fixes what is reported, but
> > I'm unsure whether it is appropriate to add this tag to the patches in
> > my series, because they add the code and the fix at once. Any advice
> > here?
>
> For this we specifically amended the kernel documentation recently.
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes
>
> "The tag is intended for bugs; please do not use it to credit feature requests."

Well, no any feature request to credit here; a bug and its fix are
involved. Sounds more like a "yes" so far.. But it wouldn't be clear
what the robot did report indeed (squashed bugs and fixes).. Maybe a
"thank" in the cover letter also to it would suffice?

> --
> With Best Regards,
> Andy Shevchenko
Andy Shevchenko May 2, 2022, 8:52 a.m. UTC | #6
On Mon, May 2, 2022 at 10:31 AM Andrea Merello <andrea.merello@gmail.com> wrote:
> Il giorno lun 2 mag 2022 alle ore 09:48 Andy Shevchenko
> <andy.shevchenko@gmail.com> ha scritto:

...

> > > BTW I have also gone through some kernel-robot reports; they also
> > > state "If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kernel test robot <lkp@intel.com>". I'd say that it would
> > > be OK to add this tag to a patch that just fixes what is reported, but
> > > I'm unsure whether it is appropriate to add this tag to the patches in
> > > my series, because they add the code and the fix at once. Any advice
> > > here?
> >
> > For this we specifically amended the kernel documentation recently.
> > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes
> >
> > "The tag is intended for bugs; please do not use it to credit feature requests."
>
> Well, no any feature request to credit here; a bug and its fix are
> involved. Sounds more like a "yes" so far.. But it wouldn't be clear
> what the robot did report indeed (squashed bugs and fixes).. Maybe a
> "thank" in the cover letter also to it would suffice?

The reported from bots should be used when your patches.changes
induced by those reports, it seems otherwise in your case.,