L1A/L23 interface (L1CTL) cleanup

* introduce a new 'l1ctl_hdr' structure common to all messages
  on this interface
* use struct l1ctl_hdr in both the firmware and layer23
* add a new L1CTL_PM_REQ request for performing layer23-initiated
  power measurements (firmware does not implement them yet)

3 files changed, 58 insertions, 28 deletions

diff --git a/src/host/layer23/src/l1ctl.c b/src/host/layer23/src/l1ctl.c
index 7289e95d..5a94c111 100644
--- a/src/host/layer23/src/l1ctl.c
+++ b/src/host/layer23/src/l1ctl.c
@@ -25,6 +25,9 @@
 #include <stdint.h>
 #include <string.h>
 #include <errno.h>
+
+#include <arpa/inet.h>
+
 #include <l1a_l23_interface.h>
 
 #include <osmocore/timer.h>
@@ -43,7 +46,7 @@
 
 static struct msgb *osmo_l1_alloc(uint8_t msg_type)
 {
-	struct l1ctl_info_ul *ul;
+	struct l1ctl_hdr *l1h;
 	struct msgb *msg = msgb_alloc_headroom(256, 4, "osmo_l1");
 
 	if (!msg) {
@@ -51,9 +54,9 @@ static struct msgb *osmo_l1_alloc(uint8_t msg_type)
 		return NULL;
 	}
 
-	msg->l1h = msgb_put(msg, sizeof(*ul));
-	ul = (struct l1ctl_info_ul *) msg->l1h;
-	ul->msg_type = msg_type;
+	msg->l1h = msgb_put(msg, sizeof(*l1h));
+	l1h = (struct l1ctl_hdr *) msg->l1h;
+	l1h->msg_type = msg_type;
 	
 	return msg;
 }
@@ -77,7 +80,7 @@ static int rx_l1_ccch_resp(struct osmocom_ms *ms, struct msgb *msg)
 	}
 
 	dl = (struct l1ctl_info_dl *) msg->l1h;
-	sb = (struct l1ctl_sync_new_ccch_resp *) msg->l2h;
+	sb = (struct l1ctl_sync_new_ccch_resp *) dl->payload;
 
 	gsm_fn2gsmtime(&tm, ntohl(dl->frame_nr));
 	printf("SCH: SNR: %u TDMA: (%.4u/%.2u/%.2u) bsic: %d\n",
@@ -129,6 +132,7 @@ static int rx_ph_data_ind(struct osmocom_ms *ms, struct msgb *msg)
 	}
 
 	dl = (struct l1ctl_info_dl *) msg->l1h;
+	msg->l2h = dl->payload;
 	ccch = (struct l1ctl_data_ind *) msg->l2h;
 
 	gsm_fn2gsmtime(&tm, ntohl(dl->frame_nr));
@@ -151,7 +155,7 @@ static int rx_ph_data_ind(struct osmocom_ms *ms, struct msgb *msg)
 	memcpy(&dl_cpy, dl, sizeof(dl_cpy));
 
 	/* pull the L1 header from the msgb */
-	msgb_pull(msg, msg->l2h - msg->l1h);
+	msgb_pull(msg, msg->l2h - (msg->l1h-sizeof(struct l1ctl_hdr)));
 	msg->l1h = NULL;
 
 	/* send it up into LAPDm */
@@ -164,6 +168,7 @@ static int rx_ph_data_ind(struct osmocom_ms *ms, struct msgb *msg)
 int tx_ph_data_req(struct osmocom_ms *ms, struct msgb *msg,
 		   uint8_t chan_nr, uint8_t link_id)
 {
+	struct l1ctl_hdr *l1h;
 	struct l1ctl_info_ul *l1i_ul;
 	uint8_t chan_type, chan_ts, chan_ss;
 	uint8_t gsmtap_chan_type;
@@ -184,11 +189,7 @@ int tx_ph_data_req(struct osmocom_ms *ms, struct msgb *msg,
 			0, 127, 255, msg->l2h, msgb_l2len(msg));
 
 	/* prepend uplink info header */
-	printf("sizeof(struct l1ctl_info_ul)=%lu\n", sizeof(*l1i_ul));
-	msg->l1h = msgb_push(msg, sizeof(*l1i_ul));
-	l1i_ul = (struct l1ctl_info_ul *) msg->l1h;
-
-	l1i_ul->msg_type = L1CTL_DATA_REQ;
+	l1i_ul = (struct l1ctl_info_ul *) msgb_push(msg, sizeof(*l1i_ul));
 
 	l1i_ul->chan_nr = chan_nr;
 	l1i_ul->link_id = link_id;
@@ -196,6 +197,11 @@ int tx_ph_data_req(struct osmocom_ms *ms, struct msgb *msg,
 	/* FIXME: where to get this from? */
 	l1i_ul->tx_power = 0;
 
+	/* prepend l1 header */
+	msg->l1h = msgb_push(msg, sizeof(*l1h));
+	l1h = (struct l1ctl_hdr *) msg->l1h;
+	l1h->msg_type = L1CTL_DATA_REQ;
+
 	return osmo_send_l1(ms, msg);
 }
 
@@ -220,6 +226,7 @@ static int rx_l1_reset(struct osmocom_ms *ms)
 int tx_ph_rach_req(struct osmocom_ms *ms)
 {
 	struct msgb *msg;
+	struct l1ctl_info_ul *ul;
 	struct l1ctl_rach_req *req;
 	static uint8_t i = 0;
 
@@ -228,6 +235,7 @@ int tx_ph_rach_req(struct osmocom_ms *ms)
 		return -1;
 
 	printf("RACH Req.\n");
+	ul = (struct l1ctl_info_ul *) msgb_put(msg, sizeof(*ul));
 	req = (struct l1ctl_rach_req *) msgb_put(msg, sizeof(*req));
 	req->ra = i++;
 
@@ -247,7 +255,7 @@ int tx_ph_dm_est_req(struct osmocom_ms *ms, uint16_t band_arfcn, uint8_t chan_nr
 
 	printf("Tx Dedic.Mode Est Req (arfcn=%u, chan_nr=0x%02x)\n",
 		band_arfcn, chan_nr);
-	ul = (struct l1ctl_info_ul *) msg->l1h;
+	ul = (struct l1ctl_info_ul *) msgb_put(msg, sizeof(*ul));
 	ul->chan_nr = chan_nr;
 	ul->link_id = 0;
 	ul->tx_power = 0; /* FIXME: initial TX power */
@@ -255,13 +263,13 @@ int tx_ph_dm_est_req(struct osmocom_ms *ms, uint16_t band_arfcn, uint8_t chan_nr
 	req->band_arfcn = band_arfcn;
 
 	return osmo_send_l1(ms, msg);
-
 }
 
 /* Receive incoming data from L1 using L1CTL format */
 int l1ctl_recv(struct osmocom_ms *ms, struct msgb *msg)
 {
 	int rc = 0;
+	struct l1ctl_hdr *l1h;
 	struct l1ctl_info_dl *dl;
 
 	if (msgb_l2len(msg) < sizeof(*dl)) {
@@ -269,10 +277,13 @@ int l1ctl_recv(struct osmocom_ms *ms, struct msgb *msg)
 		return -1;
 	}
 
-	dl = (struct l1ctl_info_dl *) msg->l1h;
-	msg->l2h = &msg->l1h[0] + sizeof(*dl);
+	l1h = (struct l1ctl_info_dl *) msg->l1h;
+
+	/* move the l1 header pointer to point _BEHIND_ l1ctl_hdr,
+	   as the l1ctl header is of no interest to subsequent code */
+	msg->l1h = l1h->data;
 
-	switch (dl->msg_type) {
+	switch (l1h->msg_type) {
 	case L1CTL_NEW_CCCH_RESP:
 		rc = rx_l1_ccch_resp(ms, msg);
 		break;
@@ -283,7 +294,7 @@ int l1ctl_recv(struct osmocom_ms *ms, struct msgb *msg)
 		rc = rx_l1_reset(ms);
 		break;
 	default:
-		fprintf(stderr, "Unknown MSG: %u\n", dl->msg_type);
+		fprintf(stderr, "Unknown MSG: %u\n", l1h->msg_type);
 		break;
 	}
 
diff --git a/src/target/firmware/layer1/l23_api.c b/src/target/firmware/layer1/l23_api.c
index a92a5dfb..e1c3591c 100644
--- a/src/target/firmware/layer1/l23_api.c
+++ b/src/target/firmware/layer1/l23_api.c
@@ -82,6 +82,7 @@ static enum mframe_task chan_nr2mf_task(uint8_t chan_nr)
 struct msgb *l1_create_l2_msg(int msg_type, uint32_t fn, uint16_t snr,
 			      uint16_t arfcn)
 {
+	struct l1ctl_hdr *l1h;
 	struct l1ctl_info_dl *dl;
 	struct msgb *msg;
 
@@ -94,8 +95,10 @@ struct msgb *l1_create_l2_msg(int msg_type, uint32_t fn, uint16_t snr,
 		return NULL;
 	}
 
+	l1h = (struct l1ctl_hdr *) msgb_put(msg, sizeof(*l1h));
+	l1h->msg_type = msg_type;
+
 	dl = (struct l1ctl_info_dl *) msgb_put(msg, sizeof(*dl));
-	dl->msg_type = msg_type;
 	dl->frame_nr = htonl(fn);
 	dl->snr = snr;
 	dl->band_arfcn = arfcn;
@@ -103,10 +106,19 @@ struct msgb *l1_create_l2_msg(int msg_type, uint32_t fn, uint16_t snr,
 	return msg;
 }
 
+void l1ctl_rx_pm_req(struct msgb *msg)
+{
+	struct l1ctl_hdr *l1h = (struct l1ctl_hdr *) msg->data;
+	struct l1ctl_pm_req *pm_req = (struct l1ctl_pm_req *) l1h->data;
+
+	/* FIXME */
+}
+
 /* callback from SERCOMM when L2 sends a message to L1 */
 static void l1a_l23_rx_cb(uint8_t dlci, struct msgb *msg)
 {
-	struct l1ctl_info_ul *ul = (struct l1ctl_info_ul *) msg->data;
+	struct l1ctl_hdr *l1h = (struct l1ctl_hdr *) msg->data;
+	struct l1ctl_info_ul *ul = (struct l1ctl_info_ul *) l1h->data;
 	struct l1ctl_sync_new_ccch_req *sync_req;
 	struct l1ctl_rach_req *rach_req;
 	struct l1ctl_dm_est_req *est_req;
@@ -121,19 +133,21 @@ static void l1a_l23_rx_cb(uint8_t dlci, struct msgb *msg)
 		puts("\n");
 	}
 
-	if (sizeof(*ul) > msg->len) {
+	msg->l1h = msg->data;
+
+	if (sizeof(*l1h) > msg->len) {
 		printf("l1a_l23_cb: Short message. %u\n", msg->len);
 		goto exit_msgbfree;
 	}
 
-	switch (ul->msg_type) {
+	switch (l1h->msg_type) {
 	case L1CTL_NEW_CCCH_REQ:
-		if (sizeof(*ul) + sizeof(*sync_req) > msg->len) {
+		if (sizeof(*sync_req) > msg->len) {
 			printf("Short sync msg. %u\n", msg->len);
 			break;
 		}
 
-		sync_req = (struct l1ctl_sync_new_ccch_req *) (&msg->data[0] + sizeof(*ul));
+		sync_req = (struct l1ctl_sync_new_ccch_req *) l1h->data;
 		printd("L1CTL_DM_EST_REQ (arfcn=%u)\n", sync_req->band_arfcn);
 
 		/* reset scheduler and hardware */
@@ -177,7 +191,6 @@ static void l1a_l23_rx_cb(uint8_t dlci, struct msgb *msg)
 	case L1CTL_DATA_REQ:
 		data_ind = (struct l1ctl_data_ind *) ul->payload;
 		printd("L1CTL_DATA_REQ (link_id=0x%02x)\n", ul->link_id);
-		printd("sizeof(struct l1ctl_info_ul)=%u\n", sizeof(struct l1ctl_info_ul));
 		if (ul->link_id & 0x40)
 			tx_queue = &l1s.tx_queue[L1S_CHAN_SACCH];
 		else
@@ -188,6 +201,10 @@ static void l1a_l23_rx_cb(uint8_t dlci, struct msgb *msg)
 		l1a_txq_msgb_enq(tx_queue, msg);
 		/* we have to keep the msgb, not free it! */
 		goto exit_nofree;
+	case L1CTL_PM_REQ:
+		printd("L1CTL_PM_REQ\n");
+		l1ctl_rx_pm_req(msg);
+		break;
 	}
 
 exit_msgbfree:
diff --git a/src/target/firmware/layer1/sync.c b/src/target/firmware/layer1/sync.c
index 38794158..e80756a5 100644
--- a/src/target/firmware/layer1/sync.c
+++ b/src/target/firmware/layer1/sync.c
@@ -840,8 +840,9 @@ static int l1s_nb_resp(__unused uint8_t p1, uint8_t burst_id, uint16_t p3)
 
 	/* 4th burst, get frame data */
 	if (dsp_api.db_r->d_burst_d == 3) {
+		struct l1ctl_hdr *l1h;
 		struct l1ctl_info_dl *dl;
-		struct l1ctl_data_ind *l1;
+		struct l1ctl_data_ind *di;
 		uint32_t avg_snr = 0;
 		int32_t avg_dbm8 = 0;
 		uint8_t i, j;
@@ -863,8 +864,9 @@ static int l1s_nb_resp(__unused uint8_t p1, uint8_t burst_id, uint16_t p3)
 
 		/* place it in the queue for the layer2 */
 		msg = l1_create_l2_msg(L1CTL_DATA_IND, l1s.current_time.fn-4, last_fb->snr, rf_arfcn);
-		dl = (struct l1ctl_info_dl *) msg->data;
-		l1 = (struct l1ctl_data_ind *) msgb_put(msg, sizeof(*l1));
+		l1h = (struct l1ctl_hdr *) msg->l1h;
+		dl = (struct l1ctl_info_dl *) l1h->data;
+		di = (struct l1ctl_data_ind *) msgb_put(msg, sizeof(*di));
 
 		/* Set Channel Number depending on MFrame Task ID */
 		dl->chan_nr = mframe_task2chan_nr(mf_task_id, 0); /* FIXME: TS */
@@ -885,7 +887,7 @@ static int l1s_nb_resp(__unused uint8_t p1, uint8_t burst_id, uint16_t p3)
 
 		/* copy the actual payload data */
 		for (i = 0; i < 23; ++i)
-			l1->data[i] = sig->nb.frame[i];
+			di->data[i] = sig->nb.frame[i];
 		l1_queue_for_l2(msg);
 
 		/* clear downlink task */