Re: [PATCH 2/3] tty: n_gsm: add parameters used with parameter negotiation

[Ilpo Järvinen]

On Fri, 21 Oct 2022, D. Starke wrote:

> From: Daniel Starke <daniel.starke@xxxxxxxxxxx>
> 
> n_gsm is based on the 3GPP 07.010 and its newer version is the 3GPP 27.010.
> See https://portal.3gpp.org/desktopmodules/Specifications/SpecificationDetails.aspx?specificationId=1516
> The changes from 07.010 to 27.010 are non-functional. Therefore, I refer to
> the newer 27.010 here. Chapter 5.4.6.3.1 describes the encoding of the
> parameter negotiation messages.
> 
> Add the parameters used there to 'gsm_mux' and 'gsm_dlci' and initialize both
> according to the value ranges and recommended defaults defined in chapter 5.7.
> 
> Replace the use of the DLC default values from the 'gsm_mux' fields with the DLC
> specific values from the 'gsm_dlci' fields where applicable.
> 
> Signed-off-by: Daniel Starke <daniel.starke@xxxxxxxxxxx>
> ---

> @@ -2075,6 +2085,13 @@ static struct gsm_dlci *gsm_dlci_alloc(struct gsm_mux *gsm, int addr)
>  	dlci->gsm = gsm;
>  	dlci->addr = addr;
>  	dlci->adaption = gsm->adaption;
> +	dlci->mtu = gsm->mtu;
> +	if (addr == 0)
> +		dlci->prio = 0;
> +	else
> +		dlci->prio = ((((addr / 8) + 1) * 8) - 1);

Is this doing roundup(addr, 8) - 1 (in linux/math.h)?

> +	dlci->ftype = gsm->ftype;
> +	dlci->k = gsm->k;
>  	dlci->state = DLCI_CLOSED;
>  	if (addr) {
>  		dlci->data = gsm_dlci_data;
> @@ -2650,7 +2667,9 @@ static struct gsm_mux *gsm_alloc_mux(void)
>  
>  	gsm->t1 = T1;
>  	gsm->t2 = T2;
> +	gsm->t3 = T3;
>  	gsm->n2 = N2;
> +	gsm->k = K;
>  	gsm->ftype = UIH;
>  	gsm->adaption = 1;
>  	gsm->encoding = GSM_ADV_OPT;
> @@ -2691,7 +2710,7 @@ static void gsm_copy_config_values(struct gsm_mux *gsm,
>  	c->initiator = gsm->initiator;
>  	c->t1 = gsm->t1;
>  	c->t2 = gsm->t2;
> -	c->t3 = 0;	/* Not supported */
> +	c->t3 = gsm->t3;
>  	c->n2 = gsm->n2;
>  	if (gsm->ftype == UIH)
>  		c->i = 1;
> @@ -2700,7 +2719,7 @@ static void gsm_copy_config_values(struct gsm_mux *gsm,
>  	pr_debug("Ftype %d i %d\n", gsm->ftype, c->i);
>  	c->mru = gsm->mru;
>  	c->mtu = gsm->mtu;
> -	c->k = 0;
> +	c->k = gsm->k;
>  }
>  
>  static int gsm_config(struct gsm_mux *gsm, struct gsm_config *c)
> @@ -2717,12 +2736,16 @@ static int gsm_config(struct gsm_mux *gsm, struct gsm_config *c)
>  		return -EINVAL;
>  	if (c->mru > MAX_MRU || c->mtu > MAX_MTU)
>  		return -EINVAL;
> +	if (c->t3 > 255)
> +		return -EINVAL;
>  	if (c->n2 > 255)
>  		return -EINVAL;
>  	if (c->encapsulation > 1)	/* Basic, advanced, no I */
>  		return -EINVAL;
>  	if (c->initiator > 1)
>  		return -EINVAL;
> +	if (c->k > 7)

Please don't add non-obvious literal such as this. Is this xx_MAX_WINDOW 
or something along those lines?

-- 
 i.