From 96cb0c54a97c398512e74d3fd4d14582e3ffb7e4 Mon Sep 17 00:00:00 2001 From: Pau Espin Pedrol Date: Sat, 28 Sep 2019 00:50:51 +0200 Subject: [PATCH] bts-trx: Rework code handling poweron state Use of variables in each code is confusing and mixing configuration with POWERON/POWEROFF state (which is at least per phy inst and not per TRX, since those commands are only expected on the 1st phy inst). * field "poweron" becomes "enabled", and is used as an indicator for actions to take during TRX provisioning (hether to power it on and configure it or to power it off). * poweron/poweroff state becomes "powered", and it is shared by all trx in same phy_link, and is updated only after confirmation by TRX. * poweron_set becomes poweronoff_set (because it's used by both POWERON and POWEROFF), and becomes shared by all trx in same phy_link, since those CMDs are usually sent by first phy instance of the link (the first trx). Related: OS#4215 Change-Id: Icd0b482f1454236432e1952220bbec9d178b8607 --- include/osmo-bts/phy_link.h | 2 ++ src/osmo-bts-trx/l1_if.c | 42 ++++++++++++++-------- src/osmo-bts-trx/l1_if.h | 4 +-- src/osmo-bts-trx/trx_if.c | 72 +++++++++++++++++++++---------------- src/osmo-bts-trx/trx_if.h | 5 +-- src/osmo-bts-trx/trx_vty.c | 4 +-- 6 files changed, 78 insertions(+), 51 deletions(-) diff --git a/include/osmo-bts/phy_link.h b/include/osmo-bts/phy_link.h index a06cf3fc0..316a1badb 100644 --- a/include/osmo-bts/phy_link.h +++ b/include/osmo-bts/phy_link.h @@ -51,6 +51,8 @@ struct phy_link { uint32_t rts_advance; bool use_legacy_setbsic; uint8_t trxd_hdr_ver_max; /* Maximum TRXD header version to negotiate */ + bool powered; /* last POWERON (true) or POWEROFF (false) confirmed */ + bool poweronoff_sent; /* is there a POWERON/POWEROFF in transit? (one or the other based on ->powered) */ } osmotrx; struct { char *mcast_dev; /* Network device for multicast */ diff --git a/src/osmo-bts-trx/l1_if.c b/src/osmo-bts-trx/l1_if.c index 9de0abc8a..f564dc5d4 100644 --- a/src/osmo-bts-trx/l1_if.c +++ b/src/osmo-bts-trx/l1_if.c @@ -179,6 +179,21 @@ static void l1if_setslot_cb(struct trx_l1h *l1h, uint8_t tn, uint8_t type, int r cb_ts_connected(ts, rc); } +static void l1if_poweronoff_cb(struct trx_l1h *l1h, bool poweronoff, int rc) +{ + struct phy_instance *pinst = l1h->phy_inst; + struct phy_link *plink = pinst->phy_link; + + plink->u.osmotrx.powered = poweronoff; + plink->u.osmotrx.poweronoff_sent = false; + + if (poweronoff) { + if (rc == 0 && pinst->phy_link->state != PHY_LINK_CONNECTED) + phy_link_state_set(pinst->phy_link, PHY_LINK_CONNECTED); + else if (rc != 0 && pinst->phy_link->state != PHY_LINK_SHUTDOWN) + phy_link_state_set(pinst->phy_link, PHY_LINK_SHUTDOWN); + } +} /* * transceiver provisioning @@ -192,7 +207,7 @@ int l1if_provision_transceiver_trx(struct trx_l1h *l1h) if (!transceiver_available) return -EIO; - if (l1h->config.poweron + if (l1h->config.enabled && l1h->config.tsc_valid && l1h->config.bsic_valid && l1h->config.arfcn_valid) { @@ -225,9 +240,9 @@ int l1if_provision_transceiver_trx(struct trx_l1h *l1h) l1h->config.setformat_sent = 1; } - if (!l1h->config.poweron_sent) { - trx_if_cmd_poweron(l1h); - l1h->config.poweron_sent = 1; + if (pinst->num == 0 && !plink->u.osmotrx.powered && !plink->u.osmotrx.poweronoff_sent) { + trx_if_cmd_poweron(l1h, l1if_poweronoff_cb); + plink->u.osmotrx.poweronoff_sent = true; } /* after power on */ @@ -259,9 +274,11 @@ int l1if_provision_transceiver_trx(struct trx_l1h *l1h) return 0; } - if (!l1h->config.poweron && !l1h->config.poweron_sent) { - trx_if_cmd_poweroff(l1h); - l1h->config.poweron_sent = 1; + if (!l1h->config.enabled) { + if (pinst->num == 0 && plink->u.osmotrx.powered && !plink->u.osmotrx.poweronoff_sent) { + trx_if_cmd_poweroff(l1h, l1if_poweronoff_cb); + plink->u.osmotrx.poweronoff_sent = true; + } l1h->config.rxgain_sent = 0; l1h->config.power_sent = 0; l1h->config.maxdly_sent = 0; @@ -287,7 +304,6 @@ int l1if_provision_transceiver(struct gsm_bts *bts) l1h->config.arfcn_sent = 0; l1h->config.tsc_sent = 0; l1h->config.bsic_sent = 0; - l1h->config.poweron_sent = 0; l1h->config.rxgain_sent = 0; l1h->config.power_sent = 0; l1h->config.maxdly_sent = 0; @@ -310,9 +326,8 @@ static int trx_init(struct gsm_bts_trx *trx) struct trx_l1h *l1h = pinst->u.osmotrx.hdl; /* power on transceiver, if not already */ - if (!l1h->config.poweron) { - l1h->config.poweron = 1; - l1h->config.poweron_sent = 0; + if (!l1h->config.enabled) { + l1h->config.enabled = true; l1if_provision_transceiver_trx(l1h); } @@ -343,9 +358,8 @@ int bts_model_trx_close(struct gsm_bts_trx *trx) } /* power off transceiver, if not already */ - if (l1h->config.poweron) { - l1h->config.poweron = 0; - l1h->config.poweron_sent = 0; + if (l1h->config.enabled) { + l1h->config.enabled = false; l1if_provision_transceiver_trx(l1h); } diff --git a/src/osmo-bts-trx/l1_if.h b/src/osmo-bts-trx/l1_if.h index 29bd24686..a8d40e107 100644 --- a/src/osmo-bts-trx/l1_if.h +++ b/src/osmo-bts-trx/l1_if.h @@ -54,8 +54,8 @@ struct trx_config { uint8_t trxd_hdr_ver_use; /* actual TRXD header version in use */ int setformat_sent; - uint8_t poweron; /* poweron(1) or poweroff(0) */ - int poweron_sent; + bool enabled; + int arfcn_valid; uint16_t arfcn; diff --git a/src/osmo-bts-trx/trx_if.c b/src/osmo-bts-trx/trx_if.c index 166cfe6d0..a260919af 100644 --- a/src/osmo-bts-trx/trx_if.c +++ b/src/osmo-bts-trx/trx_if.c @@ -250,23 +250,15 @@ static int trx_ctrl_cmd_cb(struct trx_l1h *l1h, int critical, void *cb, const ch #define trx_ctrl_cmd(l1h, critical, cmd, fmt, ...) trx_ctrl_cmd_cb(l1h, critical, NULL, cmd, fmt, ##__VA_ARGS__) /*! Send "POWEROFF" command to TRX */ -int trx_if_cmd_poweroff(struct trx_l1h *l1h) +int trx_if_cmd_poweroff(struct trx_l1h *l1h, trx_if_cmd_poweronoff_cb *cb) { - struct phy_instance *pinst = l1h->phy_inst; - if (pinst->num == 0) - return trx_ctrl_cmd(l1h, 1, "POWEROFF", ""); - else - return 0; + return trx_ctrl_cmd_cb(l1h, 1, cb, "POWEROFF", ""); } /*! Send "POWERON" command to TRX */ -int trx_if_cmd_poweron(struct trx_l1h *l1h) +int trx_if_cmd_poweron(struct trx_l1h *l1h, trx_if_cmd_poweronoff_cb *cb) { - struct phy_instance *pinst = l1h->phy_inst; - if (pinst->num == 0) - return trx_ctrl_cmd(l1h, 1, "POWERON", ""); - else - return 0; + return trx_ctrl_cmd_cb(l1h, 1, cb, "POWERON", ""); } /*! Send "SETFORMAT" command to TRX: change TRXD header format version */ @@ -448,6 +440,35 @@ static bool cmd_matches_rsp(struct trx_ctrl_msg *tcm, struct trx_ctrl_rsp *rsp) return true; } +static int trx_ctrl_rx_rsp_poweron(struct trx_l1h *l1h, struct trx_ctrl_rsp *rsp) +{ + trx_if_cmd_poweronoff_cb *cb = (trx_if_cmd_poweronoff_cb*) rsp->cb; + + if (rsp->status != 0) + LOGPPHI(l1h->phy_inst, DTRX, LOGL_NOTICE, + "transceiver rejected POWERON command (%d), re-trying in a few seconds\n", + rsp->status); + + if (cb) + cb(l1h, true, rsp->status); + + /* If TRX fails, try again after 5 sec */ + return rsp->status == 0 ? 0 : 5; +} + +static int trx_ctrl_rx_rsp_poweroff(struct trx_l1h *l1h, struct trx_ctrl_rsp *rsp) +{ + trx_if_cmd_poweronoff_cb *cb = (trx_if_cmd_poweronoff_cb*) rsp->cb; + + if (rsp->status == 0) { + if (cb) + cb(l1h, false, rsp->status); + return 0; + } + + return -EINVAL; +} + static int trx_ctrl_rx_rsp_setslot(struct trx_l1h *l1h, struct trx_ctrl_rsp *rsp) { trx_if_cmd_setslot_cb *cb = (trx_if_cmd_setslot_cb*) rsp->cb; @@ -525,22 +546,10 @@ static int trx_ctrl_rx_rsp(struct trx_l1h *l1h, struct trx_ctrl_rsp *rsp, struct trx_ctrl_msg *tcm) { - struct phy_instance *pinst = l1h->phy_inst; - - /* If TRX fails, try again after 1 sec */ if (strcmp(rsp->cmd, "POWERON") == 0) { - if (rsp->status == 0) { - if (pinst->phy_link->state != PHY_LINK_CONNECTED) - phy_link_state_set(pinst->phy_link, PHY_LINK_CONNECTED); - return 0; - } else { - LOGPPHI(l1h->phy_inst, DTRX, LOGL_NOTICE, - "transceiver rejected POWERON command (%d), re-trying in a few seconds\n", - rsp->status); - if (pinst->phy_link->state != PHY_LINK_SHUTDOWN) - phy_link_state_set(pinst->phy_link, PHY_LINK_SHUTDOWN); - return 5; - } + return trx_ctrl_rx_rsp_poweron(l1h, rsp); + } else if (strcmp(rsp->cmd, "POWEROFF") == 0) { + return trx_ctrl_rx_rsp_poweroff(l1h, rsp); } else if (strcmp(rsp->cmd, "SETSLOT") == 0) { return trx_ctrl_rx_rsp_setslot(l1h, rsp); /* We may get 'RSP ERR 1' if 'SETFORMAT' is not supported, @@ -1176,9 +1185,8 @@ static int trx_if_open(struct trx_l1h *l1h) /* enable all slots */ l1h->config.slotmask = 0xff; - /* FIXME: why was this only for TRX0 ? */ - //if (l1h->trx->nr == 0) - trx_if_cmd_poweroff(l1h); + if (pinst->num == 0) + trx_if_cmd_poweroff(l1h, NULL); return 0; @@ -1266,5 +1274,7 @@ cleanup: /*! determine if the TRX for given handle is powered up */ int trx_if_powered(struct trx_l1h *l1h) { - return l1h->config.poweron; + struct phy_instance *pinst = l1h->phy_inst; + struct phy_link *plink = pinst->phy_link; + return plink->u.osmotrx.powered; } diff --git a/src/osmo-bts-trx/trx_if.h b/src/osmo-bts-trx/trx_if.h index dda7116ee..3325c6256 100644 --- a/src/osmo-bts-trx/trx_if.h +++ b/src/osmo-bts-trx/trx_if.h @@ -15,11 +15,12 @@ struct trx_ctrl_msg { void *cb; }; +typedef void trx_if_cmd_poweronoff_cb(struct trx_l1h *l1h, bool poweronoff, int rc); typedef void trx_if_cmd_setslot_cb(struct trx_l1h *l1h, uint8_t tn, uint8_t type, int rc); void trx_if_init(struct trx_l1h *l1h); -int trx_if_cmd_poweroff(struct trx_l1h *l1h); -int trx_if_cmd_poweron(struct trx_l1h *l1h); +int trx_if_cmd_poweroff(struct trx_l1h *l1h, trx_if_cmd_poweronoff_cb *cb); +int trx_if_cmd_poweron(struct trx_l1h *l1h, trx_if_cmd_poweronoff_cb *cb); int trx_if_cmd_settsc(struct trx_l1h *l1h, uint8_t tsc); int trx_if_cmd_setbsic(struct trx_l1h *l1h, uint8_t bsic); int trx_if_cmd_setrxgain(struct trx_l1h *l1h, int db); diff --git a/src/osmo-bts-trx/trx_vty.c b/src/osmo-bts-trx/trx_vty.c index f554ae530..993c7809c 100644 --- a/src/osmo-bts-trx/trx_vty.c +++ b/src/osmo-bts-trx/trx_vty.c @@ -290,9 +290,9 @@ DEFUN(cfg_phyinst_power_on, cfg_phyinst_power_on_cmd, struct trx_l1h *l1h = pinst->u.osmotrx.hdl; if (strcmp(argv[0], "on")) - vty_out(vty, "OFF: %d%s", trx_if_cmd_poweroff(l1h), VTY_NEWLINE); + vty_out(vty, "OFF: %d%s", trx_if_cmd_poweroff(l1h, NULL), VTY_NEWLINE); else { - vty_out(vty, "ON: %d%s", trx_if_cmd_poweron(l1h), VTY_NEWLINE); + vty_out(vty, "ON: %d%s", trx_if_cmd_poweron(l1h, NULL), VTY_NEWLINE); } return CMD_SUCCESS;