drivers: sensor: icm42670: supports icm42670-P and icm42670-S

[afontaine-invn]

dts: supports icm42670-P/-S
samples: sensor: tdk_apex: TDK APEX generic sample
samples: sensor: 6DOF motion DRDY generic sample
module: HAL TDK module

Adds official TDK Invensense Inc. driver in TDK HAL module for icm42670-P/-S sensor.
Adds SPI and I2C interface support.
Adds APEX features, such as Pedometer, Tilt detection, Wake on Motion
and Significant Motion Detector.
Add generic samples supporting this driver.
Integration module test successful with https://github.com/tdk-invn-oss/zephyr.hal_tdk
Validated with custom setup: nrf52dk_nrf52832 + icm42670-P/-S EVB board
Build ok by running:
west twister -T samples\sensor\tdk_apex-T samples\sensor\6dof_motion_drdy -p nrf52dk/nrf52832

Signed-off-by: Aurelie Fontaine aurelie.fontaine@tdk.com

[github-actions]

Hello @afontaine-invn, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

[gmarull]

please, check Zephyr contribution guidelines. There seem to be PDFs and binary files in this PR.

[teburd]

An initial pass here (and this is large so I'll need time to look at it again) with some high level notable comments but...

I love that this is being submitted as a PR from you, with a TDK email address. I hope this is just the first of many to be frank.

That said there's some issues that will need to be addressed. Blobs are problematic, pdf's are problematic, rewriting the register map but not using the zephyr macros for describing masks and manipulating fields seems like a step backwards. Using the existing trigger API requiring a thread or work queue also feels like a step backwards, but that's not on you.

Is the code under the imu directory here something from a TDK software devkit? If so it might be worth considering looking at how ST packages and uses stmemsc for ST sensor drivers. It does not need to live or be managed then by Zephyr but can be managed entirely but TDK in something like a hal_tdk git repository.

[carlescufi]

Thanks for the PR! Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request.

[afontaine-invn]

Hi @teburd, @carlescufi, thank you very much for your review! You point many subjects to address, Find my answers below:

• Regarding PDF and blobs I'll remove it for now. We though exposing the AML library (featuring pointing, gesture recognition, ...) but it seems not feasible and compatible with Zephyr rules.

• Regarding the trigger API requiring a thread or work queue I kept it configurable by KConfig and I kept the option to not configure any trigger. I can put it as default.

• Regarding the possibility to use a hal_tdk git repository to put the imu driver directory, we look at how st handles it with @gjabouley-invn and we'll like the idea. Could you give us more details on how to put it in place? And if there is any rules or guidelines to manage it over the time?

• And just as you know I'm not the only one to push with a TDK address lately, @rbuisson-invn also updated ICP drivers with official ones. Find the PR driver: sensor: icp101xx: adds driver and sample #69282

[decsny]

any DT binding property with a default value needs justification of the default value in the description https://docs.zephyrproject.org/latest/build/dts/bindings-upstream.html

[afontaine-invn]

I updated the DT bindings with default description as required.

[decsny]

missing justification

[afontaine-invn]

Thanks for the comment. I added the default value justification for this property also.

[afontaine-invn]

I opened an issue to create the tdk_hal repository #71561

[teburd]

Sensor WG 04/22/2024

Asked the question around pros/cons of using sensor vendor supplied HALs:

Pros:

• Potentially more vendor involvement, with more drivers being pushed to the tree

Questions Posed (Potential Cons):

• Unclear how much benefit the HAL provides, e.g. abstracting bus reads/writes is already doable within Zephyr today.
• Unclear how the maintainer burden increases when using HALs, who maintains this code long term and moves it forward as sensor APIs evolve and change.
• Unclear how we move this code to the asynchronous sensor_read API which is the future of sensors to better support more use cases

addressed

[afontaine-invn]

Hi @teburd ,
Thanks for the updates on the tdk_hal repository issue.
Are there any blocking points or any changes expected to move forward with the PR?

[teburd]

Hi @teburd , Thanks for the updates on the tdk_hal repository issue. Are there any blocking points or any changes expected to move forward with the PR?

No blocking issues from me no, I expect the TSC will likely approve hal_tdk barring any major concerns, and then this can go in with the tdk hal code removed. I'd expect some more progress in a week or so as the TSC meets weekly, and our sensor maintainer (Maureen Helm at Analog) is traveling this week

[sriccardi-invn]

HI @MaureenHelm,
Any update on the MR ?
The branch is opened since a lot of time with different reviewers. My team consume is time to rebase everytime.
Do you think we could converge?
We have already a lot of sensor drivers ready to be integrated in tdk_hal.
Thanks.
Regards,
Sebastien Riccardi

[afontaine-invn]

Hello @MaureenHelm , Thanks for the approval, we're happy to achieve this step!
I rebased this PR and resolved the conflict in icm42670.c file. Could you approve again and remove the DNM and dev-review label in order to be able to merge?

[sriccardi-invn]

HI @MaureenHelm ,
I am affraid by the delay to merge a branch.
Everytime we wait, some change have beed done an another side, we have ro rebase, wait a new validation.
This is infinite.
We have a lot of sensors driver ready to be integrated waiting this first merge.
Do you have or see any remaining issue on your side?
Thanks.
Regards,
Sebastien Riccardi

[kartben]

Thank you for your patience @afontaine-invn @sriccardi-invn and team, and sorry this took so long. This will go in with the next merge batch in just a couple hours.

[github-actions]

Hi @afontaine-invn!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

[pdgendt]

Every build now prints:

CMake Warning at /home/pdgendt/zephyrproject/zephyr/CMakeLists.txt:996 (message):
  No SOURCES given to Zephyr library: ..__modules__hal__tdk

  Excluding target from build.

[pdgendt]

Created #83151 to get rid of the warning.