Skip to content

Commit 96175fc

Browse files
yperessfabiobaltieri
authored andcommitted
sensors: Fix alignment issues
Add padding to the header and remove unnecessary memset in order to fix alignment faults in cores such as M0 or ones that support CONFIG_TRAP_UNALIGNED_ACCESS Signed-off-by: Yuval Peress <[email protected]>
1 parent 3563347 commit 96175fc

File tree

3 files changed

+29
-8
lines changed

3 files changed

+29
-8
lines changed

drivers/sensor/default_rtio_sensor.c

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@
1212

1313
LOG_MODULE_REGISTER(sensor_compat, CONFIG_SENSOR_LOG_LEVEL);
1414

15+
/*
16+
* Ensure that the size of the generic header aligns with the sensor channel enum. If it doesn't,
17+
* then cores that require aligned memory access will fail to read channel[0].
18+
*/
19+
BUILD_ASSERT((sizeof(struct sensor_data_generic_header) % sizeof(enum sensor_channel)) == 0);
20+
1521
static void sensor_submit_fallback(const struct device *dev, struct rtio_iodev_sqe *iodev_sqe);
1622

1723
static void sensor_iodev_submit(struct rtio_iodev_sqe *iodev_sqe)
@@ -49,6 +55,21 @@ static inline int compute_num_samples(const enum sensor_channel *channels, size_
4955
return num_samples;
5056
}
5157

58+
/**
59+
* @brief Compute the required header size
60+
*
61+
* This function takes into account alignment of the q31 values that will follow the header.
62+
*
63+
* @param[in] num_output_samples The number of samples to represent
64+
* @return The number of bytes needed for this sample frame's header
65+
*/
66+
static inline uint32_t compute_header_size(int num_output_samples)
67+
{
68+
uint32_t size = sizeof(struct sensor_data_generic_header) +
69+
(num_output_samples * sizeof(enum sensor_channel));
70+
return (size + 3) & ~0x3;
71+
}
72+
5273
/**
5374
* @brief Compute the minimum number of bytes needed
5475
*
@@ -57,8 +78,7 @@ static inline int compute_num_samples(const enum sensor_channel *channels, size_
5778
*/
5879
static inline uint32_t compute_min_buf_len(int num_output_samples)
5980
{
60-
return sizeof(struct sensor_data_generic_header) + (num_output_samples * sizeof(q31_t)) +
61-
(num_output_samples * sizeof(enum sensor_channel));
81+
return compute_header_size(num_output_samples) + (num_output_samples * sizeof(q31_t));
6282
}
6383

6484
/**
@@ -121,8 +141,7 @@ static void sensor_submit_fallback(const struct device *dev, struct rtio_iodev_s
121141
header->num_channels = num_output_samples;
122142
header->shift = 0;
123143

124-
q31_t *q = (q31_t *)(buf + sizeof(struct sensor_data_generic_header) +
125-
num_output_samples * sizeof(enum sensor_channel));
144+
q31_t *q = (q31_t *)(buf + compute_header_size(num_output_samples));
126145

127146
/* Populate values, update shift, and set channels */
128147
for (size_t i = 0, sample_idx = 0; i < cfg->count; ++i) {

drivers/sensor/icm42688/icm42688.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -251,12 +251,11 @@ int icm42688_init(const struct device *dev)
251251
}
252252
#endif
253253

254-
memset(&data->cfg, 0, sizeof(struct icm42688_cfg));
255254
data->cfg.accel_mode = ICM42688_ACCEL_LN;
256-
data->cfg.gyro_mode = ICM42688_GYRO_LN;
257255
data->cfg.accel_fs = ICM42688_ACCEL_FS_2G;
258-
data->cfg.gyro_fs = ICM42688_GYRO_FS_125;
259256
data->cfg.accel_odr = ICM42688_ACCEL_ODR_1000;
257+
data->cfg.gyro_mode = ICM42688_GYRO_LN;
258+
data->cfg.gyro_fs = ICM42688_GYRO_FS_125;
260259
data->cfg.gyro_odr = ICM42688_GYRO_ODR_1000;
261260
data->cfg.fifo_en = false;
262261

include/zephyr/drivers/sensor.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -801,11 +801,14 @@ struct __attribute__((__packed__)) sensor_data_generic_header {
801801
* The number of channels present in the frame. This will be the true number of elements in
802802
* channel_info and in the q31 values that follow the header.
803803
*/
804-
size_t num_channels;
804+
uint32_t num_channels;
805805

806806
/* Shift value for all samples in the frame */
807807
int8_t shift;
808808

809+
/* This padding is needed to make sure that the 'channels' field is aligned */
810+
int8_t _padding[sizeof(enum sensor_channel) - 1];
811+
809812
/* Channels present in the frame */
810813
enum sensor_channel channels[0];
811814
};

0 commit comments

Comments
 (0)