From: Andrea Merello <Andrea.Merello@iit.it>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
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: Tue, 14 Jun 2022 09:11:24 +0000 [thread overview]
Message-ID: <164f663acbba481a8ea8f45f185aaf83@iit.it> (raw)
In-Reply-To: <CAHp75Vdh8kAH6540xCwzFh5uf=QMVTHC42a8pOgvkpObzjfD+w@mail.gmail.com>
Few inline comments, OK for the rest.
>...
>
>> + /* 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?
Not sure what you mean here, sorry. May you elaborate or provide an example, please?
>...
>
>> +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.
The error eventually comes out from something that has nothing to do with debugs per se (i.e. the devm stuff), but it will only affect debugfs indeed.
Assuming that we don't want to make the whole driver fail in case debugfs stuff fails (see last part of the comment above debugfs_create_file() implementation), and given that the devm_add_action_or_reset(), should indeed "reset" in case of failure (i.e. we should be in a clean situation anyway), I would say it should be OK not to propagate the error and let things go on.
However we can add a dev_warn() to report what happened.
next prev parent reply other threads:[~2022-06-14 12:35 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
2022-06-14 9:11 ` Andrea Merello [this message]
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=164f663acbba481a8ea8f45f185aaf83@iit.it \
--to=andrea.merello@iit.it \
--cc=andy.shevchenko@gmail.com \
--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).