Re: [PATCH v2 1/6] tty: n_gsm: add enumeration for gsm encodings

From: Jiri Slaby
Date: Tue Aug 30 2022 - 02:49:17 EST


On 23. 08. 22, 8:22, D. Starke wrote:
From: Daniel Starke <daniel.starke@xxxxxxxxxxx>

Add an enumeration for the gsm mux encoding types to improve code
readability and to avoid invalid values. Only two values are defined by the
standard:
- basic option mode
- advanced option mode (uses ISO HDLC standard transparency mechanism)

Also remove redundant configuration in gsmld_open(). The same value is
already set in gsm_alloc_mux() which is also called in gsmld_open().

Signed-off-by: Daniel Starke <daniel.starke@xxxxxxxxxxx>
---
drivers/tty/n_gsm.c | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)

No changes since v1.

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index caa5c14ed57f..5bf09d129357 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
...
@@ -2651,7 +2656,7 @@ static void gsm_copy_config_values(struct gsm_mux *gsm,
{
memset(c, 0, sizeof(*c));
c->adaption = gsm->adaption;
- c->encapsulation = gsm->encoding;
+ c->encapsulation = (int)gsm->encoding;

IIRC, the C standard says enum behave like int. So why is the cast needed?

c->initiator = gsm->initiator;
c->t1 = gsm->t1;
c->t2 = gsm->t2;
...
@@ -2942,8 +2947,6 @@ static int gsmld_open(struct tty_struct *tty)
tty->receive_room = 65536;
/* Attach the initial passive connection */
- gsm->encoding = 1;
-

This doesn't relate to "add enumeration for gsm encodings". Why do you remove this line?

gsmld_attach_gsm(tty, gsm);

thanks,
--
js
suse labs