[PATCH v2] iio: imu: bno055: add explicit scan buf layout

David Lechner posted 1 patch 1 month, 2 weeks ago
There is a newer version of this series
drivers/iio/imu/bno055/bno055.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
[PATCH v2] iio: imu: bno055: add explicit scan buf layout
Posted by David Lechner 1 month, 2 weeks ago
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>
Re: [PATCH v2] iio: imu: bno055: add explicit scan buf layout
Posted by Jonathan Cameron 1 month, 2 weeks ago
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,
Re: [PATCH v2] iio: imu: bno055: add explicit scan buf layout
Posted by Andy Shevchenko 1 month, 2 weeks ago
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
Re: [PATCH v2] iio: imu: bno055: add explicit scan buf layout
Posted by David Lechner 1 month ago
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.