[osmo-bts PATCH v4] misc/sysmobts_misc: function for switching off/on and requesting status power
From: Álvaro Neira Ayuso <anayuso@sysmocom.de> I have extended the principal function that we use for requesting information to the microcontroller for switching off/on the board and the PA. And i have extended too with a function for requesting the power status information of the board and the PA. Signed-off-by: Alvaro Neira Ayuso <anayuso@sysmocom.de> --- v4: Fixed the msgb_free in the function status. With the return, we never use it src/osmo-bts-sysmo/misc/sysmobts_misc.c | 81 +++++++++++++++++++++++++++++++ src/osmo-bts-sysmo/misc/sysmobts_misc.h | 10 ++++ 2 files changed, 91 insertions(+) diff --git a/src/osmo-bts-sysmo/misc/sysmobts_misc.c b/src/osmo-bts-sysmo/misc/sysmobts_misc.c index 55a7a2a..897d605 100644 --- a/src/osmo-bts-sysmo/misc/sysmobts_misc.c +++ b/src/osmo-bts-sysmo/misc/sysmobts_misc.c @@ -120,6 +120,12 @@ static struct msgb *sbts2050_ucinfo_get(struct uc *ucontrol, struct ucinfo info) case SBTS2050_TEMP_RQT: num = sizeof(command->cmd.tempGet); break; + case SBTS2050_PWR_RQT: + num = sizeof(command->cmd.pwrSetState); + break; + case SBTS2050_PWR_STATUS: + num = sizeof(command->cmd.pwrGetStatus); + break; default: return NULL; } @@ -138,6 +144,17 @@ static struct msgb *sbts2050_ucinfo_get(struct uc *ucontrol, struct ucinfo info) command->u8Id = info.id; command->u8Len = sizeof(command->cmd.tempGet); break; + case SBTS2050_PWR_RQT: + command->u8Id = info.id; + command->u8Len = sizeof(command->cmd.pwrSetState); + command->cmd.pwrSetState.u1MasterEn = !!info.master; + command->cmd.pwrSetState.u1SlaveEn = !!info.slave; + command->cmd.pwrSetState.u1PwrAmpEn = !!info.pa; + break; + case SBTS2050_PWR_STATUS: + command->u8Id = info.id; + command->u8Len = sizeof(command->cmd.pwrGetStatus); + break; default: goto err; } @@ -182,6 +199,70 @@ err: #endif /********************************************************************** + * Get power status function + *********************************************************************/ +int sbts2050_uc_status(struct uc *ucontrol, enum sbts2050_status_rqt status) +{ + struct msgb *msg; + struct ucinfo info = { + .id = SBTS2050_PWR_STATUS, + }; + rsppkt_t *response; + + msg = sbts2050_ucinfo_get(ucontrol, info); + + if (msg == NULL) { + LOGP(DTEMP, LOGL_ERROR, "Error switching off some unit"); + return -1; + } + + response = (rsppkt_t *)msg->data; + + msgb_free(msg); + switch (status) { + case SBTS2050_STATUS_MASTER: + return response->rsp.pwrGetStatus.u1MasterEn; + case SBTS2050_STATUS_SLAVE: + return response->rsp.pwrGetStatus.u1SlaveEn; + case SBTS2050_STATUS_PA: + return response->rsp.pwrGetStatus.u1PwrAmpEn; + default: + return -1; + } +} + +/********************************************************************** + * Uc Power Switching handling + *********************************************************************/ +void sbts2050_uc_power(struct uc *ucontrol, int pmaster, int pslave, int ppa) +{ + struct msgb *msg; + struct ucinfo info = { + .id = 0x00, + .master = pmaster, + .slave = pslave, + .pa = ppa + }; + + msg = sbts2050_ucinfo_get(ucontrol, info); + + if (msg == NULL) { + LOGP(DTEMP, LOGL_ERROR, "Error switching off some unit"); + return; + } + + LOGP(DTEMP, LOGL_DEBUG, "Switch off/on success:\n" + "MASTER %s\n" + "SLAVE %s\n" + "PA %s\n", + pmaster ? "ON" : "OFF", + pslave ? "ON" : "OFF", + ppa ? "ON" : "OFF"); + + msgb_free(msg); +} + +/********************************************************************** * Uc temperature handling *********************************************************************/ void sbts2050_uc_check_temp(struct uc *ucontrol, int *temp_pa, int *temp_board) diff --git a/src/osmo-bts-sysmo/misc/sysmobts_misc.h b/src/osmo-bts-sysmo/misc/sysmobts_misc.h index 3c6513e..01878f2 100644 --- a/src/osmo-bts-sysmo/misc/sysmobts_misc.h +++ b/src/osmo-bts-sysmo/misc/sysmobts_misc.h @@ -13,6 +13,12 @@ enum sysmobts_temp_type { _NUM_TEMP_TYPES }; +enum sbts2050_status_rqt { + SBTS2050_STATUS_MASTER, + SBTS2050_STATUS_SLAVE, + SBTS2050_STATUS_PA +}; + struct uc { int id; int fd; @@ -33,6 +39,10 @@ void sysmobts_check_temp(int no_eeprom_write); void sbts2050_uc_check_temp(struct uc *ucontrol, int *temp_pa, int *temp_board); +void sbts2050_uc_power(struct uc *ucontrol, int pmaster, int pslave, int ppa); + +int sbts2050_uc_status(struct uc *ucontrol, enum sbts2050_status_rqt status); + int sysmobts_update_hours(int no_epprom_write); enum sysmobts_firmware_type {
From: Álvaro Neira Ayuso <anayuso@sysmocom.de> I have extended the principal function that we use for requesting information to the microcontroller for switching off/on the board and the PA. And I have extended it for requesting the power status information of the board and the PA. Signed-off-by: Alvaro Neira Ayuso <anayuso@sysmocom.de> --- v5: Fixed the switch in the status function, for freeing the msg. src/osmo-bts-sysmo/misc/sysmobts_misc.c | 87 +++++++++++++++++++++++++++++++ src/osmo-bts-sysmo/misc/sysmobts_misc.h | 10 ++++ 2 files changed, 97 insertions(+) diff --git a/src/osmo-bts-sysmo/misc/sysmobts_misc.c b/src/osmo-bts-sysmo/misc/sysmobts_misc.c index 55a7a2a..9ed528e 100644 --- a/src/osmo-bts-sysmo/misc/sysmobts_misc.c +++ b/src/osmo-bts-sysmo/misc/sysmobts_misc.c @@ -120,6 +120,12 @@ static struct msgb *sbts2050_ucinfo_get(struct uc *ucontrol, struct ucinfo info) case SBTS2050_TEMP_RQT: num = sizeof(command->cmd.tempGet); break; + case SBTS2050_PWR_RQT: + num = sizeof(command->cmd.pwrSetState); + break; + case SBTS2050_PWR_STATUS: + num = sizeof(command->cmd.pwrGetStatus); + break; default: return NULL; } @@ -138,6 +144,17 @@ static struct msgb *sbts2050_ucinfo_get(struct uc *ucontrol, struct ucinfo info) command->u8Id = info.id; command->u8Len = sizeof(command->cmd.tempGet); break; + case SBTS2050_PWR_RQT: + command->u8Id = info.id; + command->u8Len = sizeof(command->cmd.pwrSetState); + command->cmd.pwrSetState.u1MasterEn = !!info.master; + command->cmd.pwrSetState.u1SlaveEn = !!info.slave; + command->cmd.pwrSetState.u1PwrAmpEn = !!info.pa; + break; + case SBTS2050_PWR_STATUS: + command->u8Id = info.id; + command->u8Len = sizeof(command->cmd.pwrGetStatus); + break; default: goto err; } @@ -182,6 +199,76 @@ err: #endif /********************************************************************** + * Get power status function + *********************************************************************/ +int sbts2050_uc_status(struct uc *ucontrol, enum sbts2050_status_rqt status) +{ + struct msgb *msg; + struct ucinfo info = { + .id = SBTS2050_PWR_STATUS, + }; + rsppkt_t *response; + int val_status; + + msg = sbts2050_ucinfo_get(ucontrol, info); + + if (msg == NULL) { + LOGP(DTEMP, LOGL_ERROR, "Error switching off some unit"); + return -1; + } + + response = (rsppkt_t *)msg->data; + + switch (status) { + case SBTS2050_STATUS_MASTER: + val_status = response->rsp.pwrGetStatus.u1MasterEn; + break; + case SBTS2050_STATUS_SLAVE: + val_status = response->rsp.pwrGetStatus.u1SlaveEn; + break; + case SBTS2050_STATUS_PA: + val_status = response->rsp.pwrGetStatus.u1PwrAmpEn; + break; + default: + msgb_free(msg); + return -1; + } + msgb_free(msg); + return val_status; +} + +/********************************************************************** + * Uc Power Switching handling + *********************************************************************/ +void sbts2050_uc_power(struct uc *ucontrol, int pmaster, int pslave, int ppa) +{ + struct msgb *msg; + struct ucinfo info = { + .id = 0x00, + .master = pmaster, + .slave = pslave, + .pa = ppa + }; + + msg = sbts2050_ucinfo_get(ucontrol, info); + + if (msg == NULL) { + LOGP(DTEMP, LOGL_ERROR, "Error switching off some unit"); + return; + } + + LOGP(DTEMP, LOGL_DEBUG, "Switch off/on success:\n" + "MASTER %s\n" + "SLAVE %s\n" + "PA %s\n", + pmaster ? "ON" : "OFF", + pslave ? "ON" : "OFF", + ppa ? "ON" : "OFF"); + + msgb_free(msg); +} + +/********************************************************************** * Uc temperature handling *********************************************************************/ void sbts2050_uc_check_temp(struct uc *ucontrol, int *temp_pa, int *temp_board) diff --git a/src/osmo-bts-sysmo/misc/sysmobts_misc.h b/src/osmo-bts-sysmo/misc/sysmobts_misc.h index 3c6513e..01878f2 100644 --- a/src/osmo-bts-sysmo/misc/sysmobts_misc.h +++ b/src/osmo-bts-sysmo/misc/sysmobts_misc.h @@ -13,6 +13,12 @@ enum sysmobts_temp_type { _NUM_TEMP_TYPES }; +enum sbts2050_status_rqt { + SBTS2050_STATUS_MASTER, + SBTS2050_STATUS_SLAVE, + SBTS2050_STATUS_PA +}; + struct uc { int id; int fd; @@ -33,6 +39,10 @@ void sysmobts_check_temp(int no_eeprom_write); void sbts2050_uc_check_temp(struct uc *ucontrol, int *temp_pa, int *temp_board); +void sbts2050_uc_power(struct uc *ucontrol, int pmaster, int pslave, int ppa); + +int sbts2050_uc_status(struct uc *ucontrol, enum sbts2050_status_rqt status); + int sysmobts_update_hours(int no_epprom_write); enum sysmobts_firmware_type { -- 1.7.10.4
On Wed, Mar 12, 2014 at 06:15:28PM +0100, Alvaro Neira Ayuso wrote:
From: Álvaro Neira Ayuso <anayuso@sysmocom.de>
Dear Alvaro,
/********************************************************************** + * Get power status function + *********************************************************************/ +int sbts2050_uc_status(struct uc *ucontrol, enum sbts2050_status_rqt status)
I think you need to guard this with the BUILD_SBTS2050 define as well as I am getting a build error with the sbts2050 header not being installed. Please fix the build and re-submit (with in-reply-to). holger
From: Álvaro Neira Ayuso <anayuso@sysmocom.de> I have extended the principal function that we use for requesting information to the microcontroller for switching off/on the board and the PA. And I have extended it for requesting the power status information of the board and the PA. Signed-off-by: Alvaro Neira Ayuso <anayuso@sysmocom.de> --- v6: Added the BUILD_SBTS2050 for fixing the error when we don't have the header. src/osmo-bts-sysmo/misc/sysmobts_misc.c | 93 +++++++++++++++++++++++++++++++ src/osmo-bts-sysmo/misc/sysmobts_misc.h | 10 ++++ 2 files changed, 103 insertions(+) diff --git a/src/osmo-bts-sysmo/misc/sysmobts_misc.c b/src/osmo-bts-sysmo/misc/sysmobts_misc.c index 55a7a2a..9ea26c2 100644 --- a/src/osmo-bts-sysmo/misc/sysmobts_misc.c +++ b/src/osmo-bts-sysmo/misc/sysmobts_misc.c @@ -120,6 +120,12 @@ static struct msgb *sbts2050_ucinfo_get(struct uc *ucontrol, struct ucinfo info) case SBTS2050_TEMP_RQT: num = sizeof(command->cmd.tempGet); break; + case SBTS2050_PWR_RQT: + num = sizeof(command->cmd.pwrSetState); + break; + case SBTS2050_PWR_STATUS: + num = sizeof(command->cmd.pwrGetStatus); + break; default: return NULL; } @@ -138,6 +144,17 @@ static struct msgb *sbts2050_ucinfo_get(struct uc *ucontrol, struct ucinfo info) command->u8Id = info.id; command->u8Len = sizeof(command->cmd.tempGet); break; + case SBTS2050_PWR_RQT: + command->u8Id = info.id; + command->u8Len = sizeof(command->cmd.pwrSetState); + command->cmd.pwrSetState.u1MasterEn = !!info.master; + command->cmd.pwrSetState.u1SlaveEn = !!info.slave; + command->cmd.pwrSetState.u1PwrAmpEn = !!info.pa; + break; + case SBTS2050_PWR_STATUS: + command->u8Id = info.id; + command->u8Len = sizeof(command->cmd.pwrGetStatus); + break; default: goto err; } @@ -182,6 +199,82 @@ err: #endif /********************************************************************** + * Get power status function + *********************************************************************/ +int sbts2050_uc_status(struct uc *ucontrol, enum sbts2050_status_rqt status) +{ +#ifdef BUILD_SBTS2050 + struct msgb *msg; + struct ucinfo info = { + .id = SBTS2050_PWR_STATUS, + }; + rsppkt_t *response; + int val_status; + + msg = sbts2050_ucinfo_get(ucontrol, info); + + if (msg == NULL) { + LOGP(DTEMP, LOGL_ERROR, "Error switching off some unit"); + return -1; + } + + response = (rsppkt_t *)msg->data; + + switch (status) { + case SBTS2050_STATUS_MASTER: + val_status = response->rsp.pwrGetStatus.u1MasterEn; + break; + case SBTS2050_STATUS_SLAVE: + val_status = response->rsp.pwrGetStatus.u1SlaveEn; + break; + case SBTS2050_STATUS_PA: + val_status = response->rsp.pwrGetStatus.u1PwrAmpEn; + break; + default: + msgb_free(msg); + return -1; + } + msgb_free(msg); + return val_status; +#else + return -1; +#endif +} + +/********************************************************************** + * Uc Power Switching handling + *********************************************************************/ +void sbts2050_uc_power(struct uc *ucontrol, int pmaster, int pslave, int ppa) +{ +#ifdef BUILD_SBTS2050 + struct msgb *msg; + struct ucinfo info = { + .id = 0x00, + .master = pmaster, + .slave = pslave, + .pa = ppa + }; + + msg = sbts2050_ucinfo_get(ucontrol, info); + + if (msg == NULL) { + LOGP(DTEMP, LOGL_ERROR, "Error switching off some unit"); + return; + } + + LOGP(DTEMP, LOGL_DEBUG, "Switch off/on success:\n" + "MASTER %s\n" + "SLAVE %s\n" + "PA %s\n", + pmaster ? "ON" : "OFF", + pslave ? "ON" : "OFF", + ppa ? "ON" : "OFF"); + + msgb_free(msg); +#endif +} + +/********************************************************************** * Uc temperature handling *********************************************************************/ void sbts2050_uc_check_temp(struct uc *ucontrol, int *temp_pa, int *temp_board) diff --git a/src/osmo-bts-sysmo/misc/sysmobts_misc.h b/src/osmo-bts-sysmo/misc/sysmobts_misc.h index 3c6513e..01878f2 100644 --- a/src/osmo-bts-sysmo/misc/sysmobts_misc.h +++ b/src/osmo-bts-sysmo/misc/sysmobts_misc.h @@ -13,6 +13,12 @@ enum sysmobts_temp_type { _NUM_TEMP_TYPES }; +enum sbts2050_status_rqt { + SBTS2050_STATUS_MASTER, + SBTS2050_STATUS_SLAVE, + SBTS2050_STATUS_PA +}; + struct uc { int id; int fd; @@ -33,6 +39,10 @@ void sysmobts_check_temp(int no_eeprom_write); void sbts2050_uc_check_temp(struct uc *ucontrol, int *temp_pa, int *temp_board); +void sbts2050_uc_power(struct uc *ucontrol, int pmaster, int pslave, int ppa); + +int sbts2050_uc_status(struct uc *ucontrol, enum sbts2050_status_rqt status); + int sysmobts_update_hours(int no_epprom_write); enum sysmobts_firmware_type { -- 1.7.10.4
Alvaro Neira Ayuso wrote:
+++ b/src/osmo-bts-sysmo/misc/sysmobts_misc.c @@ -120,6 +120,12 @@ static struct msgb *sbts2050_ucinfo_get(struct uc *ucontrol, struct ucinfo info) case SBTS2050_TEMP_RQT: num = sizeof(command->cmd.tempGet); break; + case SBTS2050_PWR_RQT: + num = sizeof(command->cmd.pwrSetState); + break;
A small style remark is that I like to use: sizeof command->cmd.pwerSetState since sizeof isn't really a function. Similar to return.
@@ -138,6 +144,17 @@ static struct msgb *sbts2050_ucinfo_get(struct uc *ucontrol, struct ucinfo info) command->u8Id = info.id; command->u8Len = sizeof(command->cmd.tempGet); break; + case SBTS2050_PWR_RQT: + command->u8Id = info.id; + command->u8Len = sizeof(command->cmd.pwrSetState); + command->cmd.pwrSetState.u1MasterEn = !!info.master; + command->cmd.pwrSetState.u1SlaveEn = !!info.slave; + command->cmd.pwrSetState.u1PwrAmpEn = !!info.pa; + break; + case SBTS2050_PWR_STATUS: + command->u8Id = info.id; + command->u8Len = sizeof(command->cmd.pwrGetStatus);
The above lines have a couple of extra spaces before the = which may or may not be worth fixing.
+int sbts2050_uc_status(struct uc *ucontrol, enum sbts2050_status_rqt status) .. + msg = sbts2050_ucinfo_get(ucontrol, info); + + if (msg == NULL) { + LOGP(DTEMP, LOGL_ERROR, "Error switching off some unit");
This error message doesn't contain very much information. It might be nice to know both which unit could not be switched off *and* what the error was. Also, is DTEMP guaranteed to always be the appropriate subsystem for this function? Maybe yes, but this code is in sysmobts_misc.c which suggests that it may be something more general?
+++ b/src/osmo-bts-sysmo/misc/sysmobts_misc.h @@ -13,6 +13,12 @@ enum sysmobts_temp_type { _NUM_TEMP_TYPES };
+enum sbts2050_status_rqt { + SBTS2050_STATUS_MASTER, + SBTS2050_STATUS_SLAVE, + SBTS2050_STATUS_PA
Maybe add a , after the last enum to make it easier to add more values in the future. //Peter
Hello Peter El 15/03/14 02:45, Peter Stuge escribió:
Alvaro Neira Ayuso wrote:
+++ b/src/osmo-bts-sysmo/misc/sysmobts_misc.c @@ -120,6 +120,12 @@ static struct msgb *sbts2050_ucinfo_get(struct uc *ucontrol, struct ucinfo info) case SBTS2050_TEMP_RQT: num = sizeof(command->cmd.tempGet); break; + case SBTS2050_PWR_RQT: + num = sizeof(command->cmd.pwrSetState); + break;
A small style remark is that I like to use: sizeof command->cmd.pwerSetState since sizeof isn't really a function. Similar to return.
I have followed the kernel coding style, and he said doesn't speak about use sizeof without bracket.
@@ -138,6 +144,17 @@ static struct msgb *sbts2050_ucinfo_get(struct uc *ucontrol, struct ucinfo info) command->u8Id = info.id; command->u8Len = sizeof(command->cmd.tempGet); break; + case SBTS2050_PWR_RQT: + command->u8Id = info.id; + command->u8Len = sizeof(command->cmd.pwrSetState); + command->cmd.pwrSetState.u1MasterEn = !!info.master; + command->cmd.pwrSetState.u1SlaveEn = !!info.slave; + command->cmd.pwrSetState.u1PwrAmpEn = !!info.pa; + break; + case SBTS2050_PWR_STATUS: + command->u8Id = info.id; + command->u8Len = sizeof(command->cmd.pwrGetStatus);
The above lines have a couple of extra spaces before the = which may or may not be worth fixing.
Yes, it's true, I don't know why I have done that...
+int sbts2050_uc_status(struct uc *ucontrol, enum sbts2050_status_rqt status) .. + msg = sbts2050_ucinfo_get(ucontrol, info); + + if (msg == NULL) { + LOGP(DTEMP, LOGL_ERROR, "Error switching off some unit");
This error message doesn't contain very much information. It might be nice to know both which unit could not be switched off *and* what the error was.
Also, is DTEMP guaranteed to always be the appropriate subsystem for this function? Maybe yes, but this code is in sysmobts_misc.c which suggests that it may be something more general?
I have used DTEMP, because i'm going to have a relation between the temperature and this function but it's true that somebody can use it in other cases. I don't know what do you think about that, if somebody can help me.
+++ b/src/osmo-bts-sysmo/misc/sysmobts_misc.h @@ -13,6 +13,12 @@ enum sysmobts_temp_type { _NUM_TEMP_TYPES };
+enum sbts2050_status_rqt { + SBTS2050_STATUS_MASTER, + SBTS2050_STATUS_SLAVE, + SBTS2050_STATUS_PA
Maybe add a , after the last enum to make it easier to add more values in the future.
//Peter
Thanks a lot Peter
Álvaro Neira Ayuso wrote:
+ case SBTS2050_PWR_RQT: + num = sizeof(command->cmd.pwrSetState); + break;
A small style remark is that I like to use: sizeof command->cmd.pwerSetState since sizeof isn't really a function. Similar to return.
I have followed the kernel coding style, and he said doesn't speak about use sizeof without bracket.
All right!
+int sbts2050_uc_status(struct uc *ucontrol, enum sbts2050_status_rqt status) .. + msg = sbts2050_ucinfo_get(ucontrol, info); + + if (msg == NULL) { + LOGP(DTEMP, LOGL_ERROR, "Error switching off some unit");
This error message doesn't contain very much information. It might be nice to know both which unit could not be switched off *and* what the error was.
Also, is DTEMP guaranteed to always be the appropriate subsystem for this function? Maybe yes, but this code is in sysmobts_misc.c which suggests that it may be something more general?
I have used DTEMP, because i'm going to have a relation between the temperature and this function but it's true that somebody can use it in other cases. I don't know what do you think about that, if somebody can help me.
I also don't know enough to make a suggestion here - it depends on how versatile these new sbts2050_uc_* functions should be. Maybe they are only intended to be called from the TEMP subsystem, then the current patch is fine. Hopefully someone with more overview can comment on this? //Peter
participants (5)
-
Alvaro Neira -
Alvaro Neira Ayuso
-
Holger Hans Peter Freyther -
Peter Stuge -
Álvaro Neira Ayuso