diff mbox

[osmo-bts,3/3,v6] main: Added support for changing the max_power_red in sbts2050

Message ID 1397650179-17828-1-git-send-email-anayuso@sysmocom.de
State Superseded
Headers show

Commit Message

Alvaro Neira April 16, 2014, 12:09 p.m. UTC 
From: Álvaro Neira Ayuso <anayuso@sysmocom.de>

Added functions for changing the max_power_red in case of we receive
a Failure Event report from the manager. The sbts2050 change the
max_power_red to the value that we have configured in the manager
and the sbts2050 restore this value to the initial value if the bts
don't receive another Failure Event report in the same time that we
check the temperature in the manager.

Signed-off-by: Alvaro Neira Ayuso <anayuso@sysmocom.de>
---
[changes in v6]
* Added the floor 0 when the reduce_power is bigger than
  nominal_power - max_power_red

 src/osmo-bts-sysmo/main.c |   80 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

Comments

Holger Freyther April 16, 2014, 9:20 p.m. UTC | #1 
On Wed, Apr 16, 2014 at 02:09:39PM +0200, Alvaro Neira Ayuso wrote:

> +static void update_transmiter_power(struct gsm_bts_trx *trx, int reduce_power)
> +{
> +	struct femtol1_hdl *fl1h = trx_femtol1_hdl(trx);
> +	int power_transmitter = trx->nominal_power - trx->max_power_red;
> +
> +	power_transmitter -= reduce_power;
> +	if (power_transmitter < 0)
> +		power_transmitter = 0;

This would be third place we calculate "trx->nominal_power -
trx->max_power_red".  The code is also fragile in regard to a sequence
like this:

1.) BTS is powering up with lower power output
2.) It gets too hot already and the manager reduces the power
3.) BTS is increasing the power.

With 3rd your reduction in 2nd would be undone. The right and safe
way to handle it is to:

* Store the manager power reduction in another variable (e.g. inside
  the fl1h or the trx)
* Assign this variable
* Have one function that works on the trx and returns you the
  nominal power to be used by the DSP/FPGA.


This way the order of power reduction changes does not matter.


