Attention is currently required from: neels, laforge, fixeria.
10 comments:
File src/osmo-bsc/pcu_sock.c:
Patch Set #2, 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?
I think its correct to drop it here. The code was ported from osmo-bts some years ago and I think this printf was left there as a placeholder for the code we now add with this patch.
Patch Set #2, Line 794: for (i = 0; i < PCU_IF_NUM_TRX; i++) {
this is a fix of a magic number, very good but rather in a separate patch
Done
Patch Set #2, 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 […]
I have added a comment. I hope it makes it clearer now. As far as I know GSM_PCHAN_TCH_F_PDCH is the ip.access style of dynamic timeslots. This is not applicable here.
Patch Set #2, Line 802: ts->fi->state == TS_ST_PDCH
generally true, but here i agree with checking the state explicitly: […]
Done
Patch Set #2, Line 935: struct gsm_bts_trx_ts *ts;
This variable should be moved to the inner for-loop.
Done
Patch Set #2, Line 941: bts = llist_entry(state->net->bts_list.next, struct gsm_bts, list);
rather use llist_first_entry_or_null() […]
The PCU still has a limitation that it can only handle one BTS at a time. Thats at least what it is tested for. It indeed has a list with BTSs, but it never ran with multiple BTSs. Also the PHY implementations pick only the first BTS in that list.
As far as the BSC is concerned we can fix this. For the sock_accept and sock_close function we have to run over all BTSs in the network. When we get the primitive from the PCU it has the BTS number in it. I will fix this in a different patch since it is a bit out of scope here. I will also keep the llist_entry() to keep consistency with the existing code.
Patch Set #2, Line 973: gsm_bts_trx_num
agree
Done
could also use TRX_NR_TS, but ARRAY_SIZE is also my favorite; TRX_NR_TS is our legacy way, these day […]
Done
Patch Set #2, Line 979: && ts->pchan_on_init == GSM_PCHAN_OSMO_DYN && ts->fi->state == TS_ST_UNUSED) {
Also for GSM_PCHAN_TCH_F_PDCH ? […]
Done
File src/osmo-bsc/timeslot_fsm.c:
Patch Set #2, Line 326: static void ts_fsm_unused_pdch_act(struct osmo_fsm_inst *fi)
it would be cool to add this function in a separate patch before this patch, so that in this patch t […]
lets skip this part, we are a bit under pressure to get this patch set merged.
To view, visit change 31497. To unsubscribe, or for help writing mail filters, visit settings.