fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/39830?usp=email )
Change subject: fixup: rsl: properly initialize MS/BS Power Control state ......................................................................
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);