Skip to content

modules: chre #41735

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 18, 2022
Merged

modules: chre #41735

merged 2 commits into from
Apr 18, 2022

Conversation

yperess
Copy link
Collaborator

@yperess yperess commented Jan 12, 2022

Add initial build rules for CHRE. This change includes a Kconfig and
CMakeLists.txt to begin compiling code from the CHRE module.
Additional files are included to bridge the APIs from CHRE's to
Zephyr's. These can be found in modules/chre/include and
modules/chre/src.

Additionally, add a sample to make sure that the module builds. It can
be built via:

$ west build -b native_posix -p=always samples/application_development/chre

Issue #37223
Signed-off-by: Yuval Peress [email protected]

Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution.

A couple of blockers that must be addressed:

@yperess
Copy link
Collaborator Author

yperess commented Jan 14, 2022

OK, I've update the example in this PR. I'm working with the CHRE team to see if they're OK owning the build files. I think that will be the best approach. The new sample actually runs the full CHRE, loads a nanoapp, sends an event, etc.

The goal is to have everything under modules/chre/include and modules/chre/src actually live upstream so please ignore any style issues

@yperess
Copy link
Collaborator Author

yperess commented Jan 14, 2022

Marking as WIP as I'm moving the build files over to AOSP. When done this PR will only be the added module and the sample app. At that point I'll remove the WIP label.

@yperess yperess marked this pull request as draft January 14, 2022 04:55
@github-actions
Copy link

github-actions bot commented Jan 18, 2022

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
chre N/A zephyrproject-rtos/chre@0edfe2c (zephyr) N/A

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@yperess
Copy link
Collaborator Author

yperess commented Jan 18, 2022

PR updated now that AOSP is willing to own all the build rules for Zephyr as well as adding this to their CI. To verify this PR please also cherry-pick the 2 changes at https://android-review.googlesource.com/c/platform/system/chre/+/1952616 (@MaureenHelm this is why this PR wasn't building for you, you needed the upstream change to fix the attribute order)

@yperess
Copy link
Collaborator Author

yperess commented Feb 1, 2022

I'd like to remind anyone interested in this PR to please review the change in https://android-review.googlesource.com/c/platform/system/chre/+/1952616/

@yperess yperess marked this pull request as ready for review March 8, 2022 06:28
@yperess yperess force-pushed the chre branch 3 times, most recently from 7fe5fa4 to f586cb7 Compare March 8, 2022 07:26
@yperess yperess requested a review from MaureenHelm March 8, 2022 07:33
@yperess yperess requested review from carlescufi and galak as code owners April 4, 2022 17:21
@github-actions github-actions bot added the area: API Changes to public APIs label Apr 4, 2022
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.

I got a new linker warning when building for frdm_k64f:

$ west build -p auto -b frdm_k64f samples/modules/chre/ -- -DCONFIG_NEWLIB_LIBC=y
<snip>
[164/174] Linking CXX executable zephyr/zephyr_pre0.elf
/home/mhelm/.local/zephyr-sdk-0.13.1/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/10.3.0/../../../../arm-zephyr-eabi/bin/ld.bfd: warning: orphan section `.unstable_id' from `modules/chre/lib..__modules__lib__chre__platform__zephyr.a(version.cc.obj)' being placed in section `.unstable_id'

[168/174] Linking CXX executable zephyr/zephyr_pre1.elf
/home/mhelm/.local/zephyr-sdk-0.13.1/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/10.3.0/../../../../arm-zephyr-eabi/bin/ld.bfd: warning: orphan section `.unstable_id' from `modules/chre/lib..__modules__lib__chre__platform__zephyr.a(version.cc.obj)' being placed in section `.unstable_id'

[174/174] Linking CXX executable zephyr/zephyr.elf
/home/mhelm/.local/zephyr-sdk-0.13.1/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/10.3.0/../../../../arm-zephyr-eabi/bin/ld.bfd: warning: orphan section `.unstable_id' from `modules/chre/lib..__modules__lib__chre__platform__zephyr.a(version.cc.obj)' being placed in section `.unstable_id'
Memory region         Used Size  Region Size  %age Used
           FLASH:       33454 B         1 MB      3.19%
            SRAM:       17088 B       192 KB      8.69%
           SRAML:          0 GB        64 KB      0.00%
        IDT_LIST:          0 GB         2 KB      0.00%

