* [PATCH v2 0/3] TTY improve n_gsm support @ 2020-05-12 11:53 Gregory CLEMENT 2020-05-12 11:53 ` [PATCH v2 1/3] tty: n_gsm: Improve debug output Gregory CLEMENT ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Gregory CLEMENT @ 2020-05-12 11:53 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel Cc: Thomas Petazzoni, Gregory CLEMENT Hello, This series is the second version of patch improving n_gms support especially with TELIT LE910. However the fix should benefit to any modem supporting cmux. The first patch is just about improving debugging output. The second one removes a tty optimization which make the LE910 hang. The last one fixes an issue observed on the LE910 but should benefit to all the modem. We observed that pretty quickly the transfer done using the virtual tty were blocked. We found that it was due of a wakeup to the real tty. Without this fix, the real tty wait for indefinitely. Thanks to Jiri Slaby for the review. Changelog: v1 -> v2: - don't replace the pr_info by pr_debug - remove the superfluous printk("\n"); - use --follow option with git log to find the original commit to fix - use tty_port_tty_wakeup - use 'for' loop instead of 'while' Gregory Gregory CLEMENT (3): tty: n_gsm: Improve debug output tty: n_gsm: Fix SOF skipping tty: n_gsm: Fix waking up upper tty layer when room available drivers/tty/n_gsm.c | 48 +++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 23 deletions(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/3] tty: n_gsm: Improve debug output 2020-05-12 11:53 [PATCH v2 0/3] TTY improve n_gsm support Gregory CLEMENT @ 2020-05-12 11:53 ` Gregory CLEMENT 2020-05-18 6:40 ` Jiri Slaby 2020-05-12 11:53 ` [PATCH v2 2/3] tty: n_gsm: Fix SOF skipping Gregory CLEMENT 2020-05-12 11:53 ` [PATCH v2 3/3] tty: n_gsm: Fix waking up upper tty layer when room available Gregory CLEMENT 2 siblings, 1 reply; 13+ messages in thread From: Gregory CLEMENT @ 2020-05-12 11:53 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel Cc: Thomas Petazzoni, Gregory CLEMENT Use appropriate print helpers for debug messages. Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> --- drivers/tty/n_gsm.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index d77ed82a4840..67c8f8173023 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -504,18 +504,8 @@ static void gsm_print_packet(const char *hdr, int addr, int cr, else pr_cont("(F)"); - if (dlen) { - int ct = 0; - while (dlen--) { - if (ct % 8 == 0) { - pr_cont("\n"); - pr_debug(" "); - } - pr_cont("%02X ", *data++); - ct++; - } - } - pr_cont("\n"); + if (dlen) + print_hex_dump_bytes("", DUMP_PREFIX_NONE, data, dlen); } -- 2.26.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] tty: n_gsm: Improve debug output 2020-05-12 11:53 ` [PATCH v2 1/3] tty: n_gsm: Improve debug output Gregory CLEMENT @ 2020-05-18 6:40 ` Jiri Slaby 2020-05-18 7:33 ` Gregory CLEMENT 0 siblings, 1 reply; 13+ messages in thread From: Jiri Slaby @ 2020-05-18 6:40 UTC (permalink / raw) To: Gregory CLEMENT, Greg Kroah-Hartman, linux-kernel; +Cc: Thomas Petazzoni On 12. 05. 20, 13:53, Gregory CLEMENT wrote: > Use appropriate print helpers for debug messages. > > Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> > --- > drivers/tty/n_gsm.c | 14 ++------------ > 1 file changed, 2 insertions(+), 12 deletions(-) > > diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c > index d77ed82a4840..67c8f8173023 100644 > --- a/drivers/tty/n_gsm.c > +++ b/drivers/tty/n_gsm.c > @@ -504,18 +504,8 @@ static void gsm_print_packet(const char *hdr, int addr, int cr, > else > pr_cont("(F)"); > > - if (dlen) { > - int ct = 0; > - while (dlen--) { > - if (ct % 8 == 0) { > - pr_cont("\n"); > - pr_debug(" "); > - } > - pr_cont("%02X ", *data++); > - ct++; > - } > - } > - pr_cont("\n"); > + if (dlen) This test is superfluous. print_hex_dump_* won't write anything when zero length is passed to it. > + print_hex_dump_bytes("", DUMP_PREFIX_NONE, data, dlen); > } thanks, -- js suse labs ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] tty: n_gsm: Improve debug output 2020-05-18 6:40 ` Jiri Slaby @ 2020-05-18 7:33 ` Gregory CLEMENT 2020-05-18 7:38 ` Greg Kroah-Hartman 0 siblings, 1 reply; 13+ messages in thread From: Gregory CLEMENT @ 2020-05-18 7:33 UTC (permalink / raw) To: Jiri Slaby, Greg Kroah-Hartman, linux-kernel; +Cc: Thomas Petazzoni Hello Jiri, > On 12. 05. 20, 13:53, Gregory CLEMENT wrote: >> Use appropriate print helpers for debug messages. >> >> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> >> --- >> drivers/tty/n_gsm.c | 14 ++------------ >> 1 file changed, 2 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c >> index d77ed82a4840..67c8f8173023 100644 >> --- a/drivers/tty/n_gsm.c >> +++ b/drivers/tty/n_gsm.c >> @@ -504,18 +504,8 @@ static void gsm_print_packet(const char *hdr, int addr, int cr, >> else >> pr_cont("(F)"); >> >> - if (dlen) { >> - int ct = 0; >> - while (dlen--) { >> - if (ct % 8 == 0) { >> - pr_cont("\n"); >> - pr_debug(" "); >> - } >> - pr_cont("%02X ", *data++); >> - ct++; >> - } >> - } >> - pr_cont("\n"); >> + if (dlen) > > This test is superfluous. print_hex_dump_* won't write anything when > zero length is passed to it. As I will send a v3 due to the issue found on the last patch, I am also going to fix this. Gregory > >> + print_hex_dump_bytes("", DUMP_PREFIX_NONE, data, dlen); >> } > > thanks, > -- > js > suse labs -- Gregory Clement, Bootlin Embedded Linux and Kernel engineering http://bootlin.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] tty: n_gsm: Improve debug output 2020-05-18 7:33 ` Gregory CLEMENT @ 2020-05-18 7:38 ` Greg Kroah-Hartman 2020-05-18 7:44 ` Gregory CLEMENT 0 siblings, 1 reply; 13+ messages in thread From: Greg Kroah-Hartman @ 2020-05-18 7:38 UTC (permalink / raw) To: Gregory CLEMENT; +Cc: Jiri Slaby, linux-kernel, Thomas Petazzoni On Mon, May 18, 2020 at 09:33:57AM +0200, Gregory CLEMENT wrote: > Hello Jiri, > > > On 12. 05. 20, 13:53, Gregory CLEMENT wrote: > >> Use appropriate print helpers for debug messages. > >> > >> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> > >> --- > >> drivers/tty/n_gsm.c | 14 ++------------ > >> 1 file changed, 2 insertions(+), 12 deletions(-) > >> > >> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c > >> index d77ed82a4840..67c8f8173023 100644 > >> --- a/drivers/tty/n_gsm.c > >> +++ b/drivers/tty/n_gsm.c > >> @@ -504,18 +504,8 @@ static void gsm_print_packet(const char *hdr, int addr, int cr, > >> else > >> pr_cont("(F)"); > >> > >> - if (dlen) { > >> - int ct = 0; > >> - while (dlen--) { > >> - if (ct % 8 == 0) { > >> - pr_cont("\n"); > >> - pr_debug(" "); > >> - } > >> - pr_cont("%02X ", *data++); > >> - ct++; > >> - } > >> - } > >> - pr_cont("\n"); > >> + if (dlen) > > > > This test is superfluous. print_hex_dump_* won't write anything when > > zero length is passed to it. > > As I will send a v3 due to the issue found on the last patch, I am also > going to fix this. Ugh, as I already applied this series, should I just revert them all, or are you going to send fix-ups on top of what I have applied instead? thanks, greg k-h ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] tty: n_gsm: Improve debug output 2020-05-18 7:38 ` Greg Kroah-Hartman @ 2020-05-18 7:44 ` Gregory CLEMENT 2020-05-18 7:48 ` Greg Kroah-Hartman 0 siblings, 1 reply; 13+ messages in thread From: Gregory CLEMENT @ 2020-05-18 7:44 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Thomas Petazzoni Hello Greg, > On Mon, May 18, 2020 at 09:33:57AM +0200, Gregory CLEMENT wrote: >> Hello Jiri, >> >> > On 12. 05. 20, 13:53, Gregory CLEMENT wrote: >> >> Use appropriate print helpers for debug messages. >> >> >> >> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> >> >> --- >> >> drivers/tty/n_gsm.c | 14 ++------------ >> >> 1 file changed, 2 insertions(+), 12 deletions(-) >> >> >> >> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c >> >> index d77ed82a4840..67c8f8173023 100644 >> >> --- a/drivers/tty/n_gsm.c >> >> +++ b/drivers/tty/n_gsm.c >> >> @@ -504,18 +504,8 @@ static void gsm_print_packet(const char *hdr, int addr, int cr, >> >> else >> >> pr_cont("(F)"); >> >> >> >> - if (dlen) { >> >> - int ct = 0; >> >> - while (dlen--) { >> >> - if (ct % 8 == 0) { >> >> - pr_cont("\n"); >> >> - pr_debug(" "); >> >> - } >> >> - pr_cont("%02X ", *data++); >> >> - ct++; >> >> - } >> >> - } >> >> - pr_cont("\n"); >> >> + if (dlen) >> > >> > This test is superfluous. print_hex_dump_* won't write anything when >> > zero length is passed to it. >> >> As I will send a v3 due to the issue found on the last patch, I am also >> going to fix this. > > Ugh, as I already applied this series, should I just revert them all, or > are you going to send fix-ups on top of what I have applied instead? I was about to send a new series, but I can just send fix-ups. It's up to you. Gregory > > thanks, > > greg k-h -- Gregory Clement, Bootlin Embedded Linux and Kernel engineering http://bootlin.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] tty: n_gsm: Improve debug output 2020-05-18 7:44 ` Gregory CLEMENT @ 2020-05-18 7:48 ` Greg Kroah-Hartman 2020-05-18 17:50 ` [PATCH] tty: n_gsm: gsm_print_packet: use scnprintf to avoid pr_cont Joe Perches 0 siblings, 1 reply; 13+ messages in thread From: Greg Kroah-Hartman @ 2020-05-18 7:48 UTC (permalink / raw) To: Gregory CLEMENT; +Cc: Jiri Slaby, linux-kernel, Thomas Petazzoni On Mon, May 18, 2020 at 09:44:52AM +0200, Gregory CLEMENT wrote: > Hello Greg, > > > On Mon, May 18, 2020 at 09:33:57AM +0200, Gregory CLEMENT wrote: > >> Hello Jiri, > >> > >> > On 12. 05. 20, 13:53, Gregory CLEMENT wrote: > >> >> Use appropriate print helpers for debug messages. > >> >> > >> >> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> > >> >> --- > >> >> drivers/tty/n_gsm.c | 14 ++------------ > >> >> 1 file changed, 2 insertions(+), 12 deletions(-) > >> >> > >> >> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c > >> >> index d77ed82a4840..67c8f8173023 100644 > >> >> --- a/drivers/tty/n_gsm.c > >> >> +++ b/drivers/tty/n_gsm.c > >> >> @@ -504,18 +504,8 @@ static void gsm_print_packet(const char *hdr, int addr, int cr, > >> >> else > >> >> pr_cont("(F)"); > >> >> > >> >> - if (dlen) { > >> >> - int ct = 0; > >> >> - while (dlen--) { > >> >> - if (ct % 8 == 0) { > >> >> - pr_cont("\n"); > >> >> - pr_debug(" "); > >> >> - } > >> >> - pr_cont("%02X ", *data++); > >> >> - ct++; > >> >> - } > >> >> - } > >> >> - pr_cont("\n"); > >> >> + if (dlen) > >> > > >> > This test is superfluous. print_hex_dump_* won't write anything when > >> > zero length is passed to it. > >> > >> As I will send a v3 due to the issue found on the last patch, I am also > >> going to fix this. > > > > Ugh, as I already applied this series, should I just revert them all, or > > are you going to send fix-ups on top of what I have applied instead? > > I was about to send a new series, but I can just send fix-ups. It's up > to you. fix-ups are less work for me :) ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] tty: n_gsm: gsm_print_packet: use scnprintf to avoid pr_cont 2020-05-18 7:48 ` Greg Kroah-Hartman @ 2020-05-18 17:50 ` Joe Perches 0 siblings, 0 replies; 13+ messages in thread From: Joe Perches @ 2020-05-18 17:50 UTC (permalink / raw) To: Greg Kroah-Hartman, Gregory CLEMENT Cc: Jiri Slaby, linux-kernel, Thomas Petazzoni Use a temporary buffer to avoid multiple pr_cont uses. Signed-off-by: Joe Perches <joe@perches.com> --- > > > Ugh, as I already applied this series, should I just revert them all, or > > > are you going to send fix-ups on top of what I have applied instead? > > > > I was about to send a new series, but I can just send fix-ups. It's up > > to you. > > fix-ups are less work for me :) Perhaps use something like this instead? It does increase object size a tiny bit. drivers/tty/n_gsm.c | 71 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 42 insertions(+), 29 deletions(-) diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 69200bd411f7..7d7820aeb57b 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -454,58 +454,71 @@ static u8 gsm_encode_modem(const struct gsm_dlci *dlci) */ static void gsm_print_packet(const char *hdr, int addr, int cr, - u8 control, const u8 *data, int dlen) + u8 control, const u8 *data, int dlen) { + char buf[100]; + char *p; + char *type; + if (!(debug & 1)) return; - pr_info("%s %d) %c: ", hdr, addr, "RC"[cr]); + p = buf; + p += scnprintf(p, buf - p, "%s %d) %c: ", hdr, addr, "RC"[cr]); switch (control & ~PF) { case SABM: - pr_cont("SABM"); + type = "SABM"; break; case UA: - pr_cont("UA"); + type = "UA"; break; case DISC: - pr_cont("DISC"); + type = "DISC"; break; case DM: - pr_cont("DM"); + type = "DM"; break; case UI: - pr_cont("UI"); + type = "UI"; break; case UIH: - pr_cont("UIH"); + type = "UIH"; break; default: - if (!(control & 0x01)) { - pr_cont("I N(S)%d N(R)%d", - (control & 0x0E) >> 1, (control & 0xE0) >> 5); - } else switch (control & 0x0F) { - case RR: - pr_cont("RR(%d)", (control & 0xE0) >> 5); - break; - case RNR: - pr_cont("RNR(%d)", (control & 0xE0) >> 5); - break; - case REJ: - pr_cont("REJ(%d)", (control & 0xE0) >> 5); - break; - default: - pr_cont("[%02X]", control); + type = NULL; + break; + } + + if (type) { + p += scnprintf(p, buf - p, "%s", type); + } else if (!(control & 0x01)) { + p += scnprintf(p, buf - p, "I N(S)%d N(R)%d", + (control & 0x0E) >> 1, (control & 0xE0) >> 5); + } else { + switch (control & 0x0F) { + case RR: + p += scnprintf(p, buf - p, "RR(%d)", + (control & 0xE0) >> 5); + break; + case RNR: + p += scnprintf(p, buf - p, "RNR(%d)", + (control & 0xE0) >> 5); + break; + case REJ: + p += scnprintf(p, buf - p, "REJ(%d)", + (control & 0xE0) >> 5); + break; + default: + p += scnprintf(p, buf - p, "[%02X]", control); + break; } } - if (control & PF) - pr_cont("(P)"); - else - pr_cont("(F)"); + p += scnprintf(p, buf - p, "(%c)", control & PF ? 'P' : 'F'); - if (dlen) - print_hex_dump_bytes("", DUMP_PREFIX_NONE, data, dlen); + pr_info("%s\n", buf); + print_hex_dump_bytes("", DUMP_PREFIX_NONE, data, dlen); } ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/3] tty: n_gsm: Fix SOF skipping 2020-05-12 11:53 [PATCH v2 0/3] TTY improve n_gsm support Gregory CLEMENT 2020-05-12 11:53 ` [PATCH v2 1/3] tty: n_gsm: Improve debug output Gregory CLEMENT @ 2020-05-12 11:53 ` Gregory CLEMENT 2020-05-12 11:53 ` [PATCH v2 3/3] tty: n_gsm: Fix waking up upper tty layer when room available Gregory CLEMENT 2 siblings, 0 replies; 13+ messages in thread From: Gregory CLEMENT @ 2020-05-12 11:53 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel Cc: Thomas Petazzoni, Gregory CLEMENT For at least some modems like the TELIT LE910, skipping SOF makes transfers blocking indefinitely after a short amount of data transferred. Given the small improvement provided by skipping the SOF (just one byte on about 100 bytes), it seems better to completely remove this "feature" than make it optional. Fixes: e1eaea46bb40 ("tty: n_gsm line discipline") Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> --- drivers/tty/n_gsm.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 67c8f8173023..d8d196645500 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -667,7 +667,6 @@ static void gsm_data_kick(struct gsm_mux *gsm) { struct gsm_msg *msg, *nmsg; int len; - int skip_sof = 0; list_for_each_entry_safe(msg, nmsg, &gsm->tx_list, list) { if (gsm->constipated && msg->addr) @@ -689,15 +688,10 @@ static void gsm_data_kick(struct gsm_mux *gsm) print_hex_dump_bytes("gsm_data_kick: ", DUMP_PREFIX_OFFSET, gsm->txframe, len); - - if (gsm->output(gsm, gsm->txframe + skip_sof, - len - skip_sof) < 0) + if (gsm->output(gsm, gsm->txframe, len) < 0) break; /* FIXME: Can eliminate one SOF in many more cases */ gsm->tx_bytes -= msg->len; - /* For a burst of frames skip the extra SOF within the - burst */ - skip_sof = 1; list_del(&msg->list); kfree(msg); -- 2.26.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/3] tty: n_gsm: Fix waking up upper tty layer when room available 2020-05-12 11:53 [PATCH v2 0/3] TTY improve n_gsm support Gregory CLEMENT 2020-05-12 11:53 ` [PATCH v2 1/3] tty: n_gsm: Improve debug output Gregory CLEMENT 2020-05-12 11:53 ` [PATCH v2 2/3] tty: n_gsm: Fix SOF skipping Gregory CLEMENT @ 2020-05-12 11:53 ` Gregory CLEMENT 2020-05-18 6:44 ` Jiri Slaby 2 siblings, 1 reply; 13+ messages in thread From: Gregory CLEMENT @ 2020-05-12 11:53 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel Cc: Thomas Petazzoni, Gregory CLEMENT Warn the upper layer when n_gms is ready to receive data again. Without this the associated virtual tty remains blocked indefinitely. Fixes: e1eaea46bb40 ("tty: n_gsm line discipline") Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> --- drivers/tty/n_gsm.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index d8d196645500..69200bd411f7 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -663,7 +663,7 @@ static struct gsm_msg *gsm_data_alloc(struct gsm_mux *gsm, u8 addr, int len, * FIXME: lock against link layer control transmissions */ -static void gsm_data_kick(struct gsm_mux *gsm) +static void gsm_data_kick(struct gsm_mux *gsm, struct gsm_dlci *dlci) { struct gsm_msg *msg, *nmsg; int len; @@ -695,6 +695,24 @@ static void gsm_data_kick(struct gsm_mux *gsm) list_del(&msg->list); kfree(msg); + + if (dlci) { + tty_port_tty_wakeup(&dlci->port); + } else { + int i = 0; + + for (i = 0; i < NUM_DLCI; i++) { + struct gsm_dlci *dlci; + + dlci = gsm->dlci[i]; + if (dlci == NULL) { + i++; + continue; + } + + tty_port_tty_wakeup(&dlci->port); + } + } } } @@ -746,7 +764,7 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg) /* Add to the actual output queue */ list_add_tail(&msg->list, &gsm->tx_list); gsm->tx_bytes += msg->len; - gsm_data_kick(gsm); + gsm_data_kick(gsm, dlci); } /** @@ -1207,7 +1225,7 @@ static void gsm_control_message(struct gsm_mux *gsm, unsigned int command, gsm_control_reply(gsm, CMD_FCON, NULL, 0); /* Kick the link in case it is idling */ spin_lock_irqsave(&gsm->tx_lock, flags); - gsm_data_kick(gsm); + gsm_data_kick(gsm, NULL); spin_unlock_irqrestore(&gsm->tx_lock, flags); break; case CMD_FCOFF: @@ -2529,7 +2547,7 @@ static void gsmld_write_wakeup(struct tty_struct *tty) /* Queue poll */ clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags); spin_lock_irqsave(&gsm->tx_lock, flags); - gsm_data_kick(gsm); + gsm_data_kick(gsm, NULL); if (gsm->tx_bytes < TX_THRESH_LO) { gsm_dlci_data_sweep(gsm); } -- 2.26.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/3] tty: n_gsm: Fix waking up upper tty layer when room available 2020-05-12 11:53 ` [PATCH v2 3/3] tty: n_gsm: Fix waking up upper tty layer when room available Gregory CLEMENT @ 2020-05-18 6:44 ` Jiri Slaby 2020-05-18 7:42 ` Gregory CLEMENT 0 siblings, 1 reply; 13+ messages in thread From: Jiri Slaby @ 2020-05-18 6:44 UTC (permalink / raw) To: Gregory CLEMENT, Greg Kroah-Hartman, linux-kernel; +Cc: Thomas Petazzoni On 12. 05. 20, 13:53, Gregory CLEMENT wrote: > Warn the upper layer when n_gms is ready to receive data > again. Without this the associated virtual tty remains blocked > indefinitely. > > Fixes: e1eaea46bb40 ("tty: n_gsm line discipline") > Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> > --- > drivers/tty/n_gsm.c | 26 ++++++++++++++++++++++---- > 1 file changed, 22 insertions(+), 4 deletions(-) > > diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c > index d8d196645500..69200bd411f7 100644 > --- a/drivers/tty/n_gsm.c > +++ b/drivers/tty/n_gsm.c > @@ -663,7 +663,7 @@ static struct gsm_msg *gsm_data_alloc(struct gsm_mux *gsm, u8 addr, int len, > * FIXME: lock against link layer control transmissions > */ > > -static void gsm_data_kick(struct gsm_mux *gsm) > +static void gsm_data_kick(struct gsm_mux *gsm, struct gsm_dlci *dlci) > { > struct gsm_msg *msg, *nmsg; > int len; > @@ -695,6 +695,24 @@ static void gsm_data_kick(struct gsm_mux *gsm) > > list_del(&msg->list); > kfree(msg); > + > + if (dlci) { > + tty_port_tty_wakeup(&dlci->port); > + } else { > + int i = 0; > + > + for (i = 0; i < NUM_DLCI; i++) { > + struct gsm_dlci *dlci; > + > + dlci = gsm->dlci[i]; > + if (dlci == NULL) { > + i++; This "i++" looks bogus here. > + continue; > + } > + > + tty_port_tty_wakeup(&dlci->port); So simply: for (i = 0; i < NUM_DLCI; i++) { struct gsm_dlci *dlci = gsm->dlci[i]; if (dlci) tty_port_tty_wakeup(&dlci->port); } ? Or even maybe directly: for (i = 0; i < NUM_DLCI; i++) if (gsm->dlci[i]) tty_port_tty_wakeup(&gsm->dlci[i]->port); thanks, -- js suse labs ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/3] tty: n_gsm: Fix waking up upper tty layer when room available 2020-05-18 6:44 ` Jiri Slaby @ 2020-05-18 7:42 ` Gregory CLEMENT 0 siblings, 0 replies; 13+ messages in thread From: Gregory CLEMENT @ 2020-05-18 7:42 UTC (permalink / raw) To: Jiri Slaby, Greg Kroah-Hartman, linux-kernel; +Cc: Thomas Petazzoni Hi Jiri, > On 12. 05. 20, 13:53, Gregory CLEMENT wrote: >> Warn the upper layer when n_gms is ready to receive data >> again. Without this the associated virtual tty remains blocked >> indefinitely. >> >> Fixes: e1eaea46bb40 ("tty: n_gsm line discipline") >> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> >> --- >> drivers/tty/n_gsm.c | 26 ++++++++++++++++++++++---- >> 1 file changed, 22 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c >> index d8d196645500..69200bd411f7 100644 >> --- a/drivers/tty/n_gsm.c >> +++ b/drivers/tty/n_gsm.c >> @@ -663,7 +663,7 @@ static struct gsm_msg *gsm_data_alloc(struct gsm_mux *gsm, u8 addr, int len, >> * FIXME: lock against link layer control transmissions >> */ >> >> -static void gsm_data_kick(struct gsm_mux *gsm) >> +static void gsm_data_kick(struct gsm_mux *gsm, struct gsm_dlci *dlci) >> { >> struct gsm_msg *msg, *nmsg; >> int len; >> @@ -695,6 +695,24 @@ static void gsm_data_kick(struct gsm_mux *gsm) >> >> list_del(&msg->list); >> kfree(msg); >> + >> + if (dlci) { >> + tty_port_tty_wakeup(&dlci->port); >> + } else { >> + int i = 0; >> + >> + for (i = 0; i < NUM_DLCI; i++) { >> + struct gsm_dlci *dlci; >> + >> + dlci = gsm->dlci[i]; >> + if (dlci == NULL) { >> + i++; > > This "i++" looks bogus here. You're right! Sorry for this. > >> + continue; >> + } >> + >> + tty_port_tty_wakeup(&dlci->port); > > > So simply: > for (i = 0; i < NUM_DLCI; i++) { > struct gsm_dlci *dlci = gsm->dlci[i]; > if (dlci) > tty_port_tty_wakeup(&dlci->port); > } > > ? Or even maybe directly: > for (i = 0; i < NUM_DLCI; i++) > if (gsm->dlci[i]) > tty_port_tty_wakeup(&gsm->dlci[i]->port); I will do this, thanks, Gregory > > thanks, > -- > js > suse labs -- Gregory Clement, Bootlin Embedded Linux and Kernel engineering http://bootlin.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 0/2] TTY fix-ups for "TTY improve n_gsm support" series @ 2020-05-18 8:45 Gregory CLEMENT 2020-05-18 8:45 ` [PATCH v2 2/3] tty: n_gsm: Fix SOF skipping Gregory CLEMENT 0 siblings, 1 reply; 13+ messages in thread From: Gregory CLEMENT @ 2020-05-18 8:45 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel Cc: Thomas Petazzoni, Gregory CLEMENT Hello, Jiri found issues or things to improve in the v2 of the series "TTY improve n_gsm support". However it was already applied. So this series implements the needed changes. For the second patch, as the commit "tty: n_gsm: Fix waking up upper tty layer when room available" was not yet in mainline, it has no SHA-1 ID yet. That's why I reference the same commit that the one fixed by "TTY improve n_gsm support", as they should be applied in the same order in stable branch than in mainline. Gregory Gregory CLEMENT (2): tty: n_gsm: Remove unnecessary test in gsm_print_packet() tty: n_gsm: Fix bogus i++ in gsm_data_kick drivers/tty/n_gsm.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/3] tty: n_gsm: Fix SOF skipping 2020-05-18 8:45 [PATCH 0/2] TTY fix-ups for "TTY improve n_gsm support" series Gregory CLEMENT @ 2020-05-18 8:45 ` Gregory CLEMENT 0 siblings, 0 replies; 13+ messages in thread From: Gregory CLEMENT @ 2020-05-18 8:45 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel Cc: Thomas Petazzoni, Gregory CLEMENT For at least some modems like the TELIT LE910, skipping SOF makes transfers blocking indefinitely after a short amount of data transferred. Given the small improvement provided by skipping the SOF (just one byte on about 100 bytes), it seems better to completely remove this "feature" than make it optional. Fixes: e1eaea46bb40 ("tty: n_gsm line discipline") Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> --- drivers/tty/n_gsm.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 67c8f8173023..d8d196645500 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -667,7 +667,6 @@ static void gsm_data_kick(struct gsm_mux *gsm) { struct gsm_msg *msg, *nmsg; int len; - int skip_sof = 0; list_for_each_entry_safe(msg, nmsg, &gsm->tx_list, list) { if (gsm->constipated && msg->addr) @@ -689,15 +688,10 @@ static void gsm_data_kick(struct gsm_mux *gsm) print_hex_dump_bytes("gsm_data_kick: ", DUMP_PREFIX_OFFSET, gsm->txframe, len); - - if (gsm->output(gsm, gsm->txframe + skip_sof, - len - skip_sof) < 0) + if (gsm->output(gsm, gsm->txframe, len) < 0) break; /* FIXME: Can eliminate one SOF in many more cases */ gsm->tx_bytes -= msg->len; - /* For a burst of frames skip the extra SOF within the - burst */ - skip_sof = 1; list_del(&msg->list); kfree(msg); -- 2.26.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-05-18 17:50 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-12 11:53 [PATCH v2 0/3] TTY improve n_gsm support Gregory CLEMENT 2020-05-12 11:53 ` [PATCH v2 1/3] tty: n_gsm: Improve debug output Gregory CLEMENT 2020-05-18 6:40 ` Jiri Slaby 2020-05-18 7:33 ` Gregory CLEMENT 2020-05-18 7:38 ` Greg Kroah-Hartman 2020-05-18 7:44 ` Gregory CLEMENT 2020-05-18 7:48 ` Greg Kroah-Hartman 2020-05-18 17:50 ` [PATCH] tty: n_gsm: gsm_print_packet: use scnprintf to avoid pr_cont Joe Perches 2020-05-12 11:53 ` [PATCH v2 2/3] tty: n_gsm: Fix SOF skipping Gregory CLEMENT 2020-05-12 11:53 ` [PATCH v2 3/3] tty: n_gsm: Fix waking up upper tty layer when room available Gregory CLEMENT 2020-05-18 6:44 ` Jiri Slaby 2020-05-18 7:42 ` Gregory CLEMENT 2020-05-18 8:45 [PATCH 0/2] TTY fix-ups for "TTY improve n_gsm support" series Gregory CLEMENT 2020-05-18 8:45 ` [PATCH v2 2/3] tty: n_gsm: Fix SOF skipping Gregory CLEMENT
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).