mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] iio: imu: bno055: add explicit scan buf layout
@ 2026-02-28 20:15 David Lechner
  2026-03-02  8:59 ` Andy Shevchenko
  2026-03-02 21:14 ` Jonathan Cameron
  0 siblings, 2 replies; 4+ messages in thread
From: David Lechner @ 2026-02-28 20:15 UTC (permalink / raw)
  To: Jonathan Cameron, Nuno Sá, Andy Shevchenko
  Cc: linux-iio, linux-kernel, David Lechner

Move the scan buf.chans array into a union along with a struct that
gives the layout of the buffer with all channels enabled.

Although not technically required in this case, if there had been a
different number of items before the quaternion, there could have been
a subtle bug with the special alignment needed for the quaternion
channel data and the array would have been too small.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
This depends on [1] that introduces the IIO_DECLARE_QUATERNION()
macro. I didn't include it in the same series because the other series
contains a fix and should be backported while this one ins't a fix.

[1]: https://lore.kernel.org/linux-iio/20260214-iio-fix-repeat-alignment-v1-1-47f01288c803@baylibre.com/
---
Changes in v2:
- Rename IIO_DECLARE_REPEATED_ELEMENT() to IIO_DECLARE_QUATERNION().
- Link to v1: https://lore.kernel.org/r/20260214-iio-imu-bno055-repeated-element-v1-1-b57b08efd566@baylibre.com
---
 drivers/iio/imu/bno055/bno055.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/imu/bno055/bno055.c b/drivers/iio/imu/bno055/bno055.c
index 303bc308f80a..d91558e100cc 100644
--- a/drivers/iio/imu/bno055/bno055.c
+++ b/drivers/iio/imu/bno055/bno055.c
@@ -211,7 +211,25 @@ struct bno055_priv {
 	struct gpio_desc *reset_gpio;
 	bool sw_reset;
 	struct {
-		__le16 chans[BNO055_SCAN_CH_COUNT];
+		union {
+			__le16 chans[BNO055_SCAN_CH_COUNT];
+			/*
+			 * Struct is to ensure proper size in case any padding
+			 * is needed. Technically not needed in this case, but
+			 * better to be explicit about the requirements.
+			 */
+			struct {
+				__le16 acc[3];
+				__le16 magn[3];
+				__le16 gyr[3];
+				__le16 yaw;
+				__le16 pitch;
+				__le16 roll;
+				IIO_DECLARE_QUATERNION(__le16, quaternion);
+				__le16 lia[3];
+				__le16 gravity[3];
+			};
+		};
 		aligned_s64 timestamp;
 	} buf;
 	struct dentry *debugfs;

---
base-commit: 0b9f2af96d9dc898af3aa58bd55bf62768229b92
change-id: 20260214-iio-imu-bno055-repeated-element-1c1552ea74be
prerequisite-change-id: 20260214-iio-fix-repeat-alignment-575b2c009e25:v2
prerequisite-patch-id: e155a526d57c5759a2fcfbfca7f544cb419addfd
prerequisite-patch-id: 6c69eaad0dd2ae69bd2745e7d387f739fc1a9ba0

Best regards,
-- 
David Lechner <dlechner@baylibre.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] iio: imu: bno055: add explicit scan buf layout
  2026-02-28 20:15 [PATCH v2] iio: imu: bno055: add explicit scan buf layout David Lechner
@ 2026-03-02  8:59 ` Andy Shevchenko
  2026-03-14 21:39   ` David Lechner
  2026-03-02 21:14 ` Jonathan Cameron
  1 sibling, 1 reply; 4+ messages in thread
From: Andy Shevchenko @ 2026-03-02  8:59 UTC (permalink / raw)
  To: David Lechner
  Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, linux-iio, linux-kernel

On Sat, Feb 28, 2026 at 02:15:51PM -0600, David Lechner wrote:
> Move the scan buf.chans array into a union along with a struct that
> gives the layout of the buffer with all channels enabled.
> 
> Although not technically required in this case, if there had been a
> different number of items before the quaternion, there could have been
> a subtle bug with the special alignment needed for the quaternion
> channel data and the array would have been too small.

Cool thing!
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] iio: imu: bno055: add explicit scan buf layout
  2026-02-28 20:15 [PATCH v2] iio: imu: bno055: add explicit scan buf layout David Lechner
  2026-03-02  8:59 ` Andy Shevchenko
