LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Andrea Merello <andrea.merello@gmail.com>
Cc: mchehab+huawei@kernel.org, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	lars@metafoo.de, robh+dt@kernel.org, andy.shevchenko@gmail.com,
	matt.ranostay@konsulko.com, ardeleanalex@gmail.com,
	jacopo@jmondi.org, Andrea Merello <andrea.merello@iit.it>
Subject: Re: [v2 06/10] iio: document bno055 private sysfs attributes
Date: Thu, 28 Oct 2021 12:04:05 +0100	[thread overview]
Message-ID: <20211028120405.6ffb01d1@jic23-huawei> (raw)
In-Reply-To: <20211028101840.24632-7-andrea.merello@gmail.com>

On Thu, 28 Oct 2021 12:18:36 +0200
Andrea Merello <andrea.merello@gmail.com> wrote:

> This patch adds ABI documentation for bno055 driver private sysfs
> attributes.

Hohum. As normal I dislike custom attributes but reality is these
don't map to anything 'standard' and I don't want them getting adopted
in places where the 'standard' approach works.

So thinking a bit more on this, I wonder if we can fit it into standard
ABI.

We can't use the normal range specification method of
_scale because it's internal to the device and the output reading is
unaffected.  The range specification via _raw_available would let us know
the range, but it is not writeable so..

A control that changes the internal scaling of the sensor in a fashion
that is not visible to the outside world maps to calibscale.  Whilst
that was intended for little tweaks to the input signal (often front
end amplifier gain tweak) it works here.  It doesn't map through to
anything userspace is expected to apply.  That combined with
_raw_available to let us know what the result is should work?

What do you think of that approach?  It's obviously a little more complex
to handle in the driver, but it does map to existing ABI and avoids
custom attributes etc.

> 
> Signed-off-by: Andrea Merello <andrea.merello@iit.it>
> ---
>  .../ABI/testing/sysfs-bus-iio-bno055          | 84 +++++++++++++++++++
>  1 file changed, 84 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-bno055
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-bno055 b/Documentation/ABI/testing/sysfs-bus-iio-bno055
> new file mode 100644
> index 000000000000..930a70c5a858
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-bno055
> @@ -0,0 +1,84 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/in_accel_range
> +KernelVersion:	5.15
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Range for acceleration readings in G. Note that this does not
> +		affects the scale (which should be used when changing the
> +		maximum and minimum readable value affects also the reading
> +		scaling factor).

Having this in G but the sensor output in m/s^2 seems inconsistent.

> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_range
> +KernelVersion:	5.15
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Range for angular velocity readings in dps. Note that this does
> +		not affects the scale (which should be used when changing the
> +		maximum	and minimum readable value affects also the reading
> +		scaling	factor).

Again, units need to match or this is going to be really confusing.

> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_accel_range_available
> +KernelVersion:	5.15
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		List of allowed values for in_accel_range attribute
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_range_available
> +KernelVersion:	5.15
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		List of allowed values for in_anglvel_range attribute
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/fast_magnetometer_calibration_enable
> +KernelVersion:	5.15
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Can be 1 or 0. Enables/disables the "Fast Magnetometer
> +		Calibration" HW function.

Naming needs to be consistent with the ABI.  This is a channel type specific function
and to match existing calibration related ABI naming it would be.

in_magn_calibration_fast_enable

Some of the others need renaming in a similar way.

> +
> +What:		/sys/bus/iio/devices/iio:deviceX/fusion_enable
> +KernelVersion:	5.15
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Can be 1 or 0. Enables/disables the "sensor fusion" (a.k.a.
> +		NDOF) HW function.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_calibration_data
> +KernelVersion:	5.15
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Reports the binary calibration data blob for the IMU sensors.

Why in_ ?  What channels does this apply to?

> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_accel
> +KernelVersion:	5.15
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Can be "Idle", "Bad", "Barely enough", "Fair", or "Good".
> +		Report the autocalibration status for the accelerometer sensor.

