Skip to content

sensor: adxl345: fix Streaming mode #88393

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 16, 2025
Merged

sensor: adxl345: fix Streaming mode #88393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cd12454
sensor: adxl345: Fix conditional instantiation of RTIO ctx and IODEV
ubieda Apr 9, 2025
9ecaca7
sensor: adxl345: Prevent clearing SQE flags set while prepping SQEs
ubieda Apr 9, 2025
51e60ce
sensor: adxl345: fix: Overriding of ODR setting in DTS property
ubieda Apr 10, 2025
bce2d15
sensor: adxl345: rename ADXL345_ODR_12HZ to ADXL345_ODR_12_5HZ
ubieda Apr 10, 2025
a9c0a0d
sensor: adxl345: Allow fifo-watermark configurable through dts
ubieda Apr 10, 2025
1f5e990
sensor: adxl345: Optimize RTIO SQE/CQE pool based on fifo-watermark
ubieda Apr 10, 2025
ebd4680
sensor: adxl345: Formatting improvements on macrobatics
ubieda Apr 11, 2025
873b090
shields: pmod_acl: Set default ODR and watermark values
ubieda Apr 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions boards/shields/pmod_acl/pmod_acl.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,7 @@
reg = <0x0>;
spi-max-frequency = <DT_FREQ_M(1)>;
status = "okay";
odr = <25>;
fifo-watermark = <31>;
};
};
158 changes: 84 additions & 74 deletions drivers/sensor/adi/adxl345/adxl345.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ int adxl345_set_op_mode(const struct device *dev, enum adxl345_op_mode op_mode)
* Set Output data rate.
* @param dev - The device structure.
* @param odr - Output data rate.
* Accepted values: ADXL345_ODR_12HZ
* Accepted values: ADXL345_ODR_12_5HZ
* ADXL345_ODR_25HZ
* ADXL345_ODR_50HZ
* ADXL345_ODR_100HZ
Expand All @@ -233,7 +233,7 @@ static int adxl345_attr_set_odr(const struct device *dev,

switch (val->val1) {
case 12:
odr = ADXL345_ODR_12HZ;
odr = ADXL345_ODR_12_5HZ;
break;
case 25:
odr = ADXL345_ODR_25HZ;
Expand Down Expand Up @@ -482,7 +482,7 @@ static int adxl345_init(const struct device *dev)
#ifdef CONFIG_ADXL345_TRIGGER
rc = adxl345_configure_fifo(dev, ADXL345_FIFO_STREAMED,
ADXL345_INT2,
SAMPLE_NUM);
cfg->fifo_config.fifo_samples);
if (rc) {
return rc;
}
Expand Down Expand Up @@ -519,90 +519,100 @@ static int adxl345_init(const struct device *dev)
}

#ifdef CONFIG_ADXL345_TRIGGER
#define ADXL345_CFG_IRQ(inst) \
#define ADXL345_CFG_IRQ(inst) \
.interrupt = GPIO_DT_SPEC_INST_GET(inst, int2_gpios),
#else
#define ADXL345_CFG_IRQ(inst)
#endif /* CONFIG_ADXL345_TRIGGER */

