Skip to content

drivers: sensor: add bma400 accelerometer driver - #104178

Open
lucagessi wants to merge 2 commits into
zephyrproject-rtos:mainfrom
lucagessi:feature_driver_sensor_bma400
Open

drivers: sensor: add bma400 accelerometer driver#104178
lucagessi wants to merge 2 commits into
zephyrproject-rtos:mainfrom
lucagessi:feature_driver_sensor_bma400

Conversation

@lucagessi

Copy link
Copy Markdown

Add initial support for the Bosch BMA400 accelerometer. The existing bma4xx driver is not compatible with the BMA400 due to significant register map differences.

Supported features:

  • On-motion auto wakeup
  • Inactivity auto low-power mode
  • Motion detection trigger (normal mode)
  • FIFO data streaming (normal mode)

#85462 highlights that bma4xx driver is not compatible with bma400.

#8139 asks for bma400 driver implementation

@github-actions

Copy link
Copy Markdown

Hello @lucagessi, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@lucagessi

lucagessi commented Feb 23, 2026

Copy link
Copy Markdown
Author

Hi! Following up on this PR since all CI checks are green. Let me know if you have any questions.

@lucagessi

Copy link
Copy Markdown
Author

Hi @MaureenHelm,

Just a quick follow-up since the CI checks are all green. Is there anything else you need me to address before review? I'm also happy to split this into smaller commits if the current one is too large.

Let me know what you think—I'm open to any changes. Thanks!

@MaureenHelm

Copy link
Copy Markdown
Member

Hi @MaureenHelm,

Just a quick follow-up since the CI checks are all green. Is there anything else you need me to address before review? I'm also happy to split this into smaller commits if the current one is too large.

Let me know what you think—I'm open to any changes. Thanks!

I took a quick look and saw two things:

  • We're trying to avoid introducing new device-specific sensor samples. Would like to see you use one of the existing accel samples rather than introducing a new one
  • Please add the new sensor to tests/drivers/build_all/sensor so it gets covered in CI/CD build testing

I'll try to come back soon and do a more in-depth review.

@lucagessi
lucagessi force-pushed the feature_driver_sensor_bma400 branch from 2f26e80 to 430dfd8 Compare March 14, 2026 14:53
@zephyrbot zephyrbot added the area: Tests Issues related to a particular existing or missing test label Mar 14, 2026
@lucagessi
lucagessi force-pushed the feature_driver_sensor_bma400 branch 2 times, most recently from 6b7d437 to 14ec47c Compare March 14, 2026 17:59
@lucagessi
lucagessi marked this pull request as draft March 14, 2026 18:14
@lucagessi
lucagessi marked this pull request as ready for review March 14, 2026 18:14
@lucagessi

Copy link
Copy Markdown
Author

@MaureenHelm I made the changes you suggested.
Hope is fine.
Thank you

@lucagessi
lucagessi requested a review from teburd March 24, 2026 05:42
@sonarqubecloud

Copy link
Copy Markdown

@lucagessi

Copy link
Copy Markdown
Author

Hi @MaureenHelm @teburd, I implemented the requested changes. All checks are green. Let me know if everything is ok now!🙏

@ubieda ubieda left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

First-pass review.

Comment thread drivers/sensor/bosch/bma400/bma400_rtio_stream.c Outdated
Comment thread drivers/sensor/bosch/bma400/bma400_rtio_stream.c Outdated
Comment thread drivers/sensor/bosch/bma400/bma400_rtio_stream.c Outdated
Comment thread drivers/sensor/bosch/bma400/bma400.c
Comment thread drivers/sensor/bosch/bma400/bma400.c Outdated
Comment thread dts/bindings/sensor/bosch,bma400-i2c.yaml Outdated
@lucagessi
lucagessi force-pushed the feature_driver_sensor_bma400 branch 2 times, most recently from e00c3c6 to 4a1706a Compare April 23, 2026 14:15
@sonarqubecloud

Copy link
Copy Markdown

@lucagessi
lucagessi requested a review from ubieda April 24, 2026 07:41
@lucagessi

Copy link
Copy Markdown
Author

@ubieda I made the changes you asked. Are ok for you?
Thank you

@ubieda

ubieda commented Apr 28, 2026

Copy link
Copy Markdown
Member

Hi, @lucagessi I'll take another look in the next couple of days! Thanks for your patience.

Comment thread drivers/sensor/bosch/bma400/Kconfig Outdated
depends on SENSOR_ASYNC_API
select SPI if $(dt_compat_on_bus,$(DT_COMPAT_BOSCH_BMA400),spi)
select SPI_RTIO if $(dt_compat_on_bus,$(DT_COMPAT_BOSCH_BMA400),spi)
select RTIO_WORKQ

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's drop this as it isn't required

struct bma400_decoder_header header;
uint8_t int_status;
uint16_t accel_odr: 4;
uint16_t fifo_count: 10;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IIUC from the datasheet - The FIFO can hold 1024 bytes. Don't we, then, need 11-bits to hold the max value?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You are right my fault

Comment on lines +89 to +90
const bool has_accel = (BMA400_FIFO_EMPTY_FRAME & buffer[0]) != 0;
const bool is_empty = (BMA400_FIFO_EMPTY_FRAME == buffer[0]);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm confused because I'm failing to see how has_accel can be false and is_empty true at the same time (See if-elseif-else branching below). Please explain the intend.

In general: please double-check the header classification and properly handle the flags critical for the features supported.

struct rtio_iodev_sqe *streaming_sqe = drv_data->streaming_sqe;

ARG_UNUSED(sqe);
ARG_UNUSED(result);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Handle result, as it may indicate a failure (applies to all completion cb's)

Comment thread drivers/sensor/bosch/bma400/bma400.c Outdated
uint8_t value;
uint8_t power_mode_conf = 0;

/* If ODR is greater than 25Hz, enable normal mode */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would you mind clarifying how we enforce this comment? Is it stale?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I Will remove It. The sensor works only in normal mode or low power with auto sleep.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok - In general: I advise to look over the driver and simplify it to cover the features that are supported. I'd rather work with a driver that is limited functionality but reliable than one that has many features but not sure what is well-supported and what isn't.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I understand your point. I needed these features for a project so I tried to implement: auto wake up, auto sleep and motion event.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Is not clear how it works? Do I need to say it clearly?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need to ellaborate: If you need all the features that's okay!

I'd simply suggest to step back, take another pass at the PR and address what you consider could use some improvement (e.g: const usage, error-handling, variables-sizing, etc).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ok. I will do a review of the code by myself taking into account your suggestions. Thank you very much ubieda, I'll do my best ;)