For interfaces that really don't have any chance of generalising this one is terrible.
Any hope at all of mapping this to something numeric?

in_accel_calibration_auto_status


> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_gyro
> +KernelVersion:	5.15
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Can be "Idle", "Bad", "Barely enough", "Fair", or "Good".
> +		Reports the autocalibration status for the gyroscope sensor.

in_angvel_calibration_auto_status
etc.

> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_magn
> +KernelVersion:	5.15
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Can be "Idle", "Bad", "Barely enough", "Fair", or "Good".
> +		Reports the autocalibration status for the magnetometer sensor.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_sys
> +KernelVersion:	5.15
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Can be "Idle", "Bad", "Barely enough", "Fair", or "Good".
> +		Reports the status for the IMU overall autocalibration.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/unique_id

Hmm. So normally we just dump these in the kernel log.  I guess you need it
here to associate a calibration blob with a particular sensor?

We could put it in label, but that would stop us using that for things like
positioning of the sensor.  So perhaps this is something that we should add
to the main ABI doc.  Probably as serial_number rather than unique ID though.

> +KernelVersion:	5.15
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		16-bytes, 2-digits-per-byte, HEX-string representing the sensor
> +		unique ID number.


  reply	other threads:[~2021-10-28 10:59 UTC|newest]

Thread overview: 89+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-15 14:17 [PATCH 0/4] Add support for Bosch BNO055 IMU Andrea Merello
2021-07-15 14:17 ` [PATCH 1/4] iio: add modifiers for linear acceleration Andrea Merello
2021-07-17 14:32   ` Jonathan Cameron
2021-07-15 14:17 ` [PATCH 2/4] iio: imu: add Bosch Sensortec BNO055 core driver Andrea Merello
2021-07-15 16:49   ` Andy Shevchenko
2021-07-16  9:19     ` Andrea Merello
2021-07-16 12:39       ` Andy Shevchenko
2021-07-19  7:12         ` Andrea Merello
2021-07-19 10:33         ` Andrea Merello
2021-07-19  9:02     ` Andrea Merello
2021-07-19 11:48       ` Andy Shevchenko
2021-07-19 13:13         ` Andrea Merello
2021-07-16  7:24   ` Alexandru Ardelean
2021-07-16  9:49     ` Andrea Merello
2021-07-17 15:32   ` Jonathan Cameron
2021-07-19  8:30     ` Andrea Merello
2021-07-24 17:08       ` Jonathan Cameron
2021-07-26 14:36         ` Andrea Merello
2021-07-31 18:01           ` Jonathan Cameron
2021-08-04 10:06             ` Andrea Merello
2021-08-04 16:50               ` Jonathan Cameron
2021-08-04 19:27                 ` Andy Shevchenko
2021-07-15 14:17 ` [PATCH 3/4] dt-bindings: iio: imu: add bosch BNO055 serdev driver bindings Andrea Merello
2021-07-17 15:39   ` Jonathan Cameron
2021-07-19  8:44     ` Andrea Merello
2021-07-15 14:17 ` [PATCH 4/4] iio: imu: add BNO055 serdev driver Andrea Merello
2021-07-15 17:08   ` kernel test robot
2021-07-15 18:14   ` kernel test robot
2021-07-17 15:50   ` Jonathan Cameron
2021-07-19  8:49     ` Andrea Merello
2021-07-19 11:55       ` Andy Shevchenko
2021-07-19 12:59         ` Andrea Merello
2021-07-19 14:15           ` Andy Shevchenko
2021-07-19 15:07             ` Andrea Merello
2021-10-28 10:18 ` [v2 00/10] Add support for Bosch BNO055 IMU Andrea Merello
2021-10-28 10:18   ` [v2 01/10] utils_macro: introduce find_closest_unsorted() Andrea Merello
2021-10-28 10:25     ` Andy Shevchenko
2021-11-08 11:05       ` Andrea Merello
2021-10-28 10:18   ` [v2 02/10] iio: document linear acceleration modifiers Andrea Merello
2021-10-28 10:31     ` Andy Shevchenko
2021-11-09  7:48       ` Andrea Merello
2021-10-28 10:40     ` Jonathan Cameron
2021-11-09  8:00       ` Andrea Merello
2021-11-09 17:00         ` Jonathan Cameron
2021-10-28 10:18   ` [v2 03/10] iio: document euler angles modifiers Andrea Merello
2021-10-28 10:33     ` Andy Shevchenko
2021-10-28 10:41     ` Jonathan Cameron
2021-11-09  8:15       ` Andrea Merello
2021-11-09 17:03         ` Jonathan Cameron
2021-10-28 10:18   ` [v2 04/10] iio: add modifiers for linear acceleration Andrea Merello
2021-10-28 10:45     ` Jonathan Cameron
2021-11-09  9:58       ` Andrea Merello
2021-11-09 17:05         ` Jonathan Cameron
2021-10-28 10:18   ` [v2 05/10] iio: add modifers for pitch, yaw, roll Andrea Merello
2021-10-28 10:47     ` Jonathan Cameron
2021-10-28 10:18   ` [v2 06/10] iio: document bno055 private sysfs attributes Andrea Merello
2021-10-28 11:04     ` Jonathan Cameron [this message]
2021-11-09 10:22       ` Andrea Merello
2021-11-14 16:20         ` Jonathan Cameron
2022-01-04 11:42           ` Andrea Merello
2022-01-15 15:27             ` Jonathan Cameron
2022-01-17  9:37               ` Andrea Merello
2022-01-22 18:08                 ` Jonathan Cameron
2021-10-28 10:18   ` [v2 07/10] iio: imu: add Bosch Sensortec BNO055 core driver Andrea Merello
2021-10-28 13:31     ` Jonathan Cameron
2021-11-09 11:52       ` Andrea Merello
2021-11-14 16:33         ` Jonathan Cameron
2021-10-28 10:18   ` [v2 08/10] dt-bindings: iio: imu: add documentation for Bosch BNO055 bindings Andrea Merello
2021-10-28 12:25     ` Rob Herring
2021-10-28 10:18   ` [v2 09/10] iio: imu: add BNO055 serdev driver Andrea Merello
2021-10-28 12:31     ` Jonathan Cameron
2021-11-09 15:33       ` Andrea Merello
2021-11-14 16:37         ` Jonathan Cameron
2021-10-29  7:09     ` kernel test robot
2021-10-29 12:59     ` kernel test robot
2021-10-28 10:18   ` [v2 10/10] iio: imu: add BNO055 I2C driver Andrea Merello
2021-10-28 11:10     ` Jonathan Cameron
2021-11-11 10:12       ` Andrea Merello
2021-11-14 16:38         ` Jonathan Cameron
2021-10-28 22:04     ` Randy Dunlap
2021-11-09 11:56       ` Andrea Merello
2021-11-09 15:47         ` Randy Dunlap
2021-11-09 18:21           ` Joe Perches
2021-11-09 19:11             ` Randy Dunlap
2021-11-09 20:46               ` Joe Perches
2021-11-09 21:20                 ` Randy Dunlap
2021-10-29 13:30     ` kernel test robot
2021-10-28 10:35   ` [v2 00/10] Add support for Bosch BNO055 IMU Jonathan Cameron
2021-10-28 10:33     ` Andy Shevchenko

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=20211028120405.6ffb01d1@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=andrea.merello@gmail.com \
    --cc=andrea.merello@iit.it \
    --cc=andy.shevchenko@gmail.com \
    --cc=ardeleanalex@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jacopo@jmondi.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 \
    --subject='Re: [v2 06/10] iio: document bno055 private sysfs attributes' \
    /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

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).