Commit 7cdbeb54 authored by Piotr Pryga's avatar Piotr Pryga Committed by Carles Cufi
Browse files

Bluetooth: controller: Use common mem_link_rx for node_rx_iq_report



Change use of dedicated memory pool for linked list nodes for
node_rx_iq_report to common mem_link_rx. Former solution had
a drawback. Released link nodes may be enqueued to wrong memory
pool. E.g. link related with nopde_rx_iq_report went to common
link memory pool, whereas link nodes from common pool were enqueued
to dedicated list.
The solution was working because links have the same memory layout,
just different memory pools they originated from.
The problem may occur if one of those link memory pools is reset.
Then the same link may be used by node_rx and node_rx_iq_report
at the same time, causing controller failure.

Signed-off-by: default avatarPiotr Pryga <piotr.pryga@nordicsemi.no>
parent 6c76b70a
Loading
Loading
Loading
Loading
+2 −1
Original line number Diff line number Diff line
@@ -49,7 +49,8 @@
#include "ll_sw/ull_conn_types.h"
#include "ll_sw/ull_conn_internal.h"
#include "ll_sw/ull_conn_iso_types.h"
#include "ll_sw/ull_df.h"
#include "ll_sw/ull_df_types.h"
#include "ll_sw/ull_df_internal.h"

#include "ll.h"
#include "ll_feat.h"
+12 −18
Original line number Diff line number Diff line
@@ -46,7 +46,8 @@
#include "ull_sync_types.h"
#include "ull_conn_types.h"
#include "ull_filter.h"
#include "ull_df.h"
#include "ull_df_types.h"
#include "ull_df_internal.h"

#include "isoal.h"
#include "ull_internal.h"
@@ -369,10 +370,11 @@ static struct {
 * happen due to supervision timeout and other reasons that dont have an
 * incoming Rx-ed PDU).
 */
#define LINK_RX_POOL_SIZE (sizeof(memq_link_t) * \
#define LINK_RX_POOL_SIZE                                                      \
	(sizeof(memq_link_t) *                                                 \
	 (RX_CNT + 2 + BT_CTLR_MAX_CONN + BT_CTLR_ADV_SET +                    \
			    (BT_CTLR_SCAN_SYNC_SET * 2) + \
			    (BT_CTLR_SCAN_SYNC_ISO_SET * 2)))
	  (BT_CTLR_SCAN_SYNC_SET * 2) + (BT_CTLR_SCAN_SYNC_ISO_SET * 2) +      \
	  (IQ_REPORT_CNT)))
static struct {
	uint8_t quota_pdu; /* Number of un-utilized buffers */

@@ -831,12 +833,7 @@ void ll_rx_dequeue(void)
			    (void **)&rx);
	LL_ASSERT(link);

#if defined(CONFIG_BT_CTLR_DF_SCAN_CTE_RX)
	if (rx->type != NODE_RX_TYPE_IQ_SAMPLE_REPORT)
#endif /* CONFIG_BT_CTLR_DF_SCAN_CTE_RX */
	{
	mem_release(link, &mem_link_rx.free);
	}

	/* handle object specific clean up */
	switch (rx->type) {
@@ -925,13 +922,6 @@ void ll_rx_dequeue(void)
	}
	break;
#endif /* CONFIG_BT_BROADCASTER */
#if defined(CONFIG_BT_CTLR_DF_SCAN_CTE_RX)
	case NODE_RX_TYPE_IQ_SAMPLE_REPORT:
	{
		ull_df_iq_report_link_release(link);
	}
	break;
#endif /* CONFIG_BT_CTLR_DF_SCAN_CTE_RX */
#endif /* CONFIG_BT_CTLR_ADV_EXT */

#if defined(CONFIG_BT_CONN)
@@ -1111,6 +1101,10 @@ void ll_rx_dequeue(void)
	case NODE_RX_TYPE_ISO_PDU:
#endif

#if defined(CONFIG_BT_CTLR_DF_SCAN_CTE_RX)
	case NODE_RX_TYPE_IQ_SAMPLE_REPORT:
#endif /* CONFIG_BT_CTLR_DF_SCAN_CTE_RX */

	/* Ensure that at least one 'case' statement is present for this
	 * code block.
	 */
+12 −40
Original line number Diff line number Diff line
@@ -33,7 +33,8 @@
#include "ull_sync_types.h"
#include "ull_sync_internal.h"
#include "ull_adv_types.h"
#include "ull_df.h"
#include "ull_df_types.h"
#include "ull_df_internal.h"

#include "ull_adv_internal.h"
#include "ull_internal.h"
@@ -53,18 +54,6 @@
#define IQ_REPORT_STRUCT_OVERHEAD  (IQ_REPORT_HEADER_SIZE)
#define IQ_SAMPLE_SIZE (sizeof(struct iq_sample))

/* TODO Verify required number of IQ reports.
 * At the moment it is set to 2 (if CONFIG_BT_PER_ADV_SYNC_MAX is set the value
 * is multiplied by 2):
 * - for LLL -> ULL
 * - for ULL -> LL(HCI).
 */
#if defined(CONFIG_BT_PER_ADV_SYNC_MAX)
#define IQ_REPORT_CNT (CONFIG_BT_PER_ADV_SYNC_MAX * 2)
#else
#define IQ_REPORT_CNT 2
#endif

#define IQ_REPORT_RX_NODE_POOL_ELEMENT_SIZE              \
	MROUND(IQ_REPORT_STRUCT_OVERHEAD + (IQ_SAMPLE_TOTAL_CNT * IQ_SAMPLE_SIZE))
#define IQ_REPORT_POOL_SIZE (IQ_REPORT_RX_NODE_POOL_ELEMENT_SIZE * IQ_REPORT_CNT)
@@ -75,18 +64,11 @@ static struct {
	uint8_t pool[IQ_REPORT_POOL_SIZE];
} mem_iq_report;

