Change in osmo-bsc[master]: Implement MS Uplink Power Control Loop.

[pespin]

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/25654 )

Change subject: Implement MS Uplink Power Control Loop.
......................................................................


Patch Set 7:

(8 comments)

https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7//COMMIT_MSG 
Commit Message:

https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7//COMMIT_MSG@10 
PS7, Line 10: * Imports power_control.c from osmo-bts project.
Please add a commit reference here.


https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7//COMMIT_MSG@11 
PS7, Line 11: * Removes unused C/I code from power_control.
from osmo-bts's power_control.c


https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7/include/osmocom/bsc/gsm_data.h 
File include/osmocom/bsc/gsm_data.h:

https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7/include/osmocom/bsc/gsm_data.h@825 
PS7, Line 825: 	struct gsm_power_ctrl_meas_proc_state rxlev_meas_proc;
Put this inside a struct lchan_power_ctrl_state {} similar to what's done in osmo-bts.
This way we keep similar code base and it's easier to re-use it later ifw e want to implement bs-power control.
We would ideally move 	uint8_t bs_power_db and uint8_t ms_power; from above inside this struct as "uint8_t current" too.


https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7/src/osmo-bsc/power_control.c 
File src/osmo-bsc/power_control.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7/src/osmo-bsc/power_control.c@170 
PS7, Line 170: 	// int16_t ul_lqual_cb_avg;
can be dropped


https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7/src/osmo-bsc/power_control.c@171 
PS7, Line 171: 	uint8_t ms_power_lvl = ms_pwr_ctl_lvl(band, mr->ms_l1.pwr);
I'd say this is wrong. mr->ms_l1.pwr as received from the MS contains already a MS Power Level right? not dBm. Or are we converting it to dBm somewhere?
IIUC this line should be:
uint8_t ms_power_lvl = mr->ms_l1.pwr;


https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7/src/osmo-bsc/power_control.c@183 
PS7, Line 183: 	if (bts->dtxu == GSM48_DTX_SHALL_BE_USED)
IIRC this can be obtained from the MR? it depends on the channel.


https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7/src/osmo-bsc/power_control.c@188 
PS7, Line 188: 	/* Shall we skip current block based on configured interval? */
Let's move this further up before the dtxu block.


https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7/src/osmo-bsc/power_control.c@258 
PS7, Line 258: 	lchan->ms_power = new_power_lvl;
Here for sure lchan_update_ms_power_ctrl_level() must be called instead.



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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ibc307e758697eb5ca3fb86622f35709d6077db9e
Gerrit-Change-Number: 25654
Gerrit-PatchSet: 7
Gerrit-Owner: keith <keith at rhizomatica.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-CC: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Fri, 01 Oct 2021 09:42:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20211001/0f2bd767/attachment.htm>