trxcon: get rid of the timer driven clock module

trxcon was heavily inspired by osmo-bts-trx, and among with many other
scheduling related parts also inherited the timer driven clock module.

This clock module is driving the Uplink burst scheduling, just like it
does drive the Downlink burst scheduling in osmo-bts-trx.  Just like
in osmo-bts-trx, the clock module relies on periodic CLCK indications
from the PHY, which are needed to compensate for the clock drifting.

The key difference is that trxcon is using Downlink bursts as the CLCK
indications, see 'bi.fn % 51' in trx_data_rx_cb().  This is possible
because the MS is a clock slave of the BTS: the MS PHY needs to sync
its freq. and clock first, and only after that it can Rx and Tx.

So far we've had no problems with the clock module in trxcon until we
started adding GPRS support and integrated the l1gprs.  While the CS
domain is quite flexible in terms of timings and delays, the PS domain
is a lot more sensetive to the timing issues.

Sometimes it happens that the trxcon's clock module is ticking quicker
than it should, resulting in Uplink PDCH blocks being scheduled earlier
than the respective Downlink PDCH blocks are received:

20230502021957724 l1sched_pull_burst(): PDTCH/U Tx time (fn=56103)
20230502021957744 (PDCH-7) Rx DL BLOCK.ind (fn=56103, len=23): ...
20230502021957747 l1sched_pull_burst(): PDTCH/U Tx time (fn=56108)
20230502021957765 l1sched_pull_burst(): PDTCH/U Tx time (fn=56112)
20230502021957767 (PDCH-7) Rx DL BLOCK.ind (fn=56108, len=23): ...
20230502021957768 (PDCH-7) Rx UL BLOCK.req (fn=56112, len=54): ...
20230502021957784 l1sched_pull_burst(): PDTCH/U Tx time (fn=56116)
20230502021957784 TS7-PDTCH dropping Tx primitive (current Fn=56116, prim Fn=56112)

This is impossible in reality, because Uplink is intentionally lagging
behind Downlink by 3 TDMA timeslot periods.  In a virtual setup this
causes sporadic dropping of Uplink PDCH blocks, as can be seen from
the logging snippet above, and significantly degrades the RLC/MAC
performance for GPRS.

Let's remove the internal clock module and trigger the Uplink burst
transmission each time we receive a Downlink burst.  This helps to
overcome the GPRS scheduling issues and replicates the approach of
osmo-trx-ms more closely.

Change-Id: Ic8a5b6277c6b16392026e0557376257d71c9d230
Related: OS#5500

11 files changed, 16 insertions, 263 deletions

diff --git a/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h b/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h
index 39aeeb92..d3b079ca 100644
--- a/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h
+++ b/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h
@@ -41,11 +41,6 @@ struct l1sched_meas_set;
 struct l1sched_state;
 struct l1sched_ts;
 
-enum l1sched_clck_state {
-	L1SCHED_CLCK_ST_WAIT,
-	L1SCHED_CLCK_ST_OK,
-};
-
 enum l1sched_burst_type {
 	L1SCHED_BURST_GMSK,
 	L1SCHED_BURST_8PSK,
@@ -319,24 +314,10 @@ struct l1sched_ts {
 struct l1sched_cfg {
 	/*! Logging context (used as prefix for messages) */
 	const char *log_prefix;
-	/*! TDMA frame-number advance */
-	uint32_t fn_advance;
 };
 
 /*! One scheduler instance */
 struct l1sched_state {
-	/*! Clock state */
-	enum l1sched_clck_state clck_state;
-	/*! Local clock source */
-	struct timespec clock;
-	/*! Count of processed frames */
-	uint32_t fn_counter_proc;
-	/*! Local frame counter advance */
-	uint32_t fn_counter_advance;
-	/*! Count of lost frames */
-	uint32_t fn_counter_lost;
-	/*! Frame callback timer */
-	struct osmo_timer_list clock_timer;
 	/*! List of timeslots maintained by this scheduler */
 	struct l1sched_ts *ts[TRX_TS_COUNT];
 	/*! SACCH cache (common for all lchans) */
diff --git a/src/host/trxcon/include/osmocom/bb/trxcon/phyif.h b/src/host/trxcon/include/osmocom/bb/trxcon/phyif.h
index abda393e..2ad7a678 100644
--- a/src/host/trxcon/include/osmocom/bb/trxcon/phyif.h
+++ b/src/host/trxcon/include/osmocom/bb/trxcon/phyif.h
@@ -110,7 +110,6 @@ struct trxcon_phyif_burst_ind {
 
 int trxcon_phyif_handle_burst_req(void *phyif, const struct trxcon_phyif_burst_req *br);
 int trxcon_phyif_handle_burst_ind(void *priv, const struct trxcon_phyif_burst_ind *bi);
-int trxcon_phyif_handle_clock_ind(void *priv, uint32_t fn);
 
 int trxcon_phyif_handle_rts_ind(void *priv, const struct trxcon_phyif_rts_ind *rts);
 int trxcon_phyif_handle_rtr_ind(void *priv, const struct trxcon_phyif_rtr_ind *ind,
diff --git a/src/host/trxcon/include/osmocom/bb/trxcon/trx_if.h b/src/host/trxcon/include/osmocom/bb/trxcon/trx_if.h
index 0243dd56..e564fd8e 100644
--- a/src/host/trxcon/include/osmocom/bb/trxcon/trx_if.h
+++ b/src/host/trxcon/include/osmocom/bb/trxcon/trx_if.h
@@ -24,6 +24,7 @@ struct trx_instance {
 	struct osmo_timer_list trx_ctrl_timer;
 	struct llist_head trx_ctrl_list;
 	struct osmo_fsm_inst *fi;
+	uint32_t fn_advance;
 
 	/* HACK: we need proper state machines */
 	uint32_t prev_state;
@@ -45,6 +46,7 @@ struct trx_if_params {
 	const char *local_host;
 	const char *remote_host;
 	uint16_t base_port;
+	uint32_t fn_advance;
 	uint8_t instance;
 
 	struct osmo_fsm_inst *parent_fi;
diff --git a/src/host/trxcon/include/osmocom/bb/trxcon/trxcon.h b/src/host/trxcon/include/osmocom/bb/trxcon/trxcon.h
index 1d012de2..3d85d59a 100644
--- a/src/host/trxcon/include/osmocom/bb/trxcon/trxcon.h
+++ b/src/host/trxcon/include/osmocom/bb/trxcon/trxcon.h
@@ -55,7 +55,7 @@ enum trxcon_log_cat {
 
 void trxcon_set_log_cfg(const int *logc, unsigned int logc_num);
 
-struct trxcon_inst *trxcon_inst_alloc(void *ctx, unsigned int id, uint32_t fn_advance);
+struct trxcon_inst *trxcon_inst_alloc(void *ctx, unsigned int id);
 void trxcon_inst_free(struct trxcon_inst *trxcon);
 
 int trxcon_l1ctl_receive(struct trxcon_inst *trxcon, struct msgb *msg);
diff --git a/src/host/trxcon/src/Makefile.am b/src/host/trxcon/src/Makefile.am
index c96949a2..7be7de62 100644
--- a/src/host/trxcon/src/Makefile.am
+++ b/src/host/trxcon/src/Makefile.am
@@ -23,7 +23,6 @@ libl1sched_la_SOURCES = \
 	sched_lchan_rach.c \
 	sched_lchan_sch.c \
 	sched_mframe.c \
-	sched_clck.c \
 	sched_prim.c \
 	sched_trx.c \
 	$(NULL)
diff --git a/src/host/trxcon/src/sched_clck.c b/src/host/trxcon/src/sched_clck.c
deleted file mode 100644
index f9eadaf5..00000000
--- a/src/host/trxcon/src/sched_clck.c
+++ /dev/null
@@ -1,200 +0,0 @@
-/*
- * OsmocomBB <-> SDR connection bridge
- * TDMA scheduler: clock synchronization
- *
- * (C) 2013 by Andreas Eversberg <jolly@eversberg.eu>
- * (C) 2015 by Alexander Chemeris <Alexander.Chemeris@fairwaves.co>
- * (C) 2015 by Harald Welte <laforge@gnumonks.org>
- *
- * All Rights Reserved
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License as published by
- * the Free Software Foundation; either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- */
-
-#include <stdio.h>
-#include <errno.h>
-#include <stdlib.h>
-#include <stdint.h>
-#include <inttypes.h>
-#include <string.h>
-
-#include <osmocom/core/logging.h>
-#include <osmocom/core/talloc.h>
-#include <osmocom/core/msgb.h>
-#include <osmocom/core/timer.h>
-#include <osmocom/core/timer_compat.h>
-
-#include <osmocom/bb/l1sched/l1sched.h>
-#include <osmocom/bb/l1sched/logging.h>
-
-#define MAX_FN_SKEW		50
-#define TRX_LOSS_FRAMES	400
-
-static void l1sched_clck_tick(void *data)
-{
-	struct l1sched_state *sched = (struct l1sched_state *) data;
-	struct timespec tv_now, *tv_clock, elapsed;
-	int64_t elapsed_us;
-	const struct timespec frame_duration = { .tv_sec = 0, .tv_nsec = GSM_TDMA_FN_DURATION_nS };
-
-	/* Check if transceiver is still alive */
-	if (sched->fn_counter_lost++ == TRX_LOSS_FRAMES) {
-		LOGP_SCHEDC(sched, LOGL_NOTICE, "No more clock from transceiver\n");
-		sched->clck_state = L1SCHED_CLCK_ST_WAIT;
-
-		return;
-	}
-
-	/* Get actual / previous frame time */
-	osmo_clock_gettime(CLOCK_MONOTONIC, &tv_now);
-	tv_clock = &sched->clock;
-
-	timespecsub(&tv_now, tv_clock, &elapsed);
-	elapsed_us = (elapsed.tv_sec * 1000000) + (elapsed.tv_nsec / 1000);
-
-	/* If someone played with clock, or if the process stalled */
-	if (elapsed_us > GSM_TDMA_FN_DURATION_uS * MAX_FN_SKEW || elapsed_us < 0) {
-		LOGP_SCHEDC(sched, LOGL_NOTICE, "PC clock skew: "
-			    "elapsed uS %" PRId64 "\n", elapsed_us);
-
-		sched->clck_state = L1SCHED_CLCK_ST_WAIT;
-
-		return;
-	}
-
-	/* Schedule next FN clock */
-	while (elapsed_us > GSM_TDMA_FN_DURATION_uS / 2) {
-		timespecadd(tv_clock, &frame_duration, tv_clock);
-		elapsed_us -= GSM_TDMA_FN_DURATION_uS;
-
-		GSM_TDMA_FN_INC(sched->fn_counter_proc);
-
-		/* Trigger the scheduler */
-		l1sched_pull_send_frame(sched);
-	}
-
-	osmo_timer_schedule(&sched->clock_timer, 0,
-		GSM_TDMA_FN_DURATION_uS - elapsed_us);
-}
-
-static void l1sched_clck_correct(struct l1sched_state *sched,
-	struct timespec *tv_now, uint32_t fn)
-{
-	sched->fn_counter_proc = fn;
-
-	/* Trigger the scheduler */
-	l1sched_pull_send_frame(sched);
-
-	/* Schedule first FN clock */
-	sched->clock = *tv_now;
-	memset(&sched->clock_timer, 0, sizeof(sched->clock_timer));
-
-	sched->clock_timer.cb = l1sched_clck_tick;
-	sched->clock_timer.data = sched;
-	osmo_timer_schedule(&sched->clock_timer, 0, GSM_TDMA_FN_DURATION_uS);
-}
-
-int l1sched_clck_handle(struct l1sched_state *sched, uint32_t fn)
-{
-	struct timespec tv_now, *tv_clock, elapsed;
-	int64_t elapsed_us, elapsed_fn;
-
-	/* Reset lost counter */
-	sched->fn_counter_lost = 0;
-
-	/* Get actual / previous frame time */
-	osmo_clock_gettime(CLOCK_MONOTONIC, &tv_now);
-	tv_clock = &sched->clock;
-
-	/* If this is the first CLCK IND */
-	if (sched->clck_state == L1SCHED_CLCK_ST_WAIT) {
-		l1sched_clck_correct(sched, &tv_now, fn);
-
-		LOGP_SCHEDC(sched, LOGL_NOTICE, "Initial clock received: fn=%u\n", fn);
-		sched->clck_state = L1SCHED_CLCK_ST_OK;
-
-		return 0;
-	}
-
-	LOGP_SCHEDC(sched, LOGL_DEBUG, "Clock indication: fn=%u\n", fn);
-
-	osmo_timer_del(&sched->clock_timer);
-
-	/* Calculate elapsed time / frames since last processed fn */
-	timespecsub(&tv_now, tv_clock, &elapsed);
-	elapsed_us = (elapsed.tv_sec * 1000000) + (elapsed.tv_nsec / 1000);
-	elapsed_fn = GSM_TDMA_FN_SUB(fn, sched->fn_counter_proc);
-
-	if (elapsed_fn >= 135774)
-		elapsed_fn -= GSM_TDMA_HYPERFRAME;
-
-	/* Check for max clock skew */
-	if (elapsed_fn > MAX_FN_SKEW || elapsed_fn < -MAX_FN_SKEW) {
-		LOGP_SCHEDC(sched, LOGL_NOTICE, "GSM clock skew: old fn=%u, "
-			    "new fn=%u\n", sched->fn_counter_proc, fn);
-
-		l1sched_clck_correct(sched, &tv_now, fn);
-		return 0;
-	}
-
-	LOGP_SCHEDC(sched, LOGL_DEBUG, "GSM clock jitter: %" PRId64 "\n",
-		    elapsed_fn * GSM_TDMA_FN_DURATION_uS - elapsed_us);
-
-	/* Too many frames have been processed already */
-	if (elapsed_fn < 0) {
-		struct timespec duration;
-		/**
-		 * Set clock to the time or last FN should
-		 * have been transmitted
-		 */
-		duration.tv_nsec = (0 - elapsed_fn) * GSM_TDMA_FN_DURATION_nS;
-		duration.tv_sec = duration.tv_nsec / 1000000000;
-		duration.tv_nsec = duration.tv_nsec % 1000000000;
-		timespecadd(&tv_now, &duration, tv_clock);
-
-		/* Set time to the time our next FN has to be transmitted */
-		osmo_timer_schedule(&sched->clock_timer, 0,
-			GSM_TDMA_FN_DURATION_uS * (1 - elapsed_fn));
-
-		return 0;
-	}
-
-	/* Transmit what we still need to transmit */
-	while (fn != sched->fn_counter_proc) {
-		GSM_TDMA_FN_INC(sched->fn_counter_proc);
-
-		/* Trigger the scheduler */
-		l1sched_pull_send_frame(sched);
-	}
-
-	/* Schedule next FN to be transmitted */
-	*tv_clock = tv_now;
-	osmo_timer_schedule(&sched->clock_timer, 0, GSM_TDMA_FN_DURATION_uS);
-
-	return 0;
-}
-
-void l1sched_clck_reset(struct l1sched_state *sched)
-{
-	/* Reset internal state */
-	sched->clck_state = L1SCHED_CLCK_ST_WAIT;
-
-	/* Stop clock timer */
-	osmo_timer_del(&sched->clock_timer);
-
-	/* Flush counters */
-	sched->fn_counter_proc = 0;
-	sched->fn_counter_lost = 0;
-}
diff --git a/src/host/trxcon/src/sched_trx.c b/src/host/trxcon/src/sched_trx.c
index ccac32c5..3b452b16 100644
--- a/src/host/trxcon/src/sched_trx.c
+++ b/src/host/trxcon/src/sched_trx.c
@@ -152,27 +152,6 @@ void l1sched_pull_burst(struct l1sched_state *sched, struct l1sched_burst_req *b
 		l1sched_a5_burst_enc(lchan, br);
 }
 
-/* Pull *and send* Uplink bursts for all timeslots and the current TDMA Fn. */
-void l1sched_pull_send_frame(struct l1sched_state *sched)
-{
-	/* Advance TDMA frame number in order to give the transceiver
-	 * more time to handle the burst before the actual transmission. */
-	const uint32_t fn = GSM_TDMA_FN_SUM(sched->fn_counter_proc,
-					    sched->fn_counter_advance);
-
-	/* Iterate over timeslot list */
-	for (unsigned int tn = 0; tn < ARRAY_SIZE(sched->ts); tn++) {
-		struct l1sched_burst_req br = {
-			.fn = fn,
-			.tn = tn,
-			.burst_len = 0, /* NOPE.ind */
-		};
-
-		l1sched_pull_burst(sched, &br);
-		l1sched_handle_burst_req(sched, &br);
-	}
-}
-
 void l1sched_logging_init(int log_cat_common, int log_cat_data)
 {
 	l1sched_log_cat_common = log_cat_common;
@@ -188,8 +167,6 @@ struct l1sched_state *l1sched_alloc(void *ctx, const struct l1sched_cfg *cfg, vo
 		return NULL;
 
 	*sched = (struct l1sched_state) {
-		/* .clock_timer is set up in l1sched_clck_correct() */
-		.fn_counter_advance = cfg->fn_advance,
 		.priv = priv,
 	};
 
@@ -216,7 +193,6 @@ void l1sched_free(struct l1sched_state *sched)
 	for (tn = 0; tn < ARRAY_SIZE(sched->ts); tn++)
 		l1sched_del_ts(sched, tn);
 
-	l1sched_clck_reset(sched);
 	talloc_free(sched);
 }
 
@@ -234,10 +210,6 @@ void l1sched_reset(struct l1sched_state *sched, bool reset_clock)
 	for (tn = 0; tn < ARRAY_SIZE(sched->ts); tn++)
 		l1sched_del_ts(sched, tn);
 
-	/* Stop and reset clock counter if required */
-	if (reset_clock)
-		l1sched_clck_reset(sched);
-
 	memcpy(&sched->sacch_cache[0], &meas_rep_dummy[0], sizeof(meas_rep_dummy));
 }
 
diff --git a/src/host/trxcon/src/trx_if.c b/src/host/trxcon/src/trx_if.c
index 330fd62b..d98bea2a 100644
--- a/src/host/trxcon/src/trx_if.c
+++ b/src/host/trxcon/src/trx_if.c
@@ -701,10 +701,16 @@ static int trx_data_rx_cb(struct osmo_fd *ofd, unsigned int what)
 		  "RX burst tn=%u fn=%u rssi=%d toa=%d\n",
 		  bi.tn, bi.fn, bi.rssi, bi.toa256);
 
-	if (bi.fn % 51 == 0)
-		trxcon_phyif_handle_clock_ind(trx->priv, bi.fn);
+	trxcon_phyif_handle_burst_ind(trx->priv, &bi);
 
-	return trxcon_phyif_handle_burst_ind(trx->priv, &bi);
+	struct trxcon_phyif_rts_ind rts = {
+		.fn = GSM_TDMA_FN_SUM(bi.fn, trx->fn_advance),
+		.tn = bi.tn,
+	};
+
+	trxcon_phyif_handle_rts_ind(trx->priv, &rts);
+
+	return 0;
 }
 
 int trx_if_handle_phyif_burst_req(struct trx_instance *trx,
@@ -797,6 +803,7 @@ struct trx_instance *trx_if_open(const struct trx_if_params *params)
 	if (rc < 0)
 		goto udp_error;
 
+	trx->fn_advance = params->fn_advance;
 	trx->priv = params->priv;
 	fi->priv = trx;
 	trx->fi = fi;
diff --git a/src/host/trxcon/src/trxcon_inst.c b/src/host/trxcon/src/trxcon_inst.c
index 65e63b3a..b7ff5810 100644
--- a/src/host/trxcon/src/trxcon_inst.c
+++ b/src/host/trxcon/src/trxcon_inst.c
@@ -63,7 +63,7 @@ void trxcon_set_log_cfg(const int *logc, unsigned int logc_num)
 	l1sched_logging_init(schc, schd);
 }
 
-struct trxcon_inst *trxcon_inst_alloc(void *ctx, unsigned int id, uint32_t fn_advance)
+struct trxcon_inst *trxcon_inst_alloc(void *ctx, unsigned int id)
 {
 	struct trxcon_inst *trxcon;
 	struct osmo_fsm_inst *fi;
@@ -85,7 +85,6 @@ struct trxcon_inst *trxcon_inst_alloc(void *ctx, unsigned int id, uint32_t fn_ad
 
 	/* Init scheduler */
 	const struct l1sched_cfg sched_cfg = {
-		.fn_advance = fn_advance,
 		.log_prefix = trxcon->log_prefix,
 	};
 
diff --git a/src/host/trxcon/src/trxcon_main.c b/src/host/trxcon/src/trxcon_main.c
index d30c98e0..7298f5a2 100644
--- a/src/host/trxcon/src/trxcon_main.c
+++ b/src/host/trxcon/src/trxcon_main.c
@@ -127,7 +127,7 @@ static void l1ctl_conn_accept_cb(struct l1ctl_client *l1c)
 {
 	struct trxcon_inst *trxcon;
 
-	trxcon = trxcon_inst_alloc(l1c, l1c->id, app_data.trx_fn_advance);
+	trxcon = trxcon_inst_alloc(l1c, l1c->id);
 	if (trxcon == NULL) {
 		l1ctl_client_conn_close(l1c);
 		return;
@@ -141,6 +141,7 @@ static void l1ctl_conn_accept_cb(struct l1ctl_client *l1c)
 		.local_host = app_data.trx_bind_ip,
 		.remote_host = app_data.trx_remote_ip,
 		.base_port = app_data.trx_base_port,
+		.fn_advance = app_data.trx_fn_advance,
 		.instance = trxcon->id,
 
 		.parent_fi = trxcon->fi,
diff --git a/src/host/trxcon/src/trxcon_shim.c b/src/host/trxcon/src/trxcon_shim.c
index 8b42efe2..54d976a1 100644
--- a/src/host/trxcon/src/trxcon_shim.c
+++ b/src/host/trxcon/src/trxcon_shim.c
@@ -237,13 +237,6 @@ int trxcon_phyif_handle_burst_ind(void *priv, const struct trxcon_phyif_burst_in
 	return l1sched_handle_rx_burst(trxcon->sched, &bi);
 }
 
-int trxcon_phyif_handle_clock_ind(void *priv, uint32_t fn)
-{
-	struct trxcon_inst *trxcon = priv;
-
-	return l1sched_clck_handle(trxcon->sched, fn);
-}
-
 int trxcon_phyif_handle_rsp(void *priv, const struct trxcon_phyif_rsp *rsp)
 {
 	struct trxcon_inst *trxcon = priv;