Change in osmo-bts[master]: sysmo, oc2g, lc15: Make RadioChannel MO depend on RadioCarrier MO

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

pespin gerrit-no-reply at lists.osmocom.org
Tue May 18 08:13:33 UTC 2021


pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/24245 )

Change subject: sysmo,oc2g,lc15: Make RadioChannel MO depend on RadioCarrier MO
......................................................................

sysmo,oc2g,lc15: Make RadioChannel MO depend on RadioCarrier MO

lower layer specific APIs require first to enable the TRX object
(GsmL1_PrimId_MphInitReq, which requires ARFCN received during Set
Radio Carrier Attributes) before enabling the per-TS structure.
Hence, OPSTART must happen in RCARRIER MO before OPSTART can be sent to
the Radio Channel MOs, otherwise the initialization of the TS objet will
fail and OPSTART for the RadioChannel MO will send back a NACK.
In order to avoid this, we need to keep the RadioChannel MO announced as
"Disabled Dependency" until RCARRIER is OPSTARTed.

Related: OS#5157
Change-Id: I8c6e5ff98c32a3cd5006f5e5ed6875bcabb1d85f
---
M include/osmo-bts/bts.h
M include/osmo-bts/nm_common_fsm.h
M src/common/nm_channel_fsm.c
M src/common/nm_radio_carrier_fsm.c
M src/osmo-bts-lc15/main.c
M src/osmo-bts-oc2g/main.c
M src/osmo-bts-sysmo/main.c
7 files changed, 55 insertions(+), 7 deletions(-)

Approvals:
  fixeria: Looks good to me, but someone else must approve; Verified
  laforge: Looks good to me, but someone else must approve
  pespin: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/include/osmo-bts/bts.h b/include/osmo-bts/bts.h
index 7a1085e..141dce2 100644
--- a/include/osmo-bts/bts.h
+++ b/include/osmo-bts/bts.h
@@ -57,6 +57,9 @@
  * measurement data is passed using a separate MPH INFO MEAS IND.
  * (See also ticket: OS#2977) */
 #define BTS_INTERNAL_FLAG_MEAS_PAYLOAD_COMB		(1 << 1)
+/* Whether the BTS model requires RadioCarrier MO to be in Enabled state
+ * (OPSTARTed) before OPSTARTing the RadioChannel MOs. See OS#5157 */
+#define BTS_INTERNAL_FLAG_NM_RCHANNEL_DEPENDS_RCARRIER	(1 << 2)
 
 /* BTS implementation flags (internal use, not exposed via OML) */
 #define bts_internal_flag_get(bts, flag) \
diff --git a/include/osmo-bts/nm_common_fsm.h b/include/osmo-bts/nm_common_fsm.h
index 4a82757..4679b23 100644
--- a/include/osmo-bts/nm_common_fsm.h
+++ b/include/osmo-bts/nm_common_fsm.h
@@ -39,7 +39,8 @@
 	NM_EV_BBTRANSC_INSTALLED, /* Radio Channel only */
 	NM_EV_BBTRANSC_ENABLED, /* Radio Channel only */
 	NM_EV_BBTRANSC_DISABLED, /* Radio Channel only */
-
+	NM_EV_RCARRIER_ENABLED, /* Radio Channel only */
+	NM_EV_RCARRIER_DISABLED, /* Radio Channel only */
 };
 extern const struct value_string nm_fsm_event_names[];
 
diff --git a/src/common/nm_channel_fsm.c b/src/common/nm_channel_fsm.c
index 4925959..15be6c6 100644
--- a/src/common/nm_channel_fsm.c
+++ b/src/common/nm_channel_fsm.c
@@ -40,6 +40,14 @@
 #define nm_chan_fsm_state_chg(fi, NEXT_STATE) \
 	osmo_fsm_inst_state_chg(fi, NEXT_STATE, 0, 0)
 
+/* Can the TS be enabled (OPSTARTed)? aka should it stay in "Disabled Dependency" state? */
+static bool ts_can_be_enabled(const struct gsm_bts_trx_ts *ts)
+{
+	return (ts->trx->bb_transc.mo.nm_state.operational == NM_OPSTATE_ENABLED &&
+		(!bts_internal_flag_get(ts->trx->bts, BTS_INTERNAL_FLAG_NM_RCHANNEL_DEPENDS_RCARRIER) ||
+		 ts->trx->mo.nm_state.operational == NM_OPSTATE_ENABLED));
+}
+
 //////////////////////////
 // FSM STATE ACTIONS
 //////////////////////////
