LSM6DSL Driver Limitations - advice to expand functionality.

[FinnBiggs]

Hi folks!

As always, I'll start with relevant environment details

===== Environment =====

nRF Connect SDK v2.5.0 (though not opposed to upgrading to one of the 2.6.X releases if helpful)

Designing for nRF52840

  - Prototyping on nRF52840DK with attached (i2c) LSM6DSL breakout (https://www.st.com/en/mems-and-sensors/lsm6dsl.html)

  - Production aimed at Xiao BLE Sense (https://wiki.seeedstudio.com/XIAO_BLE/)

===== Overview =====

I'm working on a project that involves significant use of many different IMU functionalities.

I need:

    - Control over data rate and scale (very possible with current drivers)

    - Control over interrupt source (harder, but possible through register setting via hw_tf)

    - Ability to go check registers based on an interrupt (e.g. query wakeup source, tap direction)

    - Ability to control power manager

As an aside, I cannot use the CAF sensor manager because:

    - I cannot sync the CAF sensor manager frequency to the IMU's interrupt channel (constraint by integer req. on sampling ms) ( Common Application Framework and custom Sensor with Sensor Manager Module)

===== Questions =====

I'll have to write an expansion to the existing drivers (currently at ncs/v2.5.0/zephyr/drivers/sensor/lsm6dsl/<these files>)

I'm quite experienced with this IMU in particular. I'd be very willing to write up a driver that adds handles to the functionalities I need, but I'd like some preemptive structural guidance.

At first, I was just going to pull the drivers into my application and rework them there (alongside modules), but when including them in the CMake.txt I got quite a few problems regarding duplicate definitions.

As I read it, the build system always includes /drivers (/zephyr/CMakeLists.txt), and then dives into more detail based on KConfig options. (/zephyr/drivers/CMakeLists.txt) if sensor and (/zephyr/drivers/sensor/CMakeLists.txt) if lsm6dsl. The conclusion I draw there is that the right way to do it (modify sensor drivers to my own needs) is to modify it in /drivers/ and leave it out of my application modules.

If I were to go this route, (making my own driver with handles on the functionality I need), I have a couple questions:

1. Where can I find style guides to make sure I'm conforming to zephyr best practices for a driver
2. How can I best make that work available to other zephyr developers?
   1. PR into nrfconnect/sdk-zephyr? (not ideal -- I'd like to use it in my own project alongside v2.5.0)
   2. Fork ncs/zephyr? (not ideal -- I'd like to be able to transition my project forward to later Zephyr releases without having to fold in my own block of driver code)

I share a lot of the same needs and concerns as the author of this post: (Migrating LSM6DSL Custom FIFO and Interrupts Implementation from nRF to Zephyr SDK: Seeking Guidance)

And I'll call out here that ST Did publish drivers that I'm considering using as an external lib (https://github.com/STMicroelectronics/lsm6ds3tr-c-pid/tree/master).

However, I would like to utilize Zephyr's modularity and tie the LSM6DSL drivers in to the tools provided by sensor.h.

I'd be grateful for any advice, words of warning, or pointers to wisdom you think I'll need.

Best Regards,

    - Finn

[AHaug]

Hi,

In general w.r.t contributing to NCS and Zephyr we recommend that you comply with the tips and guidelines mentioned in https://docs.nordicsemi.com/bundle/ncs-latest/page/nrf/dev_model_and_contributions/contributions.html and https://docs.nordicsemi.com/bundle/ncs-latest/page/zephyr/contribute/index.html 

Other than this I would like to recommend you to also reach out on the zephyr discord forums to discuss contribution to the SDKs there as well.

I will leave this case open here for now in case other users would like to chime in with their own experience

Kind regards,
Andreas 

[FinnBiggs]

So I've spent some time working on this now, and I've concluded that what I need isn't an expansion to the existing device drivers, but a set of tools that controls the device configuration directly during runtime.

A lot like in this post (Migrating LSM6DSL Custom FIFO and Interrupts Implementation from nRF to Zephyr SDK: Seeking Guidance), I've folded in the ST drivers to my project's /lib directory -- but this segues to a new challenge.

To use the ST drivers, one must implement a "platform read" and "platform write" method.

Could I please get advice on implementing these functions:

I'd like to use my device tree entry to populate my i2c instance

My devicetree (overlay) is as follows:

&i2c0 {
    lsm6ds3tr_c: lsm6ds3tr-c@6a {
		compatible = "st,lsm6dsl";
		reg = <0x6a>;
		irq-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
		status = "okay";
	};
};

I'd like to use my lsm6ds3tr_c node label to establish an i2c device, and then I'd like to use Zephyr's i2c functions to implement a platform-read and -write that will enable the ST drivers.

I think this approach will be better than augmenting Zephyr with a new driver because what I need differs from the typical format of sensor drivers.

Thank you for any guidance you can provide,

    - Finn

[AHaug]

Hi Finn,

I see now that I might have been too quick when reading your initial post and only focused on the part regarding "contributing to Zephyr". You are right that if you want to modify the existing driver you must either change the existing one or create your own custom version of the driver either in-tree or out-of-tree that includes what you need. From my understanding of your comments it looks like you may already be aware of this part, but I'll write it out anyways:

This device needs to have a binding similar to the lsm6ds3tr has in this binding github.com/.../st,lsm6dsl-i2c.yaml. If you have a for instance a binding named "my_lsm6dsl-i2c.yaml" located in your project_folder/bindings/dts. As an example that showcases the project-specific device, you can have a look at this repository where I've created a custom pwm device binding.

From there on you include compatible in your overlay.

FinnBiggs said:

To use the ST drivers, one must implement a "platform read" and "platform write" method.

If I understand this correct, what you want is to implement an API that allows you to actively read data from the sensor over I2C and to send data to the sensor (commands to for instance change sampling rate or to poll the sensor) over I2C. Is this correct? If so I would instead recommend that you investigate the I2C API where you will find functions to read and write as well as this I2C course which showcases how to do read and write over I2C from your controller to the I2C device: https://academy.nordicsemi.com/courses/nrf-connect-sdk-fundamentals/lessons/lesson-6-serial-com-i2c/ 

Let me know if this clarifies things for you and please expand if there is anything you believe I have misunderstood

Kind regards,
Andreas

[FinnBiggs]

Hi Andreas,

Thank you for your follow-up. Yes, I understand that to modify drivers I either need to make my own (in or out of tree). When writing my first post, I thought that to get the functionality I needed, I would have to add generic control functions to the driver for functions like Tap Detection, Free Fall, Significant Motion, or Pedometer.

However, those functions are not typical for the sensor driver, so I started looking at direct i2c controls.

Down that path, I discovered that ST has provided a very thorough driver for the sensor -- they provide a name and bit ID for every register, as well as a getter and setter method for each register.

To use those drivers, I need to implement a platform read and write method that allows the ST drivers to communicate with the sensor on this (zephyr) platform.

What I'm trying to implement is a system where the behavior of the IMU is modifiable during run time. Specifically, I'd like to be able to use a data ready trigger to take data at 104Hz, then be able to have the device go to sleep, put the IMU in low-power mode, and configure it to wake up my application via either a tap, significant motion, or pedometer steps.

Ideally, I don't have to poll, and I can use the interrupt pin to signal data is ready for normal operation, and then configure the interrupt settings as a pre power-down action, so that waking is configured by the interrupt pin and significant motion.

The binding is a good pointer, and I'll continue from there. My current code uses i2c_burst_(read/write) targeting i2c0 and my specific device's address as the content of ST's required platform_read/write methods, and seems to be working for now.

I'll keep updating this as I come to a good solution so that others might find and use this info themselves.

Best,

    - Finn

[FinnBiggs]

So, following up.

It is as you suggest; I would like to directly grab my lsm6ds3tr_c via its node label, and then set up an i2c device instance to read and write from it.

Following the instructions you linked from DevAcademy has caused a problem I've run into a few times now that I'm hoping you may have insight on.

Here is my code and configuration (and I've trimmed it down to be nearly minimal).

==========

Module Code (lsm6dsl.c)

#include <zephyr/kernel.h>

/* IMU imports */
#include <zephyr/device.h>
#include <zephyr/sys/util.h>
#include <zephyr/drivers/sensor.h>
#include <stdio.h>


/* AEM imports */
#include "imu_event.h"
#include <caf/events/module_state_event.h>

#include <zephyr/logging/log.h>

#include <zephyr/drivers/i2c.h>

#define MODULE lsm6dsl 
LOG_MODULE_REGISTER(MODULE);

/* Acquire I2C binding */
#define LSM_I2C_NODE DT_NODELABEL(lsm6ds3tr_c)

static const struct i2c_dt_spec lsm_i2c = I2C_DT_SPEC_GET(LSM_I2C_NODE);

int setup_i2c() {
    int ret;
    ret = device_is_ready( lsm_i2c.bus );
    if( !ret ) {
        LOG_ERR("I2C Bus not ready: %s \n", lsm_i2c.bus);
        return -1;
    }

    uint8_t read_buf = 0x00;
    i2c_read(&lsm_i2c, &read_buf, 1, 0x0f); // Datasheet specifies "WHO_AM_I" as (0Fh), expect 0x6a
    
    LOG_INF("Setup I2C: WHOAMI=0x%x", read_buf);
    
    return ret;
}

static bool app_event_handler(const struct app_event_header *aeh)
{
    if(is_module_state_event(aeh)){
        struct module_state_event *ev = cast_module_state_event(aeh);
        if(check_state(ev, MODULE_ID(main), MODULE_STATE_READY)) {
            int ret = setup_i2c();
            if (ret != 0) {
                printk("Error condn %d in imu setup", ret);
            }
            return false;
        }
    }
    return false;
}

APP_EVENT_LISTENER(MODULE, app_event_handler);
APP_EVENT_SUBSCRIBE(MODULE, module_state_event);

==========

prj.conf

Note: I have a custom KConfig option in here "CONFIG_ENABLE_IMU_LOG" that just configures whether IMU events are printed. (ifdef statements in the log definitions of imu_event) (not relevant to this question)

#
# Copyright (c) 2019 Nordic Semiconductor ASA
#
# SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
#
# Enabling assert
CONFIG_ASSERT=y

# Logger configuration
CONFIG_LOG=y
# CONFIG_LOG_BACKEND_UART=y
CONFIG_USE_SEGGER_RTT=y
CONFIG_LOG_BACKEND_RTT=y

# Configuration required by Application Event Manager
CONFIG_APP_EVENT_MANAGER=y
CONFIG_CAF=y
CONFIG_HEAP_MEM_POOL_SIZE=2048
CONFIG_REBOOT=y

# IMU Configuration
CONFIG_STDOUT_CONSOLE=y

CONFIG_I2C=y
CONFIG_ENABLE_IMU_LOG=n
CONFIG_SPI=n

CONFIG_SENSOR=y
CONFIG_LSM6DSL=y
CONFIG_LSM6DSL_TRIGGER_GLOBAL_THREAD=y

CONFIG_CBPRINTF_FP_SUPPORT=y

==========

nrf52840dk_nrf52840.overlay

i2c0: &i2c0 {
    lsm6ds3tr_c: lsm6ds3tr-c@6a {
		compatible = "i2c-device";
		reg = < 0x6a >;
		irq-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
		status = "okay";
		label = "myLSM";
	};
};

==========

main.c

(Paraphrasing)

app_event_manager_init --> Success --> module_set_state(MODULE_STATE_READY)

==========

annotated error output (serial terminal)

I get "booting" / "main is ready" / "USAGE ERROR"

I took the serial output of one reboot loop, and ran addr2line on each command address in the error trace.The specific (bash) command is below:

addr2line -f -e project-directory/build_dk/zephyr/zephyr.elf <hex command address>

That information is nested below each line, and organized by hex address of calling command.

*** Booting nRF Connect SDK v2.5.0 ***
[00:00:01.750,854] <inf> app_event_manager: e:module_state_event module:main state:READY
[00:00:01.941,955] <err> os: ***** USAGE FAULT *****
[00:00:01.941,986] <err> os:   Attempt to execute undefined instruction
[00:00:01.941,986] <err> os: r0/a1:  0x00000001  r1/a2:  0x20000850  r2/a3:  0x0000fbec

    0x00000001
        i2c_dump_msgs_rw
        C:/ncs/v2.5.0/zephyr/drivers/i2c/i2c_common.c:52

    0x20000850
        $d
        C:/ncs/v2.5.0/zephyr/kernel/system_work_q.c:20

    0x0000fbec
        $d
        C:/ncs/v2.5.0/nrf/include/caf/events/module_state_event.h:300


[00:00:01.942,016] <err> os: r3/a4:  0x0000bf5b r12/ip:  0x00000000 r14/lr:  0x000009e1
    
    0x0000bf5b
        z_arm_nmi
        C:/ncs/v2.5.0/zephyr/arch/arm/core/aarch32/nmi.c:57

    0x00000000
        i2c_dump_msgs_rw
        C:/ncs/v2.5.0/zephyr/drivers/i2c/i2c_common.c:52

    0x000009e1
        setup_i2c
        C:\Users\finnb\Documents\auli\zephyr_experiments\drivers_lsm\build_dk/../src/modules/lsm6dsl.c:103
            if( !ret ) { report error on device logging }

[00:00:01.942,016] <err> os:  xpsr:  0x61000000

    0x61000000
        ??
        ??:0

[00:00:01.942,047] <err> os: Faulting instruction address (r15/pc): 0x000009e8

    0x000009e8
        z_impl_i2c_transfer
        C:/ncs/v2.5.0/zephyr/include/zephyr/drivers/i2c.h:770

[00:00:01.942,077] <err> os: >>> ZEPHYR FATAL ERROR 36: Unknown error on CPU 0
[00:00:01.942,108] <err> os: Current thread: 0x20000850 (unknown)

0x20000850
    $d
    C:/ncs/v2.5.0/zephyr/kernel/system_work_q.c:20

[00:00:03.076,751] <err> fatal_error: Resetting system

==========

How might I correct this?

Note that I did get i2c reading and writing working by referencing the i2c0 node directly, and using i2c burst read/write while specifying the lsm's address in the command e.g.

    i2c_burst_read(i2c_dev, 0x6a, 0x0E, &buf, 1);
    printk("Addr: 0x%02x, Buf: 0x%02x \n", 0x0E, buf);

    uint8_t write_buf = 0x04;
    i2c_burst_write(i2c_dev, 0x6a, 0x0E, &write_buf, 1);

    i2c_burst_read(i2c_dev, 0x6a, 0x0E, &buf, 1);
    printk("Addr: 0x%02x, Buf: 0x%02x \n", 0x0E, buf);

works as expected (where i2c_dev is set up as below, then treated the same as in above module, substituting this device for the lsm_i2c device in my module)

#define I2C_NODE DT_NODELABEL(i2c0)
static struct device const *i2c_dev = DEVICE_DT_GET(I2C_NODE);

==========

Apologies for the long post, I'm trying to learn about the best practice here. As always, I'm grateful for your advice and time.

Best,

    - Finn

[FinnBiggs]

Hi Andreas,

I'm still struggling in a couple places (posts below) - any updates or advice?

Thank you

    - Finn

[AHaug]

Hi Finn, 

Apologies for the long response time. 

I have unfortunately not been able to find any concrete solutions as of now. Initially I thought it could be due to a thread/heap sizes not beeing sufficiently large, but seeing htat none of the faults you check with addr_2_line comes from a stack overflow I doubt this is the issue. You could however try to increase the stack size anyways.

Another thought is that the i2c device is already initialized or that the custom device is slightly different defined w.r.t what the api expects (thus the usage error, just speculation from my part here..)

I will continue looking into this on Monday, apologies for not beeing of more help any sooner

Kind regards,
Andreas