This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
pespin gerrit-no-reply at lists.osmocom.orgpespin has submitted this change and it was merged. ( https://gerrit.osmocom.org/c/osmo-bts/+/15615 ) Change subject: bts-trx: Get rid of messy transceiver_available state handler ...................................................................... bts-trx: Get rid of messy transceiver_available state handler This variable meaning has been changing its exact meaning over time until finally not being really clear which kind of state it holds. Initially it seemed to be used to identfy whether CLOCK IND were being received. However, over time both osmo-bts and osmo-trx have evolved and were fixed so that clock indications are only sent by osmo-trx after POWERON command is sent. As a result, this state can be checked simply by looking at the "powered" phy_link variable, which is only set to true once the TRX has confirmed the POWERON command, and hence it is sending CLOCK IND. On the other hand, at some point in time "available" started to be set to 1 in bts_model_phy_link_open(), which means available is nowadays almost always 1 from startup until the end (only dropped during bts_shutdown(), when we are already exiting the process anyway). As a result, !available condition in scheduler_trx.c:trx_fn_timer_cb can almost never happen, because available is set to true already. Only possibility would be if an old process of osmo-trx (not set up by this BTS process) is still sending CLOCK INDs, but in that case we for sure don't want to configure the BTS based on that, but ignore them until this BTS process has again configured the TRX. So that whole check can be dropped. We are better checking for "powered" state, which is far more accurate, and we better do that in trx_if.c before calling trx_fn_timer_cb(). Other uses of "transceiver_available" being used can be changed to use plink->state!= PHY_LINK_SHUTDOWN, since available was already being set to 1 at the same time the plink->state was being set to PHY_LINK_CONNECTING. As a result of this state handling re-arrangement, OS#4215 is fixed since trx_if_powered() is used instead of previous state condition to check whether data frames should be sent. Related: OS#4215 Change-Id: I35f4697bd33dbe8a4c76c9500b82c16589c701d4 --- M src/osmo-bts-trx/l1_if.c M src/osmo-bts-trx/scheduler_trx.c M src/osmo-bts-trx/trx_if.c M src/osmo-bts-trx/trx_if.h M src/osmo-bts-trx/trx_vty.c 5 files changed, 21 insertions(+), 40 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, but someone else must approve pespin: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/osmo-bts-trx/l1_if.c b/src/osmo-bts-trx/l1_if.c index f564dc5..221d88b 100644 --- a/src/osmo-bts-trx/l1_if.c +++ b/src/osmo-bts-trx/l1_if.c @@ -204,8 +204,18 @@ struct phy_instance *pinst = l1h->phy_inst; struct phy_link *plink = pinst->phy_link; - if (!transceiver_available) + /* During setup, pinst may still not be associated to a TRX nr */ + if (!pinst->trx) { + LOGPPHI(pinst, DL1C, LOGL_INFO, + "Delaying provision, TRX not yet assigned to phy instance\n"); return -EIO; + } + + if (phy_link_state_get(plink) == PHY_LINK_SHUTDOWN) { + LOGPPHI(pinst, DL1C, LOGL_INFO, + "Delaying provision, TRX not yet available\n"); + return -EIO; + } if (l1h->config.enabled && l1h->config.tsc_valid @@ -390,6 +400,7 @@ llist_for_each_entry(trx, &bts->trx_list, list) { struct phy_instance *pinst = trx_phy_instance(trx); + struct phy_link *plink = pinst->phy_link; struct trx_l1h *l1h = pinst->u.osmotrx.hdl; if (l1h->config.bsic != bsic || !l1h->config.bsic_valid) { l1h->config.bsic = bsic; @@ -397,7 +408,7 @@ l1h->config.bsic_sent = 0; l1if_provision_transceiver_trx(l1h); } - check_transceiver_availability_trx(l1h, transceiver_available); + check_transceiver_availability_trx(l1h, phy_link_state_get(plink) != PHY_LINK_SHUTDOWN); } return 0; diff --git a/src/osmo-bts-trx/scheduler_trx.c b/src/osmo-bts-trx/scheduler_trx.c index b3b656a..af639e2 100644 --- a/src/osmo-bts-trx/scheduler_trx.c +++ b/src/osmo-bts-trx/scheduler_trx.c @@ -1721,21 +1721,6 @@ clock_gettime(CLOCK_MONOTONIC, &tv_now); - /* clock becomes valid */ - if (!transceiver_available) { - LOGP(DL1C, LOGL_NOTICE, "initial GSM clock received: fn=%u\n", fn); - - transceiver_available = 1; - - /* start provisioning transceiver */ - l1if_provision_transceiver(bts); - - /* tell BSC */ - check_transceiver_availability(bts, 1); - - return trx_setup_clock(bts, tcs, &tv_now, &interval, fn); - } - /* calculate elapsed time +fn since last timer */ elapsed_us = compute_elapsed_us(&tcs->last_fn_timer.tv, &tv_now); elapsed_fn = compute_elapsed_fn(tcs->last_fn_timer.fn, fn); diff --git a/src/osmo-bts-trx/trx_if.c b/src/osmo-bts-trx/trx_if.c index a260919..9933109 100644 --- a/src/osmo-bts-trx/trx_if.c +++ b/src/osmo-bts-trx/trx_if.c @@ -48,8 +48,6 @@ #include "l1_if.h" #include "trx_if.h" -int transceiver_available = 0; - /* * socket helper functions */ @@ -125,6 +123,10 @@ "wrapping correctly, correcting to fn=%u\n", fn); } + if (!plink->u.osmotrx.powered) { + LOGPPHI(pinst, DTRX, LOGL_NOTICE, "Ignoring CLOCK IND %u, TRX not yet powered on\n", fn); + return 0; + } /* inform core TRX clock handling code that a FN has been received */ trx_sched_clock(pinst->trx->bts, fn); @@ -201,13 +203,6 @@ va_list ap; int pending; - if (!transceiver_available && - !(!strcmp(cmd, "POWEROFF") || !strcmp(cmd, "POWERON"))) { - LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR, "CTRL %s ignored: No clock from " - "transceiver, please fix!\n", cmd); - return -EIO; - } - pending = !llist_empty(&l1h->trx_ctrl_list); /* create message */ @@ -1097,12 +1092,11 @@ /* copy ubits {0,1} */ memcpy(buf + 6, bits, nbits); - /* we must be sure that we have clock, and we have sent all control - * data */ - if (transceiver_available && llist_empty(&l1h->trx_ctrl_list)) { + /* we must be sure that TRX is on */ + if (trx_if_powered(l1h)) { send(l1h->trx_ofd_data.fd, buf, nbits + 6, 0); } else - LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR, "Ignoring TX data, transceiver offline.\n"); + LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR, "Ignoring TX data, transceiver powered off.\n"); return 0; } @@ -1255,8 +1249,7 @@ if (trx_phy_inst_open(pinst) < 0) goto cleanup; } - /* FIXME: is there better way to check/report TRX availability? */ - transceiver_available = 1; + return 0; cleanup: diff --git a/src/osmo-bts-trx/trx_if.h b/src/osmo-bts-trx/trx_if.h index 3325c62..fd0077d 100644 --- a/src/osmo-bts-trx/trx_if.h +++ b/src/osmo-bts-trx/trx_if.h @@ -1,8 +1,6 @@ #ifndef TRX_IF_H #define TRX_IF_H -extern int transceiver_available; - struct trx_l1h; struct trx_ctrl_msg { diff --git a/src/osmo-bts-trx/trx_vty.c b/src/osmo-bts-trx/trx_vty.c index 993c780..86f5712 100644 --- a/src/osmo-bts-trx/trx_vty.c +++ b/src/osmo-bts-trx/trx_vty.c @@ -58,12 +58,6 @@ struct gsm_bts_trx *trx; struct trx_l1h *l1h; - if (!transceiver_available) { - vty_out(vty, "transceiver is not connected%s", VTY_NEWLINE); - } else { - vty_out(vty, "transceiver is connected%s", VTY_NEWLINE); - } - llist_for_each_entry(trx, &bts->trx_list, list) { struct phy_instance *pinst = trx_phy_instance(trx); struct phy_link *plink = pinst->phy_link; -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/15615 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I35f4697bd33dbe8a4c76c9500b82c16589c701d4 Gerrit-Change-Number: 15615 Gerrit-PatchSet: 6 Gerrit-Owner: pespin <pespin at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <axilirator at gmail.com> Gerrit-Reviewer: laforge <laforge at osmocom.org> Gerrit-Reviewer: pespin <pespin at sysmocom.de> Gerrit-MessageType: merged -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20191005/d927db90/attachment.htm>