@ 2026-03-02 21:14 ` Jonathan Cameron
  1 sibling, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2026-03-02 21:14 UTC (permalink / raw)
  To: David Lechner; +Cc: Nuno Sá, Andy Shevchenko, linux-iio, linux-kernel

On Sat, 28 Feb 2026 14:15:51 -0600
David Lechner <dlechner@baylibre.com> wrote:

> Move the scan buf.chans array into a union along with a struct that
> gives the layout of the buffer with all channels enabled.
> 
> Although not technically required in this case, if there had been a
> different number of items before the quaternion, there could have been
> a subtle bug with the special alignment needed for the quaternion
> channel data and the array would have been too small.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
> This depends on [1] that introduces the IIO_DECLARE_QUATERNION()
> macro. I didn't include it in the same series because the other series
> contains a fix and should be backported while this one ins't a fix.
> 
> [1]: https://lore.kernel.org/linux-iio/20260214-iio-fix-repeat-alignment-v1-1-47f01288c803@baylibre.com/
> ---
> Changes in v2:
> - Rename IIO_DECLARE_REPEATED_ELEMENT() to IIO_DECLARE_QUATERNION().
> - Link to v1: https://lore.kernel.org/r/20260214-iio-imu-bno055-repeated-element-v1-1-b57b08efd566@baylibre.com
> ---
>  drivers/iio/imu/bno055/bno055.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/imu/bno055/bno055.c b/drivers/iio/imu/bno055/bno055.c
> index 303bc308f80a..d91558e100cc 100644
> --- a/drivers/iio/imu/bno055/bno055.c
> +++ b/drivers/iio/imu/bno055/bno055.c
> @@ -211,7 +211,25 @@ struct bno055_priv {
>  	struct gpio_desc *reset_gpio;
>  	bool sw_reset;
>  	struct {
> -		__le16 chans[BNO055_SCAN_CH_COUNT];
> +		union {
> +			__le16 chans[BNO055_SCAN_CH_COUNT];
> +			/*
> +			 * Struct is to ensure proper size in case any padding
> +			 * is needed. Technically not needed in this case, but
> +			 * better to be explicit about the requirements.
> +			 */
> +			struct {
> +				__le16 acc[3];
> +				__le16 magn[3];
> +				__le16 gyr[3];
> +				__le16 yaw;
> +				__le16 pitch;
> +				__le16 roll;
> +				IIO_DECLARE_QUATERNION(__le16, quaternion);
> +				__le16 lia[3];
> +				__le16 gravity[3];
> +			};
> +		};
>  		aligned_s64 timestamp;
>  	} buf;

It's not a new problem, but I'm not sure a structure is valid here at all
as the driver isn't using avail_scan_masks to enforce all channels are enabled.

If I read it right we can get any subset of those channels.

Generally for these we've been using the macro you added rather than
an explicit structure, but if we do that we don't get the align to 8 of the
quaternion as an explicit thing.  Maybe we make an exception and use
a structure here but if we do should we pull the timestamp into that
struct and use IIO_DECLARE_BUFFER_WITH_TS) for the chans array so
that we make it clear everything floats around depending on what
is in use.

Jonathan




>  	struct dentry *debugfs;
> 
> ---
> base-commit: 0b9f2af96d9dc898af3aa58bd55bf62768229b92
> change-id: 20260214-iio-imu-bno055-repeated-element-1c1552ea74be
> prerequisite-change-id: 20260214-iio-fix-repeat-alignment-575b2c009e25:v2
> prerequisite-patch-id: e155a526d57c5759a2fcfbfca7f544cb419addfd
> prerequisite-patch-id: 6c69eaad0dd2ae69bd2745e7d387f739fc1a9ba0
> 
> Best regards,


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] iio: imu: bno055: add explicit scan buf layout
  2026-03-02  8:59 ` Andy Shevchenko
@ 2026-03-14 21:39   ` David Lechner
  0 siblings, 0 replies; 4+ messages in thread
From: David Lechner @ 2026-03-14 21:39 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, linux-iio, linux-kernel

On 3/2/26 2:59 AM, Andy Shevchenko wrote:
> On Sat, Feb 28, 2026 at 02:15:51PM -0600, David Lechner wrote:
>> Move the scan buf.chans array into a union along with a struct that
>> gives the layout of the buffer with all channels enabled.
>>
>> Although not technically required in this case, if there had been a
>> different number of items before the quaternion, there could have been
>> a subtle bug with the special alignment needed for the quaternion
>> channel data and the array would have been too small.
> 
> Cool thing!
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> 

Forgot to mention in v3 that I didn't pick up the tag because I
changed it significantly, so worth another look.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-03-14 21:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-02-28 20:15 [PATCH v2] iio: imu: bno055: add explicit scan buf layout David Lechner
2026-03-02  8:59 ` Andy Shevchenko
2026-03-14 21:39   ` David Lechner
2026-03-02 21:14 ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®