On Mon, 24 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.1.8.1.1 describes the parameter negotiation > messages and parameters. Chapter 5.4.1 states that the default parameters > are to be used if no negotiation is performed. Chapter 5.4.6.3.1 describes > the encoding of the parameter negotiation message. The meaning of the > parameters and allowed value ranges can be found in chapter 5.7. > > Add parameter negotiation support accordingly. DLCI specific parameter > configuration by the user requires additional ioctls. This is subject to > another patch. > > Signed-off-by: Daniel Starke <daniel.starke@xxxxxxxxxxx> > --- > drivers/tty/n_gsm.c | 335 ++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 327 insertions(+), 8 deletions(-) > > v1 -> v2: > Incorporated review comments. > Simplification of command retry handling remains subject to future patches. > > Link: https://lore.kernel.org/all/8c2b9492-caf4-7a48-3a7b-da939a4ac8b6@xxxxxxxxxxxxxxx/ > > diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c > index c217013b3e16..11d3730bf436 100644 > --- a/drivers/tty/n_gsm.c > +++ b/drivers/tty/n_gsm.c > @@ -38,6 +38,7 @@ > #include <linux/sched/signal.h> > #include <linux/interrupt.h> > #include <linux/tty.h> > +#include <linux/bitfield.h> > #include <linux/ctype.h> > #include <linux/mm.h> > #include <linux/math.h> > @@ -127,6 +128,7 @@ struct gsm_msg { > > enum gsm_dlci_state { > DLCI_CLOSED, > + DLCI_CONFIGURE, /* Sending PN (for adaption > 1) */ > DLCI_OPENING, /* Sending SABM not seen UA */ > DLCI_OPEN, /* SABM/UA complete */ > DLCI_CLOSING, /* Sending DISC not seen UA/DM */ > @@ -184,6 +186,30 @@ struct gsm_dlci { > struct net_device *net; /* network interface, if created */ > }; > > +/* > + * Parameter bits used for parameter negotiation according to 3GPP 27.010 > + * chapter 5.4.6.3.1. > + */ > + > +struct gsm_dlci_param_bits { > + u8 d_bits; > + u8 i_cl_bits; > + u8 p_bits; > + u8 t_bits; > + __le16 n_bits; > + u8 na_bits; > + u8 k_bits; > +} __packed; > + > +#define PN_D_FIELD_DLCI GENMASK(5, 0) > +#define PN_I_CL_FIELD_FTYPE GENMASK(3, 0) > +#define PN_I_CL_FIELD_ADAPTION GENMASK(7, 4) > +#define PN_P_FIELD_PRIO GENMASK(5, 0) > +#define PN_T_FIELD_T1 GENMASK(7, 0) > +#define PN_N_FIELD_N1 GENMASK(15, 0) > +#define PN_NA_FIELD_N2 GENMASK(7, 0) I guess there three would not be strictly necessary since they match to the full type size but then they're not harmful either. > +#define PN_K_FIELD_K GENMASK(2, 0) > + > /* Total number of supported devices */ > #define GSM_TTY_MINORS 256 > > @@ -411,6 +437,7 @@ static const u8 gsm_fcs8[256] = { > #define INIT_FCS 0xFF > #define GOOD_FCS 0xCF > > +static void gsm_dlci_close(struct gsm_dlci *dlci); > static int gsmld_output(struct gsm_mux *gsm, u8 *data, int len); > static int gsm_modem_update(struct gsm_dlci *dlci, u8 brk); > static struct gsm_msg *gsm_data_alloc(struct gsm_mux *gsm, u8 addr, int len, > @@ -533,6 +560,59 @@ static void gsm_hex_dump_bytes(const char *fname, const u8 *data, > kfree(prefix); > } > > +/** > + * gsm_encode_params - encode DLCI parameters > + * @dlci: DLCI to encode from > + * @params: buffer to fill with the encoded parameters > + * > + * Encodes the parameters according to GSM 07.10 section 5.4.6.3.1 > + * table 3. > + */ > +static int gsm_encode_params(const struct gsm_dlci *dlci, > + struct gsm_dlci_param_bits *params) > +{ > + const struct gsm_mux *gsm = dlci->gsm; > + unsigned int i, cl; > + > + switch (dlci->ftype) { > + case UIH: > + i = 0; /* UIH */ > + break; > + case UI: > + i = 1; /* UI */ > + break; > + default: > + pr_err("%s: unsupported frame type %d\n", __func__, > + dlci->ftype); > + return -EINVAL; > + } > + > + switch (dlci->adaption) { > + case 1: /* Unstructured */ > + cl = 0; /* convergence layer type 1 */ > + break; > + case 2: /* Unstructured with modem bits. */ > + cl = 1; /* convergence layer type 2 */ > + break; > + default: > + pr_err("%s: unsupported adaption %d\n", __func__, > + dlci->adaption); > + return -EINVAL; > + } > + > + params->d_bits = FIELD_PREP(PN_D_FIELD_DLCI, dlci->addr); > + /* UIH, convergence layer type 1 */ > + params->i_cl_bits = FIELD_PREP(PN_I_CL_FIELD_FTYPE, i) | > + FIELD_PREP(PN_I_CL_FIELD_ADAPTION, cl); > + params->p_bits = FIELD_PREP(PN_P_FIELD_PRIO, dlci->prio); > + params->t_bits = FIELD_PREP(PN_T_FIELD_T1, gsm->t1); > + params->n_bits = cpu_to_le16(FIELD_PREP(PN_N_FIELD_N1, dlci->mtu)); > + params->na_bits = FIELD_PREP(PN_NA_FIELD_N2, gsm->n2); > + params->k_bits = FIELD_PREP(PN_K_FIELD_K, dlci->k); > + > + return 0; > +} > + > /** > * gsm_register_devices - register all tty devices for a given mux index > * > @@ -1450,6 +1530,116 @@ static void gsm_process_modem(struct tty_struct *tty, struct gsm_dlci *dlci, > dlci->modem_rx = mlines; > } > > +/** > + * gsm_process_negotiation - process received parameters > + * @gsm: GSM channel > + * @addr: DLCI address > + * @cr: command/response > + * @params: encoded parameters from the parameter negotiation message > + * > + * Used when the response for our parameter negotiation command was > + * received. > + */ > +static int gsm_process_negotiation(struct gsm_mux *gsm, unsigned int addr, > + unsigned int cr, > + const struct gsm_dlci_param_bits *params) > +{ > + struct gsm_dlci *dlci = gsm->dlci[addr]; > + unsigned int ftype, i, adaption, prio, n1, k; > + > + i = FIELD_GET(PN_I_CL_FIELD_FTYPE, params->i_cl_bits); > + adaption = FIELD_GET(PN_I_CL_FIELD_ADAPTION, params->i_cl_bits) + 1; > + prio = FIELD_GET(PN_P_FIELD_PRIO, params->p_bits); > + n1 = FIELD_GET(PN_N_FIELD_N1, le16_to_cpu(params->n_bits)); Should this be using get_unaligned...()? -- i.