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>