@@ -58,7 +66,7 @@
 	switch (event) {
 	case NM_EV_BBTRANSC_INSTALLED:
 		oml_mo_tx_sw_act_rep(&ts->mo);
-		if (ts->trx->bb_transc.mo.nm_state.operational == NM_OPSTATE_ENABLED)
+		if (ts_can_be_enabled(ts))
 			nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_OFFLINE);
 		else
 			nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_DEPENDENCY);
@@ -92,7 +100,13 @@
 		oml_mo_opstart_nack(&ts->mo, (int)(intptr_t)data);
 		return;
 	case NM_EV_BBTRANSC_ENABLED:
-		nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_OFFLINE);
+	case NM_EV_RCARRIER_ENABLED:
+		if (ts_can_be_enabled(ts))
+			nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_OFFLINE);
+		return;
+	case NM_EV_BBTRANSC_DISABLED:
+	case NM_EV_RCARRIER_DISABLED:
+		/* do nothing, we are simply waiting for (potentially) both to be enabled */
 		return;
 	default:
 		OSMO_ASSERT(0);
@@ -121,7 +135,9 @@
 		oml_mo_opstart_nack(&ts->mo, (int)(intptr_t)data);
 		return;
 	case NM_EV_BBTRANSC_DISABLED:
-		nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_DEPENDENCY);
+	case NM_EV_RCARRIER_DISABLED:
+		if (!ts_can_be_enabled(ts))
+			nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_DEPENDENCY);
 		return;
 	default:
 		OSMO_ASSERT(0);
@@ -136,9 +152,13 @@
 
 static void st_op_enabled(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
+	struct gsm_bts_trx_ts *ts = (struct gsm_bts_trx_ts *)fi->priv;
+
 	switch (event) {
 	case NM_EV_BBTRANSC_DISABLED:
-		nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_DEPENDENCY);
+	case NM_EV_RCARRIER_DISABLED:
+		if (!ts_can_be_enabled(ts))
+			nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_DEPENDENCY);
 		return;
 	case NM_EV_DISABLE:
 		nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_OFFLINE);
@@ -163,7 +183,10 @@
 		.in_event_mask =
 			X(NM_EV_OPSTART_ACK) |  /* backward compatibility, buggy BSC */
 			X(NM_EV_OPSTART_NACK) |
-			X(NM_EV_BBTRANSC_ENABLED),
+			X(NM_EV_BBTRANSC_ENABLED) |
+			X(NM_EV_RCARRIER_ENABLED) |
+			X(NM_EV_BBTRANSC_DISABLED) |
+			X(NM_EV_RCARRIER_DISABLED),
 		.out_state_mask =
 			X(NM_CHAN_ST_OP_DISABLED_OFFLINE) |
 			X(NM_CHAN_ST_OP_ENABLED), /* backward compatibility, buggy BSC */
@@ -175,7 +198,8 @@
 		.in_event_mask =
 			X(NM_EV_OPSTART_ACK) |
 			X(NM_EV_OPSTART_NACK) |
-			X(NM_EV_BBTRANSC_DISABLED),
+			X(NM_EV_BBTRANSC_DISABLED) |
+			X(NM_EV_RCARRIER_DISABLED),
 		.out_state_mask =
 			X(NM_CHAN_ST_OP_ENABLED) |
 			X(NM_CHAN_ST_OP_DISABLED_DEPENDENCY),
