zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.87k stars 6.63k forks source link

i2c_nrfx_twim: Error 0x0BAE0002 if sensor is set in trigger mode and reset with nrf device #38902

Closed kclauah closed 3 years ago

kclauah commented 3 years ago

Inserted by avisconti

Explanation of the issue described below, for the benefit of future observations and questions. On LSM6DSO sensor the INT1 pin is used for both generating the drdy interrupt and for switching to I3C hotjoin mode just after reset if it is at logical '1' level.

If triggers are enabled and drdy are routed on INT1, it might happens that, after a board reset, the logical level '1' generated by the DRDY interrupt gets preserved (e.g. due to the presence of a level shifter) on the next boot forcing the LSM6DSO to enter erroneously in I3C mode. When this scenario happens, I2C can no longer communicate with the device.

Fixed by #39181


Describe the bug This issue happens if the program reset while the sensor is in trigger mode. Tested on nf52dk_nrf52832 and sparkfun thing plus nrf9160

To Reproduce Steps to reproduce the behavior:

  1. Add nrf52dk_nrf52832.overlay in project
    arduino_i2c: &i2c0 {
    zephyr,concat-buf-size = <8>;
    lsm6dso@6a {
        compatible = "st,lsm6dso";
        label = "LSM6DSO";
        reg = <0x6a>;
        irq-gpios = <&gpio0 11 GPIO_ACTIVE_HIGH>;
        int-pin = <1>;
    };
    };
  2. Run the example on nrf52dk_nrf52832 with trigger mode.
  3. Data successfully received from fetch_and_display function
  4. Reset the device

Expected behavior

