Re: [PATCH v7 5/6] iio: ABI: Add partial quaternion modifier

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 4 Mar 2026 16:35:39 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On 3/4/26 2:07 AM, Francesco Lavra wrote:
> > This modifier applies to the IIO_ROT channel type, and indicates a data
> > representation that specifies the {x, y, z} components of the normalized
> > quaternion vector.
> > 
> > Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> > ---
> >  Documentation/ABI/testing/sysfs-bus-iio | 13 +++++++++++++
> >  drivers/iio/industrialio-core.c         |  1 +
> >  include/uapi/linux/iio/types.h          |  1 +
> >  tools/iio/iio_event_monitor.c           |  1 +
> >  4 files changed, 16 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> > index bd6c3305dd2b..54e38ebb044d 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-iio
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio
> > @@ -1755,6 +1755,19 @@ Description:
> >  		measurement from channel Y. Units after application of scale and
> >  		offset are milliamps.
> >  
> > +What:		/sys/bus/iio/devices/iio:deviceX/in_rot_partial_quaternion_raw
> > +KernelVersion:	7.1
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		Raw value of {x, y, z} components of the quaternion vector. These
> > +		components represent the axis about which a rotation occurs, and are
> > +		subject to the following costraints:  
> 
> s/costraints/constraints/
> 
> > +		- the quaternion vector is normalized, i.e. x^2 + y^2 + z^2 + w^2 = 1  
> 
> Isn't this usually written with w first?
> 
> > +		- the rotation angle is within the [-180, 180] range, i.e. the w  
> 
> Best to say the angle units. IIO standard unit for angle is radians.
> 
> > +		  component (which represents the amount of rotation) is non-negative
> > +		These constraints allow the w value to be calculated from the other
> > +		components: w = sqrt(1 - (x^2 + y^2 + z^2)).
> > +
> >  What:		/sys/.../iio:deviceX/in_energy_en
> >  What:		/sys/.../iio:deviceX/in_distance_en
> >  What:		/sys/.../iio:deviceX/in_velocity_sqrt(x^2+y^2+z^2)_en
> > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > index 22eefd048ba9..792fbeb0dfa8 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -125,6 +125,7 @@ static const char * const iio_modifier_names[] = {
> >  	[IIO_MOD_LIGHT_UVB] = "uvb",
> >  	[IIO_MOD_LIGHT_DUV] = "duv",
> >  	[IIO_MOD_QUATERNION] = "quaternion",
> > +	[IIO_MOD_PARTIAL_QUATERNION] = "partial_quaternion",  
> 
> We've found that using `_` in components of the attribute name makes it
> difficult to parse since you have to know all component names to know
> where the break between components is. Each time we add something with
> an `_` means all parsers need to be updated to handle the new name.
> 
> In other components, we've just squashed the two words together without
> any space or punctuation to get around this problem. I find that rather hard
> to read though.
> 
> I wonder if we could propose something different going forward to do a better
> job being consistent and readable. For example, use a `-` at the word break
> in components that have more than one word in the name.
Whilst I can see the argument, I'm not keen on more punctuation in the filenames.
May well trip up someone's parser.  For now squashing the spaces is probably
the best option.

This made me spot a major issue though. We can only add to end of these
enums, not the middle!

> 
> >  	[IIO_MOD_TEMP_AMBIENT] = "ambient",
> >  	[IIO_MOD_TEMP_OBJECT] = "object",
> >  	[IIO_MOD_NORTH_MAGN] = "from_north_magnetic",