From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Andrea Merello <andrea.merello@iit.it>
Cc: Jonathan Cameron <jic23@kernel.org>,
Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
linux-iio <linux-iio@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
devicetree <devicetree@vger.kernel.org>,
Lars-Peter Clausen <lars@metafoo.de>,
Rob Herring <robh+dt@kernel.org>,
Matt Ranostay <matt.ranostay@konsulko.com>,
Alexandru Ardelean <ardeleanalex@gmail.com>,
jmondi <jacopo@jmondi.org>
Subject: Re: [v6 08/14] iio: imu: add Bosch Sensortec BNO055 core driver
Date: Mon, 13 Jun 2022 18:44:09 +0200 [thread overview]
Message-ID: <CAHp75Vdh8kAH6540xCwzFh5uf=QMVTHC42a8pOgvkpObzjfD+w@mail.gmail.com> (raw)
In-Reply-To: <20220613120534.36991-9-andrea.merello@iit.it>
On Mon, Jun 13, 2022 at 2:05 PM <andrea.merello@iit.it> wrote:
>
> From: Andrea Merello <andrea.merello@iit.it>
>
> 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 does 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.
...
> +config BOSCH_BNO055_IIO
Does it need _IIO suffix? Any name collision?
...
> +static int bno055_acc_lpf_vals[] = {
> + 7, 810000, 15, 630000, 31, 250000, 62, 500000,
> + 125, 0, 250, 0, 500, 0, 1000, 0
+ Comma?
> +};
...
> + /* G: 2, 4, 8, 16 */
Indentation of this comment is a bit off.
> +static int bno055_acc_range_vals[] = {1962, 3924, 7848, 15696};
Perhaps split this to 4 lines and put the comment on top of the third line?
...
> +static int bno055_gyr_scale_vals[] = {
> + 125, 1877467, 250, 1877467, 500, 1877467,
> + 1000, 1877467, 2000, 1877467
+ Comma?
> +};
...
> +#ifdef CONFIG_DEBUG_FS
> + struct dentry *debugfs;
> +#endif
...
> + /*
> + * IMU reports sensor offests; IIO wants correction
offsets
> + * offsets, thus we need the 'minus' here.
> + */
...
> + if (kstrtobool(buf, &en))
> + return -EINVAL;
Why shadow an actual error code(s)?
...
> + ret = kstrtoul(buf, 10, &val);
> + if (ret)
> + return ret;
Here it's done properly (see just above).
...
> +static void bno055_debugfs_init(struct iio_dev *iio_dev)
> +{
> + struct bno055_priv *priv = iio_priv(iio_dev);
> +
> + priv->debugfs = debugfs_create_file("firmware_version", 0400,
> + iio_get_debugfs_dentry(iio_dev),
> + priv, &bno055_fw_version_ops);
> + devm_add_action_or_reset(priv->dev, bno055_debugfs_remove, priv->debugfs);
Shouldn't we report the potential error here? It's not directly
related to debugfs, but something which is not directly related.
> +}
...
> +static IIO_DEVICE_ATTR(fusion_enable, 0644,
> + bno055_fusion_enable_show,
> + bno055_fusion_enable_store, 0);
IIO_DEVICE_ATTR_RW()
> +static IIO_DEVICE_ATTR(in_magn_calibration_fast_enable, 0644,
> + bno055_fmc_enable_show,
> + bno055_fmc_enable_store, 0);
> +
> +static IIO_DEVICE_ATTR(in_accel_range_raw, 0644,
> + bno055_in_accel_range_show,
> + bno055_in_accel_range_store, 0);
Ditto for above.
...
> + /*
> + * All chans are made up 1 16-bit sample, except for quaternion that is
channels
> + * made up 4 16-bit values.
> + * For us the quaternion CH is just like 4 regular CHs.
> + * If our read starts past the quaternion make sure to adjust the
> + * starting offset; if the quaternion is contained in our scan then make
> + * sure to adjust the read len.
> + */
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2022-06-13 19:04 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-13 12:05 [v6 00/14] Add support for Bosch BNO055 IMU andrea.merello
2022-06-13 12:05 ` [v6 01/14] iio: add modifiers for linear acceleration andrea.merello
2022-06-13 12:05 ` [v6 02/14] iio: document linear acceleration modifiers andrea.merello
2022-06-13 12:05 ` [v6 03/14] iio: event_monitor: add " andrea.merello
2022-06-13 12:05 ` [v6 04/14] iio: add modifers for pitch, yaw, roll andrea.merello
2022-06-13 12:05 ` [v6 05/14] iio: document pitch, yaw, roll modifiers andrea.merello
2022-06-13 12:05 ` [v6 06/14] iio: event_monitor: add pitch, yaw and " andrea.merello
2022-06-13 12:05 ` [v6 07/14] iio: add support for binary attributes andrea.merello
2022-06-13 12:05 ` [v6 08/14] iio: imu: add Bosch Sensortec BNO055 core driver andrea.merello
2022-06-13 16:44 ` Andy Shevchenko [this message]
2022-06-14 9:11 ` Andrea Merello
2022-06-14 10:58 ` Andy Shevchenko
2022-06-14 12:15 ` Andrea Merello
2022-06-14 15:10 ` Andy Shevchenko
2022-06-14 15:27 ` Andrea Merello
2022-06-18 17:26 ` Jonathan Cameron
2022-06-13 12:05 ` [v6 09/14] iio: document bno055 private sysfs attributes andrea.merello
2022-06-13 12:05 ` [v6 10/14] iio: document "serialnumber" sysfs attribute andrea.merello
2022-06-13 12:05 ` [v6 11/14] dt-bindings: iio/imu: Add Bosch BNO055 andrea.merello
2022-06-13 12:05 ` [v6 12/14] iio: imu: add BNO055 serdev driver andrea.merello
2022-06-13 16:52 ` Andy Shevchenko
2022-06-15 20:57 ` kernel test robot
2022-06-15 21:13 ` Andy Shevchenko
2022-06-15 21:15 ` Andy Shevchenko
2022-07-03 1:52 ` kernel test robot
2022-06-13 12:05 ` [v6 13/14] iio: imu: add BNO055 I2C driver andrea.merello
2022-06-13 12:05 ` [v6 14/14] docs: iio: add documentation for BNO055 driver andrea.merello
2022-07-03 7:58 ` kernel test robot
2022-07-03 13:11 ` Bagas Sanjaya
2022-07-03 14:00 ` Andy Shevchenko
2022-07-04 1:21 ` Bagas Sanjaya
2022-07-04 3:40 ` [PATCH v2] Documentation: bno055: separate SPDX identifier and page title Bagas Sanjaya
2022-07-04 19:49 ` Andy Shevchenko
2022-07-05 1:13 ` Bagas Sanjaya
2022-07-05 9:02 ` Andy Shevchenko
2022-07-13 16:09 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAHp75Vdh8kAH6540xCwzFh5uf=QMVTHC42a8pOgvkpObzjfD+w@mail.gmail.com' \
--to=andy.shevchenko@gmail.com \
--cc=andrea.merello@iit.it \
--cc=ardeleanalex@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=jacopo@jmondi.org \
--cc=jic23@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=matt.ranostay@konsulko.com \
--cc=mchehab+huawei@kernel.org \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).