Bluetooth: Controller: Fix LLL events pending check from ULL Low #37757
Conversation
cvinayak
force-pushed
the
github_lll_pending_check
branch
3 times, most recently
from
b6fa31e to
005a50d
Compare
cvinayak
requested review from
carlescufi,
kruithofa and
thoh-ot
as code owners
|
This is a complex set of changes. Will need to take some more time to review. |
@mtpr-ot The changes in this PR are to make access to ULL reference count context safe with ULL_HIGH priority != ULL_LOW priority, in which scenarios ULL_HIGH where reference count gets decremented would pre-empt the ULL_LOW code after |
There was a problem hiding this comment.
Yes, this is correct. For some reason the calling context was assumed to be ULL_LOW, but it's called from conn_cleanup, which is ULL_HIGH.
There was a problem hiding this comment.
Calling cig_disabled_cb here directly in ULL_HIGH seems like the correct thing to do. However, this will cause recursing call to cis_disabled_cb, when more than one CIS is associated with an ACL connection - which it cannot handle. Either we need to make sure cis_released_cb is deferred, or for efficiency, we will need to guard for re-entrance into cis_disabled_cb to prevent stopping ticker twice.
There was a problem hiding this comment.
Give me a solution ;-), will chaining the call to cig_disabled_cb using mayfly help?
There was a problem hiding this comment.
In cis_disabled_cb, check at entry if this is the last CIS, and only then stop ticker at end of function. This ensures that (efficient) handling of disabling multiple CISes associated with same ACL conn, where this function would be called recursively in certain cases.
E.g.:
is_last_cis = cig->lll.num_cis == 1;
.
.
if (is_last_cis && cig->lll.num_cis == 0) {
ticker_status = ticker_stop(TICKER_INSTANCE_ID_CTLR,
.
.
As an alternative, pull out the ull_con_iso commit, and I'll do it in a separate PR.
There was a problem hiding this comment.
As an alternative, pull out the ull_con_iso commit, and I'll do it in a separate PR.
Ok, seems best, will remove the commit from the PR, you can clean up the commit and send it in a new PR.
Use consistent naming for ticker operation callback functions. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
ULL reference count is checked in ULL_LOW context to decide if LLL events are pending, but the reference count can be decremented by the ULL HIGH execution context which can prevent the set `disabled_cb` function not being called due to no pending event to produce the done events. Fixed by checking the reference count in the ULL HIGH execution context using a mayfly to schedule the check. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Minor rename of Connection ISO disabled callback function names. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
cvinayak
force-pushed
the
github_lll_pending_check
branch
from
005a50d to
f6628d8
Compare
4 participants
ULL reference count is checked in ULL_LOW context to decide
if LLL events are pending, but the reference count can be
decremented by the ULL HIGH execution context which can
prevent the set
disabled_cbfunction not being called dueto no pending event to produce the done events.
Fixed by checking the reference count in the ULL HIGH
execution context using a mayfly to schedule the check.
Signed-off-by: Vinayak Kariappa Chettimada [email protected]