Patch Set 2:
Line 1: /* (C) 2018 Stefan Sperling <ssperl...@sysmocom.de>
> (C) sysmocom / Author: Stefan Sperling, please look at other examples, than
Fine, fixed in next patch set.
Line 41: struct gsm_bts *bts; /* backpointer to BTS using this ACC ramp
> one could avoid the back-pointer if we used offsetof() to go back from acc_
I agree. I will investigate this idea later and provide a follow-up patch if I
find a nicer way of doing this.
Line 68: static void allow_all_allowed_accs(struct acc_ramp *acc_ramp)
> the naming is a bit ... odd.
I will change the naming in the next patch set to make a distinction between
"allowed ACC" (this ACC is not currently barred) and "enabled ACC" (the user
has not permanently barred this ACC via VTY). I hope that will be clearer.
PS2, Line 93: DRLL
> this is not related to the Radio Link Layer (which is a sub-layer of RSL).
I will switch these to DRSL in the next patch set. I could also introduce a
separate log category in a follow-up patch if desired.
Line 98: static void send_bts_system_info(struct gsm_bts *bts)
> this should become a public function that's part of the general bts/trx/sys
There already is one in bsc_init.c, gsm_bts_set_system_infos(). I will use that
instead in the next patch set.
Line 107: static void do_ramping_step(void *data)
> 'acc' in the name might be useful in backtraces/debugging/... to indicate w
Agreed. Will rename in next patch set.
Line 335: acc_ramp_start(&trx->bts->acc_ramp);
> your first ramp step is executed before the call to gsm_bts_trx_set_system_
Indeed, thanks for catching this!
I think we can solve this by again calling acc_ramp_init() when we enter this
function, to ensure ACC barring overrides take effect in the initial system
infos bootstrap_rsl() sends via gsm_bts_trx_set_system_infos().
And then start the actual ramping process at the end of bootstrap_rsl(). This
call might update system infos again if needed, but that's what we do as part
of each ramping step anyway.
Calling acc_ramp_init() in bootstrap_rsl() will also help in case ACC ramp VTY
configuration has changed when a BTS reconnects, which happens without going
through bootstrap_bts() again as far as can I tell.
PS2, Line 3135: enabled (0|1)
> while we have some commands like this dating back to the early days, it is
Changed to work as suggested (Switch/Router style) in the next patch set. The
commands which modify step size and interval were also changed slightly as a
Line 662: static void apply_acc_ramp(struct gsm48_rach_control *rach_control,
struct acc_ramp *acc_ramp)
> I think that should be part of the acc related code, not here? At that poi
In the next patch set I am moving this to acc_ramp.h as an inline function,
named as suggested.
To view, visit https://gerrit.osmocom.org/6324
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-Owner: Stefan Sperling <ssperl...@sysmocom.de>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Stefan Sperling <ssperl...@sysmocom.de>
Gerrit-Reviewer: Vadim Yanitskiy <axilira...@gmail.com>