#define LINK_IQ_REPORT_POOL_SIZE (sizeof(memq_link_t) * IQ_REPORT_CNT)

/* Linked list to store pointers to mem_iq_report memory pool for IQ reports. */
static struct {
	uint8_t quota_pdu; /* Number of un-utilized buffers */

	void *free;
	uint8_t pool[LINK_IQ_REPORT_POOL_SIZE];
} mem_link_iq_report;

/* FIFO to store free IQ report norde_rx objects. */
static MFIFO_DEFINE(iq_report_free, sizeof(void *), IQ_REPORT_CNT);

/* Number of available instance of linked list to be used for node_rx_iq_reports. */
static uint8_t mem_link_iq_report_quota_pdu;
#endif /* CONFIG_BT_CTLR_DF_SCAN_CTE_RX */

/* ToDo:
@@ -168,13 +150,8 @@ static int init_reset(void)
		 sizeof(mem_iq_report.pool) / (IQ_REPORT_RX_NODE_POOL_ELEMENT_SIZE),
		 &mem_iq_report.free);

	/* Initialize IQ report link pool. */
	mem_init(mem_link_iq_report.pool, sizeof(memq_link_t),
		 sizeof(mem_link_iq_report.pool) / sizeof(memq_link_t),
		 &mem_link_iq_report.free);

	/* Allocate free IQ report node rx */
	mem_link_iq_report.quota_pdu = IQ_REPORT_CNT;
	mem_link_iq_report_quota_pdu = IQ_REPORT_CNT;
	ull_df_rx_iq_report_alloc(UINT8_MAX);
#endif /* CONFIG_BT_CTLR_DF_SCAN_CTE_RX */
	return 0;
@@ -582,11 +559,6 @@ void *ull_df_iq_report_alloc(void)
	return MFIFO_DEQUEUE(iq_report_free);
}

void ull_df_iq_report_link_release(memq_link_t *link)
{
	mem_release(link, &mem_link_iq_report.free);
}

void ull_df_iq_report_mem_release(struct node_rx_hdr *rx)
{
	mem_release(rx, &mem_iq_report.free);
@@ -594,30 +566,30 @@ void ull_df_iq_report_mem_release(struct node_rx_hdr *rx)

void ull_iq_report_link_inc_quota(int8_t delta)
{
	LL_ASSERT(delta <= 0 || mem_link_iq_report.quota_pdu < IQ_REPORT_CNT);
	mem_link_iq_report.quota_pdu += delta;
	LL_ASSERT(delta <= 0 || mem_link_iq_report_quota_pdu < IQ_REPORT_CNT);
	mem_link_iq_report_quota_pdu += delta;
}

void ull_df_rx_iq_report_alloc(uint8_t max)
{
	uint8_t idx;

	if (max > mem_link_iq_report.quota_pdu) {
		max = mem_link_iq_report.quota_pdu;
	if (max > mem_link_iq_report_quota_pdu) {
		max = mem_link_iq_report_quota_pdu;
	}

	while ((max--) && MFIFO_ENQUEUE_IDX_GET(iq_report_free, &idx)) {
		memq_link_t *link;
		struct node_rx_hdr *rx;

		link = mem_acquire(&mem_link_iq_report.free);
		link = ll_rx_link_alloc();
		if (!link) {
			return;
		}

		rx = mem_acquire(&mem_iq_report.free);
		if (!rx) {
			mem_release(link, &mem_link_iq_report.free);
			ll_rx_link_release(link);
			return;
		}

+0 −17
Original line number Diff line number Diff line
@@ -4,23 +4,6 @@
 * SPDX-License-Identifier: Apache-2.0
 */

/* @brief Direction Finding switching sampling rates
 *
 * The enum provides information about supported switching
 * and sampling rates in different Direction Finding types:
 * - Angle of departure: 1us switching for transmission
 * - Angle of departure 1us sampling for reception
 * - Angle of arrival 1us switching-sampling for reception.
 *
 * @note Pay attention that both types AoD and AoA
 * support 2US switching-sampling as mandatory.
 */
enum df_switch_sample_support {
	DF_AOD_1US_TX   = BIT(0),
	DF_AOD_1US_RX   = BIT(1),
	DF_AOA_1US      = BIT(2)
};

int ull_df_init(void);
int ull_df_reset(void);

+34 −0
Original line number Diff line number Diff line
/*
 * Copyright (c) 2021 Nordic Semiconductor ASA
 *
 * SPDX-License-Identifier: Apache-2.0
 */

/* @brief Direction Finding switching sampling rates
 *
 * The enum provides information about supported switching
 * and sampling rates in different Direction Finding types:
 * - Angle of departure: 1us switching for transmission
 * - Angle of departure 1us sampling for reception
 * - Angle of arrival 1us switching-sampling for reception.
 *
 * @note Pay attention that both types AoD and AoA
 * support 2US switching-sampling as mandatory.
 */
enum df_switch_sample_support {
	DF_AOD_1US_TX = BIT(0),
	DF_AOD_1US_RX = BIT(1),
	DF_AOA_1US = BIT(2)
};

/* TODO Verify required number of IQ reports.
 * At the moment it is set to 2 (if CONFIG_BT_PER_ADV_SYNC_MAX is set the value
 * is multiplied by 2):
 * - for LLL -> ULL
 * - for ULL -> LL(HCI).
 */
#if defined(CONFIG_BT_PER_ADV_SYNC_MAX)
#define IQ_REPORT_CNT (CONFIG_BT_PER_ADV_SYNC_MAX * 2)
#else
#define IQ_REPORT_CNT 0
#endif
Loading