Attention is currently required from: neels, dexter. fixeria 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 2: (4 comments) File src/osmo-bsc/pcu_sock.c: https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/0a901719_39feab04 PS2, Line 802: ts->fi->state == TS_ST_PDCH Could you clarify why checking the FSM state is necessary? It's a bit weird to see code checking `fi->state` before sending an event. The FSM itself should take care of handling events in different states. https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/ea0d7c3d_136a90a1 PS2, Line 935: struct gsm_bts_trx_ts *ts; This variable should be moved to the inner for-loop. https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/3074cb99_b60f9c88 PS2, Line 973: gsm_bts_trx_num As I already commented on one of your previous patches, using `gsm_bts_trx_num()` in a loop is not a good idea. Better iterate over the `&bts->trx_list` directly, like you did in `pcu_tx_e1_ccu_ind()`. https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/28e6e22b_01c1f963 PS2, Line 976: 8 ARRAY_SIZE(trx->ts) -- 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: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Fri, 24 Feb 2023 22:19:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment