Re: [PATCH 2/2] docs: abi: iio: Add event when offset changes

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 28 Aug 2020 16:31:56 -0700
Gwendal Grignou <gwendal@chromium.org> wrote:

> Some sensors/sensorhubs can calculate drift or hard iron offsets to
> apply to raw data to get the true measure data.
> These offsets are applied by the user space application.
> When these offsets change, events are raised to tell the application
> to update the cached offset values.
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Hi Gwendal

This strikes me as rather prone to races.  I guess if the updates
tend to be fairly minor we will just have slightly less accurate data
until the update gets picked up by userspace.

We have had some discussions about how to handle meta data updates
in the past.  One option was to provide a metadata index channel
that could be used to indicate there was an update to something
sat in a separate fifo.

> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 33 +++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 47df16c87862d..40da602e7a555 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -1716,3 +1716,36 @@ Description:
>  		Mass concentration reading of particulate matter in ug / m3.
>  		pmX consists of particles with aerodynamic diameter less or
>  		equal to X micrometers.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_x_offset
> +What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_y_offset
> +What:		/sys/bus/iio/devices/iio:deviceX/in_anglvel_z_offset
> +KernelVersion:	x.y
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Gyroscope drift calculated by the sensor. In addition to factory
> +		calibration, sensor or sensorhub can
> +		detect gyroscope drift and report it to userspace.

This looks like standard ABI that should probably already be documented,
unrelated to the controversial part of this patch. I would split it out into
it's own patch a I can pick that up much faster.


> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_magn_x_offset
> +What:		/sys/bus/iio/devices/iio:deviceX/in_magn_y_offset
> +What:		/sys/bus/iio/devices/iio:deviceX/in_magn_z_offset
> +KernelVersion:	x.y
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Hard Iron bias calculated by the sensor or sensorhub. To be applied by
> +		user space application to the raw data to obtain the geomagnetic field.

Same as above.

> +
> +What:		/sys/.../iio:deviceX/events/in_accel_offset_change_en
> +What:		/sys/.../iio:deviceX/events/in_magn_offset_change_en
> +What:		/sys/.../iio:deviceX/events/in_magn_scale_change_en
> +What:		/sys/.../iio:deviceX/events/in_anglvel_offset_change_en
> +KernelVersion:	x.y
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Some sensors internally calculate offset to apply to remove bias (for
> +		instance, hard/soft-iron bias for magnetometer, online calibration bias for
> +		gyroscope or accelerometer).
> +		When the sensor computes a new set of offset values, it generates an
> +		event for the userspace application to refresh the offsets to apply to raw
> +		data.

I'm not totally sold on this idea, though would like inputs from other people before
ruling it out.

One minor change I'd make would be to have a single event to indicate that something
userspace might care about in the way of metadata for this channel has changed.
I guess something like

in_accel_metachange_en etc with a single new event code.  For events, it's the event
code mapping that normally matters more than sysfs binding as that is much more
tightly restricted.

Jonathan