Bluetooth: Audio: MCC - serialized subscription after discovery

The media control client used to subscribe to GMCS characteristics as
the characteristics were discovered, in effect queuing ATT requests.

Since then, the ATT MTU exchange has become automatic, leading to
larger ATT MTUs and more discovered characteristics per ATT response,
which again lead to the client enqueuing more subscriptions and
running out of buffers, with discovery failing as a result.

This commit moves the subscription to GMCS characteristics to a
separate sequence, so that it happens serially after discovery, rather
than in parallel with the discovery.

- Remove separate discovery parameters for the characteristics - they
  are no longer needed.
- Add a subscription callback and a couple of subscription functions

The subscription for characteristics for the included OTS has not been
modified.  Only two of these are subscribed to, and doing that is
currently not a problem.  And I need to better understand the
interaction with OTS and how these subscriptipons are used.

Signed-off-by: Asbjørn Sæbø <asbjorn.sabo@nordicsemi.no>

Refactor subscription to get chaining into callback

subsys/bluetooth/audio/mcc.c

@@ -74,42 +74,25 @@ struct mcs_instance_t {
 	uint16_t content_control_id_handle;
 
 	struct bt_gatt_subscribe_params player_name_sub_params;
-	struct bt_gatt_discover_params player_name_sub_disc_params;
 	struct bt_gatt_subscribe_params track_changed_sub_params;
-	struct bt_gatt_discover_params track_changed_sub_disc_params;
 	struct bt_gatt_subscribe_params track_title_sub_params;
-	struct bt_gatt_discover_params track_title_sub_disc_params;
 	struct bt_gatt_subscribe_params track_duration_sub_params;
-	struct bt_gatt_discover_params track_duration_sub_disc_params;
 	struct bt_gatt_subscribe_params track_position_sub_params;
-	struct bt_gatt_discover_params track_position_sub_disc_params;
 	struct bt_gatt_subscribe_params playback_speed_sub_params;
-	struct bt_gatt_discover_params playback_speed_sub_disc_params;
 	struct bt_gatt_subscribe_params seeking_speed_sub_params;
-	struct bt_gatt_discover_params seeking_speed_sub_disc_params;
 #ifdef CONFIG_BT_MCC_OTS
 	struct bt_gatt_subscribe_params current_track_obj_sub_params;
-	struct bt_gatt_discover_params current_track_sub_disc_params;
 	struct bt_gatt_subscribe_params next_track_obj_sub_params;
-	struct bt_gatt_discover_params next_track_obj_sub_disc_params;
 	struct bt_gatt_subscribe_params parent_group_obj_sub_params;
-	struct bt_gatt_discover_params parent_group_obj_sub_disc_params;
 	struct bt_gatt_subscribe_params current_group_obj_sub_params;
-	struct bt_gatt_discover_params current_group_obj_sub_disc_params;
 #endif /* CONFIG_BT_MCC_OTS */
 	struct bt_gatt_subscribe_params playing_order_sub_params;
-	struct bt_gatt_discover_params playing_order_sub_disc_params;
 	struct bt_gatt_subscribe_params media_state_sub_params;
-	struct bt_gatt_discover_params media_state_sub_disc_params;
 	struct bt_gatt_subscribe_params cp_sub_params;
-	struct bt_gatt_discover_params cp_sub_disc_params;
 	struct bt_gatt_subscribe_params opcodes_supported_sub_params;
-	struct bt_gatt_discover_params opcodes_supported_sub_disc_params;
 #ifdef CONFIG_BT_MCC_OTS
 	struct bt_gatt_subscribe_params scp_sub_params;
-	struct bt_gatt_discover_params scp_sub_disc_params;
 	struct bt_gatt_subscribe_params search_results_obj_sub_params;
-	struct bt_gatt_discover_params search_results_obj_sub_disc_params;
 #endif /* CONFIG_BT_MCC_OTS */
 
 	/* The write buffer is used for
@@ -1049,6 +1032,14 @@ static uint8_t mcs_notify_handler(struct bt_conn *conn,
 	return BT_GATT_ITER_CONTINUE;
 }
 
+static void discovery_failure(struct bt_conn *conn, int err)
+{
+	cur_mcs_inst = NULL;
+	if (mcc_cb && mcc_cb->discover_mcs) {
+		mcc_cb->discover_mcs(conn, err);
+	}
+}
+
 #ifdef CONFIG_BT_MCC_OTS
 static uint8_t discover_otc_char_func(struct bt_conn *conn,
 				      const struct bt_gatt_attr *attr,
@@ -1129,7 +1120,6 @@ static uint8_t discover_otc_char_func(struct bt_conn *conn,
 }
 #endif /* CONFIG_BT_MCC_OTS */
 
-
 #ifdef CONFIG_BT_MCC_OTS
 /* This function is called when an included service is found.
  * The function will store the start and end handle for the service,
@@ -1175,11 +1165,8 @@ static uint8_t discover_include_func(struct bt_conn *conn,
 		BT_DBG("Start discovery of OTS characteristics");
 		err = bt_gatt_discover(conn, &discover_params);
 		if (err) {
-			BT_DBG("Discovery of OTS chars. failed (err %d)",
+			BT_DBG("Discovery of OTS chars. failed");
-			       err);
+			discovery_failure(conn, err);
-			if (mcc_cb && mcc_cb->discover_mcs) {
-				mcc_cb->discover_mcs(conn, err);
-			}
 		}
 		return BT_GATT_ITER_STOP;
 	}
@@ -1207,14 +1194,47 @@ static void discover_included(struct bt_conn *conn)
 	BT_DBG("Start discovery of included services");
 	err = bt_gatt_discover(conn, &discover_params);
 	if (err) {
-		BT_DBG("Discover of included service failed (err %d)", err);
+		BT_DBG("Discovery of included service failed: %d", err);
-		if (mcc_cb && mcc_cb->discover_mcs) {
+		discovery_failure(conn, err);
-			mcc_cb->discover_mcs(conn, err);
-		}
 	}
 }
 #endif /* CONFIG_BT_MCC_OTS */
 
+static bool subscribe_next_mcs_char(struct bt_conn *conn);
+
+/* This function will subscribe to GMCS CCCDs.
+ * After this, the function will start discovery of included services.
+ */
+static void subscribe_mcs_char_func(struct bt_conn *conn, uint8_t err,
+				    struct bt_gatt_subscribe_params *params)
+{
+	bool subscription_done;
+
+	if (err) {
+		BT_DBG("Subscription callback error: %u", err);
+		discovery_failure(conn, err);
+		return;
+	}
+
+	BT_DBG("Subscribed: value handle: %d, ccc handle: %d",
+	       params->value_handle, params->ccc_handle);
+
+	/* Subscribe to next characteristic */
+	subscription_done = subscribe_next_mcs_char(conn);
+
+	if (subscription_done) {
+#ifdef CONFIG_BT_MCC_OTS
+		/* Start discovery of included services to find OTS */
+		discover_included(conn);
+#else
+		/* If OTS is not configured, discovery ends here */
+		if (mcc_cb && mcc_cb->discover_mcs) {
+			mcc_cb->discover_mcs(conn, 0);
+		}
+#endif /* CONFIG_BT_MCC_OTS */
+	}
+}
+
 /* Subscribe to a characteristic - helper function */
 static int do_subscribe(struct bt_conn *conn, uint16_t handle,
 			 struct bt_gatt_subscribe_params *sub_params)
@@ -1225,22 +1245,237 @@ static int do_subscribe(struct bt_conn *conn, uint16_t handle,
 	sub_params->value = BT_GATT_CCC_NOTIFY;
 	sub_params->value_handle = handle;
 	sub_params->notify = mcs_notify_handler;
+	sub_params->subscribe = subscribe_mcs_char_func;
+	/* disc_params pointer is also used as subscription flag */
+	sub_params->disc_params = &discover_params;
 
+	BT_DBG("Subscring to handle %d", handle);
 	return bt_gatt_subscribe(conn, sub_params);
 }
 
+/* Subscribe to the next GMCS CCCD.
+ * @return true if there are no more characteristics to subscribe to
+ */
+static bool subscribe_next_mcs_char(struct bt_conn *conn)
+{
+	struct bt_gatt_subscribe_params *sub_params = NULL;
+	int err = 0;
+
+	/* The characteristics may be in any order on the server, and
+	 * not all of them may exist => need to check all.
+	 * For each of the subscribable characteristics
+	 * - check if we have a handle for it
+	 * - check sub_params.disc_params pointer to see if we have
+	 *   already subscribed to it (set in do_subscribe() ).
+	 */
+
+	if (cur_mcs_inst->player_name_handle &&
+	    cur_mcs_inst->player_name_sub_params.disc_params == NULL) {
+		sub_params = &cur_mcs_inst->player_name_sub_params;
+		err = do_subscribe(conn, cur_mcs_inst->player_name_handle, sub_params);
+		if (err) {
+			BT_DBG("Could not subscribe: %d", err);
+			discovery_failure(conn, err);
+		}
+		return false;
+	}
+
+	if (cur_mcs_inst->track_changed_handle &&
+	    cur_mcs_inst->track_changed_sub_params.disc_params == NULL) {
+		sub_params = &cur_mcs_inst->track_changed_sub_params;
+		err = do_subscribe(conn, cur_mcs_inst->track_changed_handle, sub_params);
+		if (err) {
+			BT_DBG("Could not subscribe: %d", err);
+			discovery_failure(conn, err);
+		}
+		return false;
+	}
+	if (cur_mcs_inst->track_title_handle &&
+	    cur_mcs_inst->track_title_sub_params.disc_params == NULL) {
+		sub_params = &cur_mcs_inst->track_title_sub_params;
+		err = do_subscribe(conn, cur_mcs_inst->track_title_handle, sub_params);
+		if (err) {
+			BT_DBG("Could not subscribe: %d", err);
+			discovery_failure(conn, err);
+		}
+		return false;
+	}
+
+	if (cur_mcs_inst->track_duration_handle &&
+	    cur_mcs_inst->track_duration_sub_params.disc_params == NULL) {
+		sub_params = &cur_mcs_inst->track_duration_sub_params;
+		err = do_subscribe(conn, cur_mcs_inst->track_duration_handle, sub_params);
+		if (err) {
+			BT_DBG("Could not subscribe: %d", err);
+			discovery_failure(conn, err);
+		}
+		return false;
+	}
+
+	if (cur_mcs_inst->track_position_handle &&
+	    cur_mcs_inst->track_position_sub_params.disc_params == NULL) {
+		sub_params = &cur_mcs_inst->track_position_sub_params;
+		err = do_subscribe(conn, cur_mcs_inst->track_position_handle, sub_params);
+		if (err) {
+			BT_DBG("Could not subscribe: %d", err);
+			discovery_failure(conn, err);
+		}
+		return false;
+	}
+
+	if (cur_mcs_inst->playback_speed_handle &&
+	    cur_mcs_inst->playback_speed_sub_params.disc_params == NULL) {
+		sub_params = &cur_mcs_inst->playback_speed_sub_params;
+		err = do_subscribe(conn, cur_mcs_inst->playback_speed_handle, sub_params);
+		if (err) {
+			BT_DBG("Could not subscribe: %d", err);
+			discovery_failure(conn, err);
+		}
+		return false;
+	}
+
+	if (cur_mcs_inst->seeking_speed_handle &&
+	    cur_mcs_inst->seeking_speed_sub_params.disc_params == NULL) {
+		sub_params = &cur_mcs_inst->seeking_speed_sub_params;
+		err = do_subscribe(conn, cur_mcs_inst->seeking_speed_handle, sub_params);
+		if (err) {
+			BT_DBG("Could not subscribe: %d", err);
+			discovery_failure(conn, err);
+		}
+		return false;
+	}
+
+#ifdef CONFIG_BT_MCC_OTS
+	if (cur_mcs_inst->current_track_obj_id_handle &&
+	    cur_mcs_inst->current_track_obj_sub_params.disc_params == NULL) {
+		sub_params = &cur_mcs_inst->current_track_obj_sub_params;
+		err = do_subscribe(conn, cur_mcs_inst->current_track_obj_id_handle, sub_params);
+		if (err) {
+			BT_DBG("Could not subscribe: %d", err);
+			discovery_failure(conn, err);
+		}
+		return false;
+	}
+
+	if (cur_mcs_inst->next_track_obj_id_handle &&
+	    cur_mcs_inst->next_track_obj_sub_params.disc_params == NULL) {
+		sub_params = &cur_mcs_inst->next_track_obj_sub_params;
+		err = do_subscribe(conn, cur_mcs_inst->next_track_obj_id_handle, sub_params);
+		if (err) {
+			BT_DBG("Could not subscribe: %d", err);
+			discovery_failure(conn, err);
+		}
+		return false;
+	}
+
+	if (cur_mcs_inst->parent_group_obj_id_handle &&
+	    cur_mcs_inst->parent_group_obj_sub_params.disc_params == NULL) {
+		sub_params = &cur_mcs_inst->parent_group_obj_sub_params;
+		err = do_subscribe(conn, cur_mcs_inst->parent_group_obj_id_handle, sub_params);
+		if (err) {
+			BT_DBG("Could not subscribe: %d", err);
+			discovery_failure(conn, err);
+		}
+		return false;
+	}
+
+	if (cur_mcs_inst->current_group_obj_id_handle &&
+	    cur_mcs_inst->parent_group_obj_sub_params.disc_params == NULL) {
+		sub_params = &cur_mcs_inst->current_group_obj_sub_params;
+		err = do_subscribe(conn, cur_mcs_inst->current_group_obj_id_handle, sub_params);
+		if (err) {
+			BT_DBG("Could not subscribe: %d", err);
+			discovery_failure(conn, err);
+		}
+		return false;
+	}
+
+#endif /* CONFIG_BT_MCC_OTS */
+
+	if (cur_mcs_inst->playing_order_handle &&
+	    cur_mcs_inst->playing_order_sub_params.disc_params == NULL) {
+		sub_params = &cur_mcs_inst->playing_order_sub_params;
+		err = do_subscribe(conn, cur_mcs_inst->playing_order_handle, sub_params);
+		if (err) {
+			BT_DBG("Could not subscribe: %d", err);
+			discovery_failure(conn, err);
+		}
+		return false;
+	}
+
+	if (cur_mcs_inst->media_state_handle &&
+	    cur_mcs_inst->media_state_sub_params.disc_params == NULL) {
+		sub_params = &cur_mcs_inst->media_state_sub_params;
+		err = do_subscribe(conn, cur_mcs_inst->media_state_handle, sub_params);
+		if (err) {
+			BT_DBG("Could not subscribe: %d", err);
+			discovery_failure(conn, err);
+		}
+		return false;
+	}
+
+	if (cur_mcs_inst->cp_handle &&
+	    cur_mcs_inst->cp_sub_params.disc_params == NULL) {
+		sub_params = &cur_mcs_inst->cp_sub_params;
+		err = do_subscribe(conn, cur_mcs_inst->cp_handle, sub_params);
+		if (err) {
+			BT_DBG("Could not subscribe: %d", err);
+			discovery_failure(conn, err);
+		}
+		return false;
+	}
+
+	if (cur_mcs_inst->opcodes_supported_handle &&
+	    cur_mcs_inst->opcodes_supported_sub_params.disc_params == NULL) {
+		sub_params = &cur_mcs_inst->opcodes_supported_sub_params;
+		err = do_subscribe(conn, cur_mcs_inst->opcodes_supported_handle, sub_params);
+		if (err) {
+			BT_DBG("Could not subscribe: %d", err);
+			discovery_failure(conn, err);
+		}
+		return false;
+	}
+
+#ifdef CONFIG_BT_MCC_OTS
+	if (cur_mcs_inst->scp_handle &&
+	    cur_mcs_inst->scp_sub_params.disc_params == NULL) {
+		sub_params = &cur_mcs_inst->scp_sub_params;
+		err = do_subscribe(conn, cur_mcs_inst->scp_handle, sub_params);
+		if (err) {
+			BT_DBG("Could not subscribe: %d", err);
+			discovery_failure(conn, err);
+		}
+		return false;
+	}
+
+	if (cur_mcs_inst->search_results_obj_id_handle &&
+	    cur_mcs_inst->search_results_obj_sub_params.disc_params == NULL) {
+		sub_params = &cur_mcs_inst->search_results_obj_sub_params;
+		err = do_subscribe(conn, cur_mcs_inst->search_results_obj_id_handle, sub_params);
+		if (err) {
+			BT_DBG("Could not subscribe: %d", err);
+			discovery_failure(conn, err);
+		}
+		return false;
+	}
+#endif /* CONFIG_BT_MCC_OTS */
+
+	/* If we have come here, there are no more characteristics to
+	 * subscribe to, and we are done.
+	 */
+	return true;
+}
+
 /* This function is called when characteristics are found.
- * The function will store handles, and optionally subscribe to, GMCS
+ * The function will store handles to GMCS characteristics.
- * characteristics.
+ * After this, the function will start subscription to characteristics
- * After this, the function will start discovery of included services.
  */
 static uint8_t discover_mcs_char_func(struct bt_conn *conn,
 				      const struct bt_gatt_attr *attr,
 				      struct bt_gatt_discover_params *params)
 {
 	struct bt_gatt_chrc *chrc;
-	struct bt_gatt_subscribe_params *sub_params = NULL;
+	bool subscription_done = true;
-	int err = 0;
 
 	if (attr) {
 		/* Found an attribute */
@@ -1258,8 +1493,8 @@ static uint8_t discover_mcs_char_func(struct bt_conn *conn,
 		if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_PLAYER_NAME)) {
 			BT_DBG("Player name, UUID: %s", bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->player_name_handle = chrc->value_handle;
-			sub_params = &cur_mcs_inst->player_name_sub_params;
+			/* Use discovery params pointer as subscription flag */
-			sub_params->disc_params = &cur_mcs_inst->player_name_sub_disc_params;
+			cur_mcs_inst->player_name_sub_params.disc_params = NULL;
 #ifdef CONFIG_BT_MCC_OTS
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_ICON_OBJ_ID)) {
 			BT_DBG("Icon Object, UUID: %s", bt_uuid_str(chrc->uuid));
@@ -1271,33 +1506,27 @@ static uint8_t discover_mcs_char_func(struct bt_conn *conn,
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_TRACK_CHANGED)) {
 			BT_DBG("Track Changed, UUID: %s", bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->track_changed_handle = chrc->value_handle;
-			sub_params = &cur_mcs_inst->track_changed_sub_params;
+			cur_mcs_inst->track_changed_sub_params.disc_params = NULL;
-			sub_params->disc_params = &cur_mcs_inst->track_changed_sub_disc_params;
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_TRACK_TITLE)) {
 			BT_DBG("Track Title, UUID: %s", bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->track_title_handle = chrc->value_handle;
-			sub_params = &cur_mcs_inst->track_title_sub_params;
+			cur_mcs_inst->track_title_sub_params.disc_params = NULL;
-			sub_params->disc_params = &cur_mcs_inst->track_title_sub_disc_params;
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_TRACK_DURATION)) {
 			BT_DBG("Track Duration, UUID: %s", bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->track_duration_handle = chrc->value_handle;
-			sub_params = &cur_mcs_inst->track_duration_sub_params;
+			cur_mcs_inst->track_duration_sub_params.disc_params = NULL;
-			sub_params->disc_params = &cur_mcs_inst->track_duration_sub_disc_params;
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_TRACK_POSITION)) {
 			BT_DBG("Track Position, UUID: %s", bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->track_position_handle = chrc->value_handle;
-			sub_params = &cur_mcs_inst->track_position_sub_params;
+			cur_mcs_inst->track_position_sub_params.disc_params = NULL;
-			sub_params->disc_params = &cur_mcs_inst->track_position_sub_disc_params;
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_PLAYBACK_SPEED)) {
 			BT_DBG("Playback Speed, UUID: %s", bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->playback_speed_handle = chrc->value_handle;
-			sub_params = &cur_mcs_inst->playback_speed_sub_params;
+			cur_mcs_inst->playback_speed_sub_params.disc_params = NULL;
-			sub_params->disc_params = &cur_mcs_inst->playback_speed_sub_disc_params;
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_SEEKING_SPEED)) {
 			BT_DBG("Seeking Speed, UUID: %s", bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->seeking_speed_handle = chrc->value_handle;
-			sub_params = &cur_mcs_inst->seeking_speed_sub_params;
+			cur_mcs_inst->seeking_speed_sub_params.disc_params = NULL;
-			sub_params->disc_params = &cur_mcs_inst->seeking_speed_sub_disc_params;
 #ifdef CONFIG_BT_MCC_OTS
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_TRACK_SEGMENTS_OBJ_ID)) {
 			BT_DBG("Track Segments Object, UUID: %s", bt_uuid_str(chrc->uuid));
@@ -1305,59 +1534,49 @@ static uint8_t discover_mcs_char_func(struct bt_conn *conn,
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_CURRENT_TRACK_OBJ_ID)) {
 			BT_DBG("Current Track Object, UUID: %s", bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->current_track_obj_id_handle = chrc->value_handle;
-			sub_params = &cur_mcs_inst->current_track_obj_sub_params;
+			cur_mcs_inst->current_track_obj_sub_params.disc_params = NULL;
-			sub_params->disc_params = &cur_mcs_inst->current_track_sub_disc_params;
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_NEXT_TRACK_OBJ_ID)) {
 			BT_DBG("Next Track Object, UUID: %s", bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->next_track_obj_id_handle = chrc->value_handle;
-			sub_params = &cur_mcs_inst->next_track_obj_sub_params;
+			cur_mcs_inst->next_track_obj_sub_params.disc_params = NULL;
-			sub_params->disc_params = &cur_mcs_inst->next_track_obj_sub_disc_params;
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_PARENT_GROUP_OBJ_ID)) {
 			BT_DBG("Parent Group Object, UUID: %s", bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->parent_group_obj_id_handle = chrc->value_handle;
-			sub_params = &cur_mcs_inst->parent_group_obj_sub_params;
+			cur_mcs_inst->parent_group_obj_sub_params.disc_params = NULL;
-			sub_params->disc_params = &cur_mcs_inst->parent_group_obj_sub_disc_params;
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_CURRENT_GROUP_OBJ_ID)) {
 			BT_DBG("Group Object, UUID: %s", bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->current_group_obj_id_handle = chrc->value_handle;
-			sub_params = &cur_mcs_inst->current_group_obj_sub_params;
+			cur_mcs_inst->current_group_obj_sub_params.disc_params = NULL;
-			sub_params->disc_params = &cur_mcs_inst->current_group_obj_sub_disc_params;
 #endif /* CONFIG_BT_MCC_OTS */
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_PLAYING_ORDER)) {
 			BT_DBG("Playing Order, UUID: %s", bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->playing_order_handle = chrc->value_handle;
-			sub_params = &cur_mcs_inst->playing_order_sub_params;
+			cur_mcs_inst->playing_order_sub_params.disc_params = NULL;
-			sub_params->disc_params = &cur_mcs_inst->playing_order_sub_disc_params;
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_PLAYING_ORDERS)) {
 			BT_DBG("Playing Orders supported, UUID: %s", bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->playing_orders_supported_handle = chrc->value_handle;
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_MEDIA_STATE)) {
 			BT_DBG("Media State, UUID: %s", bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->media_state_handle = chrc->value_handle;
-			sub_params = &cur_mcs_inst->media_state_sub_params;
+			cur_mcs_inst->media_state_sub_params.disc_params = NULL;
-			sub_params->disc_params = &cur_mcs_inst->media_state_sub_disc_params;
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_MEDIA_CONTROL_POINT)) {
 			BT_DBG("Media Control Point, UUID: %s", bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->cp_handle = chrc->value_handle;
-			sub_params = &cur_mcs_inst->cp_sub_params;
+			cur_mcs_inst->cp_sub_params.disc_params = NULL;
-			sub_params->disc_params = &cur_mcs_inst->cp_sub_disc_params;
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_MEDIA_CONTROL_OPCODES)) {
 			BT_DBG("Media control opcodes supported, UUID: %s",
 			       bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->opcodes_supported_handle = chrc->value_handle;
-			sub_params = &cur_mcs_inst->opcodes_supported_sub_params;
+			cur_mcs_inst->opcodes_supported_sub_params.disc_params = NULL;
-			sub_params->disc_params = &cur_mcs_inst->opcodes_supported_sub_disc_params;
 #ifdef CONFIG_BT_MCC_OTS
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_SEARCH_CONTROL_POINT)) {
 			BT_DBG("Search control point, UUID: %s", bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->scp_handle = chrc->value_handle;
-			sub_params = &cur_mcs_inst->scp_sub_params;
+			cur_mcs_inst->scp_sub_params.disc_params = NULL;
-			sub_params->disc_params = &cur_mcs_inst->scp_sub_disc_params;
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_MCS_SEARCH_RESULTS_OBJ_ID)) {
 			BT_DBG("Search Results object, UUID: %s", bt_uuid_str(chrc->uuid));
 			cur_mcs_inst->search_results_obj_id_handle = chrc->value_handle;
-			sub_params = &cur_mcs_inst->search_results_obj_sub_params;
+			cur_mcs_inst->search_results_obj_sub_params.disc_params = NULL;
-			sub_params->disc_params = &cur_mcs_inst->search_results_obj_sub_disc_params;
 #endif /* CONFIG_BT_MCC_OTS */
 		} else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_CCID)) {
 			BT_DBG("Content Control ID, UUID: %s", bt_uuid_str(chrc->uuid));
@@ -1365,43 +1584,39 @@ static uint8_t discover_mcs_char_func(struct bt_conn *conn,
 		}
 
 
-		if (subscribe_all && sub_params) {
-			BT_DBG("Subscribing - handle: 0x%04x", attr->handle);
-			err = do_subscribe(conn, chrc->value_handle, sub_params);
-			if (err) {
-				BT_WARN("Subscription failed (err %d)", err);
-				if (mcc_cb && mcc_cb->discover_mcs) {
-					mcc_cb->discover_mcs(conn, err);
-				}
-			}
-		}
-
 		/* Continue to search for more attributes */
 		return BT_GATT_ITER_CONTINUE;
 	}
 
 	/* No more attributes found */
