sensor: adxl345: fix Streaming mode

[ubieda]

Description

This PR contains fixes found while troubleshooting the ADXL345 driver in order to get it working in Streaming mode.

Specific changes:

• Fix run-time issues with Streaming mode.
• Eliminate overriding of ODR DTS setting.
• Add enums for dt-bindings so it's easier to select between ODRs.
• Add configurability of fifo-watermark through a DTS property

Note

While digging into this, some fixes were applied to NXP I2C RTIO driver (#88315) and nRF I2C RTIO driver (#88380 and #88390). They're required in order to prove this fix works.

Testing

Build Shell sample for nRF52840DK with Streaming commands enabled and validate sensor data is coming through (To replicate, please go to the branch with the I2C RTIO related fixes: https://github.com/croxel/zephyr/tree/fix-working-with-nrf52840dk/adxl345-streaming).

• Build command:

west build -b nrf52840dk/nrf52840 samples/subsys/shell/shell_module

• Configuration Overlay (boards/nrf52840dk_nrf52840.conf):

CONFIG_SENSOR=y
CONFIG_SENSOR_ASYNC_API=y

CONFIG_ADXL345_STREAM=y
CONFIG_I2C_RTIO=y

CONFIG_SENSOR_SHELL=y
CONFIG_SENSOR_SHELL_STREAM=y

• DTS overlay (boards/nrf52840dk_nrf52840.overlay):

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

&arduino_i2c {
	status = "okay";
	
	adxl345: adxl345@53 {
		compatible = "adi,adxl345";
		reg = <0x53>;
		status = "okay";
		odr = <ADXL345_DT_ODR_25>;
		fifo-watermark = <31>;
		int2-gpios = <&gpio1 3 GPIO_ACTIVE_HIGH>;
	};
};

• Console output:

uart:~$ sensor get adxl345@53 
channel type=3(accel_xyz) index=0 shift=7 num_samples=1 value=5990661621ns, (0.000000, -0.306457, 8.887274)
...
uart:~$ sensor stream adxl345@53 on fifo_wm incl
Enabling stream...
Trigger (10 / fifo_wm) detected
channel type=3(accel_xyz) index=0 shift=7 num_samples=31 value=22069793701ns, (-0.004942, -0.291629, 8.911989)
Trigger (10 / fifo_wm) detected
channel type=3(accel_xyz) index=0 shift=7 num_samples=31 value=23299346923ns, (0.000000, -0.296571, 8.902103)
Trigger (10 / fifo_wm) detected
channel type=3(accel_xyz) index=0 shift=7 num_samples=31 value=24528808593ns, (0.000000, -0.286686, 8.897160)
Trigger (10 / fifo_wm) detected
channel type=3(accel_xyz) index=0 shift=7 num_samples=31 value=25758331298ns, (0.000000, -0.301514, 8.897160)
Trigger (10 / fifo_wm) detected
channel type=3(accel_xyz) index=0 shift=7 num_samples=31 value=26987854003ns, (0.000000, -0.291629, 8.916932)
Trigger (10 / fifo_wm) detected
channel type=3(accel_xyz) index=0 shift=7 num_samples=31 value=28217376708ns, (0.000000, -0.296571, 8.897160)
Trigger (10 / fifo_wm) detected
channel type=3(accel_xyz) index=0 shift=7 num_samples=31 value=29446838378ns, (0.000000, -0.296571, 8.926817)

• Logic Analyzer capture with interrupt triggers and I2C transfers gathering data:
  

[ubieda]

I've pushed additional changes to this branch:

• Remove overriding of DTS ODR setting.
• Create enum for DT-bindings to improve clarity on ODR settings.
• Expose fifo-watermark through a DTS property.

[ubieda]

I've pushed once more to fix the CI failures, which now seems to be green. PR is up for review!

[dimitrije-lilic]

Hi all,
I've tested this PR and it works.
We discussed with @ubieda and it definitely is not possible to do a multiple samples burst read so the size of sq and cq are now not hardcoded to largest possible watermark and are now function of watermark dt value. This will at least create smaller memory footprint for sensors that have smaller watermark set.
Good changes overall :)

[yperess]

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.

[ubieda]

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

Thanks in advance!

[ubieda]

FYI - I'm planning to come back to this early next week. Stay tuned
Per Sensor WG discussion: It does not seem required to iterate on this PR any further, rather have reviewers just take another look.

[ubieda]

Per today's Sensor WG discussion, adding a DNM label until @MaureenHelm @yperess take another look

[MaureenHelm]

@yperess please revisit