-
Notifications
You must be signed in to change notification settings - Fork 8.4k
Bluetooth: Controller: Only select BT_TICKER_LAZY_GET for ZLL #89249
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
Bluetooth: Controller: Only select BT_TICKER_LAZY_GET for ZLL #89249
Conversation
| bool "Periodic Advertising Sync Transfer sender" | ||
| depends on BT_CTLR_SYNC_TRANSFER_SENDER_SUPPORT | ||
| select BT_TICKER_LAZY_GET | ||
| select BT_TICKER_LAZY_GET if BT_LL_SW_SPLIT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already covered here:
| select BT_TICKER_LAZY_GET if BT_CTLR_ADV_PERIODIC || BT_CTLR_CONN_ISO || BT_CTLR_SYNC_TRANSFER_SENDER |
| select BT_TICKER_LAZY_GET if BT_LL_SW_SPLIT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, that is insufficient since it only applies for Nordic controllers. The original change is correct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But @Tronil , dont you have a custom choice on the same line as config BT_LLL_VENDOR_NORDIC/OPENISA where this select be present?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could have (we don't at the moment), but why not just have it in the one place instead of once per vendor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will revert to the original solution then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could have (we don't at the moment), but why not just have it in the one place instead of once per vendor?
Just hopping to keep the specifics inside ll_sw_split Kconfig file. Not able to think of an alternative for now, approving the current change in this PR.
|
nitpick, prefer the commit title be prefixed as |
593716d to
35c428f
Compare
BT_TICKER_LAZY_GET is a ZLL only kconfig and should not be selected for other controllers. Signed-off-by: Timothy Keys <timothy.keys@nordicsemi.no>
35c428f to
d82bf44
Compare
| bool "Periodic Advertising Sync Transfer sender" | ||
| depends on BT_CTLR_SYNC_TRANSFER_SENDER_SUPPORT | ||
| select BT_TICKER_LAZY_GET | ||
| select BT_TICKER_LAZY_GET if BT_LL_SW_SPLIT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could have (we don't at the moment), but why not just have it in the one place instead of once per vendor?
Just hopping to keep the specifics inside ll_sw_split Kconfig file. Not able to think of an alternative for now, approving the current change in this PR.
rugeGerritsen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternative approach is to add another declaration of BT_CTLR_SYNC_TRANSFER_SENDER inside the bt_ll_sw_split file
BT_TICKER_LAZY_GET is a ZLL only kconfig and should not be selected for other controllers.