Features/add icm456xx #9
Conversation
* Added ICM45686 in README.md * Update ICM456xx driver (Newport A) version to 2.4.0 icm45605_github package Update inv_imu .h to support different variants * Fix https://invensense.atlassian.net/browse/SWNEW-612 --------- Co-authored-by: JH Kim <jh.kim@tdk.com>
afontaine-invn
requested review from
nashif,
thelinuxfoundation,
stephanosio and
carlescufi
|
Hi @nashif , hi @thelinuxfoundation, hi @stephanosio , hi @carlescufi, do you have any chances to review and add approval? I'm usually maintainer of the tdk_hal so this time I cannot do the review myself. |
| /** @brief Function pointer to read register(s). | ||
| * @param[in] reg Register address to be read. | ||
| * @param[out] buf Output data from the register. | ||
| * @param[in] len Number of byte to be read. | ||
| * @return 0 on success, negative value on error. | ||
| */ | ||
| typedef int (*inv_imu_read_reg_t)(void * context, uint8_t reg, uint8_t *buf, uint32_t len); | ||
|
|
||
| /** @brief Function pointer to write register(s). | ||
| * @param[in] reg Register address to be written. | ||
| * @param[in] buf Input data to write. | ||
| * @param[in] len Number of byte to be written. | ||
| * @return 0 on success, negative value on error. | ||
| */ | ||
| typedef int (*inv_imu_write_reg_t)(void * context, uint8_t reg, const uint8_t *buf, uint32_t len); |
There was a problem hiding this comment.
Same question I think I've left in another PR: how will this work with async transfers? (e.g: Streaming mode). Is there async variant that provides the result through a callback?
There was a problem hiding this comment.
For sure, with the current driver version, async transfer is not supported. We currently use a callback at higher level to be notified in case of FIFO interrupt for example.
But I start to understand that this is a crucial question for next sensor driver generation, so we need to dig into v2 version and think about how to be able to support such case.
There was a problem hiding this comment.
Ok, please keep in mind the upstreamed version of ICM45686 already supports v2: https://github.com/zephyrproject-rtos/zephyr/tree/main/drivers/sensor/tdk/icm45686
There was a problem hiding this comment.
Hi @ubieda , I'm still stuck to merge this. Could you approve it? As said above, the reviewers should be tdk folks but it is zephyr members who are not really available I supposed ... I don't know how to move forward.
|
@afontaine-invn as you can see, my +1 does not count towards the requirements. @rbuisson-invn please review this PR as it seems you have collaborator access to this HAL repo. |
|
Hi @nashif , hi @thelinuxfoundation, hi @stephanosio , hi @carlescufi, could you help to merge this PR as it seems you are the only ones to be able to do so? There is an issue with the PR process here. Do you know who I should ping to get it resolve? |
|
@kartben it seems the TDK team does not have the rights to approve their own HAL PRs. According to zephyrproject-rtos/zephyr#71561 the following users should have R/W access:
Could you help us over here? Thanks! |
|
@afontaine-invn you should be able to merge this now. |
|
Thanks ! |
Support ICM45686 sensor from the icm456xx family
Update README, Cmakelist and drivers folder