<p>pespin <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bts/+/24245">View Change</a></p><div style="white-space:pre-wrap">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
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">sysmo,oc2g,lc15: Make RadioChannel MO depend on RadioCarrier MO<br><br>lower layer specific APIs require first to enable the TRX object<br>(GsmL1_PrimId_MphInitReq, which requires ARFCN received during Set<br>Radio Carrier Attributes) before enabling the per-TS structure.<br>Hence, OPSTART must happen in RCARRIER MO before OPSTART can be sent to<br>the Radio Channel MOs, otherwise the initialization of the TS objet will<br>fail and OPSTART for the RadioChannel MO will send back a NACK.<br>In order to avoid this, we need to keep the RadioChannel MO announced as<br>"Disabled Dependency" until RCARRIER is OPSTARTed.<br><br>Related: OS#5157<br>Change-Id: I8c6e5ff98c32a3cd5006f5e5ed6875bcabb1d85f<br>---<br>M include/osmo-bts/bts.h<br>M include/osmo-bts/nm_common_fsm.h<br>M src/common/nm_channel_fsm.c<br>M src/common/nm_radio_carrier_fsm.c<br>M src/osmo-bts-lc15/main.c<br>M src/osmo-bts-oc2g/main.c<br>M src/osmo-bts-sysmo/main.c<br>7 files changed, 55 insertions(+), 7 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmo-bts/bts.h b/include/osmo-bts/bts.h</span><br><span>index 7a1085e..141dce2 100644</span><br><span>--- a/include/osmo-bts/bts.h</span><br><span>+++ b/include/osmo-bts/bts.h</span><br><span>@@ -57,6 +57,9 @@</span><br><span> * measurement data is passed using a separate MPH INFO MEAS IND.</span><br><span> * (See also ticket: OS#2977) */</span><br><span> #define BTS_INTERNAL_FLAG_MEAS_PAYLOAD_COMB (1 << 1)</span><br><span style="color: hsl(120, 100%, 40%);">+/* Whether the BTS model requires RadioCarrier MO to be in Enabled state</span><br><span style="color: hsl(120, 100%, 40%);">+ * (OPSTARTed) before OPSTARTing the RadioChannel MOs. See OS#5157 */</span><br><span style="color: hsl(120, 100%, 40%);">+#define BTS_INTERNAL_FLAG_NM_RCHANNEL_DEPENDS_RCARRIER (1 << 2)</span><br><span> </span><br><span> /* BTS implementation flags (internal use, not exposed via OML) */</span><br><span> #define bts_internal_flag_get(bts, flag) \</span><br><span>diff --git a/include/osmo-bts/nm_common_fsm.h b/include/osmo-bts/nm_common_fsm.h</span><br><span>index 4a82757..4679b23 100644</span><br><span>--- a/include/osmo-bts/nm_common_fsm.h</span><br><span>+++ b/include/osmo-bts/nm_common_fsm.h</span><br><span>@@ -39,7 +39,8 @@</span><br><span> NM_EV_BBTRANSC_INSTALLED, /* Radio Channel only */</span><br><span> NM_EV_BBTRANSC_ENABLED, /* Radio Channel only */</span><br><span> NM_EV_BBTRANSC_DISABLED, /* Radio Channel only */</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(120, 100%, 40%);">+ NM_EV_RCARRIER_ENABLED, /* Radio Channel only */</span><br><span style="color: hsl(120, 100%, 40%);">+ NM_EV_RCARRIER_DISABLED, /* Radio Channel only */</span><br><span> };</span><br><span> extern const struct value_string nm_fsm_event_names[];</span><br><span> </span><br><span>diff --git a/src/common/nm_channel_fsm.c b/src/common/nm_channel_fsm.c</span><br><span>index 4925959..15be6c6 100644</span><br><span>--- a/src/common/nm_channel_fsm.c</span><br><span>+++ b/src/common/nm_channel_fsm.c</span><br><span>@@ -40,6 +40,14 @@</span><br><span> #define nm_chan_fsm_state_chg(fi, NEXT_STATE) \</span><br><span> osmo_fsm_inst_state_chg(fi, NEXT_STATE, 0, 0)</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/* Can the TS be enabled (OPSTARTed)? aka should it stay in "Disabled Dependency" state? */</span><br><span style="color: hsl(120, 100%, 40%);">+static bool ts_can_be_enabled(const struct gsm_bts_trx_ts *ts)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ return (ts->trx->bb_transc.mo.nm_state.operational == NM_OPSTATE_ENABLED &&</span><br><span style="color: hsl(120, 100%, 40%);">+ (!bts_internal_flag_get(ts->trx->bts, BTS_INTERNAL_FLAG_NM_RCHANNEL_DEPENDS_RCARRIER) ||</span><br><span style="color: hsl(120, 100%, 40%);">+ ts->trx->mo.nm_state.operational == NM_OPSTATE_ENABLED));</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> //////////////////////////</span><br><span> // FSM STATE ACTIONS</span><br><span> //////////////////////////</span><br><span>@@ -58,7 +66,7 @@</span><br><span> switch (event) {</span><br><span> case NM_EV_BBTRANSC_INSTALLED:</span><br><span> oml_mo_tx_sw_act_rep(&ts->mo);</span><br><span style="color: hsl(0, 100%, 40%);">- if (ts->trx->bb_transc.mo.nm_state.operational == NM_OPSTATE_ENABLED)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ts_can_be_enabled(ts))</span><br><span> nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_OFFLINE);</span><br><span> else</span><br><span> nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_DEPENDENCY);</span><br><span>@@ -92,7 +100,13 @@</span><br><span> oml_mo_opstart_nack(&ts->mo, (int)(intptr_t)data);</span><br><span> return;</span><br><span> case NM_EV_BBTRANSC_ENABLED:</span><br><span style="color: hsl(0, 100%, 40%);">- nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_OFFLINE);</span><br><span style="color: hsl(120, 100%, 40%);">+ case NM_EV_RCARRIER_ENABLED:</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ts_can_be_enabled(ts))</span><br><span style="color: hsl(120, 100%, 40%);">+ nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_OFFLINE);</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span style="color: hsl(120, 100%, 40%);">+ case NM_EV_BBTRANSC_DISABLED:</span><br><span style="color: hsl(120, 100%, 40%);">+ case NM_EV_RCARRIER_DISABLED:</span><br><span style="color: hsl(120, 100%, 40%);">+ /* do nothing, we are simply waiting for (potentially) both to be enabled */</span><br><span> return;</span><br><span> default:</span><br><span> OSMO_ASSERT(0);</span><br><span>@@ -121,7 +135,9 @@</span><br><span> oml_mo_opstart_nack(&ts->mo, (int)(intptr_t)data);</span><br><span> return;</span><br><span> case NM_EV_BBTRANSC_DISABLED:</span><br><span style="color: hsl(0, 100%, 40%);">- nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_DEPENDENCY);</span><br><span style="color: hsl(120, 100%, 40%);">+ case NM_EV_RCARRIER_DISABLED:</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!ts_can_be_enabled(ts))</span><br><span style="color: hsl(120, 100%, 40%);">+ nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_DEPENDENCY);</span><br><span> return;</span><br><span> default:</span><br><span> OSMO_ASSERT(0);</span><br><span>@@ -136,9 +152,13 @@</span><br><span> </span><br><span> static void st_op_enabled(struct osmo_fsm_inst *fi, uint32_t event, void *data)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm_bts_trx_ts *ts = (struct gsm_bts_trx_ts *)fi->priv;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> switch (event) {</span><br><span> case NM_EV_BBTRANSC_DISABLED:</span><br><span style="color: hsl(0, 100%, 40%);">- nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_DEPENDENCY);</span><br><span style="color: hsl(120, 100%, 40%);">+ case NM_EV_RCARRIER_DISABLED:</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!ts_can_be_enabled(ts))</span><br><span style="color: hsl(120, 100%, 40%);">+ nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_DEPENDENCY);</span><br><span> return;</span><br><span> case NM_EV_DISABLE:</span><br><span> nm_chan_fsm_state_chg(fi, NM_CHAN_ST_OP_DISABLED_OFFLINE);</span><br><span>@@ -163,7 +183,10 @@</span><br><span> .in_event_mask =</span><br><span> X(NM_EV_OPSTART_ACK) | /* backward compatibility, buggy BSC */</span><br><span> X(NM_EV_OPSTART_NACK) |</span><br><span style="color: hsl(0, 100%, 40%);">- X(NM_EV_BBTRANSC_ENABLED),</span><br><span style="color: hsl(120, 100%, 40%);">+ X(NM_EV_BBTRANSC_ENABLED) |</span><br><span style="color: hsl(120, 100%, 40%);">+ X(NM_EV_RCARRIER_ENABLED) |</span><br><span style="color: hsl(120, 100%, 40%);">+ X(NM_EV_BBTRANSC_DISABLED) |</span><br><span style="color: hsl(120, 100%, 40%);">+ X(NM_EV_RCARRIER_DISABLED),</span><br><span> .out_state_mask =</span><br><span> X(NM_CHAN_ST_OP_DISABLED_OFFLINE) |</span><br><span> X(NM_CHAN_ST_OP_ENABLED), /* backward compatibility, buggy BSC */</span><br><span>@@ -175,7 +198,8 @@</span><br><span> .in_event_mask =</span><br><span> X(NM_EV_OPSTART_ACK) |</span><br><span> X(NM_EV_OPSTART_NACK) |</span><br><span style="color: hsl(0, 100%, 40%);">- X(NM_EV_BBTRANSC_DISABLED),</span><br><span style="color: hsl(120, 100%, 40%);">+ X(NM_EV_BBTRANSC_DISABLED) |</span><br><span style="color: hsl(120, 100%, 40%);">+ X(NM_EV_RCARRIER_DISABLED),</span><br><span> .out_state_mask =</span><br><span> X(NM_CHAN_ST_OP_ENABLED) |</span><br><span> X(NM_CHAN_ST_OP_DISABLED_DEPENDENCY),</span><br><span>@@ -186,6 +210,7 @@</span><br><span> [NM_CHAN_ST_OP_ENABLED] = {</span><br><span> .in_event_mask =</span><br><span> X(NM_EV_BBTRANSC_DISABLED) |</span><br><span style="color: hsl(120, 100%, 40%);">+ X(NM_EV_RCARRIER_DISABLED) |</span><br><span> X(NM_EV_DISABLE),</span><br><span> .out_state_mask =</span><br><span> X(NM_CHAN_ST_OP_DISABLED_OFFLINE) |</span><br><span>diff --git a/src/common/nm_radio_carrier_fsm.c b/src/common/nm_radio_carrier_fsm.c</span><br><span>index d8291f8..4cbdf68 100644</span><br><span>--- a/src/common/nm_radio_carrier_fsm.c</span><br><span>+++ b/src/common/nm_radio_carrier_fsm.c</span><br><span>@@ -79,8 +79,17 @@</span><br><span> static void st_op_disabled_offline_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state)</span><br><span> {</span><br><span> struct gsm_bts_trx *trx = (struct gsm_bts_trx *)fi->priv;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int i;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> trx->mo.opstart_success = false;</span><br><span> oml_mo_state_chg(&trx->mo, NM_OPSTATE_DISABLED, NM_AVSTATE_OFF_LINE);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (prev_state == NM_RCARRIER_ST_OP_ENABLED) {</span><br><span style="color: hsl(120, 100%, 40%);">+ for (i = 0; i < TRX_NR_TS; i++) {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm_bts_trx_ts *ts = &trx->ts[i];</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_inst_dispatch(ts->mo.fi, NM_EV_RCARRIER_DISABLED, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> </span><br><span> static void st_op_disabled_offline(struct osmo_fsm_inst *fi, uint32_t event, void *data)</span><br><span>@@ -136,7 +145,14 @@</span><br><span> static void st_op_enabled_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state)</span><br><span> {</span><br><span> struct gsm_bts_trx *trx = (struct gsm_bts_trx *)fi->priv;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int tn;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> oml_mo_state_chg(&trx->mo, NM_OPSTATE_ENABLED, NM_AVSTATE_OK);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Mark Dependency TS as Offline (ready to be Opstarted) */</span><br><span style="color: hsl(120, 100%, 40%);">+ for (tn = 0; tn < TRX_NR_TS; tn++) {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm_bts_trx_ts *ts = &trx->ts[tn];</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_inst_dispatch(ts->mo.fi, NM_EV_RCARRIER_ENABLED, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> </span><br><span> static void st_op_enabled(struct osmo_fsm_inst *fi, uint32_t event, void *data)</span><br><span>diff --git a/src/osmo-bts-lc15/main.c b/src/osmo-bts-lc15/main.c</span><br><span>index 985d7db..3e119f6 100644</span><br><span>--- a/src/osmo-bts-lc15/main.c</span><br><span>+++ b/src/osmo-bts-lc15/main.c</span><br><span>@@ -111,6 +111,7 @@</span><br><span> osmo_bts_set_feature(bts->features, BTS_FEAT_SPEECH_H_AMR);</span><br><span> </span><br><span> bts_internal_flag_set(bts, BTS_INTERNAL_FLAG_MS_PWR_CTRL_DSP);</span><br><span style="color: hsl(120, 100%, 40%);">+ bts_internal_flag_set(bts, BTS_INTERNAL_FLAG_NM_RCHANNEL_DEPENDS_RCARRIER);</span><br><span> </span><br><span> return 0;</span><br><span> }</span><br><span>diff --git a/src/osmo-bts-oc2g/main.c b/src/osmo-bts-oc2g/main.c</span><br><span>index 12c489f..9a4bbfe 100644</span><br><span>--- a/src/osmo-bts-oc2g/main.c</span><br><span>+++ b/src/osmo-bts-oc2g/main.c</span><br><span>@@ -112,6 +112,7 @@</span><br><span> osmo_bts_set_feature(bts->features, BTS_FEAT_SPEECH_H_AMR);</span><br><span> </span><br><span> bts_internal_flag_set(bts, BTS_INTERNAL_FLAG_MS_PWR_CTRL_DSP);</span><br><span style="color: hsl(120, 100%, 40%);">+ bts_internal_flag_set(bts, BTS_INTERNAL_FLAG_NM_RCHANNEL_DEPENDS_RCARRIER);</span><br><span> </span><br><span> return 0;</span><br><span> }</span><br><span>diff --git a/src/osmo-bts-sysmo/main.c b/src/osmo-bts-sysmo/main.c</span><br><span>index 2057a05..2bf1068 100644</span><br><span>--- a/src/osmo-bts-sysmo/main.c</span><br><span>+++ b/src/osmo-bts-sysmo/main.c</span><br><span>@@ -79,6 +79,7 @@</span><br><span> </span><br><span> bts_internal_flag_set(bts, BTS_INTERNAL_FLAG_MS_PWR_CTRL_DSP);</span><br><span> bts_internal_flag_set(bts, BTS_INTERNAL_FLAG_MEAS_PAYLOAD_COMB);</span><br><span style="color: hsl(120, 100%, 40%);">+ bts_internal_flag_set(bts, BTS_INTERNAL_FLAG_NM_RCHANNEL_DEPENDS_RCARRIER);</span><br><span> </span><br><span> return 0;</span><br><span> }</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bts/+/24245">change 24245</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-bts/+/24245"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-bts </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I8c6e5ff98c32a3cd5006f5e5ed6875bcabb1d85f </div>
<div style="display:none"> Gerrit-Change-Number: 24245 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>