Skip to content

Conversation

@pabigot
Copy link
Contributor

@pabigot pabigot commented Oct 15, 2019

Correct IRQ active level to default active-low, switch to new interrupt configuration.

Tested with frdm_k64f and nucleo_l476rg with x-nucleo-iks01a2.

NOTE This is based on #19862, #19866, and #19870 on which it depends. It's DNM solely for those inclusions.

@pabigot pabigot changed the title drivers: sensor: lsm6dsl: update to new GPIO API [TOPIC-GPIO] drivers: sensor: lsm6dsl: update to new GPIO API Oct 15, 2019
@zephyrbot
Copy link

zephyrbot commented Oct 15, 2019

All checks passed.

checkpatch (informational only, not a failure)

-:89: WARNING:LONG_LINE: line over 80 characters
#89: FILE: drivers/sensor/lsm6dsl/lsm6dsl_trigger.c:59:
+	if (gpio_pin_get(drv_data->gpio, DT_INST_0_ST_LSM6DSL_IRQ_GPIOS_PIN) > 0) {

- total: 0 errors, 1 warnings, 140 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

@erwango
Copy link
Member

erwango commented Oct 16, 2019

@pabigot, can't get it work on disco_l475_iot1 (working on master).
Tested on #19579 + current PR.
Not much time right now for analysis, but if that can help, here is a back trcae of the issue:

0x080055d6 in LL_I2C_IsActiveFlag_ARLO (I2Cx=0x40005800) at /local/mcu/zephyr/modules/hal/stm32/stm32cube/stm32l4xx/drivers/include/stm32l4xx_ll_i2c.h:1630
1630	  return ((READ_BIT(I2Cx->ISR, I2C_ISR_ARLO) == (I2C_ISR_ARLO)) ? 1UL : 0UL);
(gdb) bt
#0  0x080055d6 in LL_I2C_IsActiveFlag_ARLO (I2Cx=0x40005800) at /local/mcu/zephyr/modules/hal/stm32/stm32cube/stm32l4xx/drivers/include/stm32l4xx_ll_i2c.h:1630
#1  check_errors (funcname=<optimized out>, dev=<optimized out>) at /local/mcu/zephyr/zephyr-project/drivers/i2c/i2c_ll_stm32_v2.c:488
#2  0x080056f2 in stm32_i2c_msg_read (dev=0x2000170c <__device_i2c_stm32_I2C_2>, msg=0x200014d4 <sys_work_q_stack+916>, next_msg_flags=<optimized out>, slave=<optimized out>)
    at /local/mcu/zephyr/zephyr-project/drivers/i2c/i2c_ll_stm32_v2.c:581
#3  0x080057fe in i2c_stm32_transfer (dev=0x2000170c <__device_i2c_stm32_I2C_2>, msg=<optimized out>, num_msgs=1 '\001', slave=<optimized out>) at /local/mcu/zephyr/zephyr-project/drivers/i2c/i2c_ll_stm32.c:142
#4  0x08005a8c in z_impl_i2c_transfer (addr=106, num_msgs=2 '\002', msgs=0x200014c8 <sys_work_q_stack+904>, dev=<optimized out>) at ../../../../../include/drivers/i2c.h:252
#5  i2c_transfer (addr=106, num_msgs=2 '\002', msgs=0x200014c8 <sys_work_q_stack+904>, dev=<optimized out>) at zephyr/include/generated/syscalls/i2c.h:46
#6  i2c_write_read (num_write=1, num_read=<optimized out>, read_buf=<optimized out>, write_buf=0x200014c7 <sys_work_q_stack+903>, addr=106, dev=<optimized out>) at ../../../../../include/drivers/i2c.h:454
#7  i2c_burst_read (num_bytes=<optimized out>, buf=<optimized out>, start_addr=<optimized out>, dev_addr=106, dev=<optimized out>) at ../../../../../include/drivers/i2c.h:480
#8  lsm6dsl_i2c_read_data (data=<optimized out>, reg_addr=<optimized out>, value=<optimized out>, len=<optimized out>) at /local/mcu/zephyr/zephyr-project/drivers/sensor/lsm6dsl/lsm6dsl_i2c.c:26
#9  0x0800582e in lsm6dsl_sample_fetch_accel (dev=<optimized out>) at /local/mcu/zephyr/zephyr-project/drivers/sensor/lsm6dsl/lsm6dsl.c:321
#10 0x080058d4 in lsm6dsl_sample_fetch (dev=0x2000173c <__device_lsm6dsl>, chan=<optimized out>) at /local/mcu/zephyr/zephyr-project/drivers/sensor/lsm6dsl/lsm6dsl.c:423
#11 0x080017d6 in z_impl_sensor_sample_fetch_chan (type=SENSOR_CHAN_ACCEL_XYZ, dev=0x2000173c <__device_lsm6dsl>) at /local/mcu/zephyr/zephyr-project/include/drivers/sensor.h:439
#12 sensor_sample_fetch_chan (type=SENSOR_CHAN_ACCEL_XYZ, dev=0x2000173c <__device_lsm6dsl>) at zephyr/include/generated/syscalls/sensor.h:59
#13 lsm6dsl_trigger_handler (dev=0x2000173c <__device_lsm6dsl>, trig=<optimized out>) at ../../src/main.c:44
#14 0x08005ac6 in lsm6dsl_thread_cb (arg=<optimized out>) at /local/mcu/zephyr/zephyr-project/drivers/sensor/lsm6dsl/lsm6dsl_trigger.c:85
#15 lsm6dsl_work_cb (work=<optimized out>) at /local/mcu/zephyr/zephyr-project/drivers/sensor/lsm6dsl/lsm6dsl_trigger.c:113
#16 0x08004ce2 in z_work_q_main (work_q_ptr=0x2000037c <k_sys_work_q>, p2=<optimized out>, p3=<optimized out>) at /local/mcu/zephyr/zephyr-project/lib/os/work_q.c:32
#17 0x08004ca2 in z_thread_entry (entry=0x8004cab <z_work_q_main>, p1=<optimized out>, p2=<optimized out>, p3=<optimized out>) at /local/mcu/zephyr/zephyr-project/lib/os/thread_entry.c:29
#18 0x08004ca2 in z_thread_entry (entry=0x8004cab <z_work_q_main>, p1=<optimized out>, p2=<optimized out>, p3=<optimized out>) at /local/mcu/zephyr/zephyr-project/lib/os/thread_entry.c:29
#19 0x08001cb8 in stm32_exti_unset_callback (line=134244516) at /local/mcu/zephyr/zephyr-project/drivers/interrupt_controller/exti_stm32.c:406

@pabigot
Copy link
Contributor Author

pabigot commented Oct 16, 2019

@erwango Sorry, that was a simple error of re-invoking the fetch operation instead of re-enabling the interrupt once the worker completed. Should work now.

@pabigot pabigot requested a review from nashif as a code owner October 16, 2019 17:53
@zephyrbot zephyrbot added the area: Tests Issues related to a particular existing or missing test label Oct 16, 2019
Copy link
Contributor

Choose a reason for hiding this comment

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

I already addressed it in #21203. I think it is the proper place (96b_argonkey changes)

Copy link
Contributor

Choose a reason for hiding this comment

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

But can be done also in this commit honestly.
What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't care where it's done. I'll leave it in here for now, there should be no conflict no matter which gets merged first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@avisconti Do you have objections to this PR other than that it updates the devicetree binding that it depends on? I don't think I can remove that, unless #21203 is merged first and then this is rebased on it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@avisconti Do you have objections to this PR other than that it updates the devicetree binding that it depends on? I don't think I can remove that, unless #21203 is merged first and then this is rebased on it.

Sure.

Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

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

doc change LGTM

@pabigot pabigot removed the DNM This PR should not be merged (Do Not Merge) label Dec 13, 2019
Copy link
Contributor

@avisconti avisconti left a comment

Choose a reason for hiding this comment

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

No, it is good!

Copy link
Member

@MaureenHelm MaureenHelm left a comment

Choose a reason for hiding this comment

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

Please update the commit message to say active high rather than active low.

Correct IRQ active level to default active-high, switch to new
interrupt configuration.

Signed-off-by: Peter Bigot <[email protected]>
@pabigot
Copy link
Contributor Author

pabigot commented Dec 13, 2019

Please update the commit message to say active high rather than active low.

Done.

@carlescufi carlescufi merged commit 226a9f2 into zephyrproject-rtos:topic-gpio Dec 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Boards area: Devicetree area: GPIO 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