fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/21014 )
Change subject: l1sap: add repeated downlink FACCH ...................................................................... Patch Set 2: (7 comments) https://gerrit.osmocom.org/c/osmo-bts/+/21014/2/include/osmo-bts/gsm_data.h File include/osmo-bts/gsm_data.h: https://gerrit.osmocom.org/c/osmo-bts/+/21014/2/include/osmo-bts/gsm_data.h@159 PS2, Line 159: gsm_rep_facch I guess you could just store fn within msgb->cb? https://gerrit.osmocom.org/c/osmo-bts/+/21014/2/src/common/bts.c File src/common/bts.c: https://gerrit.osmocom.org/c/osmo-bts/+/21014/2/src/common/bts.c@458 PS2, Line 458: if (lchan->repeated_acch_capability && (lchan->type == GSM_LCHAN_TCH_F || lchan->type == GSM_LCHAN_TCH_H)) Not sure if we would ever have other scale values, so I would move this condition after the next block and simply do: t200_ms_acch[DL_SAPI0] *= 2; t200_ms_acch[DL_SAPI3] *= 2; https://gerrit.osmocom.org/c/osmo-bts/+/21014/2/src/common/l1sap.c File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/21014/2/src/common/l1sap.c@141 PS2, Line 141: LOGPLCHAN(lchan, DRTP, LOGL_ERROR, "RTP clock out of sync with lower layer:" Why? https://gerrit.osmocom.org/c/osmo-bts/+/21014/2/src/common/l1sap.c@909 PS2, Line 909: } missing new line https://gerrit.osmocom.org/c/osmo-bts/+/21014/2/src/common/l1sap.c@925 PS2, Line 925: fn + 8 <= fn Are you sure this is safe? Don't we need GSM_TDMA_FN_SUM() / GSM_TDMA_FN_DIFF() here? And where this +8 is coming from? https://gerrit.osmocom.org/c/osmo-bts/+/21014/2/src/common/l1sap.c@929 PS2, Line 929: fn + 8 <= fn Same here. https://gerrit.osmocom.org/c/osmo-bts/+/21014/2/src/common/vty.c File src/common/vty.c: https://gerrit.osmocom.org/c/osmo-bts/+/21014/2/src/common/vty.c@817 PS2, Line 817: "disable downlink FACCH repetition\n", missing NO_STR -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/21014 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I72f0cf7eaaef9f80fc35e752c90ae0e2d24d0c75 Gerrit-Change-Number: 21014 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <[email protected]> Gerrit-CC: laforge <[email protected]> Gerrit-Comment-Date: Mon, 09 Nov 2020 14:19:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
