Add support for Bosch BNO055 IMU

From: Andrea Merello <[email protected]>

Add the core driver for the BNO055 IMU from Bosch. This IMU can be connected via both serial and I2C busses; separate patches will add support for them.

The driver supports "AMG" (Accelerometer, Magnetometer, Gyroscope) mode, that provides raw data from the said internal sensors, and a couple of "fusion" modes (i.e. the IMU also do calculations in order to provide euler

angles, quaternions, linear acceleration and gravity measurements).

In fusion modes the AMG data is still available (with some calibration refinements done by the IMU), but certain settings such as low pass filters cut-off frequency and sensors ranges are fixed, while in AMG mode they can

be customized; this is why AMG mode can still be interesting.

+#define BNO055_FW_GENERIC_NAME (BNO055_FW_NAME BNO055_FW_EXT)

+/*
+ * Theoretically the IMU should return data in a given (i.e. fixed) unit
+ * regardless the range setting. This happens for the accelerometer, but not for

+ * the gyroscope; the gyroscope range setting affects the scale.
+ * This is probably due to this[0] bug.
+ * For this reason we map the internal range setting onto the standard IIO scale
+ * attribute for gyro.
+ * Since the bug[0] may be fixed in future, we check for the IMU FW version and
+ * eventually warn the user.
+ * Currently we just don't care about "range" attributes for gyro.
+ *
+ * [0] https://community.bosch-sensortec.com/t5/MEMS-sensors-forum/BNO055-Wrong-sensitivity-resolution-in-datasheet/td-p/10266
+ */

+}

+ },

+ msleep(20);

drivers/iio/imu/bno055/bno055.c new file +11 -0

+               ret = regmap_write(priv->regmap, BNO055_SYS_TRIGGER_REG,
+                                  BNO055_SYS_TRIGGER_RST_SYS);
+               if (ret)
+                       return ret;
+       }
+
+       regcache_drop_region(priv->regmap, 0x0, 0xff);
+       usleep_range(650000, 700000);
+
+       return 0;
+}

+exit:

drivers/iio/imu/bno055/bno055.c new file +3 -0

+       mutex_unlock(&priv->lock);
+       return ret;
+}

+ }, \

+ },

drivers/iio/imu/bno055/bno055.c new file +3 -0

+               len /= 2;
+               req_val = val * 1000000 + val2;
+       }

+ if (first || delta < best_delta) {

drivers/iio/imu/bno055/bno055.c new file +4 -0

+                       best_delta = delta;
+                       hwval = i;
+                       first = false;
+               }

(quoted diff) +2 -0

+                        * offset, thus we need the 'minus' here.
+                        */

drivers/iio/imu/bno055/bno055.c new file +4 -0

+                       return bno055_get_regmask(priv, val, val2,
+                                                 BNO055_MAG_CONFIG_REG,
+                                                 BNO055_MAG_CONFIG_ODR_MASK,
+                                                 &bno055_mag_odr);

+ int ret = 0;

+ } else {

drivers/iio/imu/bno055/bno055.c new file +13 -0

+               /*
+                * Coming from AMG means the FMC was off, just switch to fusion
+                * but don't change anything that doesn't belong to us (i.e let
+                * FMC stay off.
+                * Coming from any other fusion mode means we don't need to do
+                * anything.
+                */
+               if (priv->operation_mode == BNO055_OPR_MODE_AMG)
+                       ret = bno055_operation_mode_set(priv, BNO055_OPR_MODE_FUSION_FMC_OFF);
+       }
+       if (ret)
+               return ret;
+       return len;

+ if (count < BNO055_CALDATA_LEN || pos != 0)

+ return -EINVAL;

+unlock:

(quoted diff) +2 -0

+       mutex_unlock(&priv->lock);
+       return ret;

drivers/iio/imu/bno055/bno055.c new file +4 -0

+        * will be included in same transfer.
+        * Every time the bitmap contains a gap wider than xfer_burst_break_thr
+        * then we split the transfer, skipping the gap.
+        */

+ }

+}

drivers/iio/imu/bno055/bno055.c new file +2 -0

+                       dev_warn(dev, "No usable reset method; IMU may be unreliable");
+       }

+ * know what other FW version might do..

+ */

drivers/iio/imu/bno055/bno055.c new file +4 -0

