* [PATCH] tty: n_gsm: Fix CR bit value when initiator=0 @ 2021-06-16 2:56 Zhenguo Zhao 2021-06-16 6:19 ` Greg KH 0 siblings, 1 reply; 6+ messages in thread From: Zhenguo Zhao @ 2021-06-16 2:56 UTC (permalink / raw) To: zhenguo6858, gregkh, jirislaby; +Cc: linux-kernel From: Zhenguo Zhao <zhenguo.zhao1@unisoc.com> When set initiator=0,switch to Responder,gsmld received dlci SABM/DISC frame,CR bit should be 0 by calculation. receive DLC0 SABM CMD: [69.740263] c1 gsmld_receive: 00000000: f9 03 3f 01 1c f9 [69.893247] c1 gsm_queue cr:1 [69.897629] c1 <-- 0) C: SABM(P) [69.907516] c1 gsm_queue cr:0 Signed-off-by: Zhenguo Zhao <zhenguo.zhao1@unisoc.com> --- drivers/tty/n_gsm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 5fea02c..becca2c 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -1779,7 +1779,7 @@ static void gsm_queue(struct gsm_mux *gsm) switch (gsm->control) { case SABM|PF: - if (cr == 0) + if (cr == 1) goto invalid; if (dlci == NULL) dlci = gsm_dlci_alloc(gsm, address); @@ -1793,7 +1793,7 @@ static void gsm_queue(struct gsm_mux *gsm) } break; case DISC|PF: - if (cr == 0) + if (cr == 1) goto invalid; if (dlci == NULL || dlci->state == DLCI_CLOSED) { gsm_response(gsm, address, DM); -- 1.9.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] tty: n_gsm: Fix CR bit value when initiator=0 2021-06-16 2:56 [PATCH] tty: n_gsm: Fix CR bit value when initiator=0 Zhenguo Zhao @ 2021-06-16 6:19 ` Greg KH 2021-06-16 7:29 ` 赵振国 0 siblings, 1 reply; 6+ messages in thread From: Greg KH @ 2021-06-16 6:19 UTC (permalink / raw) To: Zhenguo Zhao; +Cc: jirislaby, linux-kernel On Wed, Jun 16, 2021 at 10:56:39AM +0800, Zhenguo Zhao wrote: > From: Zhenguo Zhao <zhenguo.zhao1@unisoc.com> > > When set initiator=0,switch to Responder,gsmld received dlci SABM/DISC > frame,CR bit should be 0 by calculation. > > receive DLC0 SABM CMD: > [69.740263] c1 gsmld_receive: 00000000: f9 03 3f 01 1c f9 > [69.893247] c1 gsm_queue cr:1 > [69.897629] c1 <-- 0) C: SABM(P) > [69.907516] c1 gsm_queue cr:0 Why is this changelog text indented by tabs? And I do not understand the changelog text here, what is this showing? What is wrong here and what is being fixed? > Signed-off-by: Zhenguo Zhao <zhenguo.zhao1@unisoc.com> Does this fix a long-standing issue? Should a "Fixes:" tag go here? If so, please provide it. Should it also be sent to stable kernels? > --- > drivers/tty/n_gsm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c > index 5fea02c..becca2c 100644 > --- a/drivers/tty/n_gsm.c > +++ b/drivers/tty/n_gsm.c > @@ -1779,7 +1779,7 @@ static void gsm_queue(struct gsm_mux *gsm) > > switch (gsm->control) { > case SABM|PF: > - if (cr == 0) > + if (cr == 1) How did the original code ever work properly? > goto invalid; > if (dlci == NULL) > dlci = gsm_dlci_alloc(gsm, address); > @@ -1793,7 +1793,7 @@ static void gsm_queue(struct gsm_mux *gsm) > } > break; > case DISC|PF: > - if (cr == 0) > + if (cr == 1) Same here, how did this ever work? Are you sure this change is correct? thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tty: n_gsm: Fix CR bit value when initiator=0 2021-06-16 6:19 ` Greg KH @ 2021-06-16 7:29 ` 赵振国 2021-06-16 7:39 ` Greg KH 0 siblings, 1 reply; 6+ messages in thread From: 赵振国 @ 2021-06-16 7:29 UTC (permalink / raw) To: Greg KH; +Cc: jirislaby, linux-kernel Dear gregkh 1: Documentation/driver-api/serial/n_gsm.rst The text introduces the config of master ( c.initiator = 1), but the config of as responder is different. when set gsm->initiator=0 by GSMIOC_SETCONF ,ngsm driver should be responder(slaver) config： c.initiator = 0; // set initiator=0，ngsm as responder ioctl(fd, GSMIOC_SETCONF, &c); 2: if master side send SABM/DISC frame data by uart dev DLC0 control data frame:f9 03 3f 01 1c f9 kernel log： gsmld_receive: 00000000: f9 03 3f 01 1c f9 { cr = gsm->address & 1; /* C/R bit */ //CR value=1 gsm_print_packet("<--", address, cr, gsm->control, gsm->buf, gsm->len); cr ^= 1 - gsm->initiator; /* Flip so 1 always means command */ //when gsm->initiator is 0, CR value=0 by "^=" calculation dlci = gsm->dlci[address]; switch (gsm->control) { case SABM|PF: if (cr == 0) goto invalid; //if CR value=0,ngsm will goto invalid,but the dlc0 control frame data is right,if we can't modify ,ngsm can't send UA response data } 2021-06-16 14:19 GMT+08:00, Greg KH <gregkh@linuxfoundation.org>: > On Wed, Jun 16, 2021 at 10:56:39AM +0800, Zhenguo Zhao wrote: >> From: Zhenguo Zhao <zhenguo.zhao1@unisoc.com> >> >> When set initiator=0,switch to Responder,gsmld received dlci SABM/DISC >> frame,CR bit should be 0 by calculation. >> >> receive DLC0 SABM CMD: >> [69.740263] c1 gsmld_receive: 00000000: f9 03 3f 01 1c f9 >> [69.893247] c1 gsm_queue cr:1 >> [69.897629] c1 <-- 0) C: SABM(P) >> [69.907516] c1 gsm_queue cr:0 > > Why is this changelog text indented by tabs? > > And I do not understand the changelog text here, what is this showing? > What is wrong here and what is being fixed? > >> Signed-off-by: Zhenguo Zhao <zhenguo.zhao1@unisoc.com> > > Does this fix a long-standing issue? Should a "Fixes:" tag go here? If > so, please provide it. > > Should it also be sent to stable kernels? > >> --- >> drivers/tty/n_gsm.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c >> index 5fea02c..becca2c 100644 >> --- a/drivers/tty/n_gsm.c >> +++ b/drivers/tty/n_gsm.c >> @@ -1779,7 +1779,7 @@ static void gsm_queue(struct gsm_mux *gsm) >> >> switch (gsm->control) { >> case SABM|PF: >> - if (cr == 0) >> + if (cr == 1) > > How did the original code ever work properly? > >> goto invalid; >> if (dlci == NULL) >> dlci = gsm_dlci_alloc(gsm, address); >> @@ -1793,7 +1793,7 @@ static void gsm_queue(struct gsm_mux *gsm) >> } >> break; >> case DISC|PF: >> - if (cr == 0) >> + if (cr == 1) > > Same here, how did this ever work? Are you sure this change is correct? > > thanks, > > greg k-h > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tty: n_gsm: Fix CR bit value when initiator=0 2021-06-16 7:29 ` 赵振国 @ 2021-06-16 7:39 ` Greg KH 2021-06-16 7:45 ` Jiri Slaby 0 siblings, 1 reply; 6+ messages in thread From: Greg KH @ 2021-06-16 7:39 UTC (permalink / raw) To: 赵振国; +Cc: jirislaby, linux-kernel A: http://en.wikipedia.org/wiki/Top_post Q: Were do I find info about this thing called top-posting? A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? A: No. Q: Should I include quotations after my reply? http://daringfireball.net/2007/07/on_top On Wed, Jun 16, 2021 at 03:29:11PM +0800, 赵振国 wrote: > Dear gregkh > > 1: Documentation/driver-api/serial/n_gsm.rst > > The text introduces the config of master ( c.initiator = 1), but the > config of as responder is different. > when set gsm->initiator=0 by GSMIOC_SETCONF ,ngsm driver should be > responder(slaver) > > config： > c.initiator = 0; // set initiator=0，ngsm as responder > ioctl(fd, GSMIOC_SETCONF, &c); > > 2: if master side send SABM/DISC frame data by uart dev > DLC0 control data frame:f9 03 3f 01 1c f9 > kernel log： gsmld_receive: 00000000: f9 03 3f 01 1c f9 > > { > cr = gsm->address & 1; /* C/R bit */ > //CR value=1 > > gsm_print_packet("<--", address, cr, gsm->control, gsm->buf, gsm->len); > > cr ^= 1 - gsm->initiator; /* Flip so 1 always means command */ > //when gsm->initiator is 0, CR value=0 by "^=" calculation > dlci = gsm->dlci[address]; > > switch (gsm->control) { > case SABM|PF: > if (cr == 0) > goto invalid; //if CR value=0,ngsm will goto > invalid,but the dlc0 control frame data is right,if we can't modify > ,ngsm can't send UA response data > } I am sorry, but I really do not understand what you are saying here. Please resubmit your patch with an updated changelog that explains why this change is needed and what it does. thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tty: n_gsm: Fix CR bit value when initiator=0 2021-06-16 7:39 ` Greg KH @ 2021-06-16 7:45 ` Jiri Slaby 2021-06-16 8:01 ` 赵振国 0 siblings, 1 reply; 6+ messages in thread From: Jiri Slaby @ 2021-06-16 7:45 UTC (permalink / raw) To: Greg KH, 赵振国; +Cc: linux-kernel On 16. 06. 21, 9:39, Greg KH wrote: > > A: http://en.wikipedia.org/wiki/Top_post > Q: Were do I find info about this thing called top-posting? > A: Because it messes up the order in which people normally read text. > Q: Why is top-posting such a bad thing? > A: Top-posting. > Q: What is the most annoying thing in e-mail? > > A: No. > Q: Should I include quotations after my reply? > > http://daringfireball.net/2007/07/on_top > > On Wed, Jun 16, 2021 at 03:29:11PM +0800, 赵振国 wrote: >> Dear gregkh >> >> 1: Documentation/driver-api/serial/n_gsm.rst >> >> The text introduces the config of master ( c.initiator = 1), but the >> config of as responder is different. >> when set gsm->initiator=0 by GSMIOC_SETCONF ,ngsm driver should be >> responder(slaver) >> >> config： >> c.initiator = 0; // set initiator=0，ngsm as responder >> ioctl(fd, GSMIOC_SETCONF, &c); >> >> 2: if master side send SABM/DISC frame data by uart dev >> DLC0 control data frame:f9 03 3f 01 1c f9 >> kernel log： gsmld_receive: 00000000: f9 03 3f 01 1c f9 >> >> { >> cr = gsm->address & 1; /* C/R bit */ >> //CR value=1 >> >> gsm_print_packet("<--", address, cr, gsm->control, gsm->buf, gsm->len); >> >> cr ^= 1 - gsm->initiator; /* Flip so 1 always means command */ >> //when gsm->initiator is 0, CR value=0 by "^=" calculation >> dlci = gsm->dlci[address]; >> >> switch (gsm->control) { >> case SABM|PF: >> if (cr == 0) >> goto invalid; //if CR value=0,ngsm will goto >> invalid,but the dlc0 control frame data is right,if we can't modify >> ,ngsm can't send UA response data >> } > > I am sorry, but I really do not understand what you are saying here. > Please resubmit your patch with an updated changelog that explains why > this change is needed and what it does. And why it was able to work until now. I.e. isn't the Documentation wrong? thanks, -- js suse labs ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tty: n_gsm: Fix CR bit value when initiator=0 2021-06-16 7:45 ` Jiri Slaby @ 2021-06-16 8:01 ` 赵振国 0 siblings, 0 replies; 6+ messages in thread From: 赵振国 @ 2021-06-16 8:01 UTC (permalink / raw) To: Jiri Slaby; +Cc: Greg KH, linux-kernel Dear gregkh，Jiri I think that linux ngsm is cp cmux function (config slaver,c.initiator = 0),pc side or ubuntu or terminal will send "AT+CMUX" by tty dev, and send SABM.DISC.CLD command。 because project is YOCTO,YOCTO is IOT open project,need config slaver，after config “c.initiator = 0”，ngsm can‘t work normally. slaver config ,it doesn't need to send "AT+CMUX" ---------------------------------------------------------- 2.1 switch the serial line to using the n_gsm line discipline by using TIOCSETD ioctl. 2.2 configure the mux using GSMIOC_GETCONF / GSMIOC_SETCONF ioctl. 2.3 obtain base gsmtty number for the used serial port, #include <stdio.h> #include <stdint.h> #include <linux/gsmmux.h> #include <linux/tty.h> #define DEFAULT_SPEED B115200 #define SERIAL_PORT /dev/ttyS0 int ldisc = N_GSM0710; struct gsm_config c; struct termios configuration; uint32_t first; /* open the serial port */ fd = open(SERIAL_PORT, O_RDWR | O_NOCTTY | O_NDELAY); /* configure the serial port : speed, flow control ... */ /* use n_gsm line discipline */ ioctl(fd, TIOCSETD, &ldisc); /* get n_gsm configuration */ ioctl(fd, GSMIOC_GETCONF, &c); /* we are responter and need encoding 0 (basic) */ c.initiator = 0; c.encapsulation = 0; /* our modem defaults to a maximum size of 127 bytes */ c.mru = 127; c.mtu = 127; /* set the new configuration */ ioctl(fd, GSMIOC_SETCONF, &c); /* get first gsmtty device node */ ioctl(fd, GSMIOC_GETFIRST, &first); printf("first muxed line: /dev/gsmtty%i\n", first); /* and wait for ever to keep the line discipline enabled */ daemon(0,0); pause(); 2021-06-16 15:45 GMT+08:00, Jiri Slaby <jirislaby@kernel.org>: > On 16. 06. 21, 9:39, Greg KH wrote: >> >> A: http://en.wikipedia.org/wiki/Top_post >> Q: Were do I find info about this thing called top-posting? >> A: Because it messes up the order in which people normally read text. >> Q: Why is top-posting such a bad thing? >> A: Top-posting. >> Q: What is the most annoying thing in e-mail? >> >> A: No. >> Q: Should I include quotations after my reply? >> >> http://daringfireball.net/2007/07/on_top >> >> On Wed, Jun 16, 2021 at 03:29:11PM +0800, 赵振国 wrote: >>> Dear gregkh >>> >>> 1: Documentation/driver-api/serial/n_gsm.rst >>> >>> The text introduces the config of master ( c.initiator = 1), but the >>> config of as responder is different. >>> when set gsm->initiator=0 by GSMIOC_SETCONF ,ngsm driver should be >>> responder(slaver) >>> >>> config： >>> c.initiator = 0; // set initiator=0，ngsm as responder >>> ioctl(fd, GSMIOC_SETCONF, &c); >>> >>> 2: if master side send SABM/DISC frame data by uart dev >>> DLC0 control data frame:f9 03 3f 01 1c f9 >>> kernel log： gsmld_receive: 00000000: f9 03 3f 01 1c f9 >>> >>> { >>> cr = gsm->address & 1; /* C/R bit */ >>> //CR value=1 >>> >>> gsm_print_packet("<--", address, cr, gsm->control, gsm->buf, gsm->len); >>> >>> cr ^= 1 - gsm->initiator; /* Flip so 1 always means command */ >>> //when gsm->initiator is 0, CR value=0 by "^=" calculation >>> dlci = gsm->dlci[address]; >>> >>> switch (gsm->control) { >>> case SABM|PF: >>> if (cr == 0) >>> goto invalid; //if CR value=0,ngsm will goto >>> invalid,but the dlc0 control frame data is right,if we can't modify >>> ,ngsm can't send UA response data >>> } >> >> I am sorry, but I really do not understand what you are saying here. >> Please resubmit your patch with an updated changelog that explains why >> this change is needed and what it does. > > And why it was able to work until now. I.e. isn't the Documentation wrong? > > thanks, > -- > js > suse labs > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-06-16 8:01 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-16 2:56 [PATCH] tty: n_gsm: Fix CR bit value when initiator=0 Zhenguo Zhao 2021-06-16 6:19 ` Greg KH 2021-06-16 7:29 ` 赵振国 2021-06-16 7:39 ` Greg KH 2021-06-16 7:45 ` Jiri Slaby 2021-06-16 8:01 ` 赵振国
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.