<p>pespin has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bts/+/15629">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">bts-trx: Rework code handling poweron state<br><br>Use of variables in each code is confusing and mixing configuration with<br>POWERON/POWEROFF state (which is at least per phy inst and not per TRX,<br>since those commands are only expected on the 1st phy inst).<br><br>* field "poweron" becomes "enabled", and is used as an indicator for<br>actions to take during TRX provisioning (hether to power it on and<br>configure it or to power it off).<br>* poweron/poweroff state becomes "powered", and it is shared by all trx<br>in same phy_link, and is updated only after confirmation by TRX.<br>* poweron_set becomes poweronoff_set (because it's used by both POWERON<br>and POWEROFF), and becomes shared by all trx in same phy_link, since<br>those CMDs are usually sent by first phy instance of the link (the first<br>trx).<br><br>Related: OS#4215<br>Change-Id: Icd0b482f1454236432e1952220bbec9d178b8607<br>---<br>M include/osmo-bts/phy_link.h<br>M src/osmo-bts-trx/l1_if.c<br>M src/osmo-bts-trx/l1_if.h<br>M src/osmo-bts-trx/trx_if.c<br>M src/osmo-bts-trx/trx_if.h<br>M src/osmo-bts-trx/trx_vty.c<br>6 files changed, 78 insertions(+), 51 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/29/15629/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmo-bts/phy_link.h b/include/osmo-bts/phy_link.h</span><br><span>index a06cf3f..316a1ba 100644</span><br><span>--- a/include/osmo-bts/phy_link.h</span><br><span>+++ b/include/osmo-bts/phy_link.h</span><br><span>@@ -51,6 +51,8 @@</span><br><span> uint32_t rts_advance;</span><br><span> bool use_legacy_setbsic;</span><br><span> uint8_t trxd_hdr_ver_max; /* Maximum TRXD header version to negotiate */</span><br><span style="color: hsl(120, 100%, 40%);">+ bool powered; /* last POWERON (true) or POWEROFF (false) confirmed */</span><br><span style="color: hsl(120, 100%, 40%);">+ bool poweronoff_sent; /* is there a POWERON/POWEROFF in transit? (one or the other based on ->powered) */</span><br><span> } osmotrx;</span><br><span> struct {</span><br><span> char *mcast_dev; /* Network device for multicast */</span><br><span>diff --git a/src/osmo-bts-trx/l1_if.c b/src/osmo-bts-trx/l1_if.c</span><br><span>index 9de0abc..f564dc5 100644</span><br><span>--- a/src/osmo-bts-trx/l1_if.c</span><br><span>+++ b/src/osmo-bts-trx/l1_if.c</span><br><span>@@ -179,6 +179,21 @@</span><br><span> cb_ts_connected(ts, rc);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static void l1if_poweronoff_cb(struct trx_l1h *l1h, bool poweronoff, int rc)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ struct phy_instance *pinst = l1h->phy_inst;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct phy_link *plink = pinst->phy_link;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ plink->u.osmotrx.powered = poweronoff;</span><br><span style="color: hsl(120, 100%, 40%);">+ plink->u.osmotrx.poweronoff_sent = false;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (poweronoff) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc == 0 && pinst->phy_link->state != PHY_LINK_CONNECTED)</span><br><span style="color: hsl(120, 100%, 40%);">+ phy_link_state_set(pinst->phy_link, PHY_LINK_CONNECTED);</span><br><span style="color: hsl(120, 100%, 40%);">+ else if (rc != 0 && pinst->phy_link->state != PHY_LINK_SHUTDOWN)</span><br><span style="color: hsl(120, 100%, 40%);">+ phy_link_state_set(pinst->phy_link, PHY_LINK_SHUTDOWN);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span> </span><br><span> /*</span><br><span> * transceiver provisioning</span><br><span>@@ -192,7 +207,7 @@</span><br><span> if (!transceiver_available)</span><br><span> return -EIO;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (l1h->config.poweron</span><br><span style="color: hsl(120, 100%, 40%);">+ if (l1h->config.enabled</span><br><span> && l1h->config.tsc_valid</span><br><span> && l1h->config.bsic_valid</span><br><span> && l1h->config.arfcn_valid) {</span><br><span>@@ -225,9 +240,9 @@</span><br><span> l1h->config.setformat_sent = 1;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!l1h->config.poweron_sent) {</span><br><span style="color: hsl(0, 100%, 40%);">- trx_if_cmd_poweron(l1h);</span><br><span style="color: hsl(0, 100%, 40%);">- l1h->config.poweron_sent = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (pinst->num == 0 && !plink->u.osmotrx.powered && !plink->u.osmotrx.poweronoff_sent) {</span><br><span style="color: hsl(120, 100%, 40%);">+ trx_if_cmd_poweron(l1h, l1if_poweronoff_cb);</span><br><span style="color: hsl(120, 100%, 40%);">+ plink->u.osmotrx.poweronoff_sent = true;</span><br><span> }</span><br><span> </span><br><span> /* after power on */</span><br><span>@@ -259,9 +274,11 @@</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!l1h->config.poweron && !l1h->config.poweron_sent) {</span><br><span style="color: hsl(0, 100%, 40%);">- trx_if_cmd_poweroff(l1h);</span><br><span style="color: hsl(0, 100%, 40%);">- l1h->config.poweron_sent = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!l1h->config.enabled) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (pinst->num == 0 && plink->u.osmotrx.powered && !plink->u.osmotrx.poweronoff_sent) {</span><br><span style="color: hsl(120, 100%, 40%);">+ trx_if_cmd_poweroff(l1h, l1if_poweronoff_cb);</span><br><span style="color: hsl(120, 100%, 40%);">+ plink->u.osmotrx.poweronoff_sent = true;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> l1h->config.rxgain_sent = 0;</span><br><span> l1h->config.power_sent = 0;</span><br><span> l1h->config.maxdly_sent = 0;</span><br><span>@@ -287,7 +304,6 @@</span><br><span> l1h->config.arfcn_sent = 0;</span><br><span> l1h->config.tsc_sent = 0;</span><br><span> l1h->config.bsic_sent = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- l1h->config.poweron_sent = 0;</span><br><span> l1h->config.rxgain_sent = 0;</span><br><span> l1h->config.power_sent = 0;</span><br><span> l1h->config.maxdly_sent = 0;</span><br><span>@@ -310,9 +326,8 @@</span><br><span> struct trx_l1h *l1h = pinst->u.osmotrx.hdl;</span><br><span> </span><br><span> /* power on transceiver, if not already */</span><br><span style="color: hsl(0, 100%, 40%);">- if (!l1h->config.poweron) {</span><br><span style="color: hsl(0, 100%, 40%);">- l1h->config.poweron = 1;</span><br><span style="color: hsl(0, 100%, 40%);">- l1h->config.poweron_sent = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!l1h->config.enabled) {</span><br><span style="color: hsl(120, 100%, 40%);">+ l1h->config.enabled = true;</span><br><span> l1if_provision_transceiver_trx(l1h);</span><br><span> }</span><br><span> </span><br><span>@@ -343,9 +358,8 @@</span><br><span> }</span><br><span> </span><br><span> /* power off transceiver, if not already */</span><br><span style="color: hsl(0, 100%, 40%);">- if (l1h->config.poweron) {</span><br><span style="color: hsl(0, 100%, 40%);">- l1h->config.poweron = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- l1h->config.poweron_sent = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (l1h->config.enabled) {</span><br><span style="color: hsl(120, 100%, 40%);">+ l1h->config.enabled = false;</span><br><span> l1if_provision_transceiver_trx(l1h);</span><br><span> }</span><br><span> </span><br><span>diff --git a/src/osmo-bts-trx/l1_if.h b/src/osmo-bts-trx/l1_if.h</span><br><span>index 29bd246..a8d40e1 100644</span><br><span>--- a/src/osmo-bts-trx/l1_if.h</span><br><span>+++ b/src/osmo-bts-trx/l1_if.h</span><br><span>@@ -54,8 +54,8 @@</span><br><span> uint8_t trxd_hdr_ver_use; /* actual TRXD header version in use */</span><br><span> int setformat_sent;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t poweron; /* poweron(1) or poweroff(0) */</span><br><span style="color: hsl(0, 100%, 40%);">- int poweron_sent;</span><br><span style="color: hsl(120, 100%, 40%);">+ bool enabled;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> </span><br><span> int arfcn_valid;</span><br><span> uint16_t arfcn;</span><br><span>diff --git a/src/osmo-bts-trx/trx_if.c b/src/osmo-bts-trx/trx_if.c</span><br><span>index 166cfe6..a260919 100644</span><br><span>--- a/src/osmo-bts-trx/trx_if.c</span><br><span>+++ b/src/osmo-bts-trx/trx_if.c</span><br><span>@@ -250,23 +250,15 @@</span><br><span> #define trx_ctrl_cmd(l1h, critical, cmd, fmt, ...) trx_ctrl_cmd_cb(l1h, critical, NULL, cmd, fmt, ##__VA_ARGS__)</span><br><span> </span><br><span> /*! Send "POWEROFF" command to TRX */</span><br><span style="color: hsl(0, 100%, 40%);">-int trx_if_cmd_poweroff(struct trx_l1h *l1h)</span><br><span style="color: hsl(120, 100%, 40%);">+int trx_if_cmd_poweroff(struct trx_l1h *l1h, trx_if_cmd_poweronoff_cb *cb)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct phy_instance *pinst = l1h->phy_inst;</span><br><span style="color: hsl(0, 100%, 40%);">- if (pinst->num == 0)</span><br><span style="color: hsl(0, 100%, 40%);">- return trx_ctrl_cmd(l1h, 1, "POWEROFF", "");</span><br><span style="color: hsl(0, 100%, 40%);">- else</span><br><span style="color: hsl(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ return trx_ctrl_cmd_cb(l1h, 1, cb, "POWEROFF", "");</span><br><span> }</span><br><span> </span><br><span> /*! Send "POWERON" command to TRX */</span><br><span style="color: hsl(0, 100%, 40%);">-int trx_if_cmd_poweron(struct trx_l1h *l1h)</span><br><span style="color: hsl(120, 100%, 40%);">+int trx_if_cmd_poweron(struct trx_l1h *l1h, trx_if_cmd_poweronoff_cb *cb)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct phy_instance *pinst = l1h->phy_inst;</span><br><span style="color: hsl(0, 100%, 40%);">- if (pinst->num == 0)</span><br><span style="color: hsl(0, 100%, 40%);">- return trx_ctrl_cmd(l1h, 1, "POWERON", "");</span><br><span style="color: hsl(0, 100%, 40%);">- else</span><br><span style="color: hsl(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ return trx_ctrl_cmd_cb(l1h, 1, cb, "POWERON", "");</span><br><span> }</span><br><span> </span><br><span> /*! Send "SETFORMAT" command to TRX: change TRXD header format version */</span><br><span>@@ -448,6 +440,35 @@</span><br><span> return true;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static int trx_ctrl_rx_rsp_poweron(struct trx_l1h *l1h, struct trx_ctrl_rsp *rsp)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ trx_if_cmd_poweronoff_cb *cb = (trx_if_cmd_poweronoff_cb*) rsp->cb;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rsp->status != 0)</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPPHI(l1h->phy_inst, DTRX, LOGL_NOTICE,</span><br><span style="color: hsl(120, 100%, 40%);">+ "transceiver rejected POWERON command (%d), re-trying in a few seconds\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ rsp->status);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (cb)</span><br><span style="color: hsl(120, 100%, 40%);">+ cb(l1h, true, rsp->status);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* If TRX fails, try again after 5 sec */</span><br><span style="color: hsl(120, 100%, 40%);">+ return rsp->status == 0 ? 0 : 5;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+static int trx_ctrl_rx_rsp_poweroff(struct trx_l1h *l1h, struct trx_ctrl_rsp *rsp)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ trx_if_cmd_poweronoff_cb *cb = (trx_if_cmd_poweronoff_cb*) rsp->cb;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rsp->status == 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (cb)</span><br><span style="color: hsl(120, 100%, 40%);">+ cb(l1h, false, rsp->status);</span><br><span style="color: hsl(120, 100%, 40%);">+ return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static int trx_ctrl_rx_rsp_setslot(struct trx_l1h *l1h, struct trx_ctrl_rsp *rsp)</span><br><span> {</span><br><span> trx_if_cmd_setslot_cb *cb = (trx_if_cmd_setslot_cb*) rsp->cb;</span><br><span>@@ -525,22 +546,10 @@</span><br><span> struct trx_ctrl_rsp *rsp,</span><br><span> struct trx_ctrl_msg *tcm)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct phy_instance *pinst = l1h->phy_inst;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* If TRX fails, try again after 1 sec */</span><br><span> if (strcmp(rsp->cmd, "POWERON") == 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (rsp->status == 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (pinst->phy_link->state != PHY_LINK_CONNECTED)</span><br><span style="color: hsl(0, 100%, 40%);">- phy_link_state_set(pinst->phy_link, PHY_LINK_CONNECTED);</span><br><span style="color: hsl(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(0, 100%, 40%);">- } else {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPPHI(l1h->phy_inst, DTRX, LOGL_NOTICE,</span><br><span style="color: hsl(0, 100%, 40%);">- "transceiver rejected POWERON command (%d), re-trying in a few seconds\n",</span><br><span style="color: hsl(0, 100%, 40%);">- rsp->status);</span><br><span style="color: hsl(0, 100%, 40%);">- if (pinst->phy_link->state != PHY_LINK_SHUTDOWN)</span><br><span style="color: hsl(0, 100%, 40%);">- phy_link_state_set(pinst->phy_link, PHY_LINK_SHUTDOWN);</span><br><span style="color: hsl(0, 100%, 40%);">- return 5;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ return trx_ctrl_rx_rsp_poweron(l1h, rsp);</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (strcmp(rsp->cmd, "POWEROFF") == 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ return trx_ctrl_rx_rsp_poweroff(l1h, rsp);</span><br><span> } else if (strcmp(rsp->cmd, "SETSLOT") == 0) {</span><br><span> return trx_ctrl_rx_rsp_setslot(l1h, rsp);</span><br><span> /* We may get 'RSP ERR 1' if 'SETFORMAT' is not supported,</span><br><span>@@ -1176,9 +1185,8 @@</span><br><span> /* enable all slots */</span><br><span> l1h->config.slotmask = 0xff;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* FIXME: why was this only for TRX0 ? */</span><br><span style="color: hsl(0, 100%, 40%);">- //if (l1h->trx->nr == 0)</span><br><span style="color: hsl(0, 100%, 40%);">- trx_if_cmd_poweroff(l1h);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (pinst->num == 0)</span><br><span style="color: hsl(120, 100%, 40%);">+ trx_if_cmd_poweroff(l1h, NULL);</span><br><span> </span><br><span> return 0;</span><br><span> </span><br><span>@@ -1266,5 +1274,7 @@</span><br><span> /*! determine if the TRX for given handle is powered up */</span><br><span> int trx_if_powered(struct trx_l1h *l1h)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- return l1h->config.poweron;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct phy_instance *pinst = l1h->phy_inst;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct phy_link *plink = pinst->phy_link;</span><br><span style="color: hsl(120, 100%, 40%);">+ return plink->u.osmotrx.powered;</span><br><span> }</span><br><span>diff --git a/src/osmo-bts-trx/trx_if.h b/src/osmo-bts-trx/trx_if.h</span><br><span>index dda7116..3325c62 100644</span><br><span>--- a/src/osmo-bts-trx/trx_if.h</span><br><span>+++ b/src/osmo-bts-trx/trx_if.h</span><br><span>@@ -15,11 +15,12 @@</span><br><span> void *cb;</span><br><span> };</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+typedef void trx_if_cmd_poweronoff_cb(struct trx_l1h *l1h, bool poweronoff, int rc);</span><br><span> typedef void trx_if_cmd_setslot_cb(struct trx_l1h *l1h, uint8_t tn, uint8_t type, int rc);</span><br><span> </span><br><span> void trx_if_init(struct trx_l1h *l1h);</span><br><span style="color: hsl(0, 100%, 40%);">-int trx_if_cmd_poweroff(struct trx_l1h *l1h);</span><br><span style="color: hsl(0, 100%, 40%);">-int trx_if_cmd_poweron(struct trx_l1h *l1h);</span><br><span style="color: hsl(120, 100%, 40%);">+int trx_if_cmd_poweroff(struct trx_l1h *l1h, trx_if_cmd_poweronoff_cb *cb);</span><br><span style="color: hsl(120, 100%, 40%);">+int trx_if_cmd_poweron(struct trx_l1h *l1h, trx_if_cmd_poweronoff_cb *cb);</span><br><span> int trx_if_cmd_settsc(struct trx_l1h *l1h, uint8_t tsc);</span><br><span> int trx_if_cmd_setbsic(struct trx_l1h *l1h, uint8_t bsic);</span><br><span> int trx_if_cmd_setrxgain(struct trx_l1h *l1h, int db);</span><br><span>diff --git a/src/osmo-bts-trx/trx_vty.c b/src/osmo-bts-trx/trx_vty.c</span><br><span>index f554ae5..993c780 100644</span><br><span>--- a/src/osmo-bts-trx/trx_vty.c</span><br><span>+++ b/src/osmo-bts-trx/trx_vty.c</span><br><span>@@ -290,9 +290,9 @@</span><br><span> struct trx_l1h *l1h = pinst->u.osmotrx.hdl;</span><br><span> </span><br><span> if (strcmp(argv[0], "on"))</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, "OFF: %d%s", trx_if_cmd_poweroff(l1h), VTY_NEWLINE);</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, "OFF: %d%s", trx_if_cmd_poweroff(l1h, NULL), VTY_NEWLINE);</span><br><span> else {</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, "ON: %d%s", trx_if_cmd_poweron(l1h), VTY_NEWLINE);</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, "ON: %d%s", trx_if_cmd_poweron(l1h, NULL), VTY_NEWLINE);</span><br><span> }</span><br><span> </span><br><span> return CMD_SUCCESS;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bts/+/15629">change 15629</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-bts/+/15629"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-bts </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Icd0b482f1454236432e1952220bbec9d178b8607 </div>
<div style="display:none"> Gerrit-Change-Number: 15629 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>