@@ -186,6 +210,7 @@
 	[NM_CHAN_ST_OP_ENABLED] = {
 		.in_event_mask =
 			X(NM_EV_BBTRANSC_DISABLED) |
+			X(NM_EV_RCARRIER_DISABLED) |
 			X(NM_EV_DISABLE),
 		.out_state_mask =
 			X(NM_CHAN_ST_OP_DISABLED_OFFLINE) |
diff --git a/src/common/nm_radio_carrier_fsm.c b/src/common/nm_radio_carrier_fsm.c
index d8291f8..4cbdf68 100644
--- a/src/common/nm_radio_carrier_fsm.c
+++ b/src/common/nm_radio_carrier_fsm.c
@@ -79,8 +79,17 @@
 static void st_op_disabled_offline_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state)
 {
 	struct gsm_bts_trx *trx = (struct gsm_bts_trx *)fi->priv;
+	unsigned int i;
+
 	trx->mo.opstart_success = false;
 	oml_mo_state_chg(&trx->mo, NM_OPSTATE_DISABLED, NM_AVSTATE_OFF_LINE);
+
+	if (prev_state == NM_RCARRIER_ST_OP_ENABLED) {
+		for (i = 0; i < TRX_NR_TS; i++) {
+			struct gsm_bts_trx_ts *ts = &trx->ts[i];
+			osmo_fsm_inst_dispatch(ts->mo.fi, NM_EV_RCARRIER_DISABLED, NULL);
+		}
+	}
 }
 
 static void st_op_disabled_offline(struct osmo_fsm_inst *fi, uint32_t event, void *data)
@@ -136,7 +145,14 @@
 static void st_op_enabled_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state)
 {
 	struct gsm_bts_trx *trx = (struct gsm_bts_trx *)fi->priv;
+	unsigned int tn;
+
 	oml_mo_state_chg(&trx->mo, NM_OPSTATE_ENABLED, NM_AVSTATE_OK);
+	/* Mark Dependency TS as Offline (ready to be Opstarted) */
+	for (tn = 0; tn < TRX_NR_TS; tn++) {
+		struct gsm_bts_trx_ts *ts = &trx->ts[tn];
+		osmo_fsm_inst_dispatch(ts->mo.fi, NM_EV_RCARRIER_ENABLED, NULL);
+	}
 }
 
 static void st_op_enabled(struct osmo_fsm_inst *fi, uint32_t event, void *data)
diff --git a/src/osmo-bts-lc15/main.c b/src/osmo-bts-lc15/main.c
index 985d7db..3e119f6 100644
--- a/src/osmo-bts-lc15/main.c
+++ b/src/osmo-bts-lc15/main.c
@@ -111,6 +111,7 @@
 	osmo_bts_set_feature(bts->features, BTS_FEAT_SPEECH_H_AMR);
 
 	bts_internal_flag_set(bts, BTS_INTERNAL_FLAG_MS_PWR_CTRL_DSP);
+	bts_internal_flag_set(bts, BTS_INTERNAL_FLAG_NM_RCHANNEL_DEPENDS_RCARRIER);
 
 	return 0;
 }
diff --git a/src/osmo-bts-oc2g/main.c b/src/osmo-bts-oc2g/main.c
index 12c489f..9a4bbfe 100644
--- a/src/osmo-bts-oc2g/main.c
+++ b/src/osmo-bts-oc2g/main.c
@@ -112,6 +112,7 @@
 	osmo_bts_set_feature(bts->features, BTS_FEAT_SPEECH_H_AMR);
 
 	bts_internal_flag_set(bts, BTS_INTERNAL_FLAG_MS_PWR_CTRL_DSP);
+	bts_internal_flag_set(bts, BTS_INTERNAL_FLAG_NM_RCHANNEL_DEPENDS_RCARRIER);
 
 	return 0;
 }
diff --git a/src/osmo-bts-sysmo/main.c b/src/osmo-bts-sysmo/main.c
index 2057a05..2bf1068 100644
--- a/src/osmo-bts-sysmo/main.c
+++ b/src/osmo-bts-sysmo/main.c
@@ -79,6 +79,7 @@
 
 	bts_internal_flag_set(bts, BTS_INTERNAL_FLAG_MS_PWR_CTRL_DSP);
 	bts_internal_flag_set(bts, BTS_INTERNAL_FLAG_MEAS_PAYLOAD_COMB);
+	bts_internal_flag_set(bts, BTS_INTERNAL_FLAG_NM_RCHANNEL_DEPENDS_RCARRIER);
 
 	return 0;
 }

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/24245
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I8c6e5ff98c32a3cd5006f5e5ed6875bcabb1d85f
Gerrit-Change-Number: 24245
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210518/04d702ea/attachment.htm>


More information about the gerrit-log mailing list