Skip to content

Commit 771a3d7

Browse files
Thalleycarlescufi
Thalley
authored and
carlescufi
committed
Bluetooth: CSIP: Set Coordinator: Replace bools with atomic
Replace boolean flags with atomic. This also properly add guards for the multi-device procedures and reduce the number of places where the busy flag is set. Signed-off-by: Emil Gydesen <[email protected]>
1 parent 3029916 commit 771a3d7

File tree

1 file changed

+78
-52
lines changed

1 file changed

+78
-52
lines changed

‎subsys/bluetooth/audio/csip_set_coordinator.c

Lines changed: 78 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* Bluetooth Coordinated Set Identification Client
22
*
33
* Copyright (c) 2020 Bose Corporation
4-
* Copyright (c) 2021-2022 Nordic Semiconductor ASA
4+
* Copyright (c) 2021-2024 Nordic Semiconductor ASA
55
*
66
* SPDX-License-Identifier: Apache-2.0
77
*
@@ -67,9 +67,14 @@ static struct active_members {
6767
bt_csip_set_coordinator_ordered_access_t oap_cb;
6868
} active;
6969

70+
enum set_coordinator_flag {
71+
SET_COORDINATOR_FLAG_BUSY,
72+
73+
SET_COORDINATOR_FLAG_NUM_FLAGS, /* keep as last */
74+
};
75+
7076
struct bt_csip_set_coordinator_inst {
7177
uint8_t inst_count;
72-
bool busy;
7378
uint8_t gatt_write_buf[1];
7479

7580
struct bt_csip_set_coordinator_svc_inst
@@ -80,6 +85,8 @@ struct bt_csip_set_coordinator_inst {
8085
struct bt_gatt_discover_params discover_params;
8186
struct bt_gatt_read_params read_params;
8287
struct bt_gatt_write_params write_params;
88+
89+
ATOMIC_DEFINE(flags, SET_COORDINATOR_FLAG_NUM_FLAGS);
8390
};
8491

8592
static struct bt_uuid_16 uuid = BT_UUID_INIT_16(0);
@@ -101,6 +108,14 @@ static void discover_insts_resume(struct bt_conn *conn, uint16_t sirk_handle,
101108

102109
static void active_members_reset(void)
103110
{
111+
for (size_t i = 0U; i < active.members_count; i++) {
112+
const struct bt_csip_set_coordinator_set_member *member = active.members[i];
113+
struct bt_csip_set_coordinator_inst *client =
114+
CONTAINER_OF(member, struct bt_csip_set_coordinator_inst, set_member);
115+
116+
atomic_clear_bit(client->flags, SET_COORDINATOR_FLAG_BUSY);
117+
}
118+
104119
(void)memset(&active, 0, sizeof(active));
105120
}
106121

@@ -350,7 +365,7 @@ static void discover_complete(struct bt_csip_set_coordinator_inst *client,
350365
struct bt_csip_set_coordinator_cb *listener;
351366

352367
client->cur_inst = NULL;
353-
client->busy = false;
368+
atomic_clear_bit(client->flags, SET_COORDINATOR_FLAG_BUSY);
354369

355370
SYS_SLIST_FOR_EACH_CONTAINER(&csip_set_coordinator_cbs, listener, _node) {
356371
if (listener->discover) {
@@ -656,15 +671,9 @@ static int csip_set_coordinator_read_rank(struct bt_conn *conn,
656671
static int csip_set_coordinator_discover_sets(struct bt_csip_set_coordinator_inst *client)
657672
{
658673
struct bt_csip_set_coordinator_set_member *member = &client->set_member;
659-
int err;
660674

661675
/* Start reading values and call CB when done */
662-
err = read_sirk((struct bt_csip_set_coordinator_svc_inst *)member->insts[0].svc_inst);
663-
if (err == 0) {
664-
client->busy = true;
665-
}
666-
667-
return err;
676+
return read_sirk((struct bt_csip_set_coordinator_svc_inst *)member->insts[0].svc_inst);
668677
}
669678

670679
static uint8_t discover_func(struct bt_conn *conn,
@@ -702,7 +711,6 @@ static uint8_t discover_func(struct bt_conn *conn,
702711
int err;
703712

704713
client->cur_inst = NULL;
705-
client->busy = false;
706714
err = csip_set_coordinator_discover_sets(client);
707715
if (err != 0) {
708716
LOG_DBG("Discover sets failed (err %d)", err);
@@ -868,8 +876,6 @@ static uint8_t csip_set_coordinator_discover_insts_read_rank_cb(struct bt_conn *
868876

869877
__ASSERT(client->cur_inst != NULL, "client->cur_inst must not be NULL");
870878

871-
client->busy = false;
872-
873879
if (err != 0) {
874880
LOG_DBG("err: 0x%02X", err);
875881

@@ -902,8 +908,6 @@ static uint8_t csip_set_coordinator_discover_insts_read_set_size_cb(
902908

903909
__ASSERT(client->cur_inst != NULL, "client->cur_inst must not be NULL");
904910

905-
client->busy = false;
906-
907911
if (err != 0) {
908912
LOG_DBG("err: 0x%02X", err);
909913

@@ -982,8 +986,6 @@ static uint8_t csip_set_coordinator_discover_insts_read_sirk_cb(struct bt_conn *
982986
int cb_err = err;
983987
__ASSERT(client->cur_inst != NULL, "client->cur_inst must not be NULL");
984988

985-
client->busy = false;
986-
987989
if (err != 0) {
988990
LOG_DBG("err: 0x%02X", err);
989991

@@ -1053,8 +1055,6 @@ static void discover_insts_resume(struct bt_conn *conn, uint16_t sirk_handle,
10531055

10541056
if (cb_err != 0) {
10551057
discover_complete(client, cb_err);
1056-
} else {
1057-
client->busy = true;
10581058
}
10591059
}
10601060

@@ -1064,8 +1064,6 @@ static void csip_set_coordinator_write_restore_cb(struct bt_conn *conn,
10641064
{
10651065
struct bt_csip_set_coordinator_inst *client = &client_insts[bt_conn_index(conn)];
10661066

1067-
client->busy = false;
1068-
10691067
if (err != 0) {
10701068
LOG_WRN("Could not restore (%d)", err);
10711069
release_set_complete(err);
@@ -1091,9 +1089,7 @@ static void csip_set_coordinator_write_restore_cb(struct bt_conn *conn,
10911089

10921090
csip_err = csip_set_coordinator_write_set_lock(
10931091
client->cur_inst, false, csip_set_coordinator_write_restore_cb);
1094-
if (csip_err == 0) {
1095-
client->busy = true;
1096-
} else {
1092+
if (csip_err != 0) {
10971093
LOG_DBG("Failed to release next member[%u]: %d", active.members_handled,
10981094
csip_err);
10991095

@@ -1110,8 +1106,6 @@ static void csip_set_coordinator_write_lock_cb(struct bt_conn *conn,
11101106
{
11111107
struct bt_csip_set_coordinator_inst *client = &client_insts[bt_conn_index(conn)];
11121108

1113-
client->busy = false;
1114-
11151109
if (err != 0) {
11161110
LOG_DBG("Could not lock (0x%X)", err);
11171111
if (active.members_handled > 0 && CONFIG_BT_MAX_CONN > 1) {
@@ -1131,9 +1125,7 @@ static void csip_set_coordinator_write_lock_cb(struct bt_conn *conn,
11311125

11321126
csip_err = csip_set_coordinator_write_set_lock(
11331127
client->cur_inst, false, csip_set_coordinator_write_restore_cb);
1134-
if (csip_err == 0) {
1135-
client->busy = true;
1136-
} else {
1128+
if (csip_err != 0) {
11371129
LOG_WRN("Could not release lock of previous locked member: %d",
11381130
csip_err);
11391131
active_members_reset();
@@ -1162,9 +1154,7 @@ static void csip_set_coordinator_write_lock_cb(struct bt_conn *conn,
11621154

11631155
csip_err = csip_set_coordinator_write_set_lock(client->cur_inst, true,
11641156
csip_set_coordinator_write_lock_cb);
1165-
if (csip_err == 0) {
1166-
client->busy = true;
1167-
} else {
1157+
if (csip_err != 0) {
11681158
LOG_DBG("Failed to lock next member[%u]: %d", active.members_handled,
11691159
csip_err);
11701160

@@ -1173,9 +1163,7 @@ static void csip_set_coordinator_write_lock_cb(struct bt_conn *conn,
11731163
csip_err = csip_set_coordinator_write_set_lock(
11741164
prev_inst, false,
11751165
csip_set_coordinator_write_restore_cb);
1176-
if (csip_err == 0) {
1177-
client->busy = true;
1178-
} else {
1166+
if (csip_err != 0) {
11791167
LOG_WRN("Could not release lock of previous locked member: %d",
11801168
csip_err);
11811169
active_members_reset();
@@ -1192,8 +1180,6 @@ static void csip_set_coordinator_write_release_cb(struct bt_conn *conn, uint8_t
11921180
{
11931181
struct bt_csip_set_coordinator_inst *client = &client_insts[bt_conn_index(conn)];
11941182

1195-
client->busy = false;
1196-
11971183
if (err != 0) {
11981184
LOG_DBG("Could not release lock (%d)", err);
11991185
release_set_complete(err);
@@ -1216,9 +1202,7 @@ static void csip_set_coordinator_write_release_cb(struct bt_conn *conn, uint8_t
12161202

12171203
csip_err = csip_set_coordinator_write_set_lock(
12181204
client->cur_inst, false, csip_set_coordinator_write_release_cb);
1219-
if (csip_err == 0) {
1220-
client->busy = true;
1221-
} else {
1205+
if (csip_err != 0) {
12221206
LOG_DBG("Failed to release next member[%u]: %d", active.members_handled,
12231207
csip_err);
12241208

@@ -1253,8 +1237,6 @@ static uint8_t csip_set_coordinator_read_lock_cb(struct bt_conn *conn,
12531237
struct bt_csip_set_coordinator_inst *client = &client_insts[bt_conn_index(conn)];
12541238
uint8_t value = 0;
12551239

1256-
client->busy = false;
1257-
12581240
if (err != 0) {
12591241
LOG_DBG("Could not read lock value (0x%X)", err);
12601242

@@ -1305,9 +1287,7 @@ static uint8_t csip_set_coordinator_read_lock_cb(struct bt_conn *conn,
13051287
}
13061288

13071289
csip_err = csip_set_coordinator_read_set_lock(client->cur_inst);
1308-
if (csip_err == 0) {
1309-
client->busy = true;
1310-
} else {
1290+
if (csip_err != 0) {
13111291
LOG_DBG("Failed to read next member[%u]: %d", active.members_handled,
13121292
csip_err);
13131293

@@ -1469,7 +1449,7 @@ int bt_csip_set_coordinator_discover(struct bt_conn *conn)
14691449
}
14701450

14711451
client = &client_insts[bt_conn_index(conn)];
1472-
if (client->busy) {
1452+
if (atomic_test_and_set_bit(client->flags, SET_COORDINATOR_FLAG_BUSY)) {
14731453
return -EBUSY;
14741454
}
14751455

@@ -1489,8 +1469,9 @@ int bt_csip_set_coordinator_discover(struct bt_conn *conn)
14891469
for (size_t i = 0; i < ARRAY_SIZE(client->set_member.insts); i++) {
14901470
client->set_member.insts[i].svc_inst = (void *)&client->svc_insts[i];
14911471
}
1492-
client->busy = true;
14931472
client->conn = bt_conn_ref(conn);
1473+
} else {
1474+
atomic_clear_bit(client->flags, SET_COORDINATOR_FLAG_BUSY);
14941475
}
14951476

14961477
return err;
@@ -1568,10 +1549,40 @@ static int verify_members(const struct bt_csip_set_coordinator_set_member **memb
15681549
return 0;
15691550
}
15701551

1571-
static int bt_csip_set_coordinator_get_lock_state(
1572-
const struct bt_csip_set_coordinator_set_member **members,
1573-
uint8_t count,
1574-
const struct bt_csip_set_coordinator_set_info *set_info)
1552+
static bool check_and_set_members_busy(const struct bt_csip_set_coordinator_set_member *members[],
1553+
size_t count)
1554+
{
1555+
size_t num_free;
1556+
1557+
for (num_free = 0U; num_free < count; num_free++) {
1558+
const struct bt_csip_set_coordinator_set_member *member = members[num_free];
1559+
struct bt_csip_set_coordinator_inst *client =
1560+
CONTAINER_OF(member, struct bt_csip_set_coordinator_inst, set_member);
1561+
1562+
if (atomic_test_and_set_bit(client->flags, SET_COORDINATOR_FLAG_BUSY)) {
1563+
LOG_DBG("Member[%zu] (%p) is busy", num_free, member);
1564+
break;
1565+
}
1566+
}
1567+
1568+
/* If any is busy, revert any busy states we've set */
1569+
if (num_free != count) {
1570+
for (size_t i = 0U; i < num_free; i++) {
1571+
const struct bt_csip_set_coordinator_set_member *member = members[i];
1572+
struct bt_csip_set_coordinator_inst *client = CONTAINER_OF(
1573+
member, struct bt_csip_set_coordinator_inst, set_member);
1574+
1575+
atomic_clear_bit(client->flags, SET_COORDINATOR_FLAG_BUSY);
1576+
}
1577+
}
1578+
1579+
return num_free == count;
1580+
}
1581+
1582+
static int
1583+
csip_set_coordinator_get_lock_state(const struct bt_csip_set_coordinator_set_member **members,
1584+
uint8_t count,
1585+
const struct bt_csip_set_coordinator_set_info *set_info)
15751586
{
15761587
int err;
15771588

@@ -1586,6 +1597,11 @@ static int bt_csip_set_coordinator_get_lock_state(
15861597
return err;
15871598
}
15881599

1600+
if (!check_and_set_members_busy(members, count)) {
1601+
LOG_DBG("One or more members are busy");
1602+
return -EBUSY;
1603+
}
1604+
15891605
active_members_store_ordered(members, count, set_info, true);
15901606

15911607
for (uint8_t i = 0U; i < count; i++) {
@@ -1638,7 +1654,7 @@ int bt_csip_set_coordinator_ordered_access(
16381654
/* wait for the get_lock_state to finish and then call the callback */
16391655
active.oap_cb = cb;
16401656

1641-
err = bt_csip_set_coordinator_get_lock_state(members, count, set_info);
1657+
err = csip_set_coordinator_get_lock_state(members, count, set_info);
16421658
if (err != 0) {
16431659
active.oap_cb = NULL;
16441660

@@ -1667,6 +1683,11 @@ int bt_csip_set_coordinator_lock(
16671683
return err;
16681684
}
16691685

1686+
if (!check_and_set_members_busy(members, count)) {
1687+
LOG_DBG("One or more members are busy");
1688+
return -EBUSY;
1689+
}
1690+
16701691
active_members_store_ordered(members, count, set_info, true);
16711692

16721693
svc_inst = lookup_instance_by_set_info(active.members[0], active.info);
@@ -1704,6 +1725,11 @@ int bt_csip_set_coordinator_release(const struct bt_csip_set_coordinator_set_mem
17041725
return err;
17051726
}
17061727

1728+
if (!check_and_set_members_busy(members, count)) {
1729+
LOG_DBG("One or more members are busy");
1730+
return -EBUSY;
1731+
}
1732+
17071733
active_members_store_ordered(members, count, set_info, false);
17081734

17091735
svc_inst = lookup_instance_by_set_info(active.members[0], active.info);

0 commit comments

Comments
 (0)