#define ADXL345_RTIO_SPI_DEFINE(inst) \
COND_CODE_1(CONFIG_SPI_RTIO, \
(SPI_DT_IODEV_DEFINE(adxl345_iodev_##inst, DT_DRV_INST(inst), \
SPI_WORD_SET(8) | SPI_TRANSFER_MSB | \
SPI_MODE_CPOL | SPI_MODE_CPHA, 0U);), \
#define ADXL345_RTIO_SPI_DEFINE(inst) \
COND_CODE_1(CONFIG_SPI_RTIO, \
(SPI_DT_IODEV_DEFINE(adxl345_iodev_##inst, DT_DRV_INST(inst), \
SPI_WORD_SET(8) | SPI_TRANSFER_MSB | \
SPI_MODE_CPOL | SPI_MODE_CPHA, 0U);), \
())

#define ADXL345_RTIO_I2C_DEFINE(inst) \
COND_CODE_1(CONFIG_I2C_RTIO, \
(I2C_DT_IODEV_DEFINE(adxl345_iodev_##inst, DT_DRV_INST(inst));), \
#define ADXL345_RTIO_I2C_DEFINE(inst) \
COND_CODE_1(CONFIG_I2C_RTIO, \
(I2C_DT_IODEV_DEFINE(adxl345_iodev_##inst, DT_DRV_INST(inst));), \
())

/* Conditionally set the RTIO size based on the presence of SPI/I2C
* lines 541 - 542.
* The sizes of sqe and cqe pools are increased due to the amount of
* multibyte reads needed for watermark using 31 samples
* (adx345_stram - line 203), using smaller amounts of samples
* to trigger an interrupt can decrease the pool sizes.
/** RTIO SQE/CQE pool size depends on the fifo-watermark because we
* can't just burst-read all the fifo data at once. Datasheet specifies
* we need to get one frame at a time (through the Data registers),
* therefore, we set all the sequence at once to properly pull each
* frame, and then end up calling the completion event so the
* application receives it).
*/
#define ADXL345_RTIO_DEFINE(inst) \
/* Conditionally include SPI and/or I2C parts based on their presence */ \
COND_CODE_1(DT_INST_ON_BUS(inst, spi), \
(ADXL345_RTIO_SPI_DEFINE(inst)), \
()) \
COND_CODE_1(DT_INST_ON_BUS(inst, i2c), \
(ADXL345_RTIO_I2C_DEFINE(inst)), \
()) \
COND_CODE_1(DT_INST_NODE_HAS_PROP(inst, spi_dt_spec) && \
DT_INST_NODE_HAS_PROP(inst, i2c_dt_spec), \
(RTIO_DEFINE(adxl345_rtio_ctx_##inst, 128, 128);), \
(RTIO_DEFINE(adxl345_rtio_ctx_##inst, 64, 64);)) \

#define ADXL345_CONFIG(inst) \
.odr = DT_INST_PROP(inst, odr), \
.fifo_config.fifo_mode = ADXL345_FIFO_STREAMED, \
.fifo_config.fifo_trigger = ADXL345_INT2, \
.fifo_config.fifo_samples = SAMPLE_NUM, \
.odr = ADXL345_RATE_25HZ, \

#define ADXL345_CONFIG_SPI(inst) \
{ \
.bus = {.spi = SPI_DT_SPEC_INST_GET(inst, \
SPI_WORD_SET(8) | \
SPI_TRANSFER_MSB | \
SPI_MODE_CPOL | \
SPI_MODE_CPHA, \
0)}, \
.bus_is_ready = adxl345_bus_is_ready_spi, \
.reg_access = adxl345_reg_access_spi, \
.bus_type = ADXL345_BUS_SPI, \
ADXL345_CONFIG(inst) \
COND_CODE_1(DT_INST_NODE_HAS_PROP(inst, int2_gpios), \
(ADXL345_CFG_IRQ(inst)), ()) \
#define ADXL345_RTIO_DEFINE(inst) \
/* Conditionally include SPI and/or I2C parts based on their presence */ \
COND_CODE_1(DT_INST_ON_BUS(inst, spi), \
(ADXL345_RTIO_SPI_DEFINE(inst)), \
()) \
COND_CODE_1(DT_INST_ON_BUS(inst, i2c), \
(ADXL345_RTIO_I2C_DEFINE(inst)), \
()) \
RTIO_DEFINE(adxl345_rtio_ctx_##inst, \
2 * DT_INST_PROP(inst, fifo_watermark) + 2, \
2 * DT_INST_PROP(inst, fifo_watermark) + 2);

#define ADXL345_CONFIG(inst) \
.odr = DT_INST_PROP(inst, odr), \
.fifo_config.fifo_mode = ADXL345_FIFO_STREAMED, \
.fifo_config.fifo_trigger = ADXL345_INT2, \
.fifo_config.fifo_samples = DT_INST_PROP_OR(inst, fifo_watermark, 0),

#define ADXL345_CONFIG_SPI(inst) \
{ \
.bus = {.spi = SPI_DT_SPEC_INST_GET(inst, \
SPI_WORD_SET(8) | \
SPI_TRANSFER_MSB | \
SPI_MODE_CPOL | \
SPI_MODE_CPHA, \
0)}, \
.bus_is_ready = adxl345_bus_is_ready_spi, \
.reg_access = adxl345_reg_access_spi, \
.bus_type = ADXL345_BUS_SPI, \
ADXL345_CONFIG(inst) \
COND_CODE_1(DT_INST_NODE_HAS_PROP(inst, int2_gpios), \
(ADXL345_CFG_IRQ(inst)), ()) \
}

#define ADXL345_CONFIG_I2C(inst) \
{ \
.bus = {.i2c = I2C_DT_SPEC_INST_GET(inst)}, \
.bus_is_ready = adxl345_bus_is_ready_i2c, \
.reg_access = adxl345_reg_access_i2c, \
.bus_type = ADXL345_BUS_I2C, \
ADXL345_CONFIG(inst) \
COND_CODE_1(DT_INST_NODE_HAS_PROP(inst, int2_gpios), \
(ADXL345_CFG_IRQ(inst)), ()) \
#define ADXL345_CONFIG_I2C(inst) \
{ \
.bus = {.i2c = I2C_DT_SPEC_INST_GET(inst)}, \
.bus_is_ready = adxl345_bus_is_ready_i2c, \
.reg_access = adxl345_reg_access_i2c, \
.bus_type = ADXL345_BUS_I2C, \
ADXL345_CONFIG(inst) \
COND_CODE_1(DT_INST_NODE_HAS_PROP(inst, int2_gpios), \
(ADXL345_CFG_IRQ(inst)), ()) \
}

#define ADXL345_DEFINE(inst) \
IF_ENABLED(CONFIG_ADXL345_STREAM, (ADXL345_RTIO_DEFINE(inst))); \
static struct adxl345_dev_data adxl345_data_##inst = { \
COND_CODE_1(adxl345_iodev_##inst, (.rtio_ctx = &adxl345_rtio_ctx_##inst, \
.iodev = &adxl345_iodev_##inst,), ()) \
}; \
static const struct adxl345_dev_config adxl345_config_##inst = \
COND_CODE_1(DT_INST_ON_BUS(inst, spi), (ADXL345_CONFIG_SPI(inst)), \
(ADXL345_CONFIG_I2C(inst))); \
\
SENSOR_DEVICE_DT_INST_DEFINE(inst, adxl345_init, NULL, \
&adxl345_data_##inst, &adxl345_config_##inst, POST_KERNEL,\
CONFIG_SENSOR_INIT_PRIORITY, &adxl345_api_funcs); \
#define ADXL345_DEFINE(inst) \
\
BUILD_ASSERT(!IS_ENABLED(CONFIG_ADXL345_STREAM) || \
DT_INST_NODE_HAS_PROP(inst, fifo_watermark), \
"Streaming requires fifo-watermark property. Please set it in the" \
"device-tree node properties"); \
BUILD_ASSERT(COND_CODE_1(DT_INST_NODE_HAS_PROP(inst, fifo_watermark), \
((DT_INST_PROP(inst, fifo_watermark) > 0) && \
(DT_INST_PROP(inst, fifo_watermark) < 32)), \
(true)), \
"fifo-watermark must be between 1 and 32. Please set it in " \
"the device-tree node properties"); \
\
IF_ENABLED(CONFIG_ADXL345_STREAM, (ADXL345_RTIO_DEFINE(inst))); \
static struct adxl345_dev_data adxl345_data_##inst = { \
IF_ENABLED(CONFIG_ADXL345_STREAM, (.rtio_ctx = &adxl345_rtio_ctx_##inst, \
.iodev = &adxl345_iodev_##inst,)) \
}; \
static const struct adxl345_dev_config adxl345_config_##inst = \
COND_CODE_1(DT_INST_ON_BUS(inst, spi), (ADXL345_CONFIG_SPI(inst)), \
(ADXL345_CONFIG_I2C(inst))); \
\
SENSOR_DEVICE_DT_INST_DEFINE(inst, adxl345_init, NULL, \
&adxl345_data_##inst, &adxl345_config_##inst, POST_KERNEL, \
CONFIG_SENSOR_INIT_PRIORITY, &adxl345_api_funcs);

DT_INST_FOREACH_STATUS_OKAY(ADXL345_DEFINE)
14 changes: 8 additions & 6 deletions drivers/sensor/adi/adxl345/adxl345.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include <zephyr/kernel.h>
#include <zephyr/sys/util.h>

#include <zephyr/dt-bindings/sensor/adxl345.h>

#ifdef CONFIG_ADXL345_STREAM
#include <zephyr/rtio/rtio.h>
#endif /* CONFIG_ADXL345_STREAM */
Expand Down Expand Up @@ -117,12 +119,12 @@
#define ADXL345_BUS_SPI 1

enum adxl345_odr {
ADXL345_ODR_12HZ = 0x7,
ADXL345_ODR_25HZ,
ADXL345_ODR_50HZ,
ADXL345_ODR_100HZ,
ADXL345_ODR_200HZ,
ADXL345_ODR_400HZ
ADXL345_ODR_12_5HZ = ADXL345_DT_ODR_12_5,
ADXL345_ODR_25HZ = ADXL345_DT_ODR_25,
ADXL345_ODR_50HZ = ADXL345_DT_ODR_50,
ADXL345_ODR_100HZ = ADXL345_DT_ODR_100,
ADXL345_ODR_200HZ = ADXL345_DT_ODR_200,
ADXL345_ODR_400HZ = ADXL345_DT_ODR_400,
};

enum adxl345_fifo_trigger {
Expand Down
2 changes: 1 addition & 1 deletion drivers/sensor/adi/adxl345/adxl345_decoder.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ static inline void adxl345_accel_convert_q31(q31_t *out, int16_t sample, int32_t
#define SENSOR_SCALING_FACTOR (SENSOR_G / (16 * 1000 / 100))

static const uint32_t accel_period_ns[] = {
[ADXL345_ODR_12HZ] = UINT32_C(1000000000) / 12,
[ADXL345_ODR_12_5HZ] = UINT32_C(1000000000) / 12,
[ADXL345_ODR_25HZ] = UINT32_C(1000000000) / 25,
[ADXL345_ODR_50HZ] = UINT32_C(1000000000) / 50,
[ADXL345_ODR_100HZ] = UINT32_C(1000000000) / 100,
Expand Down
12 changes: 6 additions & 6 deletions drivers/sensor/adi/adxl345/adxl345_stream.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ static void adxl345_process_fifo_samples_cb(struct rtio *r, const struct rtio_sq

rtio_sqe_prep_tiny_write(write_fifo_addr, data->iodev, RTIO_PRIO_NORM, &reg_addr,
1, NULL);
write_fifo_addr->flags = RTIO_SQE_TRANSACTION;
write_fifo_addr->flags |= RTIO_SQE_TRANSACTION;
rtio_sqe_prep_read(read_fifo_data, data->iodev, RTIO_PRIO_NORM,
read_buf + data->fifo_total_bytes,
SAMPLE_SIZE, current_sqe);
Expand All @@ -222,7 +222,7 @@ static void adxl345_process_fifo_samples_cb(struct rtio *r, const struct rtio_sq
if (i == fifo_samples-1) {
struct rtio_sqe *complete_op = rtio_sqe_acquire(data->rtio_ctx);

read_fifo_data->flags = RTIO_SQE_CHAINED;
read_fifo_data->flags |= RTIO_SQE_CHAINED;
rtio_sqe_prep_callback(complete_op, adxl345_fifo_read_cb, (void *)dev,
current_sqe);
}
Expand Down Expand Up @@ -341,10 +341,10 @@ static void adxl345_process_status1_cb(struct rtio *r, const struct rtio_sqe *sq
const uint8_t reg_addr = ADXL345_REG_READ(ADXL345_FIFO_STATUS_REG);

rtio_sqe_prep_tiny_write(write_fifo_addr, data->iodev, RTIO_PRIO_NORM, &reg_addr, 1, NULL);
write_fifo_addr->flags = RTIO_SQE_TRANSACTION;
write_fifo_addr->flags |= RTIO_SQE_TRANSACTION;
rtio_sqe_prep_read(read_fifo_data, data->iodev, RTIO_PRIO_NORM, data->fifo_ent, 1,
current_sqe);
read_fifo_data->flags = RTIO_SQE_CHAINED;
read_fifo_data->flags |= RTIO_SQE_CHAINED;
if (cfg->bus_type == ADXL345_BUS_I2C) {
read_fifo_data->iodev_flags |= RTIO_IODEV_I2C_STOP | RTIO_IODEV_I2C_RESTART;
}
Expand Down Expand Up @@ -379,9 +379,9 @@ void adxl345_stream_irq_handler(const struct device *dev)
uint8_t reg = ADXL345_REG_READ(ADXL345_INT_SOURCE);

rtio_sqe_prep_tiny_write(write_status_addr, data->iodev, RTIO_PRIO_NORM, &reg, 1, NULL);
write_status_addr->flags = RTIO_SQE_TRANSACTION;
write_status_addr->flags |= RTIO_SQE_TRANSACTION;
rtio_sqe_prep_read(read_status_reg, data->iodev, RTIO_PRIO_NORM, &data->status1, 1, NULL);
read_status_reg->flags = RTIO_SQE_CHAINED;
read_status_reg->flags |= RTIO_SQE_CHAINED;

if (cfg->bus_type == ADXL345_BUS_I2C) {
read_status_reg->iodev_flags |= RTIO_IODEV_I2C_STOP | RTIO_IODEV_I2C_RESTART;
Expand Down
40 changes: 27 additions & 13 deletions dts/bindings/sensor/adi,adxl345-common.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,41 @@
# Copyright (c) 2022 Analog Devices Inc.
# SPDX-License-Identifier: Apache-2.0

description: |
ADXL345 3-axis accelerometer
When setting the accelerometer DTS properties, make sure to include
adxl345.h and use the macros defined there.
Example:
#include <zephyr/dt-bindings/sensor/adxl345.h>
adxl345: adxl345@1d {
...
odr = <ADXL345_DT_ODR_25>;
};
include: sensor-device.yaml

properties:
odr:
type: int
default: 0
default: 10
description: |
Accelerometer sampling frequency (ODR). Default is power on reset value.
0 # 12.5Hz
1 # 25Hz
2 # 50Hz
3 # 100Hz
4 # 200Hz
5 # 400Hz
enum:
- 0
- 1
- 2
- 3
- 4
- 5
- 7 # ADXL345_DT_ODR_12_5
- 8 # ADXL345_DT_ODR_25
- 9 # ADXL345_DT_ODR_50
- 10 # ADXL345_DT_ODR_100
- 11 # ADXL345_DT_ODR_200
- 12 # ADXL345_DT_ODR_400

fifo-watermark:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should pick a direction and go with it. Most applications I've worked with (usually in AR/VR and Android) expect that the scheduling of the processing thread is predictable. So I much prefer to use SENSOR_ATTR_BATCH_DURATION or in DT some form of batch-duration-us. This way you can expect to get samples at the same time interval regardless of the ODR.

Whatever direction we go we should be consistent.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Friendly reminder @yperess: please let me know if there's anything else required for removing the NACK.

Thanks in advance!

type: int
description: |
Specify the FIFO watermark level in frame count.
Valid range: 1 - 31
int2-gpios:
type: phandle-array
Expand Down
29 changes: 29 additions & 0 deletions include/zephyr/dt-bindings/sensor/adxl345.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright (c) 2025 Croxel Inc.
*
* SPDX-License-Identifier: Apache-2.0
*/
#ifndef ZEPHYR_INCLUDE_DT_BINDINGS_ADI_ADX345_H_
#define ZEPHYR_INCLUDE_DT_BINDINGS_ADI_ADX345_H_

/**
* @defgroup ADXL345 ADI DT Options
* @ingroup sensor_interface
* @{
*/

/**
* @defgroup ADXL345_ODR Output Rate options
* @{
*/
#define ADXL345_DT_ODR_12_5 7
#define ADXL345_DT_ODR_25 8
#define ADXL345_DT_ODR_50 9
#define ADXL345_DT_ODR_100 10
#define ADXL345_DT_ODR_200 11
#define ADXL345_DT_ODR_400 12
/** @} */

/** @} */

#endif /* ZEPHYR_INCLUDE_DT_BINDINGS_ADI_ADX345_H_ */
Loading