+       if (ret)
+               dev_notice(dev, "Calibration file load failed. See instruction in kernel Documentation/iio/bno055.rst");
+
+       if (caldata) {

+#include <linux/regmap.h>

drivers/iio/imu/bno055/bno055.h new file +4 -0

+struct device;
+int bno055_probe(struct device *dev, struct regmap *regmap,
+                int xfer_burst_break_thr, bool sw_reset);
+extern const struct regmap_config bno055_regmap_config;

drivers/iio/imu/bno055/bno055.c line 129

Quoted 4 lines

drivers/iio/imu/bno055/bno055.c new file +3 -0

+#define BNO055_ATTR_VALS(...)          \
+       .vals = (int[]){ __VA_ARGS__},  \
+       .len = ARRAY_SIZE(((int[]){__VA_ARGS__}))

Not sure this adds any readability to the code. Can we simply have an array of int for each case with the explicit ARRAY_SIZE() calls?

Quoted 2 lines

+ msleep(20);

Perhaps a comment why so long sleep is needed.

drivers/iio/imu/bno055/bno055.c line 1026

Quoted 4 lines

drivers/iio/imu/bno055/bno055.c new file +3 -0

+       for (i = 0; i < bno055_acc_range.len; i++)
+               len += sysfs_emit_at(buf, len, "%d%c", bno055_acc_range.vals[i],
+                                    (i == bno055_acc_range.len - 1) ? '\n' : ' ');

You may move the condition out of the loop.

drivers/iio/imu/bno055/bno055.c line 1053

Quoted 6 lines

drivers/iio/imu/bno055/bno055.c new file +6 -0

+       if (indio_dev->active_scan_mask &&
+           !bitmap_empty(indio_dev->active_scan_mask, _BNO055_SCAN_MAX))
+               return -EBUSY;
+
+       if (sysfs_streq(buf, "0")) {
+               ret = bno055_operation_mode_set(priv, BNO055_OPR_MODE_AMG);

return bno055_operation_mode_set(...);

Quoted 1 line

+ } else {

...and drop this with the following decreasing indentation.

Can be removed to group all related checks together.

Quoted 4 lines

drivers/iio/imu/bno055/bno055.c new file +4 -0

+       if (ret)
+               dev_notice(dev, "Calibration file load failed. See instruction in kernel Documentation/iio/bno055.rst");
+
+       if (caldata) {

drivers/iio/imu/bno055/bno055.c line 1675

Quoted 10 lines

drivers/iio/imu/bno055/bno055.c new file +8 -0

+               caldata_data = caldata->data;
+               caldata_size = caldata->size;
+       }
+       ret = bno055_init(priv, caldata_data, caldata_size);
 
+       if (caldata)
+               release_firmware(caldata);
 
+       if (ret)
+               return ret;

Can be rewritten in a form of

if (caldata) {
ret = bno055_init();
release_firmware(...);
} else {
ret = bno055_init();
}
if (ret)
return ret;

?

Il giorno mer 27 apr 2022 alle ore 15:23 Andy Shevchenko <[email protected]> ha scritto:

Quoted 6 lines

drivers/iio/imu/bno055/bno055.c line 129

Quoted 3 lines

drivers/iio/imu/bno055/bno055.c new file +3 -0

+#define BNO055_ATTR_VALS(...)          \
+       .vals = (int[]){ __VA_ARGS__},  \
+       .len = ARRAY_SIZE(((int[]){__VA_ARGS__}))

Not sure this adds any readability to the code. Can we simply have an array of int for each case with the explicit ARRAY_SIZE() calls?

Do you mean moving the vals array out of the structs? Something like:

static int bno055_gyr_scale_vals[] = {125, 1877467, 250, 1877467,

        500, 1877467, 1000, 1877467, 2000, 1877467};

static struct bno055_sysfs_attr_aux_data bno055_gyr_scale_aux = {
        .fusion_vals = (int[]){1, 900},
        .hw_xlate = (int[]){4, 3, 2, 1, 0},
        .type = IIO_VAL_FRACTIONAL

?
But then I'd make also something like:

#define bno055_sysfs_attr_avail(priv, attr, vals, len) \
        _bno055_sysfs_attr_avail(priv, attr##_vals,

ARRAY_SIZE(attr##_vals), attr##_aux, vals, len)

And the same for all other users of those structs.

My point is not about readability,

but about avoiding as much as
possible bugs caused by mismatched attr_vals, attr_aux and ARRAY_SIZE() arg. e.g:
bno055_sysfs_attr_avail(priv, bno_foo_vals, ARRAY_SIZE(bno_bar_vals), bno_foobar_aux, vals, len)

I used to make quite a lot of mess until I grouped all the stuff in one struct :/

Quoted 3 lines

Quoted 1 line

+ msleep(20);

Perhaps a comment why so long sleep is needed.

DS says that switching mode can last from 7mS up to 19mS depending on the case, but I don't know _why_ it takes so long. I may add a comment that just states that it's a sensor requirement.

Quoted 5 lines

drivers/iio/imu/bno055/bno055.c line 1026

Quoted 3 lines

drivers/iio/imu/bno055/bno055.c new file +3 -0

+       for (i = 0; i < bno055_acc_range.len; i++)
+               len += sysfs_emit_at(buf, len, "%d%c", bno055_acc_range.vals[i],
+                                    (i == bno055_acc_range.len - 1) ? '\n' : ' ');

You may move the condition out of the loop.

May you elaborate, please? Do you mean something like: loop one time less, and then call sysfs_emit_at() once more outside the loop, getting rid of the conditional ternary operator at all?

Quoted 7 lines

Quoted 6 lines

drivers/iio/imu/bno055/bno055.c new file +6 -0

+       if (indio_dev->active_scan_mask &&
+           !bitmap_empty(indio_dev->active_scan_mask, _BNO055_SCAN_MAX))
+               return -EBUSY;
+
+       if (sysfs_streq(buf, "0")) {
+               ret = bno055_operation_mode_set(priv, BNO055_OPR_MODE_AMG);

Quoted 1 line

return bno055_operation_mode_set(...);

Why? bno055_operation_mode_set() returns an error code, while here we need to return the len, or propagate the error code only when it's the case

Quoted 3 lines

Quoted 1 line

+ } else {

...and drop this with the following decreasing indentation.

if you want to drop this, then I can just duplicate if(ret) return ret; i.e. add it after bno055_operation_mode_set(priv, BNO055_OPR_MODE_AMG); and get rid of the else branch (see above)

Quoted 1 line

Can be removed to group all related checks together.

I'm not sure what you mean here, but see below

Quoted 26 lines

Quoted 4 lines

drivers/iio/imu/bno055/bno055.c new file +4 -0

+       if (ret)
+               dev_notice(dev, "Calibration file load failed. See instruction in kernel Documentation/iio/bno055.rst");
+
+       if (caldata) {

drivers/iio/imu/bno055/bno055.c line 1675

Quoted 10 lines

drivers/iio/imu/bno055/bno055.c new file +8 -0

+               caldata_data = caldata->data;
+               caldata_size = caldata->size;
+       }
+       ret = bno055_init(priv, caldata_data, caldata_size);
 
+       if (caldata)
+               release_firmware(caldata);
 
+       if (ret)
+               return ret;

Can be rewritten in a form of

if (caldata) {
ret = bno055_init();
release_firmware(...);
} else {
ret = bno055_init();
}
if (ret)
return ret;

Quoted 1 line

?

Indeed I'd say it could be rewritten as:

        if (ret)
                ret = request_firmware(&caldata, BNO055_FW_GENERIC_NAME, dev);
        if (ret) {
                dev_notice(dev, "Calibration file load failed. See

instruction in kernel Documentation/iio/bno055.rst");

                ret = bno055_init(priv, NULL, 0);
        } else {
                ret = bno055_init(priv, caldata->data, caldata->size);
                release_firmware(caldata);
        }
        if (ret)
                return ret;

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

drivers/iio/imu/bno055/bno055.c new file +3 -0

+#define BNO055_ATTR_VALS(...)          \
+       .vals = (int[]){ __VA_ARGS__},  \
+       .len = ARRAY_SIZE(((int[]){__VA_ARGS__}))

And my point about readability. The reader, and even the author after some time, may have no clue in this forest of the macros and castings what's going on.

Quoted 6 lines

but about avoiding as much as
possible bugs caused by mismatched attr_vals, attr_aux and ARRAY_SIZE() arg. e.g:
bno055_sysfs_attr_avail(priv, bno_foo_vals, ARRAY_SIZE(bno_bar_vals), bno_foobar_aux, vals, len)

Quoted 2 lines

I used to make quite a lot of mess until I grouped all the stuff in one struct :/

If something you want to prevent at compile time, consider to utilize static_assert() and / or BUILD_BUG_ON() depending on the place in the code (the former is preferred).

Hi Andrea,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on linus/master linux/master v5.18-rc4 next-20220426] [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/intel-lab-lkp/linux/commits/Andrea-Merello/Add-support-for-Bosch-BNO055-IMU/20220426-212132
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220427/202204271554.EiuIRNPI-lkp(a)intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.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/intel-lab-lkp/linux/commit/675ca9cd13af45cc5943dd15caad5e866fd7c971
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Andrea-Merello/Add-support-for-Bosch-BNO055-IMU/20220426-212132 git checkout 675ca9cd13af45cc5943dd15caad5e866fd7c971
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

Quoted 1 line

Quoted 1 line

make[5]: *** No rule to make target 'drivers/iio/imu/bno055/bno055_ser_trace.o', needed by 'drivers/iio/imu/bno055/built-in.a'.

make[5]: Target '__build' not remade because of errors.

Quoted 31 lines

Hi Andrea,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on linus/master linux/master v5.18-rc4 next-20220426] [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/intel-lab-lkp/linux/commits/Andrea-Merello/Add-support-for-Bosch-BNO055-IMU/20220426-212132
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220427/202204271554.EiuIRNPI-lkp(a)intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.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/intel-lab-lkp/linux/commit/675ca9cd13af45cc5943dd15caad5e866fd7c971
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Andrea-Merello/Add-support-for-Bosch-BNO055-IMU/20220426-212132 git checkout 675ca9cd13af45cc5943dd15caad5e866fd7c971
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

Quoted 2 lines

Quoted 1 line

Quoted 1 line

make[5]: *** No rule to make target 'drivers/iio/imu/bno055/bno055_ser_trace.o', needed by 'drivers/iio/imu/bno055/built-in.a'.

make[5]: Target '__build' not remade because of errors.

You need to add a C-file with the only line

#include <..._trace.h>

And drop that include from the _core.c.

--
With Best Regards,
Andy Shevchenko

Quoted 5 lines

You need to add a C-file with the only line

#include <..._trace.h>

And drop that include from the _core.c.

Hum, I'm a bit confused here: the bno055_ser_core.c file explicitly looks for that tracepoints (e.g. it calls trace_send_chunks() and friends); dropping the include prevents build here because there would be no definition for those tracepoints.

There is already a C file bno055_ser_trace.c that just contains the said include and it defines CREATE_TRACE_POINTS; I see other drivers like dwc3 do the same..

But my problem is that I cannot reproduce the issue found by the bot:
the compiler that is downloaded by the script doesn't run on my build box because it wants a newer libc (I was hoping that those compilers were statically linked, but they aren't), while any other attempt I did with other older compilers resulted in either successful build or failed with other weird, apparently unrelated, errors about relocation issues (of course I tried with the arch and config used by the build bot).

Is there any build farm publicly available or something like that?

Quoted 3 lines

--
With Best Regards,
Andy Shevchenko

Il giorno mar 3 mag 2022 alle ore 09:48 Andrea Merello <[email protected]> ha scritto:

[...]

Quoted 6 lines

Quoted 5 lines

You need to add a C-file with the only line

#include <..._trace.h>

And drop that include from the _core.c.

Quoted 8 lines

Hum, I'm a bit confused here: the bno055_ser_core.c file explicitly looks for that tracepoints (e.g. it calls trace_send_chunks() and friends); dropping the include prevents build here because there would be no definition for those tracepoints.

There is already a C file bno055_ser_trace.c that just contains the said include and it defines CREATE_TRACE_POINTS; I see other drivers like dwc3 do the same..

Oops.. it turned out that I just had this almost-empty C file as untracked in my git tree, and it ended up not being included in patches also. Being it laying in my src tree caused the build to succeed.

I have been misled by the other problem I (still) have (below); I was focused on the wrong thing, sorry.

Quoted 8 lines

But my problem is that I cannot reproduce the issue found by the bot:
the compiler that is downloaded by the script doesn't run on my build box because it wants a newer libc (I was hoping that those compilers were statically linked, but they aren't), while any other attempt I did with other older compilers resulted in either successful build or failed with other weird, apparently unrelated, errors about relocation issues (of course I tried with the arch and config used by the build bot).

Quoted 1 line

Is there any build farm publicly available or something like that?

From: Andrea Merello <[email protected]>

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

Quoted 18 lines · Andrea Merello

From: Andrea Merello <[email protected]>

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

Quoted 4 lines

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 <[email protected]>

for non-commented patches (12 out of 14 AFAICS).

Quoted 144 lines

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

Quoted 19 lines · On Wed, 27 Apr 2022 15:42:49 +0200 Andy Shevchenko <[email protected]>…

Quoted 18 lines · Andrea Merello

From: Andrea Merello <[email protected]>

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

Quoted 9 lines

Quoted 4 lines

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 <[email protected]>

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

Il giorno dom 1 mag 2022 alle ore 18:54 Jonathan Cameron <[email protected]> ha scritto:

Quoted 3 lines

Quoted 1 line · On Wed, 27 Apr 2022 15:42:49 +0200 Andy Shevchenko <[email protected]>…

On Tue, Apr 26, 2022 at 3:11 PM Andrea Merello <[email protected]> wrote:

Quoted 8 lines

Quoted 4 lines

FWIW,

Reviewed-by Andy Shevchenko <[email protected]>

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.

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 <[email protected]>". 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?

Quoted 7 lines

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 <[email protected]>". 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."

--
With Best Regards,
Andy Shevchenko

Il giorno lun 2 mag 2022 alle ore 09:48 Andy Shevchenko <[email protected]> ha scritto:

Quoted 11 lines

Quoted 7 lines

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 <[email protected]>". 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

Quoted 1 line

"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?