From Max msuraev@sysmocom.de:
Max has uploaded a new change for review.
Change subject: Change internal API for consistency ......................................................................
Change internal API for consistency
Use uint8_t for TRX numbering everywhere (we don't expect hardware with more than 256 transceivers in the near future). This change helps to avoid unnecessary casts and make API much clearer.
Change-Id: Ic584611184b0c8b5417ecff0ddae3d526b55a079 Related: SYS#2443 --- M src/osmo-bts-sysmo/sysmo_l1_if.c M src/osmo-bts-sysmo/sysmo_l1_if.h M src/pcu_l1_if.cpp 3 files changed, 9 insertions(+), 9 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/59/59/1
diff --git a/src/osmo-bts-sysmo/sysmo_l1_if.c b/src/osmo-bts-sysmo/sysmo_l1_if.c index c7c54dd..4bfcecd 100644 --- a/src/osmo-bts-sysmo/sysmo_l1_if.c +++ b/src/osmo-bts-sysmo/sysmo_l1_if.c @@ -155,7 +155,7 @@ switch (rts_ind->sapi) { case GsmL1_Sapi_Pdtch: case GsmL1_Sapi_Pacch: - rc = pcu_rx_rts_req_pdtch((long)fl1h->priv, rts_ind->u8Tn, + rc = pcu_rx_rts_req_pdtch(fl1h->trx, rts_ind->u8Tn, rts_ind->u16Arfcn, rts_ind->u32Fn, rts_ind->u8BlockNbr); case GsmL1_Sapi_Ptcch: // FIXME @@ -215,7 +215,7 @@ != GsmL1_PdtchPlType_Full) break; /* PDTCH / PACCH frame handling */ - pcu_rx_data_ind_pdtch((long)fl1h->priv, data_ind->u8Tn, + pcu_rx_data_ind_pdtch(fl1h->trx, data_ind->u8Tn, data_ind->msgUnitParam.u8Buffer + 1, data_ind->msgUnitParam.u8Size - 1, data_ind->u32Fn, @@ -357,7 +357,7 @@ return 0; }
-void *l1if_open_pdch(void *priv, uint32_t hlayer1, struct gsmtap_inst *gsmtap) +void *l1if_open_pdch(uint8_t trx, uint32_t hlayer1, struct gsmtap_inst *gsmtap) { struct femtol1_hdl *fl1h; int rc; @@ -367,7 +367,7 @@ return NULL;
fl1h->hLayer1 = hlayer1; - fl1h->priv = priv; + fl1h->trx = trx; fl1h->clk_cal = 0; /* default clock source: OCXO */ fl1h->clk_src = SuperFemto_ClkSrcId_Ocxo; diff --git a/src/osmo-bts-sysmo/sysmo_l1_if.h b/src/osmo-bts-sysmo/sysmo_l1_if.h index 6b50d4e..83ca481 100644 --- a/src/osmo-bts-sysmo/sysmo_l1_if.h +++ b/src/osmo-bts-sysmo/sysmo_l1_if.h @@ -38,7 +38,7 @@ struct gsmtap_inst *gsmtap; uint32_t gsmtap_sapi_mask;
- void *priv; /* user reference */ + uint8_t trx;
struct osmo_timer_list alive_timer; unsigned int alive_prim_cnt; diff --git a/src/pcu_l1_if.cpp b/src/pcu_l1_if.cpp index 67272ab..17e9d75 100644 --- a/src/pcu_l1_if.cpp +++ b/src/pcu_l1_if.cpp @@ -44,7 +44,7 @@
// FIXME: move this, when changed from c++ to c. extern "C" { -void *l1if_open_pdch(void *priv, uint32_t hlayer1, struct gsmtap_inst *gsmtap); +void *l1if_open_pdch(uint8_t trx, uint32_t hlayer1, struct gsmtap_inst *gsmtap); int l1if_connect_pdch(void *obj, uint8_t ts); int l1if_pdch_req(void *obj, uint8_t ts, int is_ptcch, uint32_t fn, uint16_t arfcn, uint8_t block_nr, uint8_t *data, uint8_t len); @@ -330,8 +330,8 @@ struct gprs_bssgp_pcu *pcu; struct gprs_rlcmac_pdch *pdch; struct in_addr ia; - int rc = 0; - int trx, ts; + int rc = 0, ts; + uint8_t trx; int i;
if (info_ind->version != PCU_IF_VERSION) { @@ -450,7 +450,7 @@ info_ind->trx[trx].hlayer1); if (!bts->trx[trx].fl1h) bts->trx[trx].fl1h = l1if_open_pdch( - (void *)trx, + trx, info_ind->trx[trx].hlayer1, bts->gsmtap); if (!bts->trx[trx].fl1h) {
From Holger Freyther holger@freyther.de:
Holger Freyther has posted comments on this change.
Change subject: Change internal API for consistency ......................................................................
Patch Set 1:
(2 comments)
https://gerrit.osmocom.org/#/c/59/1//COMMIT_MSG Commit Message:
Line 7: Change internal API for consistency consistency with what?
https://gerrit.osmocom.org/#/c/59/1/src/osmo-bts-sysmo/sysmo_l1_if.h File src/osmo-bts-sysmo/sysmo_l1_if.h:
Line 41: uint8_t trx; We have a TRX object. Can you call it trx_no?
From Holger Freyther holger@freyther.de:
Holger Freyther has posted comments on this change.
Change subject: Change internal API for consistency ......................................................................
Patch Set 1: Code-Review-1
From Max msuraev@sysmocom.de:
Hello Jenkins Builder, Holger Freyther,
I'd like you to reexamine a change. Please visit
to look at the new patch set (#2).
Change subject: Change internal API for consistency ......................................................................
Change internal API for consistency
Make TRX API (void *) consistent with the way it's used (integer). Use uint8_t for TRX numbering everywhere (we don't expect hardware with more than 256 transceivers in the near future). This change helps to avoid unnecessary casts and make API much clearer.
Change-Id: Ic584611184b0c8b5417ecff0ddae3d526b55a079 Related: SYS#2443 --- M src/osmo-bts-sysmo/sysmo_l1_if.c M src/osmo-bts-sysmo/sysmo_l1_if.h M src/pcu_l1_if.cpp 3 files changed, 10 insertions(+), 9 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/59/59/2
Patch Set 2: Code-Review+2
Holger Freyther has submitted this change and it was merged.
Change subject: Change internal API for consistency ......................................................................
Change internal API for consistency
Make TRX API (void *) consistent with the way it's used (integer). Use uint8_t for TRX numbering everywhere (we don't expect hardware with more than 256 transceivers in the near future). This change helps to avoid unnecessary casts and make API much clearer.
Change-Id: Ic584611184b0c8b5417ecff0ddae3d526b55a079 Related: SYS#2443 Reviewed-on: https://gerrit.osmocom.org/59 Tested-by: Jenkins Builder Reviewed-by: Holger Freyther holger@freyther.de --- M src/osmo-bts-sysmo/sysmo_l1_if.c M src/osmo-bts-sysmo/sysmo_l1_if.h M src/pcu_l1_if.cpp 3 files changed, 10 insertions(+), 9 deletions(-)
Approvals: Jenkins Builder: Verified Holger Freyther: Looks good to me, approved
diff --git a/src/osmo-bts-sysmo/sysmo_l1_if.c b/src/osmo-bts-sysmo/sysmo_l1_if.c index c7c54dd..c072c1a 100644 --- a/src/osmo-bts-sysmo/sysmo_l1_if.c +++ b/src/osmo-bts-sysmo/sysmo_l1_if.c @@ -155,7 +155,7 @@ switch (rts_ind->sapi) { case GsmL1_Sapi_Pdtch: case GsmL1_Sapi_Pacch: - rc = pcu_rx_rts_req_pdtch((long)fl1h->priv, rts_ind->u8Tn, + rc = pcu_rx_rts_req_pdtch(fl1h->trx_no, rts_ind->u8Tn, rts_ind->u16Arfcn, rts_ind->u32Fn, rts_ind->u8BlockNbr); case GsmL1_Sapi_Ptcch: // FIXME @@ -215,7 +215,7 @@ != GsmL1_PdtchPlType_Full) break; /* PDTCH / PACCH frame handling */ - pcu_rx_data_ind_pdtch((long)fl1h->priv, data_ind->u8Tn, + pcu_rx_data_ind_pdtch(fl1h->trx_no, data_ind->u8Tn, data_ind->msgUnitParam.u8Buffer + 1, data_ind->msgUnitParam.u8Size - 1, data_ind->u32Fn, @@ -357,7 +357,7 @@ return 0; }
-void *l1if_open_pdch(void *priv, uint32_t hlayer1, struct gsmtap_inst *gsmtap) +void *l1if_open_pdch(uint8_t trx_no, uint32_t hlayer1, struct gsmtap_inst *gsmtap) { struct femtol1_hdl *fl1h; int rc; @@ -367,7 +367,7 @@ return NULL;
fl1h->hLayer1 = hlayer1; - fl1h->priv = priv; + fl1h->trx_no = trx_no; fl1h->clk_cal = 0; /* default clock source: OCXO */ fl1h->clk_src = SuperFemto_ClkSrcId_Ocxo; diff --git a/src/osmo-bts-sysmo/sysmo_l1_if.h b/src/osmo-bts-sysmo/sysmo_l1_if.h index 6b50d4e..8f1857e 100644 --- a/src/osmo-bts-sysmo/sysmo_l1_if.h +++ b/src/osmo-bts-sysmo/sysmo_l1_if.h @@ -38,7 +38,7 @@ struct gsmtap_inst *gsmtap; uint32_t gsmtap_sapi_mask;
- void *priv; /* user reference */ + uint8_t trx_no;
struct osmo_timer_list alive_timer; unsigned int alive_prim_cnt; diff --git a/src/pcu_l1_if.cpp b/src/pcu_l1_if.cpp index 67272ab..790789c 100644 --- a/src/pcu_l1_if.cpp +++ b/src/pcu_l1_if.cpp @@ -44,7 +44,8 @@
// FIXME: move this, when changed from c++ to c. extern "C" { -void *l1if_open_pdch(void *priv, uint32_t hlayer1, struct gsmtap_inst *gsmtap); +void *l1if_open_pdch(uint8_t trx_no, uint32_t hlayer1, + struct gsmtap_inst *gsmtap); int l1if_connect_pdch(void *obj, uint8_t ts); int l1if_pdch_req(void *obj, uint8_t ts, int is_ptcch, uint32_t fn, uint16_t arfcn, uint8_t block_nr, uint8_t *data, uint8_t len); @@ -330,8 +331,8 @@ struct gprs_bssgp_pcu *pcu; struct gprs_rlcmac_pdch *pdch; struct in_addr ia; - int rc = 0; - int trx, ts; + int rc = 0, ts; + uint8_t trx; int i;
if (info_ind->version != PCU_IF_VERSION) { @@ -450,7 +451,7 @@ info_ind->trx[trx].hlayer1); if (!bts->trx[trx].fl1h) bts->trx[trx].fl1h = l1if_open_pdch( - (void *)trx, + trx, info_ind->trx[trx].hlayer1, bts->gsmtap); if (!bts->trx[trx].fl1h) {