@yperess yperess requested review from MaureenHelm and tejlmand April 5, 2022 02:39
@yperess
Copy link
Collaborator Author

yperess commented Apr 5, 2022

I got a new linker warning when building for frdm_k64f:

$ west build -p auto -b frdm_k64f samples/modules/chre/ -- -DCONFIG_NEWLIB_LIBC=y
<snip>
[164/174] Linking CXX executable zephyr/zephyr_pre0.elf
/home/mhelm/.local/zephyr-sdk-0.13.1/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/10.3.0/../../../../arm-zephyr-eabi/bin/ld.bfd: warning: orphan section `.unstable_id' from `modules/chre/lib..__modules__lib__chre__platform__zephyr.a(version.cc.obj)' being placed in section `.unstable_id'

[168/174] Linking CXX executable zephyr/zephyr_pre1.elf
/home/mhelm/.local/zephyr-sdk-0.13.1/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/10.3.0/../../../../arm-zephyr-eabi/bin/ld.bfd: warning: orphan section `.unstable_id' from `modules/chre/lib..__modules__lib__chre__platform__zephyr.a(version.cc.obj)' being placed in section `.unstable_id'

[174/174] Linking CXX executable zephyr/zephyr.elf
/home/mhelm/.local/zephyr-sdk-0.13.1/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/10.3.0/../../../../arm-zephyr-eabi/bin/ld.bfd: warning: orphan section `.unstable_id' from `modules/chre/lib..__modules__lib__chre__platform__zephyr.a(version.cc.obj)' being placed in section `.unstable_id'
Memory region         Used Size  Region Size  %age Used
           FLASH:       33454 B         1 MB      3.19%
            SRAM:       17088 B       192 KB      8.69%
           SRAML:          0 GB        64 KB      0.00%
        IDT_LIST:          0 GB         2 KB      0.00%

@MaureenHelm I uploaded https://android-review.googlesource.com/c/platform/system/chre/+/2055138 which seems to fix it. I'm not 100% familiar with how the linker works on Zephyr so I would appreciate a review from someone on the Zephyr team. I got a review from Jack on it and approval from the Android team. Once it merges I'll update the chre module and let you know.

@yperess
Copy link
Collaborator Author

yperess commented Apr 6, 2022

@MaureenHelm @nashif the upstream change landed in AOSP for supporting the linker section, but I seem to have lost permissions to https://github.com/zephyrproject-rtos/chre so I can't update the module.

@yperess
Copy link
Collaborator Author

yperess commented Apr 6, 2022

Need to use zephyrproject-rtos/chre#1 to get rid of the warnings

Both the `_cpu_arch` and `k_thread_runtime_stats` structs can have a
size of 0 in C, but will fail when building with C++. Add an extra
byte in those cases.

Signed-off-by: Yuval Peress <[email protected]>
@zephyrbot zephyrbot added the DNM This PR should not be merged (Do Not Merge) label Apr 11, 2022
@yperess yperess requested a review from stephanosio April 12, 2022 04:08
MaureenHelm
MaureenHelm previously approved these changes Apr 12, 2022
Add initial build rules for CHRE. This change includes a Kconfig and
CMakeLists.txt to begin compiling code from the CHRE module.
Additional files are included to bridge the APIs from CHRE's to
Zephyr's. These can be found in modules/chre/include and
modules/chre/src.

Additionally, add a sample to make sure that the module builds. It can
be built via:

```
$ west build -b native_posix -p=always samples/application_development/chre
```

Signed-off-by: Yuval Peress <[email protected]>
@zephyrbot zephyrbot removed the DNM This PR should not be merged (Do Not Merge) label Apr 12, 2022
@yperess
Copy link
Collaborator Author

yperess commented Apr 12, 2022

@MaureenHelm I update the module commit hash from the pull

@yperess yperess requested a review from cfriedt April 14, 2022 04:51
Copy link
Member

@cfriedt cfriedt left a comment

Choose a reason for hiding this comment

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

Nice! LGTM👍

@MaureenHelm MaureenHelm dismissed tejlmand’s stale review April 18, 2022 18:20

Requested changes were made

@MaureenHelm MaureenHelm merged commit aaf0c6e into zephyrproject-rtos:main Apr 18, 2022
@yperess yperess deleted the chre branch December 6, 2022 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants