fixeria has uploaded this change for review.
fixup: rsl: properly initialize MS/BS Power Control state
Commit 75162427 fixed one problem, but introduced another one:
'''
/* Initialize MS Power Control defaults */
lchan->ms_power_ctrl = (struct lchan_power_ctrl_state) {
.max = ms_pwr_ctl_lvl(lchan->ts->trx->bts->band, 0),
.current = lchan->ms_power_ctrl.max, /* XXX */
};
'''
The intention here was to assign the same value to both 'max' and
'current' fields. However, field 'current' actually gets assigned
whatever value was assigned to field 'max' previously.
This regression was caught by running the BTS_Tests.TC_rsl_ms_pwr_ctrl
several times against the same osmo-bts-trx process.
Let's use the good-old memset() to initialize the MS/BS Power Control
state structures, and move initialization of 'max' and 'current' fields
to the else branches of the RSL_IE_{MS,BS}_POWER IE checks.
Change-Id: I46c881d5a3959c2542610ed767e0f131d01f9f98
Fixes: 75162427 ("rsl: properly initialize MS/BS Power Control state")
Related: SYS#4918, OS#6672
---
M src/common/rsl.c
1 file changed, 11 insertions(+), 14 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/30/39830/1
diff --git a/src/common/rsl.c b/src/common/rsl.c
index 1104ef8..f16fe7c 100644
--- a/src/common/rsl.c
+++ b/src/common/rsl.c
@@ -1987,17 +1987,9 @@
}
}
- /* Initialize MS Power Control defaults */
- lchan->ms_power_ctrl = (struct lchan_power_ctrl_state) {
- .max = ms_pwr_ctl_lvl(lchan->ts->trx->bts->band, 0),
- .current = lchan->ms_power_ctrl.max,
- };
-
- /* Initialize BS Power Control defaults */
- lchan->bs_power_ctrl = (struct lchan_power_ctrl_state) {
- .max = 2 * 15, /* maximum defined in 9.3.4 */
- .current = 0,
- };
+ /* Initialize MS/BS Power Control state */
+ memset(&lchan->ms_power_ctrl, 0, sizeof(lchan->ms_power_ctrl));
+ memset(&lchan->bs_power_ctrl, 0, sizeof(lchan->bs_power_ctrl));
/* 9.3.6 Channel Mode */
if (type != RSL_ACT_OSMO_PDCH) {
@@ -2054,13 +2046,18 @@
LOGPLCHAN(lchan, DRSL, LOGL_DEBUG, "BS Power attenuation %u dB\n",
lchan->bs_power_ctrl.current);
+ } else {
+ lchan->bs_power_ctrl.max = 2 * 15; /* maximum defined in 9.3.4 */
+ lchan->bs_power_ctrl.current = 0;
}
/* 9.3.13 MS Power */
- if (TLVP_PRES_LEN(&tp, RSL_IE_MS_POWER, 1)) {
+ if (TLVP_PRES_LEN(&tp, RSL_IE_MS_POWER, 1))
lchan->ms_power_ctrl.max = *TLVP_VAL(&tp, RSL_IE_MS_POWER) & 0x1F;
- lchan->ms_power_ctrl.current = lchan->ms_power_ctrl.max;
- }
+ else /* XXX: should we use the maximum power level instead of 0 dBm? */
+ lchan->ms_power_ctrl.max = ms_pwr_ctl_lvl(lchan->ts->trx->bts->band, 0);
+ lchan->ms_power_ctrl.current = lchan->ms_power_ctrl.max;
+
/* 9.3.24 Timing Advance */
if (TLVP_PRES_LEN(&tp, RSL_IE_TIMING_ADVANCE, 1))
lchan->ta_ctrl.current = *TLVP_VAL(&tp, RSL_IE_TIMING_ADVANCE);
To view, visit change 39830. To unsubscribe, or for help writing mail filters, visit settings.