| Message ID | 20230426080315.7595-8-daniel.starke@siemens.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4,1/8] tty: n_gsm: add restart flag to DLC specific ioctl config | expand |
On Wed, Apr 26, 2023 at 10:03:15AM +0200, D. Starke wrote: > From: Daniel Starke <daniel.starke@siemens.com> > > Add counters for the number of data bytes received/transmitted per DLCI in > for preparation for an upcoming patch which will expose these values to the > user. As this is patch 8/8, "upcoming patch" makes no sense, sorry. Please either drop this and add it as part of the series that provides this functionality, or add the functionality to the patch series as the next patch in it. > > Signed-off-by: Daniel Starke <daniel.starke@siemens.com> > --- > drivers/tty/n_gsm.c | 25 ++++++++++++++++++++++++- > 1 file changed, 24 insertions(+), 1 deletion(-) > > v3 -> v4: > No changes. > > Link: https://lore.kernel.org/all/20230424075251.5216-8-daniel.starke@siemens.com/ > > diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c > index 62bff4474b57..2e2e1dafcf40 100644 > --- a/drivers/tty/n_gsm.c > +++ b/drivers/tty/n_gsm.c > @@ -186,6 +186,9 @@ struct gsm_dlci { > void (*data)(struct gsm_dlci *dlci, const u8 *data, int len); > void (*prev_data)(struct gsm_dlci *dlci, const u8 *data, int len); > struct net_device *net; /* network interface, if created */ > + /* Statistics (not currently exposed) */ No blank line before this? And why isn't this structure documented in kerneldoc to make it more obvious what is happening? thanks, greg k-h
> > From: Daniel Starke <daniel.starke@siemens.com> > > > > Add counters for the number of data bytes received/transmitted per DLCI in > > for preparation for an upcoming patch which will expose these values to the > > user. > > As this is patch 8/8, "upcoming patch" makes no sense, sorry. Please > either drop this and add it as part of the series that provides this > functionality, or add the functionality to the patch series as the next > patch in it. I will add the remaining two outstanding patches on my side to this patch series. The functional addition which exposes these values to the user is included there. > > diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c > > index 62bff4474b57..2e2e1dafcf40 100644 > > --- a/drivers/tty/n_gsm.c > > +++ b/drivers/tty/n_gsm.c > > @@ -186,6 +186,9 @@ struct gsm_dlci { > > void (*data)(struct gsm_dlci *dlci, const u8 *data, int len); > > void (*prev_data)(struct gsm_dlci *dlci, const u8 *data, int len); > > struct net_device *net; /* network interface, if created */ > > + /* Statistics (not currently exposed) */ > > No blank line before this? I will add a blank line before this one. > > And why isn't this structure documented in kerneldoc to make it more > obvious what is happening? True, it would be better if this was the case. However, there was no such cleanup of this internal structure yet and this is out of scope of this patch/patch series. A future patch needs to fix this. Best regards, Daniel Starke
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 62bff4474b57..2e2e1dafcf40 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -186,6 +186,9 @@ struct gsm_dlci { void (*data)(struct gsm_dlci *dlci, const u8 *data, int len); void (*prev_data)(struct gsm_dlci *dlci, const u8 *data, int len); struct net_device *net; /* network interface, if created */ + /* Statistics (not currently exposed) */ + u64 tx; /* Data bytes sent on this DLCI */ + u64 rx; /* Data bytes received on this DLCI */ }; /* @@ -1216,6 +1219,7 @@ static int gsm_dlci_data_output(struct gsm_mux *gsm, struct gsm_dlci *dlci) tty_port_tty_wakeup(&dlci->port); __gsm_data_queue(dlci, msg); + dlci->tx += len; /* Bytes of data we used up */ return size; } @@ -1283,6 +1287,7 @@ static int gsm_dlci_data_output_framed(struct gsm_mux *gsm, memcpy(dp, dlci->skb->data, len); skb_pull(dlci->skb, len); __gsm_data_queue(dlci, msg); + dlci->tx += len; if (last) { dev_kfree_skb_any(dlci->skb); dlci->skb = NULL; @@ -1461,6 +1466,7 @@ static int gsm_control_command(struct gsm_mux *gsm, int cmd, const u8 *data, msg->data[1] = (dlen << 1) | EA; memcpy(msg->data + 2, data, dlen); gsm_data_queue(dlci, msg); + dlci->tx += dlen; return 0; } @@ -1488,6 +1494,7 @@ static void gsm_control_reply(struct gsm_mux *gsm, int cmd, const u8 *data, msg->data[1] = (dlen << 1) | EA; memcpy(msg->data + 2, data, dlen); gsm_data_queue(dlci, msg); + dlci->tx += dlen; } /** @@ -1852,10 +1859,13 @@ static void gsm_control_message(struct gsm_mux *gsm, unsigned int command, const u8 *data, int clen) { u8 buf[1]; + struct gsm_dlci *dlci = gsm->dlci[0]; + + if (dlci) + dlci->rx += clen; switch (command) { case CMD_CLD: { - struct gsm_dlci *dlci = gsm->dlci[0]; /* Modem wishes to close down */ if (dlci) { dlci->dead = true; @@ -1934,6 +1944,8 @@ static void gsm_control_response(struct gsm_mux *gsm, unsigned int command, ctrl = gsm->pending_cmd; dlci = gsm->dlci[0]; + if (dlci) + dlci->rx += clen; command |= 1; /* Does the reply match our command */ if (ctrl != NULL && (command == ctrl->cmd || command == CMD_NSC)) { @@ -2298,6 +2310,9 @@ static void gsm_dlci_begin_open(struct gsm_dlci *dlci) need_pn = true; } + dlci->tx = 0; + dlci->rx = 0; + switch (dlci->state) { case DLCI_CLOSED: case DLCI_WAITING_CONFIG: @@ -2330,6 +2345,9 @@ static void gsm_dlci_begin_open(struct gsm_dlci *dlci) */ static void gsm_dlci_set_opening(struct gsm_dlci *dlci) { + dlci->tx = 0; + dlci->rx = 0; + switch (dlci->state) { case DLCI_CLOSED: case DLCI_WAITING_CONFIG: @@ -2349,6 +2367,9 @@ static void gsm_dlci_set_opening(struct gsm_dlci *dlci) */ static void gsm_dlci_set_wait_config(struct gsm_dlci *dlci) { + dlci->tx = 0; + dlci->rx = 0; + switch (dlci->state) { case DLCI_CLOSED: case DLCI_CLOSING: @@ -2425,6 +2446,7 @@ static void gsm_dlci_data(struct gsm_dlci *dlci, const u8 *data, int clen) fallthrough; case 1: /* Line state will go via DLCI 0 controls only */ default: + dlci->rx += clen; tty_insert_flip_string(port, data, clen); tty_flip_buffer_push(port); } @@ -2785,6 +2807,7 @@ static void gsm_queue(struct gsm_mux *gsm) gsm->open_error++; return; } + dlci->rx += gsm->len; if (dlci->dead) gsm_response(gsm, address, DM|PF); else {