Bluetooth: controller: Make ULL/LLL split the default #15781

cvinayak · 2019-05-01T07:14:24Z

Make the Upper and Lower Link Layer split architecture
implementation of the controller as the default when
building Zephyr Bluetooth Low Energy controller support
in applications.

Noticeable missing feature (porting) in comparison to old
architecture implementation is, Advanced scheduling of
connection events.

The missing features will subsequently be submitted
upstream.

Signed-off-by: Vinayak Kariappa Chettimada vich@nordicsemi.no

Fixes #12681
Depends on #16691

zephyrbot · 2019-05-01T10:42:03Z

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

jhedberg · 2019-05-02T08:28:41Z

subsys/bluetooth/controller/ll_sw/ll.c

@@ -175,7 +175,10 @@ int ll_init(struct k_sem *sem_rx)
 		return -ENOMEM;
 	}

+#if defined(CONFIG_BT_CTLR_FILTER)


Possible to use IS_ENABLED() here?

I included this change commit from #15780 just to get CI tests to work (as I added new conf files to testcases.yaml). I will remove the commit when #15780 gets merged first.

I kept the #15780 with the commit seperate in order to have backport label on that commit.

aescolar

https://github.com/zephyrproject-rtos/zephyr/blob/master/tests/bluetooth/bsim_bt/bsim_test_app/prj.conf
was meant to test the legacy controller,
and
https://github.com/zephyrproject-rtos/zephyr/blob/master/tests/bluetooth/bsim_bt/bsim_test_app/prj_split.conf
the split one.
When changing the default, if we want to continue testing the legacy one,
we would need to be set the 1st one to compile with BT_LL_SW.
(And whenever we are not interested in testing the legacy one anymore, we could just remove it)

carlescufi

I will block this until we have confirmation that controller-based privacy is not a blocker for anyone, because this is a substantial feature that we are taking away from users.

cvinayak · 2019-05-03T10:06:10Z

@aescolar addressed the bsim test conf file in #15849

@aescolar @carlescufi moved commits out into new PR #15849, that are independent from switching the default controller implementation.

addressed

tedd-an · 2019-05-04T00:49:09Z

I have run PTS with #15849 + #15781 and here is the summary:

Summary:
The test result was same as the previous result which ran without this PRs and didn't find any other failure.
pts_report.xlsx

Some details about the result: The first run was based on the tip of the day + PRs and some GATT TCs failed and it seemed not related to this PR. After rebasing the baseline to the tip used on Wednesday test(and this PR), those failed tests were passed again. I will look into it separately.

sixoctets · 2019-05-04T00:52:27Z

@tedd-an Thank you for running the tests. Please upload the .config file of the zephyr build here (just to confirm it built the new link layer with CONFIG_BT_LL_SW_SPLIT=y).

tedd-an · 2019-05-06T15:49:55Z

attached .config.
_config.txt

#
# BLE Controller support
#
CONFIG_BT_LL_SW_SPLIT=y
# CONFIG_BT_LL_SW is not set
CONFIG_BT_LLL_VENDOR_NORDIC=y

vkchettimada · 2019-05-06T15:57:48Z

Thank you

joerchan

There are also a couple of bugfixes for the legacy controller, that should be ported to the new controller.

subsys/bluetooth/controller/Kconfig

Privacy for platforms with HW support added in the split controller in #16037

cvinayak · 2019-07-16T15:56:57Z

doc/guides/bluetooth/bluetooth-arch.rst

@@ -110,7 +110,7 @@ BLE-enabled builds that can be produced from the Zephyr project codebase:
  * :option:`CONFIG_BT_HCI` ``=y``
  * :option:`CONFIG_BT_HCI_RAW` ``=y``
  * :option:`CONFIG_BT_CTLR` ``=y``
-  * :option:`CONFIG_BT_LL_SW` ``=y`` (if using the open source Link Layer)
+  * :option:`CONFIG_BT_LL_SW_LEGACY` ``=y`` (if using the open source Link Layer)


fix this to use BT_LL_SW_SPLIT

cvinayak · 2019-07-16T15:57:03Z

doc/guides/bluetooth/bluetooth-arch.rst

@@ -131,7 +131,7 @@ BLE-enabled builds that can be produced from the Zephyr project codebase:
  * :option:`CONFIG_BT` ``=y``
  * :option:`CONFIG_BT_HCI` ``=y``
  * :option:`CONFIG_BT_CTLR` ``=y``
