Hi Luiz,

On Monday 18 of April 2016 18:16:03 Luiz Augusto von Dentz wrote:

From: Luiz Augusto von Dentz <luiz.von.dentz(a)intel.com>

This replaces use of delayed fiber with bt_timer which uses a common
fiber and stack with other timers used by the stack.

Change-Id: Ie8f0ea778d7a415a61bd326da70cfae4131156af
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz(a)intel.com>
---
net/bluetooth/smp.c | 40 +++++++++++++++++-----------------------
1 file changed, 17 insertions(+), 23 deletions(-)

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 42f6ecf..f54b6c9 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -50,6 +50,7 @@
#include "conn_internal.h"
#include "l2cap_internal.h"
#include "smp.h"
+#include "timer.h"

#define SMP_TIMEOUT (30 * sys_clock_ticks_per_sec)

@@ -101,9 +102,6 @@ struct bt_smp {
/* The channel this context is associated with */
struct bt_l2cap_chan chan;

- /* SMP Timeout fiber handle */
- nano_thread_id_t timeout;
-
/* Commands that remote is allowed to send */
atomic_t allowed_cmds;

@@ -155,8 +153,8 @@ struct bt_smp {
/* Remote key distribution */
uint8_t remote_dist;

- /* stack for timeout fiber */
- BT_STACK(stack, 128);
+ /* Timeout timer */
+ struct bt_timer timer;
};

/* based on table 2.8 Core Spec 2.3.5.1 Vol. 3 Part H */
@@ -571,12 +569,9 @@ static void smp_reset(struct bt_smp *smp)
{
struct bt_conn *conn = smp->chan.conn;

- if (smp->timeout) {
- fiber_fiber_delayed_start_cancel(smp->timeout);
- smp->timeout = NULL;
-
- stack_analyze("smp timeout stack", smp->stack,
- sizeof(smp->stack));
+ if (smp->timer.func) {
+ bt_timer_cancel(&smp->timer);
+ smp->timer.func = NULL;
}

smp->method = JUST_WORKS;
@@ -601,15 +596,13 @@ static void smp_reset(struct bt_smp *smp)
#endif /* CONFIG_BLUETOOTH_PERIPHERAL */
}

-static void smp_timeout(int arg1, int arg2)
+static void smp_timeout(void *user_data)
{
- struct bt_smp *smp = (struct bt_smp *)arg1;
-
- ARG_UNUSED(arg2);
+ struct bt_smp *smp = user_data;

BT_ERR("SMP Timeout");

- smp->timeout = NULL;
+ smp->timer.func = NULL;

/*
* If SMP timeout occurred during key distribution we should assume
@@ -627,13 +620,13 @@ static void smp_timeout(int arg1, int arg2)

static void smp_restart_timer(struct bt_smp *smp)
{
- if (smp->timeout) {
- fiber_fiber_delayed_start_cancel(smp->timeout);
+ if (smp->timer.func) {
+ bt_timer_cancel(&smp->timer);
+ } else {
+ bt_timer_init(&smp->timer, smp_timeout, smp);
}

- smp->timeout = fiber_delayed_start(smp->stack, sizeof(smp->stack),
- smp_timeout, (int)smp, 0, 7, 0,
- SMP_TIMEOUT);
+ bt_timer_add(&smp->timer, SMP_TIMEOUT);

This is something that will likely be common case (at least in networking)
so maybe we could have bt_timer_restart()?

}

static struct net_buf *smp_create_pdu(struct bt_conn *conn, uint8_t op,
@@ -2626,8 +2619,9 @@ static void bt_smp_disconnected(struct bt_l2cap_chan
*chan)

BT_DBG("chan %p cid 0x%04x", chan, chan->tx.cid);

- if (smp->timeout) {
- fiber_fiber_delayed_start_cancel(smp->timeout);
+ if (smp->timer.func) {
+ bt_timer_cancel(&smp->timer);
+ smp->timer.func = NULL;
}

We could have helpers like bt_timer_active/running() etc. Especially since
bt_timer_init() assigns func internally. That way all fields in struct
bt_timer would be internal only.

if (keys) {

--
BR
Szymon Janc