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

[nRF] The nRF boards should not enable networking stacks #5454

Closed
SebastianBoe opened this issue Dec 20, 2017 · 7 comments
Closed

[nRF] The nRF boards should not enable networking stacks #5454

SebastianBoe opened this issue Dec 20, 2017 · 7 comments
Labels
area: Boards Enhancement Changes/Updates/Additions to existing features

Comments

@SebastianBoe
Copy link
Collaborator

It is IMHO undesired behaviour that nrf52840_pca10056_defconfig and friends are enabling SW components.

Especially big SW components like BT.

HW Kconfig should be describing the available HW, not making assumptions about what SW is going to be used.

Networking stacks should be pulled in directly or indirectly because they are needed by applications.

@SebastianBoe SebastianBoe added area: Boards Enhancement Changes/Updates/Additions to existing features labels Dec 20, 2017
@SebastianBoe
Copy link
Collaborator Author

Kconfiglib was recently merged and it enabled warnings when configs were redundant, previously these warnings defaulted to being suppressed.

Building the Bluetooth beacon sample for nrf52_pca10040 now shows the warning:

/home/sebo/zephyr/samples/bluetooth/beacon/prj.conf:1: warning: BT set more than once. Old value: "y", new value: "y".

sebo@mach:~/zephyr/samples/bluetooth/beacon$ cb
-- Found PythonInterp: /usr/bin/python3 (found suitable version "3.5.2", minimum required is "3.4") 
-- Selected BOARD nrf52_pca10040
Zephyr version: 1.10.99
/home/sebo/zephyr/samples/bluetooth/beacon/prj.conf:1: warning: BT set more than once. Old value: "y", new value: "y".

We are redundantly setting this config because both the sample and the board is enabling BT. So fixing this issue would also fix the warning.

@pfalcon
Copy link
Contributor

pfalcon commented Jan 12, 2018

A quick note that this doesn't have to be an "issue" which needs to be fixed. It may be that such values should be logically unified, and no warning should be produced.

@ulfalizer
Copy link
Collaborator

At the moment, Kconfiglib will always warn if a value is set more than once, even if the old and new values match. This is just to flag redundant entries. I could work something out if it isn't desired.

@SebastianBoe
Copy link
Collaborator Author

A quick note that this doesn't have to be an "issue" which needs to be fixed. It may be that such values should be logically unified, and no warning should be produced.

I agree, not all warnings of this kind are "issues", but in this particular case IMHO it is.

@nashif
Copy link
Member

nashif commented Jan 24, 2018

isnt CONFIG_BT here for the controller and radio which is considered a HW component, if you apply this rule globally, then we also should not be enabling UART and GPIO and and...

@SebastianBoe
Copy link
Collaborator Author

I can't speak to the original intention.

The radio is a HW component.
The controller is a SW component that runs on top of the radio.

I am OK with board's declaring that CONFIG_HAS_BT_RADIO or something similiair if needed.

I have not looked into UART or GPIO. But there might be some exceptions to the rule when
the impact is less severe.

@SebastianBoe
Copy link
Collaborator Author

SebastianBoe commented Jan 25, 2018

Another example of this in the wild: SebastianBoe/mcuboot@f4d0e1a

carlescufi added a commit to carlescufi/zephyr that referenced this issue Feb 7, 2019
Instead of enabling Bluetooth by default on nRF5x boards, only enable
the controller if Bluetooth has been enabled by the applicaiton.

Fixes zephyrproject-rtos#5454
Fixes zephyrproject-rtos#12215

Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
nashif pushed a commit that referenced this issue Feb 7, 2019
Instead of enabling Bluetooth by default on nRF5x boards, only enable
the controller if Bluetooth has been enabled by the applicaiton.

Fixes #5454
Fixes #12215

Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Boards Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

No branches or pull requests

4 participants