/**
* Set the sensor's acceleration threshold for motion detection
*/
static int bma400_attr_set_motion_threshold(const struct sensor_value *val,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this be SI (e.g: the same unit as SENSOR_CHAN_ACCEL_XYZ)?

/**
* Set the sensor's acceleration motion event duration
*/
static int bma400_attr_set_motion_duration(const struct sensor_value *val,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this be SI (as in: Seconds)?

int (*write_reg)(const struct device *dev, uint8_t reg_addr, uint8_t value);
};

struct bma400_runtime_config {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please clairfy why the following fields should be constant:

  • auto_wakeup_en
  • auto_lowpower_en
  • int1_map
  • int2_map

The reason I ask is because this whole struct is updated at run-time.

struct rtio_sqe *write_reset_fifo = rtio_sqe_acquire(r);
const uint8_t write_buffer[] = {
BMA400_REG_CMD,
0xb0, /* Reset FIFO */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not using BMA400_FIFO_FLUSH_CMD?

@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown

This pull request has been marked as stale because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 7 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions Bot added the Stale label Jun 1, 2026
@github-actions github-actions Bot closed this Jun 9, 2026
@teburd teburd reopened this Jun 9, 2026
@teburd teburd removed the Stale label Jun 9, 2026
@sonarqubecloud

sonarqubecloud Bot commented Jun 9, 2026

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

This pull request has been marked as stale because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 7 days. Note that if it gets closed, you can ask someone to reopen it for you if you do not have the permissions to do so.

@lucagessi

Copy link
Copy Markdown
Author

Sorry for the delay, been busy with work. I'm resuming work on the driver now.

@github-actions github-actions Bot removed the Stale label Jul 18, 2026
@github-actions

Copy link
Copy Markdown

This pull request has been marked as stale because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 7 days. Note that if it gets closed, you can ask someone to reopen it for you if you do not have the permissions to do so.

@github-actions github-actions Bot added the Stale label Aug 18, 2026
Add initial support for the Bosch BMA400 accelerometer.
The existing bma4xx driver is not compatible with the BMA400
due to significant register map differences.

Supported features:
- On-motion auto wakeup
- Inactivity auto low-power mode
- Motion detection trigger (normal mode)
- FIFO data streaming (normal mode)

Signed-off-by: Luca Gessi <lucagessi90@gmail.com>
Add bma400 sensor to build test.

Signed-off-by: Luca Gessi <lucagessi90@gmail.com>
@lucagessi
lucagessi force-pushed the feature_driver_sensor_bma400 branch from 4a1706a to 26d63e2 Compare August 25, 2026 20:22
@lucagessi

Copy link
Copy Markdown
Author

@ubieda I made some changes to the driver.

Below main changes:

Single interrupt line (INT1)

  • Dropped int2-gpios, fifo-pin-sel and motion-detect-pin-sel from the bindings. All sources (FIFO watermark, FIFO full, GEN2 motion) are mapped onto INT1 and served by one RTIO chain that reads the global INT_STAT0, so the second pin was redundant. Removed the second GPIO callback, the int1_map/int2_map config fields and the pending_gpio_interrupt state.
  • The line is disabled by the ISR and re-armed at the end of the chain, bounding it to one event per service window.

Sensor API conformance (review feedback)

  • SENSOR_ATTR_FULL_SCALE get/set now use m/s² (sensor_ms2_to_g/sensor_g_to_ms2) instead of raw g.
  • Motion threshold moved to SENSOR_ATTR_SLOPE_TH, converted from m/s² at 8 mg/LSB. SENSOR_ATTR_SLOPE_DUR is now expressed in samples/ticks (same convention as lis2dh/bma280) and writes the full 16-bit GEN2INT_CONFIG3:CONFIG31 field — it was truncated to 8 bits before.
  • Removed the bogus SENSOR_CHAN_DIE_TEMP support from the decoder, and the select RTIO_WORKQ.

FIFO decoding fixes

  • Frame classification rewritten: empty/sensortime/control headers are matched exactly before testing the axis-enable bits — the control frame (0x48) overlaps the Z-axis bit, so frames were mis-sized and the parse walked off.
  • fifo_count widened to 11 bits (FIFO is up to 1024 bytes); the decode buffer end now accounts for the SPI dummy byte.
  • Removed a sys_le16_to_cpu() applied to an already CPU-order sample.

RTIO error and lifecycle handling

  • SPI results are propagated from every stage (INT_STAT0, FIFO_LENGTH, FIFO_DATA) instead of ARG_UNUSED(result); every early return clears streaming_sqe and re-arms the GPIO so the stream cannot stall.
  • INT_STAT0 is decoded with the BMA400_ASSERTED_* status masks instead of the INT_CONFIG0 enable masks (they only matched by coincidence).
  • Terminal callback uses rtio_sqe_prep_callback_no_cqe(), the FIFO-flush write is null-checked and marked RTIO_SQE_NO_RESPONSE: no CQE/SQE pool leaks.
  • The streaming SQE is armed before the GPIO is re-enabled, so an event landing in that window is serviced.

Cancellation support (new)

  • Added a stream_busy atomic, claimed with atomic_cas(), so the GPIO ISR and the submitting thread can never service the same SQE.
  • Cancelled requests are retired with -ECANCELED (never 0: the executor reclaims the mempool block on a cancelled multishot) from the ISR path, from the FIFO-complete callback, and from a cancel-then-resubmit in bma400_submit_stream(), which takes over the service window, releases the parked SQE and reprograms the sensor. After a cancel the line is deliberately left disabled until a new submission arms it.
  • The trigger set is validated before any state is touched, so a rejected configuration never tears down a running stream.

Other

  • Motion and FIFO triggers in the same stream are rejected with -ENOTSUP: on a single edge-triggered line a persistent motion condition holds the line asserted and edges are lost.
  • The FIFO is flushed when stream configuration enables it, so no pre-config samples are delivered.

@github-actions github-actions Bot removed the Stale label Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Boards/SoCs area: Devicetree Binding PR modifies or adds a Device Tree binding area: Samples Samples area: Sensors Sensors area: Tests Issues related to a particular existing or missing test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants