[v8] imu: st_lsm6dsx: Add support for rotation sensor

[PATCH v8 2/6] iio: Replace 'sign' field with union in struct iio_scan_type

Posted by Francesco Lavra 2 weeks, 6 days ago

This field is used to differentiate between signed and unsigned integers.
A following commit will extend its use in order to add support for non-
integer scan elements; therefore, replace it with a union that contains a
more generic 'format' field. This union will be dropped when all drivers
are changed to use the format field.
Opportunistically replace character literals with symbolic constants that
represent the set of allowed values for the format field.

Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 Documentation/driver-api/iio/buffers.rst |  4 ++--
 include/linux/iio/iio.h                  | 17 +++++++++++++++--
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/Documentation/driver-api/iio/buffers.rst b/Documentation/driver-api/iio/buffers.rst
index 63f364e862d1..e16abaf826fe 100644
--- a/Documentation/driver-api/iio/buffers.rst
+++ b/Documentation/driver-api/iio/buffers.rst
@@ -78,7 +78,7 @@ fields in iio_chan_spec definition::
    /* other members */
            int scan_index
            struct {
-                   char sign;
+                   char format;
                    u8 realbits;
                    u8 storagebits;
                    u8 shift;
@@ -98,7 +98,7 @@ following channel definition::
 		   /* other stuff here */
 		   .scan_index = 0,
 		   .scan_type = {
-		           .sign = 's',
+		           .format = IIO_SCAN_FORMAT_SIGNED_INT,
 			   .realbits = 12,
 			   .storagebits = 16,
 			   .shift = 4,
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index a9ecff191bd9..d48a0ab01b8d 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -176,9 +176,19 @@ struct iio_event_spec {
 	unsigned long mask_shared_by_all;
 };
 
+/*
+ * Format values in scan type
+ * @IIO_SCAN_FORMAT_SIGNED_INT: Signed integer (two's complement).
+ * @IIO_SCAN_FORMAT_UNSIGNED_INT: Unsigned integer.
+ */
+#define IIO_SCAN_FORMAT_SIGNED_INT	's'
+#define IIO_SCAN_FORMAT_UNSIGNED_INT	'u'
+
 /**
  * struct iio_scan_type - specification for channel data format in buffer
- * @sign:		's' or 'u' to specify signed or unsigned
+ * @sign:		Deprecated, use @format instead.
+ * @format:		Data format, can have any of the IIO_SCAN_FORMAT_*
+ *			values.
  * @realbits:		Number of valid bits of data
  * @storagebits:	Realbits + padding
  * @shift:		Shift right by this before masking out realbits.
@@ -189,7 +199,10 @@ struct iio_event_spec {
  * @endianness:		little or big endian
  */
 struct iio_scan_type {
-	char	sign;
+	union {
+		char sign;
+		char format;
+	};
 	u8	realbits;
 	u8	storagebits;
 	u8	shift;
-- 
2.39.5

Re: [PATCH v8 2/6] iio: Replace 'sign' field with union in struct iio_scan_type

Posted by David Lechner 2 weeks, 2 days ago

On 3/17/26 10:04 AM, Francesco Lavra wrote:
> This field is used to differentiate between signed and unsigned integers.
> A following commit will extend its use in order to add support for non-
> integer scan elements; therefore, replace it with a union that contains a
> more generic 'format' field. This union will be dropped when all drivers
> are changed to use the format field.
> Opportunistically replace character literals with symbolic constants that
> represent the set of allowed values for the format field.
> 
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> ---
>  Documentation/driver-api/iio/buffers.rst |  4 ++--
>  include/linux/iio/iio.h                  | 17 +++++++++++++++--
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/driver-api/iio/buffers.rst b/Documentation/driver-api/iio/buffers.rst
> index 63f364e862d1..e16abaf826fe 100644
> --- a/Documentation/driver-api/iio/buffers.rst
> +++ b/Documentation/driver-api/iio/buffers.rst
> @@ -78,7 +78,7 @@ fields in iio_chan_spec definition::
>     /* other members */
>             int scan_index
>             struct {
> -                   char sign;
> +                   char format;
>                     u8 realbits;
>                     u8 storagebits;
>                     u8 shift;
> @@ -98,7 +98,7 @@ following channel definition::
>  		   /* other stuff here */
>  		   .scan_index = 0,
>  		   .scan_type = {
> -		           .sign = 's',
> +		           .format = IIO_SCAN_FORMAT_SIGNED_INT,
>  			   .realbits = 12,
>  			   .storagebits = 16,
>  			   .shift = 4,
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index a9ecff191bd9..d48a0ab01b8d 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -176,9 +176,19 @@ struct iio_event_spec {
>  	unsigned long mask_shared_by_all;
>  };
>  
> +/*
> + * Format values in scan type
> + * @IIO_SCAN_FORMAT_SIGNED_INT: Signed integer (two's complement).
> + * @IIO_SCAN_FORMAT_UNSIGNED_INT: Unsigned integer.
> + */

We could make this proper kernel doc format with one comment per macro.

> +#define IIO_SCAN_FORMAT_SIGNED_INT	's'
> +#define IIO_SCAN_FORMAT_UNSIGNED_INT	'u'
> +
>  /**
>   * struct iio_scan_type - specification for channel data format in buffer
> - * @sign:		's' or 'u' to specify signed or unsigned
> + * @sign:		Deprecated, use @format instead.
> + * @format:		Data format, can have any of the IIO_SCAN_FORMAT_*
> + *			values.
>   * @realbits:		Number of valid bits of data
>   * @storagebits:	Realbits + padding
>   * @shift:		Shift right by this before masking out realbits.
> @@ -189,7 +199,10 @@ struct iio_event_spec {
>   * @endianness:		little or big endian
>   */
>  struct iio_scan_type {
> -	char	sign;
> +	union {
> +		char sign;
> +		char format;
> +	};
>  	u8	realbits;
>  	u8	storagebits;
>  	u8	shift;

Re: [PATCH v8 2/6] iio: Replace 'sign' field with union in struct iio_scan_type

Posted by Francesco Lavra 2 weeks ago

On Sat, 2026-03-21 at 12:22 -0500, David Lechner wrote:
> On 3/17/26 10:04 AM, Francesco Lavra wrote:
> > This field is used to differentiate between signed and unsigned
> > integers.
> > A following commit will extend its use in order to add support for non-
> > integer scan elements; therefore, replace it with a union that contains
> > a
> > more generic 'format' field. This union will be dropped when all
> > drivers
> > are changed to use the format field.
> > Opportunistically replace character literals with symbolic constants
> > that
> > represent the set of allowed values for the format field.
> > 
> > Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> > ---
> >  Documentation/driver-api/iio/buffers.rst |  4 ++--
> >  include/linux/iio/iio.h                  | 17 +++++++++++++++--
> >  2 files changed, 17 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/driver-api/iio/buffers.rst
> > b/Documentation/driver-api/iio/buffers.rst
> > index 63f364e862d1..e16abaf826fe 100644
> > --- a/Documentation/driver-api/iio/buffers.rst
> > +++ b/Documentation/driver-api/iio/buffers.rst
> > @@ -78,7 +78,7 @@ fields in iio_chan_spec definition::
> >     /* other members */
> >             int scan_index
> >             struct {
> > -                   char sign;
> > +                   char format;
> >                     u8 realbits;
> >                     u8 storagebits;
> >                     u8 shift;
> > @@ -98,7 +98,7 @@ following channel definition::
> >                    /* other stuff here */
> >                    .scan_index = 0,
> >                    .scan_type = {
> > -                          .sign = 's',
> > +                          .format = IIO_SCAN_FORMAT_SIGNED_INT,
> >                            .realbits = 12,
> >                            .storagebits = 16,
> >                            .shift = 4,
> > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> > index a9ecff191bd9..d48a0ab01b8d 100644
> > --- a/include/linux/iio/iio.h
> > +++ b/include/linux/iio/iio.h
> > @@ -176,9 +176,19 @@ struct iio_event_spec {
> >         unsigned long mask_shared_by_all;
> >  };
> >  
> > +/*
> > + * Format values in scan type
> > + * @IIO_SCAN_FORMAT_SIGNED_INT: Signed integer (two's complement).
> > + * @IIO_SCAN_FORMAT_UNSIGNED_INT: Unsigned integer.
> > + */
> 
> We could make this proper kernel doc format with one comment per macro.

Actually, a set of related #defines can be documented with a single
comment. I see a few examples doing that in include/linux/gfp_types.h and
include/linux/fpga/fpga-mgr.h


> > +#define IIO_SCAN_FORMAT_SIGNED_INT     's'
> > +#define IIO_SCAN_FORMAT_UNSIGNED_INT   'u'
> > +

Re: [PATCH v8 2/6] iio: Replace 'sign' field with union in struct iio_scan_type

Posted by Andy Shevchenko 2 weeks ago

On Mon, Mar 23, 2026 at 05:04:10PM +0100, Francesco Lavra wrote:
> On Sat, 2026-03-21 at 12:22 -0500, David Lechner wrote:
> > On 3/17/26 10:04 AM, Francesco Lavra wrote:

...

> > > + * @IIO_SCAN_FORMAT_SIGNED_INT: Signed integer (two's complement).
> > > + * @IIO_SCAN_FORMAT_UNSIGNED_INT: Unsigned integer.

> > We could make this proper kernel doc format with one comment per macro.
> 
> Actually, a set of related #defines can be documented with a single
> comment. I see a few examples doing that in include/linux/gfp_types.h and
> include/linux/fpga/fpga-mgr.h
> 
> > > +#define IIO_SCAN_FORMAT_SIGNED_INT     's'
> > > +#define IIO_SCAN_FORMAT_UNSIGNED_INT   'u'

...or use enum

/**
 * ...kernel-doc for enum...
 */
enum {
	IIO_SCAN_FORMAT_SIGNED_INT = 's',
	IIO_SCAN_FORMAT_UNSIGNED_INT = 'u',
};


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v8 2/6] iio: Replace 'sign' field with union in struct iio_scan_type

Posted by Francesco Lavra 2 weeks ago

On Mon, 2026-03-23 at 18:49 +0200, Andy Shevchenko wrote:
> On Mon, Mar 23, 2026 at 05:04:10PM +0100, Francesco Lavra wrote:
> > On Sat, 2026-03-21 at 12:22 -0500, David Lechner wrote:
> > > On 3/17/26 10:04 AM, Francesco Lavra wrote:
> 
> ...
> 
> > > > + * @IIO_SCAN_FORMAT_SIGNED_INT: Signed integer (two's complement).
> > > > + * @IIO_SCAN_FORMAT_UNSIGNED_INT: Unsigned integer.
> 
> > > We could make this proper kernel doc format with one comment per
> > > macro.
> > 
> > Actually, a set of related #defines can be documented with a single
> > comment. I see a few examples doing that in include/linux/gfp_types.h
> > and
> > include/linux/fpga/fpga-mgr.h
> > 
> > > > +#define IIO_SCAN_FORMAT_SIGNED_INT     's'
> > > > +#define IIO_SCAN_FORMAT_UNSIGNED_INT   'u'
> 
> ...or use enum
> 
> /**
>  * ...kernel-doc for enum...
>  */
> enum {
>         IIO_SCAN_FORMAT_SIGNED_INT = 's',
>         IIO_SCAN_FORMAT_UNSIGNED_INT = 'u',
> };

There is no standard kernel-doc format for anonymous enums.

Re: [PATCH v8 2/6] iio: Replace 'sign' field with union in struct iio_scan_type

Posted by Andy Shevchenko 1 week, 6 days ago

On Mon, Mar 23, 2026 at 06:37:38PM +0100, Francesco Lavra wrote:
> On Mon, 2026-03-23 at 18:49 +0200, Andy Shevchenko wrote:
> > On Mon, Mar 23, 2026 at 05:04:10PM +0100, Francesco Lavra wrote:
> > > On Sat, 2026-03-21 at 12:22 -0500, David Lechner wrote:
> > > > On 3/17/26 10:04 AM, Francesco Lavra wrote:

...

> > > > > + * @IIO_SCAN_FORMAT_SIGNED_INT: Signed integer (two's complement).
> > > > > + * @IIO_SCAN_FORMAT_UNSIGNED_INT: Unsigned integer.
> > 
> > > > We could make this proper kernel doc format with one comment per
> > > > macro.
> > > 
> > > Actually, a set of related #defines can be documented with a single
> > > comment. I see a few examples doing that in include/linux/gfp_types.h
> > > and
> > > include/linux/fpga/fpga-mgr.h
> > > 
> > > > > +#define IIO_SCAN_FORMAT_SIGNED_INT     's'
> > > > > +#define IIO_SCAN_FORMAT_UNSIGNED_INT   'u'
> > 
> > ...or use enum
> > 
> > /**
> >  * ...kernel-doc for enum...
> >  */
> > enum {
> >         IIO_SCAN_FORMAT_SIGNED_INT = 's',
> >         IIO_SCAN_FORMAT_UNSIGNED_INT = 'u',
> > };
> 
> There is no standard kernel-doc format for anonymous enums.

What do you mean? We have such in kernel, for example,
drivers/pinctrl/intel/pinctrl-intel.c.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v8 2/6] iio: Replace 'sign' field with union in struct iio_scan_type

Posted by Francesco Lavra 1 week, 6 days ago

On Tue, 2026-03-24 at 13:04 +0200, Andy Shevchenko wrote:
> On Mon, Mar 23, 2026 at 06:37:38PM +0100, Francesco Lavra wrote:
> > On Mon, 2026-03-23 at 18:49 +0200, Andy Shevchenko wrote:
> > > On Mon, Mar 23, 2026 at 05:04:10PM +0100, Francesco Lavra wrote:
> > > > On Sat, 2026-03-21 at 12:22 -0500, David Lechner wrote:
> > > > > On 3/17/26 10:04 AM, Francesco Lavra wrote:
> 
> ...
> 
> > > > > > + * @IIO_SCAN_FORMAT_SIGNED_INT: Signed integer (two's
> > > > > > complement).
> > > > > > + * @IIO_SCAN_FORMAT_UNSIGNED_INT: Unsigned integer.
> > > 
> > > > > We could make this proper kernel doc format with one comment per
> > > > > macro.
> > > > 
> > > > Actually, a set of related #defines can be documented with a single
> > > > comment. I see a few examples doing that in
> > > > include/linux/gfp_types.h
> > > > and
> > > > include/linux/fpga/fpga-mgr.h
> > > > 
> > > > > > +#define IIO_SCAN_FORMAT_SIGNED_INT     's'
> > > > > > +#define IIO_SCAN_FORMAT_UNSIGNED_INT   'u'
> > > 
> > > ...or use enum
> > > 
> > > /**
> > >  * ...kernel-doc for enum...
> > >  */
> > > enum {
> > >         IIO_SCAN_FORMAT_SIGNED_INT = 's',
> > >         IIO_SCAN_FORMAT_UNSIGNED_INT = 'u',
> > > };
> > 
> > There is no standard kernel-doc format for anonymous enums.
> 
> What do you mean? We have such in kernel, for example,
> drivers/pinctrl/intel/pinctrl-intel.c.

The kernel-doc guidelines at Documentation/doc-guide/kernel-doc.rst, in the
section that describe structure, union, and enumeration documentation,
include the name of the struct in the example, so I thought they wouldn't
apply to anonymous types. But now I see that anonymous enum comments are
processed just fine by the kernel-doc tool.
Anyway, in v9 I switched to one comment per macro, as suggested by David.

Re: [PATCH v8 2/6] iio: Replace 'sign' field with union in struct iio_scan_type

Posted by Andy Shevchenko 1 week, 6 days ago

On Tue, Mar 24, 2026 at 12:42:19PM +0100, Francesco Lavra wrote:
> On Tue, 2026-03-24 at 13:04 +0200, Andy Shevchenko wrote:
> > On Mon, Mar 23, 2026 at 06:37:38PM +0100, Francesco Lavra wrote:
> > > On Mon, 2026-03-23 at 18:49 +0200, Andy Shevchenko wrote:
> > > > On Mon, Mar 23, 2026 at 05:04:10PM +0100, Francesco Lavra wrote:
> > > > > On Sat, 2026-03-21 at 12:22 -0500, David Lechner wrote:
> > > > > > On 3/17/26 10:04 AM, Francesco Lavra wrote:

...

> > > > > > > + * @IIO_SCAN_FORMAT_SIGNED_INT: Signed integer (two's
> > > > > > > complement).
> > > > > > > + * @IIO_SCAN_FORMAT_UNSIGNED_INT: Unsigned integer.
> > > > 
> > > > > > We could make this proper kernel doc format with one comment per
> > > > > > macro.
> > > > > 
> > > > > Actually, a set of related #defines can be documented with a single
> > > > > comment. I see a few examples doing that in
> > > > > include/linux/gfp_types.h
> > > > > and
> > > > > include/linux/fpga/fpga-mgr.h
> > > > > 
> > > > > > > +#define IIO_SCAN_FORMAT_SIGNED_INT     's'
> > > > > > > +#define IIO_SCAN_FORMAT_UNSIGNED_INT   'u'
> > > > 
> > > > ...or use enum
> > > > 
> > > > /**
> > > >  * ...kernel-doc for enum...
> > > >  */
> > > > enum {
> > > >         IIO_SCAN_FORMAT_SIGNED_INT = 's',
> > > >         IIO_SCAN_FORMAT_UNSIGNED_INT = 'u',
> > > > };
> > > 
> > > There is no standard kernel-doc format for anonymous enums.
> > 
> > What do you mean? We have such in kernel, for example,
> > drivers/pinctrl/intel/pinctrl-intel.c.
> 
> The kernel-doc guidelines at Documentation/doc-guide/kernel-doc.rst, in the
> section that describe structure, union, and enumeration documentation,
> include the name of the struct in the example, so I thought they wouldn't
> apply to anonymous types. But now I see that anonymous enum comments are
> processed just fine by the kernel-doc tool.
> Anyway, in v9 I switched to one comment per macro, as suggested by David.

WFM, thanks.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v8 2/6] iio: Replace 'sign' field with union in struct iio_scan_type

Posted by David Lechner 2 weeks ago

On 3/23/26 11:04 AM, Francesco Lavra wrote:
> On Sat, 2026-03-21 at 12:22 -0500, David Lechner wrote:
>> On 3/17/26 10:04 AM, Francesco Lavra wrote:
>>> This field is used to differentiate between signed and unsigned
>>> integers.
>>> A following commit will extend its use in order to add support for non-
>>> integer scan elements; therefore, replace it with a union that contains
>>> a
>>> more generic 'format' field. This union will be dropped when all
>>> drivers
>>> are changed to use the format field.
>>> Opportunistically replace character literals with symbolic constants
>>> that
>>> represent the set of allowed values for the format field.
>>>
>>> Signed-off-by: Francesco Lavra <flavra@baylibre.com>
>>> ---
>>>  Documentation/driver-api/iio/buffers.rst |  4 ++--
>>>  include/linux/iio/iio.h                  | 17 +++++++++++++++--
>>>  2 files changed, 17 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/driver-api/iio/buffers.rst
>>> b/Documentation/driver-api/iio/buffers.rst
>>> index 63f364e862d1..e16abaf826fe 100644
>>> --- a/Documentation/driver-api/iio/buffers.rst
>>> +++ b/Documentation/driver-api/iio/buffers.rst
>>> @@ -78,7 +78,7 @@ fields in iio_chan_spec definition::
>>>     /* other members */
>>>             int scan_index
>>>             struct {
>>> -                   char sign;
>>> +                   char format;
>>>                     u8 realbits;
>>>                     u8 storagebits;
>>>                     u8 shift;
>>> @@ -98,7 +98,7 @@ following channel definition::
>>>                    /* other stuff here */
>>>                    .scan_index = 0,
>>>                    .scan_type = {
>>> -                          .sign = 's',
>>> +                          .format = IIO_SCAN_FORMAT_SIGNED_INT,
>>>                            .realbits = 12,
>>>                            .storagebits = 16,
>>>                            .shift = 4,
>>> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
>>> index a9ecff191bd9..d48a0ab01b8d 100644
>>> --- a/include/linux/iio/iio.h
>>> +++ b/include/linux/iio/iio.h
>>> @@ -176,9 +176,19 @@ struct iio_event_spec {
>>>         unsigned long mask_shared_by_all;
>>>  };
>>>  
>>> +/*
>>> + * Format values in scan type
>>> + * @IIO_SCAN_FORMAT_SIGNED_INT: Signed integer (two's complement).
>>> + * @IIO_SCAN_FORMAT_UNSIGNED_INT: Unsigned integer.
>>> + */
>>
>> We could make this proper kernel doc format with one comment per macro.
> 
> Actually, a set of related #defines can be documented with a single
> comment. I see a few examples doing that in include/linux/gfp_types.h and
> include/linux/fpga/fpga-mgr.h

Fancy. Although, IDEs tend to be able to handle doc comments better
if they are not combined (i.e. getting the docs when you hold the
mouse over an identifier).

> 
> 
>>> +#define IIO_SCAN_FORMAT_SIGNED_INT     's'
>>> +#define IIO_SCAN_FORMAT_UNSIGNED_INT   'u'
>>> +
>