-	BT_DBG("Setup complete for GMCS");
+	BT_DBG("GMCS characteristics found");
 	(void)memset(params, 0, sizeof(*params));
 
-#ifdef CONFIG_BT_MCC_OTS
+	/* Either subscribe to characteristics, or continue to discovery of
-
+	 *included services.
-	/* Start discovery of included services to find OTS */
+	 * Subscription is done after discovery, not in parallel with it,
-	discover_included(conn);
+	 * to avoid queuing many ATT requests that requires buffers.
-
+	 */
-#else
+	if (subscribe_all) {
-
+		subscription_done = subscribe_next_mcs_char(conn);
-	/* If OTS is not configured, discovery ends here */
-	if (mcc_cb && mcc_cb->discover_mcs) {
-		mcc_cb->discover_mcs(conn, err);
 	}
 
+	if (subscription_done) {
+		/* Not subscribing, or there was nothing to subscribe to */
+#ifdef CONFIG_BT_MCC_OTS
+		/* Start discovery of included services to find OTS */
+		discover_included(conn);
+#else
+		/* If OTS is not configured, discovery ends here */
+		if (mcc_cb && mcc_cb->discover_mcs) {
+			mcc_cb->discover_mcs(conn, 0);
+		}
 #endif /* CONFIG_BT_MCC_OTS */
+	}
 
 	return BT_GATT_ITER_STOP;
 }
 
