Attention is currently required from: neels, fixeria, dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31497 )
Change subject: pcu_sock: activate/deactivate PDCH on pcu reconnect ...................................................................... Patch Set 5: (3 comments) File src/osmo-bsc/pcu_sock.c: https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/1565225a_6f6796c5 PS2, Line 801: printf("l1sap_chan_rel(trx,gsm_lchan2chan_nr(ts->lchan));\n"); > either keep this printf, or if it should be dropped, drop it in a separate > patch? Ack https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/968de04c_416d056b PS2, Line 801: && ts->pchan_on_init == GSM_PCHAN_OSMO_DYN > Why does this apply only to dynamic timeslots? I think I understand why, but > I'd like to see a code […] Ericsson only has fully-dynamic timeslots, AFAIR. But yes, mentioning that here might make sense for the most common reader of this code who has no clue about ericsson specifics. https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/880ebec0_1f12759b PS2, Line 941: bts = llist_entry(state->net->bts_list.next, struct gsm_bts, list); > rather use llist_first_entry_or_null() […] I actually think it's a serious problem that only one BTS appears to be suppored here. Not only that, but there's an implicit assumption that the first BTS is the Ericsson BTS served by the PCU? In general, a BSC-colocated PCU can of course serve any number of BTSs. Until we address that general problem/shortcoming in our current code, we shouldn't make a blind assumption, but we should make sure that the user can operate this only in a safe configuration. In the worst case, we could check if thre are more than one (ericsson) BTS configured in the BSC, and refuse to operate the PCU socket with an associated error message. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31497 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I9ea0c53a5e68a51c781ef43bae71f947cdb95678 Gerrit-Change-Number: 31497 Gerrit-PatchSet: 5 Gerrit-Owner: dexter <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <[email protected]> Gerrit-CC: fixeria <[email protected]> Gerrit-CC: laforge <[email protected]> Gerrit-Attention: neels <[email protected]> Gerrit-Attention: fixeria <[email protected]> Gerrit-Attention: dexter <[email protected]> Gerrit-Comment-Date: Tue, 28 Feb 2023 18:05:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <[email protected]> Gerrit-MessageType: comment
