Skip to content
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

manifest: sdk-zephyr: modules: hal_nordic: Add multi-instance DPPI and PPIB drivers #79857 #18073

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

NordicBuilder
Copy link
Contributor

Automatically created by action-manifest-pr GH action from PR: nrfconnect/sdk-zephyr#2143

@NordicBuilder NordicBuilder requested a review from a team as a code owner October 22, 2024 07:02
@github-actions github-actions bot added manifest changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Oct 22, 2024
@NordicBuilder
Copy link
Contributor Author

NordicBuilder commented Oct 22, 2024

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

Name Old Revision New Revision Diff
zephyr nrfconnect/sdk-zephyr@8ceab93 (main) nrfconnect/sdk-zephyr#2143 nrfconnect/sdk-zephyr#2143/files

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

@NordicBuilder
Copy link
Contributor Author

NordicBuilder commented Oct 22, 2024

CI Information

To view the history of this post, clich the 'edited' button above
Build number: 5

Inputs:

Sources:

sdk-nrf: PR head: d3b3e83ec750631d3350c79a2cfce2e3d1ccbb99
zephyr: PR head: 9d0bae6ed294f759cd4783e19093b13958474fcd

more details

sdk-nrf:

PR head: d3b3e83ec750631d3350c79a2cfce2e3d1ccbb99
merge base: 6f5194b6cac97d8bea615bcc0629fd43dfc679d8
target head (main): 6f5194b6cac97d8bea615bcc0629fd43dfc679d8
Diff

zephyr:

PR head: 9d0bae6ed294f759cd4783e19093b13958474fcd
merge base: b063a96d60f4b881114fb99157c7b394daaa4448
target head (main): 8ceab93c866d584201621703dee15a77107e7363
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (49)
applications
│  ├── nrf5340_audio
│  │  ├── src
│  │  │  ├── modules
│  │  │  │  │ audio_sync_timer.c
samples
│  ├── bluetooth
│  │  ├── conn_time_sync
│  │  │  ├── boards
│  │  │  │  ├── nrf5340_audio_dk_nrf5340_cpuapp.conf
│  │  │  │  ├── nrf5340dk_nrf5340_cpuapp.conf
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15pdk_nrf54l15_cpuapp.conf
│  │  ├── direct_test_mode
│  │  │  ├── boards
│  │  │  │  ├── nrf5340dk_nrf5340_cpunet.conf
│  │  │  │  ├── nrf5340dk_nrf5340_cpunet_hci.conf
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15pdk_nrf54l15_cpuapp.conf
│  │  ├── iso_time_sync
│  │  │  ├── boards
│  │  │  │  ├── nrf5340_audio_dk_nrf5340_cpuapp.conf
│  │  │  │  ├── nrf5340dk_nrf5340_cpuapp.conf
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15pdk_nrf54l15_cpuapp.conf
│  ├── peripheral
│  │  ├── 802154_phy_test
│  │  │  ├── boards
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp.conf
│  │  ├── radio_test
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15pdk_nrf54l15_cpuapp.conf
subsys
│  ├── debug
│  │  ├── cpu_load
│  │  │  │ cpu_load.c
│  │  ├── ppi_trace
│  │  │  │ ppi_trace.c
│  ├── dm
│  │  │ dm.c
│  ├── esb
│  │  │ esb_dppi.c
│  ├── mpsl
│  │  ├── fem
│  │  │  ├── common
│  │  │  │  │ mpsl_fem_utils.c
│  │  ├── pin_debug
│  │  │  │ mpsl_pin_debug_radio_core.c
tests
│  ├── subsys
│  │  ├── debug
│  │  │  ├── cpu_load
│  │  │  │  ├── src
│  │  │  │  │  │ test_cpu_load.c
west.yml
zephyr
│  ├── drivers
│  │  ├── audio
│  │  │  ├── Kconfig.dmic_pdm_nrfx
│  │  │  │ dmic_nrfx_pdm.c
│  │  ├── counter
│  │  │  │ counter_nrfx_rtc.c
│  │  ├── serial
│  │  │  ├── Kconfig.nrfx_uart_instance
│  │  │  ├── uart_nrfx_uart.c
│  │  │  │ uart_nrfx_uarte.c
│  ├── dts
│  │  ├── bindings
│  │  │  ├── misc
│  │  │  │  │ nordic,nrf-ppib.yaml
│  │  ├── common
│  │  │  ├── nordic
│  │  │  │  ├── nrf54l15.dtsi
│  │  │  │  │ nrf54l20.dtsi
│  ├── modules
│  │  ├── hal_nordic
│  │  │  ├── nrfx
│  │  │  │  ├── CMakeLists.txt
│  │  │  │  ├── Kconfig
│  │  │  │  ├── Kconfig.logging
│  │  │  │  ├── nrfx_config.h
│  │  │  │  ├── nrfx_config_common.h
│  │  │  │  ├── nrfx_config_nrf5340_application.h
│  │  │  │  ├── nrfx_config_nrf54l15_application.h
│  │  │  │  ├── nrfx_config_nrf54l15_flpr.h
│  │  │  │  ├── nrfx_config_reserved_resources.h
│  │  │  │  │ nrfx_glue.h
│  ├── samples
│  │  ├── drivers
│  │  │  ├── audio
│  │  │  │  ├── dmic
│  │  │  │  │  ├── boards
│  │  │  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp.overlay
│  │  │  │  │  │ sample.yaml
│  ├── soc
│  │  ├── nordic
│  │  │  ├── common
│  │  │  │  │ Kconfig.peripherals
│  │  │  ├── nrf53
│  │  │  │  │ sync_rtc.c
│  │  │  │ validate_base_addresses.c
│  │ west.yml

