Skip to content

Conversation

@sean-madigan
Copy link
Contributor

This causes issues as we still want the net core to be set up on initialisation if CONFIG_BT is enabled in many cases.

The previous changes in
#74304 assumed that because this is also handled in
bt_hci_transport_setup that it shouldn't be done on initialisation too.

Also, I think that
bt_hci_transport_setup should not be even controlling the network core state like it is currently and should be looked at being changed in a future PR.

@zephyrbot zephyrbot added the platform: nRF Nordic nRFx label Oct 15, 2024
@sean-madigan sean-madigan force-pushed the initialise_net_core_with_bt branch from e28fa9c to 10f4857 Compare October 15, 2024 15:22
sean-madigan added a commit to sean-madigan/sdk-zephyr that referenced this pull request Oct 15, 2024
… depend on !BT

This causes issues as we still want the net core to be set
up on initialisation if CONFIG_BT is enabled in many cases.

The previous changes in
zephyrproject-rtos/zephyr#74304
assumed that because this is also handled in
`bt_hci_transport_setup` that it shouldn't be done on
initialisation too.

Also, I think that
`bt_hci_transport_setup` should not be even controlling the
network core state like it is currently and should be
looked at being changed in a future PR.

Upstream PR: zephyrproject-rtos/zephyr#79875

Signed-off-by: Sean Madigan <sean.madigan@nordicsemi.no>
@cvinayak
Copy link
Contributor

Also, I think that
bt_hci_transport_setup should not be even controlling the network core state like it is currently and should be looked at being changed in a future PR.

Please create a GH issue to follow up, so that this request is not lost in the future... or do you already have PR you can link here?

Copy link
Contributor

Choose a reason for hiding this comment

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

this is deprecated, why is it being changed?

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'm trying to restore it's functionality to the commit before it was deprecated

Copy link
Member

Choose a reason for hiding this comment

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

are you aware that this will be removed at some point because it is deprecated? Does all code ported to the new order work without any changes?

Copy link
Contributor

Choose a reason for hiding this comment

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

This kconfig is not used anywhere. So it is fine we change it here or leave it.

anangl
anangl previously approved these changes Oct 21, 2024
Copy link
Member

Choose a reason for hiding this comment

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

this should not be needed: BT already enables cpunet core. Any out of tree code should be adapted accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An example where we require 53 cpunet on boot with bluetooth - a multiprotocol application where I want to run another protocol on the cpunet before enabling bluetooth.

Also I do not believe bluetooth driver open/close should control cpunet state and if either of these should change it should be removed from this, rather than on boot. (However, this will require more work and for now I suggest keeping it how it was, with boot on init and on bt driver open)

Copy link
Member

Choose a reason for hiding this comment

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

then, why doesn't this code request cpunet as BT code does? As I understand it, cpunet just needs to be booted once by the first requester, turned off by the last releaser. This is why the soc API is ref counted.

Copy link
Member

Choose a reason for hiding this comment

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

also, what will happen once this deprecated option is removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SOC_NRF53_CPUNET_ENABLE is not being deprecated is it, so this can be used

Copy link
Contributor

Choose a reason for hiding this comment

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

@gmarull can we proceed this PR? waiting for it integrated to 2.8

The previous changes in
zephyrproject-rtos#74304
assumed that because this is also handled in
`bt_hci_transport_setup` that it shouldn't be done on
initialisation too.

However, if someone wants to develop their own app which
uses BT and also wants to enable the CPUNET by default this
KConfig should be available to them.

Signed-off-by: Sean Madigan <sean.madigan@nordicsemi.no>
@sean-madigan sean-madigan force-pushed the initialise_net_core_with_bt branch from 10f4857 to d24c9db Compare October 23, 2024 10:07
sean-madigan added a commit to sean-madigan/sdk-zephyr that referenced this pull request Oct 23, 2024
… depend on !BT

The previous changes in
zephyrproject-rtos/zephyr#74304
assumed that because this is also handled in
`bt_hci_transport_setup` that it shouldn't be done on
initialisation too.

However, if someone wants to develop their own app which
uses BT and also wants to enable the CPUNET by default this
KConfig should be available to them.

Upstream PR: zephyrproject-rtos/zephyr#79875

Signed-off-by: Sean Madigan <sean.madigan@nordicsemi.no>
sean-madigan added a commit to sean-madigan/sdk-zephyr that referenced this pull request Oct 23, 2024
… depend on !BT

The previous changes in
zephyrproject-rtos/zephyr#74304
assumed that because this is also handled in
`bt_hci_transport_setup` that it shouldn't be done on
initialisation too.

However, if someone wants to develop their own app which
uses BT and also wants to enable the CPUNET by default this
KConfig should be available to them.

Upstream PR: zephyrproject-rtos/zephyr#79875

Signed-off-by: Sean Madigan <sean.madigan@nordicsemi.no>
sean-madigan added a commit to sean-madigan/sdk-zephyr that referenced this pull request Oct 23, 2024
… depend on !BT

The previous changes in
zephyrproject-rtos/zephyr#74304
assumed that because this is also handled in
`bt_hci_transport_setup` that it shouldn't be done on
initialisation too.

However, if someone wants to develop their own app which
uses BT and also wants to enable the CPUNET by default this
KConfig should be available to them.

Upstream PR: zephyrproject-rtos/zephyr#79875

Signed-off-by: Sean Madigan <sean.madigan@nordicsemi.no>
@MaureenHelm MaureenHelm merged commit e4fa386 into zephyrproject-rtos:main Oct 23, 2024
23 checks passed
sean-madigan added a commit to sean-madigan/sdk-zephyr that referenced this pull request Nov 4, 2024
… depend on !BT

The previous changes in
zephyrproject-rtos/zephyr#74304
assumed that because this is also handled in
`bt_hci_transport_setup` that it shouldn't be done on
initialisation too.

However, if someone wants to develop their own app which
uses BT and also wants to enable the CPUNET by default this
KConfig should be available to them.

Upstream PR: zephyrproject-rtos/zephyr#79875

Signed-off-by: Sean Madigan <sean.madigan@nordicsemi.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

platform: nRF Nordic nRFx

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants