sensors/lsm6ds3trc: add FIFO drain functionality - #20036
Conversation
Interrupt-driven mode so far pushes one uORB event per physical sample: one I2C burst read and one interrupt per sample, at whatever ODR the topic is running. That's the dominant power cost for a battery-constrained use case sampling continuously -- draining the chip's FIFO in batches cuts both by roughly the watermark size. The LSM6DS3TR-C's FIFO is the older ST "pattern" style (no per-sample tag byte, unlike LSM6DSO/ISM330): FIFO_CTRL3's per-sub-sensor decimation bits choose which of gyro/accel feed the FIFO (0 = excluded, 1 = no decimation -- only 0/1 are used here), FIFO_CTRL5 sets one shared FIFO-only ODR, and FIFO_STATUS1/2's DIFF_FIFO reports how many 16-bit words are waiting. Reused the existing INT1 wiring, but switched from DRDY (per-sample) to FTH (threshold reached) when CONFIG_SENSORS_LSM6DS3TRC_FIFO is on -- new bool that's a whole-driver mode switch, not a per-instance choice, so a board doesn't change; only its Kconfig does. lsm6ds3trc_fifo_configure() re-derives and writes the decimation bits, FIFO ODR, and watermark threshold (in words = watermark-in-samples * words-per-pattern, 3 with one sub-sensor active or 6 with both) from current dev->gyro/accel enabled+odr state; called from activate() and set_interval(). Both sub-sensors are forced to the same ODR while FIFO is on -- decimation factors > 1 for independent per-topic rates is real complexity (matching ODR ratios to decimation values) left for later. lsm6ds3trc_fifo_worker() replaces lsm6ds3trc_worker() under the Kconfig guard: reads DIFF_FIFO, bursts that many words (rounded down to a whole pattern chunk, capped at 2x the configured watermark so a late drain doesn't overflow the read buffer -- whatever's left over just waits in the chip's own FIFO for the next drain), then walks the buffer decoding each chunk into a push_event() same as before. FIFO entries don't carry their own timestamp, so each one is interpolated backwards from the ISR's timestamp by the configured ODR interval. Temperature isn't part of the FIFO pattern (FIFO_TEMP_EN stays off to keep the pattern width simple); one direct OUT_TEMP_L read per drain is applied to the whole batch instead. Validated on the bench, both pattern widths: with both topics subscribed (6-word pattern) and with only the accelerometer (3-word), samples arrive in watermark-sized bursts with interpolated timestamps spaced by the exact configured ODR interval (52Hz -> 19230us between every consecutive sample, matched exactly), sane accel/gyro values, no I2C errors, no overruns, sustained for 15+ seconds continuous. Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Felipe Moura <moura.fmo@gmail.com>
Previously, activate() always fell back to a hardcoded ODR_52HZ (or the other sub-sensor's rate, in FIFO mode) whenever a sub-sensor went from disabled to enabled, discarding whatever rate the application had explicitly requested via set_interval() before disabling it. An application that only ever wants, say, 25Hz would see the sensor restart at 52Hz on every re-activation, and in FIFO mode this fills the FIFO faster than intended, defeating the point of choosing a lower ODR for power savings. Add last_odr, which -- unlike odr -- survives being disabled. activate() now restores it on the next enable, only falling back to ODR_52HZ on a sub-sensor's genuine first-ever activation. Signed-off-by: Felipe Moura <moura.fmo@gmail.com>
Hardware testing (single-topic FIFO subscriptions, register dumps via
manual i2c commands) turned up two independent bugs that together
made the FIFO silently never accumulate data for a single-topic (e.g.
accel-only) subscription, while showing FIFO_STATUS2 stuck reporting
OVER_RUN and FIFO_FULL_SMART with a simultaneous zero DIFF_FIFO count:
- The LSM6DS3TR-C's FIFO write trigger (data-ready-based, the only mode
this driver uses) only fires while BOTH the accelerometer and the
gyroscope are physically running, regardless of which one(s) are
actually decimated into the FIFO pattern -- confirmed in the
datasheet's FIFO section ("the ODR must be lower than or equal to
both the accelerometer and gyroscope ODRs") and reproduced by
register-level testing with the unsubscribed sensor powered down vs.
powered up. lsm6ds3trc_fifo_configure() now forces whichever
sub-sensor isn't subscribed to run at the shared rate anyway (still
excluded from the pattern, so this costs no extra I2C bandwidth on
drain, just that sensor's own unavoidable power draw), and brings it
back down once neither sub-sensor is subscribed.
- Separately, lsm6ds3trc_fifo_configure() reconfigured decimation,
watermark and ODR while the FIFO was still running in Continuous
mode from a previous configuration. Reproduced manually: reconfiguring
live leaves FIFO_STATUS1/2 stuck reporting a stale diff count even
once the trigger fix above is in place; resetting through Bypass mode
first (which also empties the FIFO) before writing the new settings,
the same procedure the datasheet documents for changing FIFO
settings, and only re-entering Continuous mode last, is what actually
gets the diff counter to track correctly.
Also fixes two bit-definition bugs found while cross-referencing the
real ST datasheet instead of the in-tree lsm6dsl.h header used as a
starting point: MASK_FIFO_DIFF_HI was 4 bits (0x0f) instead of the
documented 3 (0x07), and MASK_DEC_FIFO_XL/SHIFT_DEC_FIFO_GY assumed
2-bit decimation fields instead of the documented 3-bit ones.
Verified on hardware: accel-only, gyro-only, and both-topics
subscriptions all now drain cleanly at the configured watermark with
no overrun, sustained over tens of seconds of continuous streaming.
Signed-off-by: Felipe Moura <moura.fmo@gmail.com>
Adds a section covering CONFIG_SENSORS_LSM6DS3TRC_FIFO: what it changes (one interrupt/I2C read per watermark instead of per sample) and its three limitations while it's on -- both sub-sensors forced to the same ODR, temperature no longer per-sample (one read per drain applied to the whole batch), and both sub-sensors must stay physically enabled regardless of subscription, since the chip's FIFO write trigger needs both running -- plus the watermark/ORB-buffer-size relationship callers need to respect. Also documents a related chip quirk found during bench testing: diff_words reads 0 at the exact moment a real FIFO overrun occurs, even though the FIFO is still completely full of valid, retained data (a forced read past that point recovers real samples, not garbage). This isn't a bug in this driver: ST's own engineers confirm the same behavior for this chip family on their community forum (thread "LSM6DS3 FIFO status clarification", td-p/184022), and the mainline Linux st_lsm6dsx driver doesn't attempt to recover from it either -- it only special-cases an empty FIFO, not an overrun one. Documenting this in a comment rather than adding recovery logic: with the small watermark this driver uses, reaching a real overrun at all means the drain has already fallen many seconds behind, and ST's own guidance for this condition is to avoid it via watermark sizing rather than recover from it. Signed-off-by: Felipe Moura <moura.fmo@gmail.com>
|
Please provide some testing logs, and be sure to update your PR title & commit messages to follow the standard commit message format! FYI, the PR title is what we use in release notes and following the directory structure convention (i.e. |
Just a correction: there should be only two items before ":" as explained in the Documentation and the Check will require, it should be "drivers/sensors: Add driver to LSM6DS3TRC IMU sensor |
Sorry, I am working in 2 parallel projects, each one with specific doc requirements. I will update PR title and add some logs. |
|
@linguini1 , sorry for documentation and PR title, I updated both and will take care about it. |
Summary
Adds optional hardware-FIFO draining to the LSM6DS3TR-C uORB driver
(
drivers/sensors/lsm6ds3trc_uorb.c, added in #19997), as a follow-up tothat base driver.
Interrupt-driven mode currently pushes one uORB event per physical sample:
one I2C burst read and one interrupt per sample, at whatever ODR the topic
is running. That is the dominant power cost for a battery-constrained use
case that samples continuously. This PR adds a new whole-driver mode,
CONFIG_SENSORS_LSM6DS3TRC_FIFO, that instead drains the chip's hardwareFIFO in batches on a watermark interrupt, cutting both I2C transactions and
interrupt wakeups by roughly the configured watermark size.
The LSM6DS3TR-C's FIFO is the older ST "pattern" style (no per-sample tag
byte, unlike LSM6DSO/ISM330):
FIFO_CTRL3's per-sub-sensor decimation bitschoose which of gyro/accel feed the FIFO,
FIFO_CTRL5sets one sharedFIFO-only ODR, and
FIFO_STATUS1/2'sDIFF_FIFOreports how many 16-bitwords are waiting. The existing INT1 wiring is reused, switching from DRDY
(per-sample) to FTH (threshold reached) when the option is on.
Included in this PR:
lsm6ds3trc_fifo_configure(): re-derives and writes decimation bits,shared FIFO ODR, and watermark threshold from the current
gyro/accel enabled+ODR state, called from
activate()andset_interval().lsm6ds3trc_fifo_worker(): replaces the per-sample DRDY worker under theKconfig guard -- reads
DIFF_FIFO, bursts the available words (roundeddown to a whole pattern chunk, capped so a late drain can't overflow the
read buffer), and decodes each chunk into a
push_event(), withper-sample timestamps interpolated backwards from the ISR timestamp by
the configured ODR interval.
A fix so
activate()restores the last ODR explicitly requested viaset_interval()instead of always resetting to a hardcoded 52 Hz onevery re-activation -- previously this defeated the point of choosing a
lower ODR for power savings once FIFO mode was involved.
Two real FIFO-correctness fixes found via register-level hardware
testing: the FIFO's data-ready write trigger only fires while both
sub-sensors are physically running (confirmed in the datasheet and by
register dumps), so a single-topic subscription previously left the
FIFO never accumulating data; and reconfiguring FIFO settings while
still in Continuous mode left
DIFF_FIFOstuck reporting a stale count,fixed by resetting through Bypass mode (which also empties the FIFO)
before writing new settings and re-entering Continuous mode last, per
the datasheet's documented procedure. Two related bit-definition bugs
(
MASK_FIFO_DIFF_HI,MASK_DEC_FIFO_XL/SHIFT_DEC_FIFO_GY) were alsofixed against the real ST datasheet rather than the in-tree
lsm6dsl.hheader the original driver was ported from.
A documented, deliberately-not-"fixed" chip quirk:
DIFF_FIFOreads 0at the exact moment a real overrun occurs, even though the FIFO is still
full of valid data. This matches ST's own guidance for this chip family
and mainline Linux's
st_lsm6dsxdriver, which doesn't attempt torecover from it either.
Documentation additions covering FIFO mode and its three limitations:
both sub-sensors forced to the same ODR, temperature no longer
per-sample (one read per drain applied to the whole batch), and both
sub-sensors must stay physically enabled regardless of subscription.
Impact
CONFIG_SENSORS_LSM6DS3TRC_FIFOdefaultsto
n, and only takes effect for boards that already register thisdriver with a real interrupt
attach()(silently unused in kthreadpolling mode).
boards/xtensa/esp32s3/esp32s3-xiao(the only board currently usingthis driver) is unaffected unless its defconfig opts in.
SENSORS_LSM6DS3TRC_FIFO_WATERMARK(default 8samples) controls the batch size; its help text notes the relationship
to the accel/gyro uORB ring buffer sizes.
Documentation/components/drivers/special/sensors/lsm6ds3trc.rst).Testing
Host: Linux, xtensa-esp32s3-elf toolchain.
Board: Seeed XIAO ESP32-S3, LSM6DS3TR-C over I2C, interrupt-driven mode
(shared INT1 pin, per the driver's existing single-IRQ design).
Both
sensor_accel0/sensor_gyro0topics subscribed (6-wordpattern) and accel-only (3-word pattern): samples arrive in
watermark-sized bursts, interpolated timestamps spaced by the exact
configured ODR interval (52 Hz -> 19230 us between consecutive
samples, matched exactly), sane accel/gyro values, no I2C errors, no
overruns, sustained for 15+ seconds of continuous streaming.
Accel-only and gyro-only single-topic subscriptions confirmed to
drain correctly after the write-trigger fix (previously silently
never accumulated data).
Re-activation after
set_interval()confirmed to resume at thepreviously requested ODR instead of resetting to 52 Hz.
Deliberate FIFO overrun test: ~7.80 s to fill+overrun both topics at
52 Hz, ~324 samples lost per trial if left unread that long --
consistent with a healthy design given the default watermark drains
roughly every 150 ms in real use.
Console output: