Change in osmocom-bb[master]: trxcon: Suppress POWERON to TRX if we're already powered on.

[Vadim Yanitskiy]

Vadim Yanitskiy has submitted this change and it was merged. ( https://gerrit.osmocom.org/14275 )

Change subject: trxcon: Suppress POWERON to TRX if we're already powered on.
......................................................................

trxcon: Suppress POWERON to TRX if we're already powered on.

The existing logic unconditionally wants to send a POWERON command
on TRXC whenever L1CTL_FBSB_REQ is received.  That may cause some
problems when sending subsequent L1CTL_FBSB_REQ, e.g. due to signal loss.

Sending POWEROFF when transceiver is not powered on is normal though.
This can happen if trxcon is restarted while fake_trx was running.

The existing FSM state could unfortunately not been used, as it's a
mixture between the TRX connection state and the command/response state.

The current solution is just a work around. We definitely need to
introduce separate state machines for transceiver and its TRXC
interface.

Change-Id: I834e8897b95a2490811319697fc7cab6076db480
---
M src/host/trxcon/l1ctl.c
M src/host/trxcon/trx_if.c
M src/host/trxcon/trx_if.h
3 files changed, 18 insertions(+), 3 deletions(-)

Approvals:
  Vadim Yanitskiy: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/host/trxcon/l1ctl.c b/src/host/trxcon/l1ctl.c
index fff1c2b..30f43d0 100644
--- a/src/host/trxcon/l1ctl.c
+++ b/src/host/trxcon/l1ctl.c
@@ -362,7 +362,10 @@
 		trx_if_cmd_txtune(l1l->trx, band_arfcn);
 	}
 
-	trx_if_cmd_poweron(l1l->trx);
+	/* Transceiver might have been powered on before, e.g.
+	 * in case of sending L1CTL_FBSB_REQ due to signal loss. */
+	if (!l1l->trx->powered_up)
+		trx_if_cmd_poweron(l1l->trx);
 
 	/* Start FBSB expire timer */
 	l1l->fbsb_timer.data = l1l;
diff --git a/src/host/trxcon/trx_if.c b/src/host/trxcon/trx_if.c
index 35ad3c0..ca78349 100644
--- a/src/host/trxcon/trx_if.c
+++ b/src/host/trxcon/trx_if.c
@@ -254,6 +254,11 @@
 
 int trx_if_cmd_poweron(struct trx_instance *trx)
 {
+	if (trx->powered_up) {
+		/* FIXME: this should be handled by the FSM, not here! */
+		LOGP(DTRX, LOGL_ERROR, "Suppressing POWERON as we're already powered up\n");
+		return -EAGAIN;
+	}
 	return trx_ctrl_cmd(trx, 1, "POWERON", "");
 }
 
@@ -495,10 +500,14 @@
 	}
 
 	/* Trigger state machine */
-	if (!strncmp(tcm->cmd + 4, "POWERON", 7))
+	if (!strncmp(tcm->cmd + 4, "POWERON", 7)) {
+		trx->powered_up = true;
 		osmo_fsm_inst_state_chg(trx->fsm, TRX_STATE_ACTIVE, 0, 0);
-	else if (!strncmp(tcm->cmd + 4, "POWEROFF", 8))
+	}
+	else if (!strncmp(tcm->cmd + 4, "POWEROFF", 8)) {
+		trx->powered_up = false;
 		osmo_fsm_inst_state_chg(trx->fsm, TRX_STATE_IDLE, 0, 0);
+	}
 	else if (!strncmp(tcm->cmd + 4, "MEASURE", 7))
 		trx_if_measure_rsp_cb(trx, buf + 14);
 	else if (!strncmp(tcm->cmd + 4, "ECHO", 4))
diff --git a/src/host/trxcon/trx_if.h b/src/host/trxcon/trx_if.h
index 0b3f36f..a44600d 100644
--- a/src/host/trxcon/trx_if.h
+++ b/src/host/trxcon/trx_if.h
@@ -25,7 +25,10 @@
 	struct osmo_timer_list trx_ctrl_timer;
 	struct llist_head trx_ctrl_list;
 	struct osmo_fsm_inst *fsm;
+
+	/* HACK: we need proper state machines */
 	uint32_t prev_state;
+	bool powered_up;
 
 	/* GSM L1 specific */
 	uint16_t pm_band_arfcn_start;

-- 
To view, visit https://gerrit.osmocom.org/14275
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I834e8897b95a2490811319697fc7cab6076db480
Gerrit-Change-Number: 14275
Gerrit-PatchSet: 2
Gerrit-Owner: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190531/8c3bbc0a/attachment.htm>