fixeria has uploaded this change for review.

View Change

bts_shutdown_fsm: fix spurious RAMP_COMPL events in WAIT_TRX_CLOSED

With multiple TRXes ramping down in lockstep, both their final
ramp-timer callbacks fire back-to-back in the same event loop pass,
setting p_total_cur_mdBm to the target value for both before any async
hardware acknowledgement arrives.  When the first SETPOWER ack returns
and fires ramp_down_compl_cb() for TRX0, the remaining-TRX check in
st_wait_ramp_down_compl() inspects p_total_cur_mdBm for TRX1 and finds
it already at the target - concluding that all TRXes are done.  The FSM
then transitions to WAIT_TRX_CLOSED, and the second ack (for TRX1)
fires ramp_down_compl_cb() into the wrong state, producing:

  BTS_SHUTDOWN(...){WAIT_TRX_CLOSED}: Event BTS_SHUTDOWN_EV_TRX_RAMP_COMPL not permitted

The root cause is that p_total_cur_mdBm is a *requested* value set in
the timer callback, not a confirmed one.  The hardware confirmation
arrives asynchronously via power_trx_change_compl() ->
power_ramp_do_step(), which is also where compl_cb() is invoked.

Fix by adding a 'complete' flag to trx_power_params.ramp that is:

* cleared when _power_ramp_start() begins a new ramp, and
* set just before compl_cb() is called in power_ramp_do_step()

The shutdown FSM remaining-TRX count then checks !ramp.complete instead of
comparing p_total_cur_mdBm against the target, correctly reflecting which
TRXes have actually received hardware confirmation of ramp completion.

Change-Id: Ia71393e871187d6b44b7f520eb421bab354aafd1
---
M include/osmo-bts/tx_power.h
M src/common/bts_shutdown_fsm.c
M src/common/tx_power.c
3 files changed, 6 insertions(+), 1 deletion(-)

git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/24/42524/1

diff --git a/include/osmo-bts/tx_power.h b/include/osmo-bts/tx_power.h
index 73134c8..52c02f5 100644
--- a/include/osmo-bts/tx_power.h
+++ b/include/osmo-bts/tx_power.h
@@ -1,6 +1,7 @@
 #pragma once
 
 #include <stdint.h>
+#include <stdbool.h>
 #include <osmocom/core/timer.h>
 
 /* our unit is 'milli dB" or "milli dBm", i.e. 1/1000 of a dB(m) */
@@ -59,6 +60,8 @@
 		struct osmo_timer_list step_timer;
 		/* call-back called when target is reached */
 		ramp_compl_cb_t compl_cb;
+		/* set to true after compl_cb has been called (ramp target confirmed by hardware) */
+		bool complete;
 	} ramp;
 };
 
diff --git a/src/common/bts_shutdown_fsm.c b/src/common/bts_shutdown_fsm.c
index 13a0e1d..a8c8f9d 100644
--- a/src/common/bts_shutdown_fsm.c
+++ b/src/common/bts_shutdown_fsm.c
@@ -110,7 +110,7 @@
 
 		llist_for_each_entry(trx, &bts->trx_list, list) {
 			if (trx->mo.nm_state.operational == NM_OPSTATE_ENABLED &&
-			    trx->power_params.p_total_cur_mdBm > BTS_SHUTDOWN_POWER_RAMP_TGT)
+			    !trx->power_params.ramp.complete)
 				remaining++;
 		}
 
diff --git a/src/common/tx_power.c b/src/common/tx_power.c
index 1e026dd..575fe2f 100644
--- a/src/common/tx_power.c
+++ b/src/common/tx_power.c
@@ -210,6 +210,7 @@
 
 	/* we had finished in last loop iteration */
 	if (!first && tpp->ramp.attenuation_mdB == 0) {
+		tpp->ramp.complete = true;
 		if (tpp->ramp.compl_cb)
 			tpp->ramp.compl_cb(trx);
 		return;
@@ -261,6 +262,7 @@
 	/* set the new target */
 	tpp->p_total_tgt_mdBm = p_total_tgt_mdBm;
 	tpp->ramp.compl_cb = ramp_compl_cb;
+	tpp->ramp.complete = false;
 
 	if (skip_ramping) {
 		/* Jump straight to the target power */

To view, visit change 42524. To unsubscribe, or for help writing mail filters, visit settings.