-  * :option:`CONFIG_BT_LL_SW` ``=y`` (if using the open source Link Layer)
+  * :option:`CONFIG_BT_LL_SW_LEGACY` ``=y`` (if using the open source Link Layer)


fix this to use BT_LL_SW_SPLIT

cvinayak · 2019-07-16T15:57:23Z

drivers/counter/Kconfig.nrfx

@@ -13,7 +13,7 @@ config COUNTER_NRF_RTC
 config COUNTER_TIMER0
 	bool "Enable Counter on TIMER0"
 	depends on HAS_HW_NRF_TIMER0
-	depends on !BT_LL_SW
+	depends on !BT_LL_SW_LEGACY


Add BT_LL_SW_SPLIT

cvinayak · 2019-07-16T15:57:29Z

drivers/counter/Kconfig.nrfx

@@ -44,7 +44,7 @@ config COUNTER_TIMER4
 config COUNTER_RTC0
 	bool "Enable Counter on RTC0"
 	depends on HAS_HW_NRF_RTC0
-	depends on !BT_LL_SW
+	depends on !BT_LL_SW_LEGACY


Add BT_LL_SW_SPLIT

dbkinder

doc changes LGTM, other than your own comment touse BT_LL_SW_SPLIT

carlescufi · 2019-07-18T10:44:28Z

subsys/bluetooth/controller/Kconfig

@@ -16,26 +16,27 @@ if BT_CTLR

 choice BT_LL_CHOICE
 	prompt "Bluetooth Link Layer Selection"
-	default BT_LL_SW


Why did you remove this line?

BT_LL_SW_SPLIT being the first item in the choice block, it is the de facto default

Rename the controller Kconfig option BT_LL_SW to BT_LL_SW_LEGACY in preparation towards switch to new Link Layer implementation. Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>

Use legacy LL until RAM utilization is optimized in the new LL. Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>

Make the Upper and Lower Link Layer split architecture implementation of the controller as the default when building Zephyr Bluetooth Low Energy controller support in applications. Noticeable missing feature (porting) in comparison to old architecture implementation is, Advanced scheduling of connection events. The missing features will subsequently be submitted upstream. Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>

carlescufi · 2019-07-18T15:36:52Z

subsys/bluetooth/controller/Kconfig

 	help
 	  Select the Bluetooth Link Layer to compile.

-config BT_LL_SW_LEGACY
+config BT_LL_SW_SPLIT


You need to rename this to BT_LL_SW, that's what we agreed with @joerchan right? no more "split" in the name

Using the same BT_LL_SW will make users’ config file be valid after switching to new LL but without them noticing the change. Users should know the change in design. I will keep the _SPLIT until legacy is deleted.

That's really not the case if they are just using CONFIG_BT on a Nordic board, right? BT_LL_SW_SPLIT will be automatically enabled without them noticing

Those who dont use explicit BT_LL_SW dont care if the macro is called BT_LL_SW_SPLIT or anything. We get them to use the new LL. Those who did use (for any reason) BT_LL_SW will now know that it is deprecated and take an informed decision to use BT_LL_SW_SPLIT.

cvinayak requested review from jhedberg, joerchan, Vudentz, sjanc, carlescufi, aescolar, thoh-ot, mped-oticon and mtpr-ot May 1, 2019 07:14

cvinayak added area: Bluetooth Enhancement Changes/Updates/Additions to existing features Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. labels May 1, 2019

cvinayak force-pushed the github_ull_lll_default branch from 71559a4 to 2e797ac Compare May 1, 2019 10:13

cvinayak force-pushed the github_ull_lll_default branch 2 times, most recently from 72c2acb to b60e1e5 Compare May 1, 2019 13:26

cvinayak added TSC Topics that need TSC discussion and removed Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. labels May 2, 2019

jhedberg reviewed May 2, 2019

View reviewed changes

aescolar added Blocked Blocked by another PR or issue and removed TSC Topics that need TSC discussion labels May 2, 2019

aescolar previously requested changes May 2, 2019

View reviewed changes

carlescufi previously requested changes May 2, 2019

View reviewed changes

cvinayak force-pushed the github_ull_lll_default branch from b60e1e5 to cb06d6d Compare May 3, 2019 10:02