-
 /* This function is called when a (primary) GMCS service has been discovered.
  * The function will store the start and end handle for the service. It will
  * then start discovery of the characteristics of the GMCS service.
@@ -1444,11 +1659,8 @@ static uint8_t discover_primary_func(struct bt_conn *conn,
 		BT_DBG("Start discovery of GMCS characteristics");
 		err = bt_gatt_discover(conn, &discover_params);
 		if (err) {
-			BT_DBG("Discover failed (err %d)", err);
+			BT_DBG("Discovery failed: %d", err);
-			cur_mcs_inst = NULL;
+			discovery_failure(conn, err);
-			if (mcc_cb && mcc_cb->discover_mcs) {
-				mcc_cb->discover_mcs(conn, err);
-			}
 		}
 		return BT_GATT_ITER_STOP;
 	}
@@ -1492,8 +1704,9 @@ int bt_mcc_init(struct bt_mcc_cb *cb)
  * The order of discovery is follows:
  * 1: Discover GMCS primary service (started here)
  * 2: Discover characteristics of GMCS
- * 3: Discover OTS service included in GMCS
+ * 3: Subscribe to characteristics of GMCS
- * 4: Discover characteristics of OTS
+ * 4: Discover OTS service included in GMCS
+ * 5: Discover characteristics of OTS and subscribe to them
  */
 int bt_mcc_discover_mcs(struct bt_conn *conn, bool subscribe)
 {