[00:00:00.201,446] [0m<inf> LSM6DSO: Initialize device LSM6DSO[0m
[00:00:00.202,880] [0m<inf> LSM6DSO: chip id 0x6c[0m
[00:00:02.704,498] [0m<inf> LSM6DSO: lsm6dso_reset_set[0m
[00:00:02.704,803] [1;31m<err> i2c_nrfx_twim: Error 0x0BAE0002 occurred for message 1[0m
[00:00:02.704,803] [0m<dbg> LSM6DSO.lsm6dso_init_chip: reset ret:-5[0m
[00:00:02.704,803] [0m<dbg> LSM6DSO.lsm6dso_init: failed to initialize chip[0m

where 0x0BAE0002 means NRFX_ERROR_DRV_TWI_ERR_DNACK = (NRFX_ERROR_DRIVERS_BASE_NUM + 2) ///< TWI error: Data not acknowledged.

Logs and console output 20210928_121614

Impact Need to reset the device if device not found but it is not a preferred option. Example: if (dev==NULL) sys_reboot(0);

Environment (please complete the following information):

cfriedt commented 3 years ago

@kclauah - do you have a fix for this? @avisconti - do you have some suggestions for this sensor?

kclauah commented 3 years ago

My approach is to stop sensor interrupt before system reboot by disabling sensor interrupt bit or setting sample frequency to zero. However, this method cannot handle unexpected system reboot.

        struct sensor_trigger trig;
        trig.type = SENSOR_TRIG_DATA_READY;
        trig.chan = SENSOR_CHAN_ACCEL_XYZ;
        if (sensor_trigger_set(dev, &trig, NULL) != 0) {
            printf("Could not set sensor type and channel\n");
            return;
        }
    /* set accel/gyro sampling frequency to 0 Hz */
    odr_attr.val1 = 0;
    odr_attr.val2 = 0;

    ret = sensor_attr_set(dev, SENSOR_CHAN_ACCEL_XYZ,
            SENSOR_ATTR_SAMPLING_FREQUENCY, &odr_attr);
    if (ret != 0) {
        printf("Cannot set sampling frequency for accelerometer.\n");
        return ret;
    }

    ret = sensor_attr_set(dev, SENSOR_CHAN_GYRO_XYZ,
            SENSOR_ATTR_SAMPLING_FREQUENCY, &odr_attr);
    if (ret != 0) {
        printf("Cannot set sampling frequency for gyro.\n");
        return ret;
    }
avisconti commented 3 years ago

@cfriedt @kclauah I guess this issue might be same as this one #20933 (and a workaround for x-nucleo-iks01a3 shield here #20913). If so, it's a known issue.

Can you try to generate interrupt on INT2 instead of INT1?

kclauah commented 3 years ago

If so, it's a known issue. Can you try to generate interrupt on INT2 instead of INT1?

Generate DRDY interrupt on INT2 solves the problem.

from #20913: This commit switch to INT2 to generate DRDY interrupt, so that INT1 always remain to logical '0' level.

Is this a hardware issue or register issue? I don't know if it is polite to ask here because I have just started working on a project of a wearable device that last at least a week. Originally we have designed one interrupt pin is used as wake up source(embedded wake up function) and I2C driver is disabled until a wake up interrupt. The other as data interrupt(LSM6DSO and external sensor connected through sensor hub). If this is a register issue which can be fixed by firmware update, I can go ahead without any modification. Otherwise how can I distinguish between external sensor data interrupt, wake up and data ready interrupt with minimum power consumption?

  1. Modify the design to fit the requirement of the sensor if possible.(which INT1 need to be disconnected when reset.)
  2. Choose another ST sensor which do not have this problem.
avisconti commented 3 years ago

If so, it's a known issue. Can you try to generate interrupt on INT2 instead of INT1?

Oh, good!

Generate DRDY interrupt on INT2 solves the problem.

from #20913: This commit switch to INT2 to generate DRDY interrupt, so that INT1 always remain to logical '0' level.

Is this a hardware issue or register issue? I don't know if it is polite to ask here because I have just started working on a project of a wearable device that last at least a week. Originally we have designed one interrupt pin is used as wake up source(embedded wake up function) and I2C driver is disabled until a wake up interrupt. The other as data interrupt(LSM6DSO and external sensor connected through sensor hub). If this is a register issue which can be fixed by firmware update, I can go ahead without any modification.

Well, it is not a register issue, but more an electrical issue. When INT1 is selected to generate interrupts, for example for DRDY, then if you reset the device it may boot with a logical '1' on INT1 (for example in the presence of level shifter that retain the '1') and for LSMDSO having '1' on INT1 in boot phase means switch to I3C.

Otherwise how can I distinguish between external sensor data interrupt, wake up and data ready interrupt with minimum power consumption?

  1. Modify the design to fit the requirement of the sensor if possible.(which INT1 need to be disconnected when reset.)

I think that it might be solved even with a pull-down resistor on INT1. So, you might keep DRDY on INT1 and other interrupts on INT2. And you may also want to try to set I3C_disable bit in the registers, even if this bit gets cleared if you reset. Can you try with pull-down?

  1. Choose another ST sensor which do not have this problem.

I'm checking if other recent ST IMUs do not have the I3C hot-join. I'll come back to you.

avisconti commented 3 years ago
  1. Choose another ST sensor which do not have this problem.

I'm checking if other recent ST IMUs do not have the I3C hot-join. I'll come back to you.

@kclauah The ST sensor LSM6DSV16X is a IMU where I3C hot-join is not present, at least using '1' on INT1 after reset

kclauah commented 3 years ago

I am so very grateful for your time.

And you may also want to try to set I3C_disable bit in the registers, even if this bit gets cleared if you reset.

Setting I3C disable register works like a charm and this should be the model answer. Here is the code.

    if (lsm6dso_device_id_get(ctx, &chip_id) < 0) {
        LOG_DBG("Failed reading chip id");
        return -EIO;
    }

    LOG_INF("chip id 0x%x", chip_id);

    if (chip_id != LSM6DSO_ID) {
        LOG_DBG("Invalid chip id 0x%x", chip_id);
        return -EIO;
    }

        // Set i3c_disable to LSM6DSO_I3C_DISABLE
        if (lsm6dso_i3c_disable_set(ctx, 128)<0){
            LOG_DBG("Failed i3c_disable_set");
            return -EIO;
        }

        /* reset device */
    if (lsm6dso_reset_set(ctx, 1) < 0) {
        return -EIO;
    }
kclauah commented 3 years ago

I think that it might be solved even with a pull-down resistor on INT1... Can you try with pull-down?

I have tried a 100k pull down resistor on INT1 but with no luck.

The ST sensor LSM6DSV16X is a IMU where I3C hot-join is not present, at least using '1' on INT1 after reset

I think this IC is an OEM-only product because I cannot find any datasheet on Google

avisconti commented 3 years ago

I think that it might be solved even with a pull-down resistor on INT1... Can you try with pull-down?

I have tried a 100k pull down resistor on INT1 but with no luck.

Can you try 10k?

The ST sensor LSM6DSV16X is a IMU where I3C hot-join is not present, at least using '1' on INT1 after reset

I think this IC is an OEM-only product because I cannot find any datasheet on Google

It is for mass market indeed, but it is not released yet. Sorry for confusion.

avisconti commented 3 years ago

And you may also want to try to set I3C_disable bit in the registers, even if this bit gets cleared if you reset.

Setting I3C disable register works like a charm and this should be the model answer. Here is the code.

Just for my understanding, the I3C_DISABLE set to '1' works even w/o the pull-down?

kclauah commented 3 years ago

Can you try 10k?

10k make no difference to the error.

Just for my understanding, the I3C_DISABLE set to '1' works even w/o the pull-down?

This is correct. Without pull-down resistor.

carlescufi commented 3 years ago

Assigned to @avisconti since this does not seem related to the nRF i2c driver at all.

avisconti commented 3 years ago

@kclauah Could you please check and test #39181? It is basically same as you did, but just in case... I appreciate your help.

kclauah commented 3 years ago

Works on nrf52dk_nrf52832. No error message with button reset and soft reset sys_reboot(0).