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/OpenBSC@lists.osmocom.org/.
Peter Stuge peter at stuge.seÁ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