[PATCH] Support of 11 bit RACH

[Holger Freyther]

> On 08 Mar 2016, at 14:15, Bhargava Abhyankar <Bhargava.Abhyankar at radisys.com> wrote:


Dear Bhargava,


> 
> This patch is first of series of patches to support 11 bit RACH.
> This includes interface structure changes between osmo-pcu and
> osmo-bts to support 11 bit RACH.Processing of 11 bit RACH for
> GPRS is added in bts.cpp.
> Unit test for the same shall be sent as separate patch.
> EGPRS PACKET CHANNEL REQUEST is not part of this patch.Note also
> that this feature requires changes in osmo-bts and versioning needs
> to be addressed further.


could you start by sending a patch for both osmo-bts and osmo-pcu to modify the pcuif_proto.h with the new protocol and the version number increase and osmo-bts to set it as GPRS_8_BIT_RACH (to match current behavior if that makes sense)?



> ---
> src/bts.cpp       | 36 +++++++++++++++++++++++++++---------
> src/bts.h         |  9 ++++++++-
> src/pcu_l1_if.cpp |  2 +-
> src/pcuif_proto.h |  3 ++-
> 4 files changed, 38 insertions(+), 12 deletions(-)
> 
> diff --git a/src/bts.cpp b/src/bts.cpp
> index 715fb51..a999c48 100644
> --- a/src/bts.cpp
> +++ b/src/bts.cpp
> @@ -459,7 +459,7 @@ int BTS::rcv_imm_ass_cnf(const uint8_t *data, uint32_t fn)
> 	return 0;
> }
> 
> -int BTS::rcv_rach(uint8_t ra, uint32_t Fn, int16_t qta)
> +int BTS::rcv_rach(uint16_t ra, uint32_t Fn, int16_t qta, uint8_t rach_type)
> {
> 

> +	} else if(rach_type == GPRS_11_BIT_RACH ) {
> +		if((ra & RACH_CAUSE_SINGLE_BLOCK) == 0x0600) {

please have a look at the Linux Kernel CodingStyle guidelines about necessary spaces between the if keyword and the opening parenthesis.

In terms of style, could you move the code that select the 'sb' into a separate (static inline) helper function. Not that we do it right now but in general this allows us to unit test such things more easily.




> +	} else if (rach_type == EGPRS_11_BIT_RACH) {
> +		/*TODO*/
> +


Do you think a log message is necessary here? Also in terms of TODO, maybe describe what someone in the future should put there?


kind regards
	holger