Outputs:

Toolchain

Version: 3dd8985b56
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:3dd8985b56_912848a074

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ❌ Build twister
    • sdk-nrf test count: 1476
    • sdk-zephyr test count: 6281
  • ❌ Integration tests
    • ❌ test-sdk-audio
    • ❌ test_ble_nrf_config
    • ❌ test-fw-nrfconnect-ble_samples
    • ❌ test-fw-nrfconnect-chip
    • ❌ test-fw-nrfconnect-nfc
    • ❌ test-fw-nrfconnect-nrf-iot_cloud
    • ❌ test-fw-nrfconnect-nrf-iot_thingy91
    • ❌ test-fw-nrfconnect-nrf_crypto
    • ❌ test-fw-nrfconnect-proprietary_esb
    • ❌ test-fw-nrfconnect-rpc
    • ❌ test-fw-nrfconnect-rs
    • ❌ test-fw-nrfconnect-fem
    • ❌ test-fw-nrfconnect-thread
    • ❌ test-fw-nrfconnect-zigbee
    • ❌ test-sdk-find-my
    • ❌ test-fw-nrfconnect-nrf-iot_mosh
    • ❌ test-sdk-sidewalk
    • ❌ test-low-level
    • ❌ test-sdk-dfu
Disabled integration tests
    • desktop52_verification
    • doc-internal
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_mesh
    • test-fw-nrfconnect-boot
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_lwm2m
    • test-fw-nrfconnect-nrf-iot_nrf_provisioning
    • test-fw-nrfconnect-nrf-iot_positioning
    • test-fw-nrfconnect-nrf-iot_samples
    • test-fw-nrfconnect-nrf-iot_serial_lte_modem
    • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • test-fw-nrfconnect-ps
    • test-fw-nrfconnect-tfm
    • test-sdk-mcuboot
    • test-sdk-pmic-samples
    • test-sdk-wifi
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

@e-rk e-rk force-pushed the auto-manifest-sdk-zephyr-2143 branch from 98efc5f to 70653fa Compare October 22, 2024 07:08
@e-rk e-rk requested review from koffes, rick1082, alexsven and gWacey and removed request for a team October 22, 2024 07:08
@e-rk e-rk added this to the 2.8.0 milestone Oct 22, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need to enable all of the DPPIC's and PPIB's here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Without having all PPIB and DPPICs enabled, the nrfx_gppi_dppi_ppib_lumos.c won't build unfortunately. Maybe there could be a separate CONFIG_NRFX_GPPI config that automatically enables those instances on Lumos.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, then we should either use a separate CONFIG_NRFX_GPPI, or add a followup task to remove the ones that are not needed. The commit message should explain the reasoning behind adding all these

@NordicBuilder
Copy link
Contributor Author

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

Comment on lines 19 to 21
#ifdef DPPI_PRESENT
static nrfx_dppi_t dppi = NRFX_DPPI_INSTANCE(0);
#endif
Copy link
Contributor

@ankuns ankuns Oct 22, 2024

Choose a reason for hiding this comment

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

@e-rk The dppi variable is used only as a kind of a "handle" to given resources represented by nrfx_dppi_t.
See https://github.com/e-rk/hal_nordic/blob/d7317a9cd28a845dfd174e14c16ba3def1c02a78/nrfx/drivers/src/nrfx_dppi.c#L527
The nrfx_dppi_channel_alloc has const qualifier for the p_instance, so the dppi variable itself (not referring to resource it "points to") will not change.

nrfx_err_t nrfx_dppi_channel_alloc(nrfx_dppi_t const * p_instance, uint8_t * p_channel)

This is true for all other "nrfx_dppi_xxxxxx" calls.

Could the variable be moved to be inside appropriate function as a local variable?
In this case into mpsl_fem_utils_ppi_channel_alloc so that it is a local non-static variable (on stack).

This could be done for all places you introduce the dppi variable .

Justification:

  • The static nrfx_dppi_t dppi will exist for application lifetime, but the dppi object itself does not change.
  • The initializer NRFX_DPPI_INSTANCE is prepared by preprocessor.
  • There are multiple such variables, each takes space.
  • There is only one underlying allocator per given DPPIC regardless of how many nrfx_dppi_t dppi we have used.
  • They are used rather in non time-critical code, there is no need to have them precalculated.

Updated the hal_nordic revision and adjusted the code base to use
multi-instance DPPI API.
Certain applications have to explicitly enable nrfx_dppi and nrfx_ppib
support with Kconfigs.

Signed-off-by: Rafał Kuźnia <rafal.kuznia@nordicsemi.no>
@e-rk e-rk force-pushed the auto-manifest-sdk-zephyr-2143 branch from 70653fa to 2889c7e Compare October 22, 2024 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. DNM manifest manifest-zephyr
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants