Messages in this thread Patch in this message | | | From | 赵振国 <> | Date | Wed, 16 Jun 2021 15:29:11 +0800 | Subject | Re: [PATCH] tty: n_gsm: Fix CR bit value when initiator=0 |
| |
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 >
| |