: MUX n_gsm debug print improvements

Dear kernel tty maintainers,

Please review the following patch:

From 0ac5da0a4653f43ce4b0761a2be8073185c549bb Mon Sep 17 00:00:00 2001
From: Gwenn Bourree <[email protected]>
Date: Mon, 29 Jun 2015 16:09:06 +0200
Subject: [PATCH] Add Debug define

Improve the debug print out and make be clearest

---
 drivers/tty/n_gsm.c | 49
++++++++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 21 deletions(-)

index 382d3fc..d6e0ea0 100644
@@ -66,17 +66,18 @@
 static int debug;
 module_param(debug, int, 0600);
 
-/* Defaults: these are from the specification */
+#define GSMDBG_VERBOSE_PACKET_REPORT(x) ((x) &  1)
+#define GSMDBG_FORCE_CARRIER(x)         ((x) &  2)
+#define GSMDBG_DATA_FULL_REPORT(x)      ((x) &  4)
+#define GSMDBG_DLCI_STREAM_REPORT(x)    ((x) &  8)
+#define GSMDBG_DLCI_DATA_REPORT(x)      ((x) & 16)
+#define GSMDBG_DATA_LEN_REPORT(x)       ((x) & 32)
 
-#define T1	10		/* 100mS */
-#define T2	34		/* 333mS */
-#define N2	3		/* Retry 3 times */
+/* Defaults: these are from the specification */
 
-/* Use long timers for testing at low speed with debug on */
-#ifdef DEBUG_TIMING
-#define T1	100
-#define T2	200
-#endif
+#define T1     10              /* 100mS */
+#define T2     34              /* 333mS */
+#define N2     3               /* Retry 3 times */
 
 /*
  * Semi-arbitrary buffer size limits. 0710 is normally run with 32-64

byte
@@ -461,7 +462,7 @@ 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)
{
- if (!(debug & 1))
+ if (!GSMDBG_VERBOSE_PACKET_REPORT(debug))
return;

pr_info("%s %d) %c: ", hdr, addr, "RC"[cr]);
@@ -700,7 +701,7 @@ static void gsm_data_kick(struct gsm_mux *gsm)
len = msg->len + 2;
}

- if (debug & 4)
+ if (GSMDBG_DATA_FULL_REPORT(debug))
print_hex_dump_bytes("gsm_data_kick: ",
DUMP_PREFIX_OFFSET, gsm->txframe, len);
@@ -1426,7 +1427,7 @@ static int gsm_control_wait(struct gsm_mux *gsm, struct gsm_control *control)
static void gsm_dlci_close(struct gsm_dlci *dlci)
{
del_timer(&dlci->t1);
- if (debug & 8)
+ if (GSMDBG_DLCI_STREAM_REPORT(debug))
pr_debug("DLCI %d goes closed.\n", dlci->addr);
dlci->state = DLCI_CLOSED;
if (dlci->addr != 0) {
@@ -1453,7 +1454,7 @@ static void gsm_dlci_open(struct gsm_dlci *dlci)
del_timer(&dlci->t1);
/* This will let a tty open continue */
dlci->state = DLCI_OPEN;
- if (debug & 8)
+ if (GSMDBG_DLCI_STREAM_REPORT(debug))
pr_debug("DLCI %d goes open.\n", dlci->addr);
wake_up(&dlci->gsm->event);
}
@@ -1556,8 +1557,8 @@ static void gsm_dlci_data(struct gsm_dlci *dlci, u8 *data, int clen)
unsigned int modem = 0;
int len = clen;

- if (debug & 16)
- pr_debug("%d bytes for tty\n", len);
+ if (GSMDBG_DLCI_DATA_REPORT(debug))
+ pr_debug("%s: %d bytes for tty\n", __func__, len);
switch (dlci->adaption) {
/* Unsupported types */
/* Packetised interruptible data */
@@ -1758,7 +1759,7 @@ static void gsm_queue(struct gsm_mux *gsm)
}
if (gsm->fcs != GOOD_FCS) {
gsm->bad_fcs++;
- if (debug & 4)
+ if (GSMDBG_DATA_FULL_REPORT(debug))
pr_debug("BAD FCS %02x\n", gsm->fcs);
return;
}
@@ -1922,6 +1923,7 @@ static void gsm0_receive(struct gsm_mux *gsm, unsigned char c)
gsm->state = GSM_SEARCH;
break;
}
+ pr_debug("wait for GSM0_SOF, while got 0x%x\n", (u32)c);
break;
}
}
@@ -2209,9 +2211,12 @@ static int gsmld_output(struct gsm_mux *gsm, u8 *data, int len)
set_bit(TTY_DO_WRITE_WAKEUP, &gsm->tty->flags); return -ENOSPC;
}
- if (debug & 4)
- print_hex_dump_bytes("gsmld_output: ", DUMP_PREFIX_OFFSET,
+ if (GSMDBG_DATA_FULL_REPORT(debug))
+ print_hex_dump_bytes(__func__, DUMP_PREFIX_OFFSET,
data, len);
+ else if (GSMDBG_DATA_LEN_REPORT(debug))
+ pr_debug("n_gsm: >> %d bytes\n", len);
+
gsm->tty->ops->write(gsm->tty, data, len);
return len;
}
@@ -2276,9 +2281,11 @@ static void gsmld_receive_buf(struct tty_struct *tty, const unsigned char *cp,
int i;
char flags = TTY_NORMAL;

- if (debug & 4)
- print_hex_dump_bytes("gsmld_receive: ", DUMP_PREFIX_OFFSET,
+ if (GSMDBG_DATA_FULL_REPORT(debug))
+ print_hex_dump_bytes(__func__, DUMP_PREFIX_OFFSET,
cp, count);
+ else if (GSMDBG_DATA_LEN_REPORT(debug))
+ pr_debug("n_gsm: << %d bytes\n", count);

for (i = count, dp = cp, f = fp; i; i--, dp++) {
if (f)
@@ -2871,7 +2878,7 @@ static int gsm_carrier_raised(struct tty_port *port)
/* Not yet open so no carrier info */
if (dlci->state != DLCI_OPEN)
return 0;
- if (debug & 2)
+ if (GSMDBG_FORCE_CARRIER(debug))
return 1;
return dlci->modem_rx & TIOCM_CD;
}

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company) Registered headquarters: "Les Montalets"- 2, rue de Paris, 92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.

Any reason you didn't cc the linux-serial mailing list? And linux-api only cares about external api changes, which I don't think you did here.

Also, what's with the odd format, I would have to hand-edit this in order to apply it properly. Please fix it up and send it without the "Dear..." lines, that's not needed at all, nor is the "From" lines.

thanks,

greg k-h

Also, your patch is line-wrapped and also base64 encoded, making it impossible to edit, or apply. Please fix up your email client to use the proper configuration, as I'm sure Intel has documentation for how to do.

In fact, it has documentation for how to do all of this, why aren't you following it?

thanks,

greg k-h

Hi Gwenn,