cvinayak force-pushed the github_ull_lll_default branch from cb06d6d to cb21dab Compare May 7, 2019 06:00

joerchan requested changes May 7, 2019

View reviewed changes

subsys/bluetooth/controller/Kconfig Outdated Show resolved Hide resolved

cvinayak force-pushed the github_ull_lll_default branch from cb21dab to ba15b7c Compare June 11, 2019 14:34

zephyrbot added the area: Tests Issues related to a particular existing or missing test label Jun 11, 2019

cvinayak added this to the v2.0.0 milestone Jul 12, 2019

aescolar removed the Blocked Blocked by another PR or issue label Jul 16, 2019

aescolar requested a review from joerchan July 16, 2019 09:24

aescolar approved these changes Jul 16, 2019

View reviewed changes

cvinayak force-pushed the github_ull_lll_default branch from ba15b7c to 0801be2 Compare July 16, 2019 15:53

cvinayak requested review from dbkinder and nordic-krch as code owners July 16, 2019 15:53

cvinayak commented Jul 16, 2019

View reviewed changes

zephyrbot added area: Counter area: Documentation labels Jul 16, 2019

dbkinder approved these changes Jul 17, 2019

View reviewed changes

cvinayak force-pushed the github_ull_lll_default branch from 0801be2 to 9e6aca4 Compare July 18, 2019 09:23

carlescufi reviewed Jul 18, 2019

View reviewed changes

cvinayak force-pushed the github_ull_lll_default branch from 9e6aca4 to a8254d7 Compare July 18, 2019 11:25

cvinayak added 3 commits July 18, 2019 17:51

Bluetooth: controller: Rename BT_LL_SW to BT_LL_SW_LEGACY

2cf72bb

Rename the controller Kconfig option BT_LL_SW to BT_LL_SW_LEGACY in preparation towards switch to new Link Layer implementation. Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>

Bluetooth: mesh: Use legacy LL for microbit build

c35d915

Use legacy LL until RAM utilization is optimized in the new LL. Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>

cvinayak force-pushed the github_ull_lll_default branch from a8254d7 to 0bbcf2b Compare July 18, 2019 12:21

carlescufi requested changes Jul 18, 2019

View reviewed changes

carlescufi approved these changes Jul 22, 2019

View reviewed changes

joerchan approved these changes Jul 22, 2019

View reviewed changes

carlescufi merged commit 6cf26b8 into zephyrproject-rtos:master Jul 22, 2019

cvinayak deleted the github_ull_lll_default branch March 1, 2021 00:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bluetooth: controller: Make ULL/LLL split the default #15781

Bluetooth: controller: Make ULL/LLL split the default #15781

cvinayak commented May 1, 2019 •

edited

zephyrbot commented May 1, 2019 •

edited

jhedberg May 2, 2019

cvinayak May 2, 2019

aescolar left a comment •

edited

carlescufi left a comment

cvinayak commented May 3, 2019

tedd-an commented May 4, 2019 •

edited

sixoctets commented May 4, 2019

tedd-an commented May 6, 2019 •

edited

vkchettimada commented May 6, 2019

joerchan left a comment

cvinayak Jul 16, 2019

cvinayak Jul 16, 2019

cvinayak Jul 16, 2019

cvinayak Jul 16, 2019

dbkinder left a comment

carlescufi Jul 18, 2019

cvinayak Jul 18, 2019

carlescufi Jul 18, 2019

cvinayak Jul 18, 2019

carlescufi Jul 19, 2019

cvinayak Jul 19, 2019

Bluetooth: controller: Make ULL/LLL split the default #15781

Bluetooth: controller: Make ULL/LLL split the default #15781

Conversation

cvinayak commented May 1, 2019 • edited

zephyrbot commented May 1, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aescolar left a comment • edited

Choose a reason for hiding this comment

carlescufi left a comment

Choose a reason for hiding this comment

cvinayak commented May 3, 2019

tedd-an commented May 4, 2019 • edited

sixoctets commented May 4, 2019

tedd-an commented May 6, 2019 • edited

vkchettimada commented May 6, 2019

joerchan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbkinder left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cvinayak commented May 1, 2019 •

edited

zephyrbot commented May 1, 2019 •

edited

aescolar left a comment •

edited

tedd-an commented May 4, 2019 •

edited

tedd-an commented May 6, 2019 •

edited