> +
> +	if (last_power_transmitter != power_transmitter) {
> +		if (fl1h->hLayer1) {
> +			l1if_set_txpower(fl1h, power_transmitter);
> +			last_power_transmitter = power_transmitter;

global state? Why is that needed?

> +static struct osmo_timer_list power_timer;
> +static void change_transmitter_power_cb(void *data)
> +{
> +	struct gsm_bts *bts = data;
> +	struct gsm_bts_trx *trx = bts->c0;
> +
> +	if (status_change_power_red == SBTS2050_ENABLE_CHANGE_POWER) {
> +		update_transmiter_power(trx, reduce_max_power);
> +		status_change_power_red = SBTS2050_DISABLE_CHANGE_POWER;
> +		osmo_timer_schedule(&power_timer, TEMP_TIMER_SECS, 0);
> +	} else if (status_change_power_red == SBTS2050_DISABLE_CHANGE_POWER)
> +		update_transmiter_power(trx, 0);
> +}
> +
> +static int take_max_power_red(struct msgb *msg)
> +{
> +	struct sbts2050_config_info *config_info;
> +	struct tlv_parsed tlv_out;
> +	int rc;
> +
> +	rc = oml_tlv_parse(&tlv_out, msg->tail - TLV_ADD_INFO_LEN,
> +			   TLV_ADD_INFO_LEN);
> +
> +	if (rc < 0)
> +		return -1;
> +
> +	config_info =
> +	    (struct sbts2050_config_info *) TLVP_VAL(&tlv_out, NM_ATT_ADD_INFO);
> +
> +	reduce_max_power = config_info->reduce_max_power;
> +
> +	status_change_power_red = SBTS2050_ENABLE_CHANGE_POWER;
> +	change_transmitter_power_cb(bts);
> +
> +	return 0;
> +}

Why do we need a timer in the sysmobts binary? If the switching of the
power should be delayed, the manager should do this as part of the
management.
diff mbox

Patch

diff --git a/src/osmo-bts-sysmo/main.c b/src/osmo-bts-sysmo/main.c
index aafbc50..269a64d 100644
--- a/src/osmo-bts-sysmo/main.c
+++ b/src/osmo-bts-sysmo/main.c
@@ -39,6 +39,7 @@ 
 #include <osmocom/vty/telnet_interface.h>
 #include <osmocom/vty/logging.h>
 #include <osmocom/gsm/protocol/ipaccess.h>
+#include <osmocom/gsm/abis_nm.h>
 
 #include <osmo-bts/gsm_data.h>
 #include <osmo-bts/logging.h>
@@ -53,11 +54,14 @@ 
 
 #define SYSMOBTS_RF_LOCK_PATH	"/var/lock/bts_rf_lock"
 
+#define TEMP_TIMER_SECS		((6 * 3600) + 1)
+
 #include "utils.h"
 #include "eeprom.h"
 #include "l1_if.h"
 #include "hw_misc.h"
 #include "btsconfig.h"
+#include <misc/sysmobts_misc.h>
 
 /* FIXME: read from real hardware */
 const uint8_t abis_mac[6] = { 0,1,2,3,4,5 };
@@ -295,6 +299,74 @@  static int write_pid_file(char *procname)
 	return 0;
 }
 
+static int reduce_max_power;
+static int status_change_power_red;
+static int last_power_transmitter = -1;
+enum {
+	SBTS2050_DISABLE_CHANGE_POWER = 0,
+	SBTS2050_ENABLE_CHANGE_POWER,
+};
+
+#define oml_tlv_parse(dec, buf, len)	\
+	tlv_parse(dec, &abis_nm_att_tlvdef, buf, len, 0, 0)
+
+/* size of the struct plus 1 Bytes of tag and 2 Bytes for the length */
+#define TLV_ADD_INFO_LEN	(sizeof(struct sbts2050_config_info) + 3)
+
+static void update_transmiter_power(struct gsm_bts_trx *trx, int reduce_power)
+{
+	struct femtol1_hdl *fl1h = trx_femtol1_hdl(trx);
+	int power_transmitter = trx->nominal_power - trx->max_power_red;
+
+	power_transmitter -= reduce_power;
+	if (power_transmitter < 0)
+		power_transmitter = 0;
+
+	if (last_power_transmitter != power_transmitter) {
+		if (fl1h->hLayer1) {
+			l1if_set_txpower(fl1h, power_transmitter);
+			last_power_transmitter = power_transmitter;
+		}
+	}
+}
+
+static struct osmo_timer_list power_timer;
+static void change_transmitter_power_cb(void *data)
+{
+	struct gsm_bts *bts = data;
+	struct gsm_bts_trx *trx = bts->c0;
+
+	if (status_change_power_red == SBTS2050_ENABLE_CHANGE_POWER) {
+		update_transmiter_power(trx, reduce_max_power);
+		status_change_power_red = SBTS2050_DISABLE_CHANGE_POWER;
+		osmo_timer_schedule(&power_timer, TEMP_TIMER_SECS, 0);
+	} else if (status_change_power_red == SBTS2050_DISABLE_CHANGE_POWER)
+		update_transmiter_power(trx, 0);
+}
+
+static int take_max_power_red(struct msgb *msg)
+{
+	struct sbts2050_config_info *config_info;
+	struct tlv_parsed tlv_out;
+	int rc;
+
+	rc = oml_tlv_parse(&tlv_out, msg->tail - TLV_ADD_INFO_LEN,
+			   TLV_ADD_INFO_LEN);
+
+	if (rc < 0)
+		return -1;
+
+	config_info =
+	    (struct sbts2050_config_info *) TLVP_VAL(&tlv_out, NM_ATT_ADD_INFO);
+
+	reduce_max_power = config_info->reduce_max_power;
+
+	status_change_power_red = SBTS2050_ENABLE_CHANGE_POWER;
+	change_transmitter_power_cb(bts);
+
+	return 0;
+}
+
 /*
  * In succesfully case, this function set again the pointers of the l2h and l3h
  * of the OML message that we have received. In other case, we don't set the
@@ -404,6 +476,11 @@  static int read_sock(struct osmo_fd *fd, unsigned int what)
 		goto err;
 	}
 
+	if (take_max_power_red(msg) < 0) {
+		LOGP(DL1C, LOGL_ERROR, "Add_info Malformed message\n");
+		goto err;
+	}
+
 	mo = &bts->mo;
 	msg->trx = mo->bts->c0;
 
@@ -452,6 +529,9 @@  static int oml_sock_unix_init(struct osmo_fd *accept, struct osmo_fd *read)
 	read->fd = -1;
 	accept->data = read;
 
+	power_timer.cb = change_transmitter_power_cb;
+	power_timer.data = bts;
+
 	rc = osmo_sock_unix_init_ofd(accept, SOCK_SEQPACKET, 0, SOCKET_PATH,
 				     OSMO_SOCK_F_BIND | OSMO_SOCK_F_NONBLOCK);
 	return rc;