Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/34059 ) Change subject: pcu_sock: use PCU_IF_SAPI_AGCH_DT instead PCU_IF_SAPI_AGCH ...................................................................... Patch Set 1: (8 comments) File include/osmo-bts/pcu_if.h: https://gerrit.osmocom.org/c/osmo-bts/+/34059/comment/156c5e69_1cf35166 PS1, Line 26: int pcu_tx_pch_data_cnf(uint32_t fn, uint32_t tlli, uint8_t sapi); the function is named pch but we pass a sapi? are you sure this is correct? File src/common/bts.c: https://gerrit.osmocom.org/c/osmo-bts/+/34059/comment/30860e13_7dcdc4c9 PS1, Line 662: static struct msgb *bts_agch_dequeue(struct gsm_bts *bts) this can be in a separate commit apparently. https://gerrit.osmocom.org/c/osmo-bts/+/34059/comment/9849d0ce_ea8aa2f0 PS1, Line 757: unrelated? https://gerrit.osmocom.org/c/osmo-bts/+/34059/comment/5298d56e_f92f8c84 PS1, Line 765: if (msg->cb[0]) I'd love to know what these fields cb[0] and cb[1] are here. Maybe add #define getters for it? https://gerrit.osmocom.org/c/osmo-bts/+/34059/comment/1066077f_9fc84f5c PS1, Line 766: pcu_tx_pch_data_cnf(gt->fn, msg->cb[1], PCU_IF_SAPI_AGCH_DT); nice, a _pch_ function where an AGCH sapi is passed. This is obviously wrong. File src/common/paging.c: https://gerrit.osmocom.org/c/osmo-bts/+/34059/comment/296a089a_ffe5a0ec PS1, Line 737: pcu_tx_pch_data_cnf(gt->fn, pr[num_pr]->u.macblock.tlli, PCU_IF_SAPI_PCH_DT); pcu_tx_data_cnf? File src/common/pcu_sock.c: https://gerrit.osmocom.org/c/osmo-bts/+/34059/comment/09e0185d_cd88012d PS1, Line 714: msg->cb[0] = confirm; can we add a packed struct for these fields? https://gerrit.osmocom.org/c/osmo-bts/+/34059/comment/2bed15e3_ff9c03e4 PS1, Line 715: msg->cb[1] = gsm_pcu_if_agch_dt->tlli; You are assuming unsigned long is 32 bytes or more here (it probably is in most archs). -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/34059 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I29858fa20ad8bd0aefe81a5c40ad77a2559a8c10 Gerrit-Change-Number: 34059 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Fri, 04 Aug 2023 10:52:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment