[PATCH v2 0/3] TTY improve n_gsm support

[PATCH v2 0/3] TTY improve n_gsm support

[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

[PATCH v2 1/3] tty: n_gsm: Improve debug output

[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

Re: [PATCH v2 1/3] tty: n_gsm: Improve debug output

[Jiri Slaby]

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

Re: [PATCH v2 1/3] tty: n_gsm: Improve debug output

[Gregory CLEMENT]

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

Re: [PATCH v2 1/3] tty: n_gsm: Improve debug output

[Greg Kroah-Hartman]

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

Re: [PATCH v2 1/3] tty: n_gsm: Improve debug output

[Gregory CLEMENT]

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

Re: [PATCH v2 1/3] tty: n_gsm: Improve debug output

[Greg Kroah-Hartman]

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 :)

[PATCH] tty: n_gsm: gsm_print_packet: use scnprintf to avoid pr_cont

[Joe Perches]

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);
 }
 
 

[PATCH v2 2/3] tty: n_gsm: Fix SOF skipping

[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

[PATCH v2 3/3] tty: n_gsm: Fix waking up upper tty layer when room available

[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

Re: [PATCH v2 3/3] tty: n_gsm: Fix waking up upper tty layer when room available

[Jiri Slaby]

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

Re: [PATCH v2 3/3] tty: n_gsm: Fix waking up upper tty layer when room available

[Gregory CLEMENT]

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

[PATCH 0/2] TTY fix-ups for "TTY improve n_gsm support" series

[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

[PATCH v2 1/3] tty: n_gsm: Improve debug output

[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