Re: [PATCH 1/8] tty: n_gsm: fix formatting errors
On 21.02.2016 22:30, Joe Perches wrote:
On Sun, 2016-02-21 at 22:38 +0100, Andrej Krpic wrote:
Minor formatting changes to remove errors and reduce number of
warnings produced by checkpatch.pl script.
[]
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
[]
@@ -489,7 +490,8 @@ static void gsm_print_packet(const char *hdr,
int addr, int cr,
if (!(control & 0x01)) {
pr_cont("I N(S)%d N(R)%d",
(control & 0x0E) >> 1, (control & 0xE0) >> 5);
- } else switch (control & 0x0F) {
+ } else
+ switch (control & 0x0F) {
Please follow the brace rule for else uses where
if one branch has braces, the other does too.
Thank you for noticing.
Should I resend this as a single patch or wait for more comments for v2
series?
-Andrej
Re: [PATCH 1/8] tty: n_gsm: fix formatting errors
On 21.02.2016 22:30, Joe Perches wrote:
On Sun, 2016-02-21 at 22:38 +0100, Andrej Krpic wrote:
Minor formatting changes to remove errors and reduce number of
warnings produced by checkpatch.pl script.
[]
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
[]
@@ -489,7 +490,8 @@ static void gsm_print_packet(const char *hdr,
int addr, int cr,
if (!(control & 0x01)) {
pr_cont("I N(S)%d N(R)%d",
(control & 0x0E) >> 1, (control & 0xE0) >> 5);
- } else switch (control & 0x0F) {
+ } else
+ switch (control & 0x0F) {
Please follow the brace rule for else uses where
if one branch has braces, the other does too.
Thank you for noticing.
Should I resend this as a single patch or wait for more comments for v2
series?
-Andrej
Re: [PATCH 0/8] tty: n_gsm: Make mux work as a responder station
On 21.02.2016 23:42, One Thousand Gnomes wrote: On Sun, 21 Feb 2016 22:38:29 +0100 Andrej Krpic <a...@tnode.com> wrote: When using n_gsm you have to explicitly set it to work as a initiator station. This led me to believe that it can also work as a responder. snip This looks reasonable to me. It was never intended to work as a responder but it seems clean enough to do so. Have you tested it against some other modems with these changes applied ? It has been tested against SIM900 (SIMCom) and M66 (Quectel). (I'm always wary of patches to this going in without testing on actual modems, because the spec is complex and we are not the only ones with bugs) While second and third patch don't change anything for initiator mode mux, others certainly do. Also can you please cc these patches to xinhuix@intel.com This address got rejected. Yours and LKML's didn't. -Andrej
Re: [PATCH 0/8] tty: n_gsm: Make mux work as a responder station
On 21.02.2016 23:42, One Thousand Gnomes wrote: On Sun, 21 Feb 2016 22:38:29 +0100 Andrej Krpic wrote: When using n_gsm you have to explicitly set it to work as a initiator station. This led me to believe that it can also work as a responder. snip This looks reasonable to me. It was never intended to work as a responder but it seems clean enough to do so. Have you tested it against some other modems with these changes applied ? It has been tested against SIM900 (SIMCom) and M66 (Quectel). (I'm always wary of patches to this going in without testing on actual modems, because the spec is complex and we are not the only ones with bugs) While second and third patch don't change anything for initiator mode mux, others certainly do. Also can you please cc these patches to xinhuix@intel.com This address got rejected. Yours and LKML's didn't. -Andrej
Re: [PATCH 0/8] tty: n_gsm: Make mux work as a responder station *DUPLICATE*
Please ignore this duplicate thread.
Re: [PATCH 0/8] tty: n_gsm: Make mux work as a responder station *DUPLICATE*
Please ignore this duplicate thread.
[PATCH 1/8] tty: n_gsm: fix formatting errors
Minor formatting changes to remove errors and reduce number of
warnings produced by checkpatch.pl script.
Signed-off-by: Andrej Krpic <a...@tnode.com>
---
drivers/tty/n_gsm.c | 138
1 file changed, 95 insertions(+), 43 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c016207..cc3b374 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -431,6 +431,7 @@ static int gsm_read_ea(unsigned int *val, u8 c)
static u8 gsm_encode_modem(const struct gsm_dlci *dlci)
{
u8 modembits = 0;
+
/* FC is true flow control not modem bits */
if (dlci->throttled)
modembits |= MDM_FC;
@@ -489,7 +490,8 @@ static void gsm_print_packet(const char *hdr, int addr, int
cr,
if (!(control & 0x01)) {
pr_cont("I N(S)%d N(R)%d",
(control & 0x0E) >> 1, (control & 0xE0) >> 5);
- } else switch (control & 0x0F) {
+ } else
+ switch (control & 0x0F) {
case RR:
pr_cont("RR(%d)", (control & 0xE0) >> 5);
break;
@@ -511,6 +513,7 @@ static void gsm_print_packet(const char *hdr, int addr, int
cr,
if (dlen) {
int ct = 0;
+
while (dlen--) {
if (ct % 8 == 0) {
pr_cont("\n");
@@ -542,6 +545,7 @@ static void gsm_print_packet(const char *hdr, int addr, int
cr,
static int gsm_stuff_frame(const u8 *input, u8 *output, int len)
{
int olen = 0;
+
while (len--) {
if (*input == GSM1_SOF || *input == GSM1_ESCAPE
|| *input == XON || *input == XOFF) {
@@ -695,7 +699,7 @@ static void gsm_data_kick(struct gsm_mux *gsm)
len += 2;
} else {
gsm->txframe[0] = GSM0_SOF;
- memcpy(gsm->txframe + 1 , msg->data, msg->len);
+ memcpy(gsm->txframe + 1, msg->data, msg->len);
gsm->txframe[msg->len + 1] = GSM0_SOF;
len = msg->len + 2;
}
@@ -711,7 +715,8 @@ static void gsm_data_kick(struct gsm_mux *gsm)
/* 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 */
+* burst
+*/
skip_sof = 1;
list_del(>list);
@@ -750,7 +755,7 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct
gsm_msg *msg)
*--dp = (msg->addr << 2) | 2 | EA;
else
*--dp = (msg->addr << 2) | EA;
- *fcs = gsm_fcs_add_block(INIT_FCS, dp , msg->data - dp);
+ *fcs = gsm_fcs_add_block(INIT_FCS, dp, msg->data - dp);
/* Ugly protocol layering violation */
if (msg->ctrl == UI || msg->ctrl == (UI|PF))
*fcs = gsm_fcs_add_block(*fcs, msg->data, msg->len);
@@ -760,7 +765,8 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct
gsm_msg *msg)
msg->data, msg->len);
/* Move the header back and adjust the length, also allow for the FCS
- now tacked on the end */
+* now tacked on the end
+*/
msg->len += (msg->data - dp) + 1;
msg->data = dp;
@@ -783,6 +789,7 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct
gsm_msg *msg)
static void gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg)
{
unsigned long flags;
+
spin_lock_irqsave(>gsm->tx_lock, flags);
__gsm_data_queue(dlci, msg);
spin_unlock_irqrestore(>gsm->tx_lock, flags);
@@ -821,7 +828,8 @@ static int gsm_dlci_data_output(struct gsm_mux *gsm, struct
gsm_dlci *dlci)
msg = gsm_data_alloc(gsm, dlci->addr, size, gsm->ftype);
/* FIXME: need a timer or something to kick this so it can't
- get stuck with no work outstanding and no buffer free */
+* get stuck with no work outstanding and no buffer free
+*/
if (msg == NULL)
return -ENOMEM;
dp = msg->data;
@@ -829,11 +837,13 @@ static int gsm_dlci_data_output(struct gsm_mux *gsm,
struct gsm_dlci *dlci)
case 1: /* Unstructured */
break;
case 2: /* Unstructed with modem bits.
- Always one byte as we never send inline break data */
+* Always one byte as we never send inline break data
+
[PATCH 1/8] tty: n_gsm: fix formatting errors
Minor formatting changes to remove errors and reduce number of
warnings produced by checkpatch.pl script.
Signed-off-by: Andrej Krpic
---
drivers/tty/n_gsm.c | 138
1 file changed, 95 insertions(+), 43 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c016207..cc3b374 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -431,6 +431,7 @@ static int gsm_read_ea(unsigned int *val, u8 c)
static u8 gsm_encode_modem(const struct gsm_dlci *dlci)
{
u8 modembits = 0;
+
/* FC is true flow control not modem bits */
if (dlci->throttled)
modembits |= MDM_FC;
@@ -489,7 +490,8 @@ static void gsm_print_packet(const char *hdr, int addr, int
cr,
if (!(control & 0x01)) {
pr_cont("I N(S)%d N(R)%d",
(control & 0x0E) >> 1, (control & 0xE0) >> 5);
- } else switch (control & 0x0F) {
+ } else
+ switch (control & 0x0F) {
case RR:
pr_cont("RR(%d)", (control & 0xE0) >> 5);
break;
@@ -511,6 +513,7 @@ static void gsm_print_packet(const char *hdr, int addr, int
cr,
if (dlen) {
int ct = 0;
+
while (dlen--) {
if (ct % 8 == 0) {
pr_cont("\n");
@@ -542,6 +545,7 @@ static void gsm_print_packet(const char *hdr, int addr, int
cr,
static int gsm_stuff_frame(const u8 *input, u8 *output, int len)
{
int olen = 0;
+
while (len--) {
if (*input == GSM1_SOF || *input == GSM1_ESCAPE
|| *input == XON || *input == XOFF) {
@@ -695,7 +699,7 @@ static void gsm_data_kick(struct gsm_mux *gsm)
len += 2;
} else {
gsm->txframe[0] = GSM0_SOF;
- memcpy(gsm->txframe + 1 , msg->data, msg->len);
+ memcpy(gsm->txframe + 1, msg->data, msg->len);
gsm->txframe[msg->len + 1] = GSM0_SOF;
len = msg->len + 2;
}
@@ -711,7 +715,8 @@ static void gsm_data_kick(struct gsm_mux *gsm)
/* 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 */
+* burst
+*/
skip_sof = 1;
list_del(>list);
@@ -750,7 +755,7 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct
gsm_msg *msg)
*--dp = (msg->addr << 2) | 2 | EA;
else
*--dp = (msg->addr << 2) | EA;
- *fcs = gsm_fcs_add_block(INIT_FCS, dp , msg->data - dp);
+ *fcs = gsm_fcs_add_block(INIT_FCS, dp, msg->data - dp);
/* Ugly protocol layering violation */
if (msg->ctrl == UI || msg->ctrl == (UI|PF))
*fcs = gsm_fcs_add_block(*fcs, msg->data, msg->len);
@@ -760,7 +765,8 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct
gsm_msg *msg)
msg->data, msg->len);
/* Move the header back and adjust the length, also allow for the FCS
- now tacked on the end */
+* now tacked on the end
+*/
msg->len += (msg->data - dp) + 1;
msg->data = dp;
@@ -783,6 +789,7 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct
gsm_msg *msg)
static void gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg)
{
unsigned long flags;
+
spin_lock_irqsave(>gsm->tx_lock, flags);
__gsm_data_queue(dlci, msg);
spin_unlock_irqrestore(>gsm->tx_lock, flags);
@@ -821,7 +828,8 @@ static int gsm_dlci_data_output(struct gsm_mux *gsm, struct
gsm_dlci *dlci)
msg = gsm_data_alloc(gsm, dlci->addr, size, gsm->ftype);
/* FIXME: need a timer or something to kick this so it can't
- get stuck with no work outstanding and no buffer free */
+* get stuck with no work outstanding and no buffer free
+*/
if (msg == NULL)
return -ENOMEM;
dp = msg->data;
@@ -829,11 +837,13 @@ static int gsm_dlci_data_output(struct gsm_mux *gsm,
struct gsm_dlci *dlci)
case 1: /* Unstructured */
break;
case 2: /* Unstructed with modem bits.
- Always one byte as we never send inline break data */
+* Always one byte as we never send inline break data
+
[PATCH 3/8] tty: n_gsm: make mux work as a responder station
Comment suggests that cr == 1 represents a received command and cr == 0
a received response. Received frames are then filtered:
- correctly by rejection of SABM and DISC responses, they are
command only frame types and
- incorrectly by rejection of UA (a response only frame type) responses.
Mux as a initiator successfully establishes DLC by receiving UA response
frame to a previously sent open channel command (SABM). Incorrect
equation (eqA) makes UA "reject cr == 0 commands" case correct, but
filters out all received SABM and DISC command frames.
Change eqA to eqB to match the intent and fix filtering of UA frames.
This enables reception of SABM and DISC frames and consequently
makes mux work as a responder station.
receivedreceiving as eqA eqB 3GPP TS 27.010
CR bit initiator (ir)cr=CR^(1-ir) cr=CR^ir5.2.1.2
0 0 10 0 (response)
1 0 _\ 01 1 (command)
0 1 / 01 1 (command)
1 1 10 0 (response)
Signed-off-by: Andrej Krpic <a...@tnode.com>
---
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 a0fb92c..05b562d 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1798,7 +1798,7 @@ static void gsm_queue(struct gsm_mux *gsm)
gsm_print_packet("<--", address, cr, gsm->control, gsm->buf, gsm->len);
- cr ^= 1 - gsm->initiator; /* Flip so 1 always means command */
+ cr ^= gsm->initiator; /* Flip so 1 always means command */
dlci = gsm->dlci[address];
switch (gsm->control) {
@@ -1829,7 +1829,7 @@ static void gsm_queue(struct gsm_mux *gsm)
break;
case UA:
case UA|PF:
- if (cr == 0 || dlci == NULL)
+ if (cr || dlci == NULL)
break;
switch (dlci->state) {
case DLCI_CLOSING:
--
2.7.0
[PATCH 3/8] tty: n_gsm: make mux work as a responder station
Comment suggests that cr == 1 represents a received command and cr == 0
a received response. Received frames are then filtered:
- correctly by rejection of SABM and DISC responses, they are
command only frame types and
- incorrectly by rejection of UA (a response only frame type) responses.
Mux as a initiator successfully establishes DLC by receiving UA response
frame to a previously sent open channel command (SABM). Incorrect
equation (eqA) makes UA "reject cr == 0 commands" case correct, but
filters out all received SABM and DISC command frames.
Change eqA to eqB to match the intent and fix filtering of UA frames.
This enables reception of SABM and DISC frames and consequently
makes mux work as a responder station.
receivedreceiving as eqA eqB 3GPP TS 27.010
CR bit initiator (ir)cr=CR^(1-ir) cr=CR^ir5.2.1.2
0 0 10 0 (response)
1 0 _\ 01 1 (command)
0 1 / 01 1 (command)
1 1 10 0 (response)
Signed-off-by: Andrej Krpic
---
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 a0fb92c..05b562d 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1798,7 +1798,7 @@ static void gsm_queue(struct gsm_mux *gsm)
gsm_print_packet("<--", address, cr, gsm->control, gsm->buf, gsm->len);
- cr ^= 1 - gsm->initiator; /* Flip so 1 always means command */
+ cr ^= gsm->initiator; /* Flip so 1 always means command */
dlci = gsm->dlci[address];
switch (gsm->control) {
@@ -1829,7 +1829,7 @@ static void gsm_queue(struct gsm_mux *gsm)
break;
case UA:
case UA|PF:
- if (cr == 0 || dlci == NULL)
+ if (cr || dlci == NULL)
break;
switch (dlci->state) {
case DLCI_CLOSING:
--
2.7.0
[PATCH 2/8] tty: n_gsm: fix C/R bit when sending as a responder
According to the specification (3GPP TS 27.010 v12.0.0 R1, 5.2.1.2),
C/R bit must be the same for corresponding command and response.
If mux is an initiator (station that initiates DLC 0), valid sent
commands and received responses must have C/R bit set to 1.
For a station that is a responder, valid sent commands and received
responses must have C/R bit set to 0.
Change the value of C/R bit in command and response frames to
depend on whether the station is initator or not.
Signed-off-by: Andrej Krpic <a...@tnode.com>
---
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 cc3b374..a0fb92c 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -622,7 +622,7 @@ static void gsm_send(struct gsm_mux *gsm, int addr, int cr,
int control)
static inline void gsm_response(struct gsm_mux *gsm, int addr, int control)
{
- gsm_send(gsm, addr, 0, control);
+ gsm_send(gsm, addr, gsm->initiator ? 0 : 1, control);
}
/**
@@ -636,7 +636,7 @@ static inline void gsm_response(struct gsm_mux *gsm, int
addr, int control)
static inline void gsm_command(struct gsm_mux *gsm, int addr, int control)
{
- gsm_send(gsm, addr, 1, control);
+ gsm_send(gsm, addr, gsm->initiator ? 1 : 0, control);
}
/* Data transmission */
--
2.7.0
[PATCH 2/8] tty: n_gsm: fix C/R bit when sending as a responder
According to the specification (3GPP TS 27.010 v12.0.0 R1, 5.2.1.2),
C/R bit must be the same for corresponding command and response.
If mux is an initiator (station that initiates DLC 0), valid sent
commands and received responses must have C/R bit set to 1.
For a station that is a responder, valid sent commands and received
responses must have C/R bit set to 0.
Change the value of C/R bit in command and response frames to
depend on whether the station is initator or not.
Signed-off-by: Andrej Krpic
---
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 cc3b374..a0fb92c 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -622,7 +622,7 @@ static void gsm_send(struct gsm_mux *gsm, int addr, int cr,
int control)
static inline void gsm_response(struct gsm_mux *gsm, int addr, int control)
{
- gsm_send(gsm, addr, 0, control);
+ gsm_send(gsm, addr, gsm->initiator ? 0 : 1, control);
}
/**
@@ -636,7 +636,7 @@ static inline void gsm_response(struct gsm_mux *gsm, int
addr, int control)
static inline void gsm_command(struct gsm_mux *gsm, int addr, int control)
{
- gsm_send(gsm, addr, 1, control);
+ gsm_send(gsm, addr, gsm->initiator ? 1 : 0, control);
}
/* Data transmission */
--
2.7.0
[PATCH 7/8] tty: n_gsm: properly format Modem Status Command message
Change format of Modem Status Command (MSC) message that is sent to the one expected in the receive function gsm_control_modem and specified in 3GPP TS 27.010 version 12.0.0 Release 12, 5.4.6.3.7. Wrongly formatted MSC causes DLC to be marked as constipated. A bug appears after format of transmitted control messages is fixed and control messages start to be recognized. Signed-off-by: Andrej Krpic <a...@tnode.com> --- drivers/tty/n_gsm.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 8aa90e0..b0d9edd 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -2874,12 +2874,11 @@ static int gsmtty_modem_update(struct gsm_dlci *dlci, u8 brk) if (brk) len++; - modembits[0] = len << 1 | EA; /* Data bytes */ - modembits[1] = dlci->addr << 2 | 3; /* DLCI, EA, 1 */ - modembits[2] = gsm_encode_modem(dlci) << 1 | EA; + modembits[0] = dlci->addr << 2 | 3; /* DLCI, EA, 1 */ + modembits[1] = gsm_encode_modem(dlci) << 1 | EA; if (brk) - modembits[3] = brk << 4 | 2 | EA; /* Valid, EA */ - ctrl = gsm_control_send(dlci->gsm, CMD_MSC, modembits, len + 1); + modembits[2] = brk << 4 | 2 | EA; /* Valid, EA */ + ctrl = gsm_control_send(dlci->gsm, CMD_MSC, modembits, len); if (ctrl == NULL) return -ENOMEM; return gsm_control_wait(dlci->gsm, ctrl); -- 2.7.0
[PATCH 7/8] tty: n_gsm: properly format Modem Status Command message
Change format of Modem Status Command (MSC) message that is sent to the one expected in the receive function gsm_control_modem and specified in 3GPP TS 27.010 version 12.0.0 Release 12, 5.4.6.3.7. Wrongly formatted MSC causes DLC to be marked as constipated. A bug appears after format of transmitted control messages is fixed and control messages start to be recognized. Signed-off-by: Andrej Krpic --- drivers/tty/n_gsm.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 8aa90e0..b0d9edd 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -2874,12 +2874,11 @@ static int gsmtty_modem_update(struct gsm_dlci *dlci, u8 brk) if (brk) len++; - modembits[0] = len << 1 | EA; /* Data bytes */ - modembits[1] = dlci->addr << 2 | 3; /* DLCI, EA, 1 */ - modembits[2] = gsm_encode_modem(dlci) << 1 | EA; + modembits[0] = dlci->addr << 2 | 3; /* DLCI, EA, 1 */ + modembits[1] = gsm_encode_modem(dlci) << 1 | EA; if (brk) - modembits[3] = brk << 4 | 2 | EA; /* Valid, EA */ - ctrl = gsm_control_send(dlci->gsm, CMD_MSC, modembits, len + 1); + modembits[2] = brk << 4 | 2 | EA; /* Valid, EA */ + ctrl = gsm_control_send(dlci->gsm, CMD_MSC, modembits, len); if (ctrl == NULL) return -ENOMEM; return gsm_control_wait(dlci->gsm, ctrl); -- 2.7.0
[PATCH 6/8] tty: n_gsm: add missing length field in control channel commands
Observing debug output while running initator and responder using n_gsm
shows all control channel commands sent by initiator are malformed
- they don't include length field (3GPP TS 07.10 ver 7.2.0, 5.4.6.1).
Add length field to transmitted control channel commands in the
gsm_control_transmit) as it is done in gsm_control_reply and expected in
gsm_dlci_command.
Signed-off-by: Andrej Krpic <a...@tnode.com>
---
drivers/tty/n_gsm.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 3c4c521..8aa90e0 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1320,12 +1320,13 @@ static void gsm_control_response(struct gsm_mux *gsm,
unsigned int command,
static void gsm_control_transmit(struct gsm_mux *gsm, struct gsm_control *ctrl)
{
- struct gsm_msg *msg = gsm_data_alloc(gsm, 0, ctrl->len + 1, gsm->ftype);
+ struct gsm_msg *msg = gsm_data_alloc(gsm, 0, ctrl->len + 2, gsm->ftype);
if (msg == NULL)
return;
- msg->data[0] = (ctrl->cmd << 1) | 2 | EA; /* command */
- memcpy(msg->data + 1, ctrl->data, ctrl->len);
+ msg->data[0] = (ctrl->cmd << 1) | 2 | EA; /* command */
+ msg->data[1] = ((ctrl->len) << 1) | EA;
+ memcpy(msg->data + 2, ctrl->data, ctrl->len);
gsm_data_queue(gsm->dlci[0], msg);
}
--
2.7.0
[PATCH 8/8] tty: n_gsm: Enable reception of frames separated with a single SOF marker
Frames may be separated with a single SOF (5.2.6.1 of 27.010 mux spec).
While transmission of a single SOF between frames is implemented in
gsm_data_kick, the reception isn't.
As a side effect, it is now possible to receive and ignore a stream of
consecutive SOFs (5.2.5 of 27.010 mux spec).
Signed-off-by: Andrej Krpic <a...@tnode.com>
---
drivers/tty/n_gsm.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index b0d9edd..12b149d 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1895,9 +1895,14 @@ static void gsm0_receive(struct gsm_mux *gsm, unsigned
char c)
}
break;
case GSM_ADDRESS: /* Address EA */
- gsm->fcs = gsm_fcs_add(gsm->fcs, c);
+ /* Ignore (not first) GSM0_SOF as it decodes into
+* reserved DLCI 62 (5.6 of the 27.010 mux spec).
+*/
+ if (c == GSM0_SOF)
+ break;
if (gsm_read_ea(>address, c))
gsm->state = GSM_CONTROL;
+ gsm->fcs = gsm_fcs_add(gsm->fcs, c);
break;
case GSM_CONTROL: /* Control Byte */
gsm->fcs = gsm_fcs_add(gsm->fcs, c);
@@ -1944,13 +1949,10 @@ static void gsm0_receive(struct gsm_mux *gsm, unsigned
char c)
case GSM_FCS: /* FCS follows the packet */
gsm->received_fcs = c;
gsm_queue(gsm);
- gsm->state = GSM_SSOF;
- break;
- case GSM_SSOF:
- if (c == GSM0_SOF) {
- gsm->state = GSM_SEARCH;
- break;
- }
+ /* Frames can be separated with a single GSM0_SOF.
+* Deal with consecutive GSM0_SOFs in GSM_ADDRESS.
+*/
+ gsm->state = GSM_SEARCH;
break;
}
}
--
2.7.0
[PATCH 6/8] tty: n_gsm: add missing length field in control channel commands
Observing debug output while running initator and responder using n_gsm
shows all control channel commands sent by initiator are malformed
- they don't include length field (3GPP TS 07.10 ver 7.2.0, 5.4.6.1).
Add length field to transmitted control channel commands in the
gsm_control_transmit) as it is done in gsm_control_reply and expected in
gsm_dlci_command.
Signed-off-by: Andrej Krpic
---
drivers/tty/n_gsm.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 3c4c521..8aa90e0 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1320,12 +1320,13 @@ static void gsm_control_response(struct gsm_mux *gsm,
unsigned int command,
static void gsm_control_transmit(struct gsm_mux *gsm, struct gsm_control *ctrl)
{
- struct gsm_msg *msg = gsm_data_alloc(gsm, 0, ctrl->len + 1, gsm->ftype);
+ struct gsm_msg *msg = gsm_data_alloc(gsm, 0, ctrl->len + 2, gsm->ftype);
if (msg == NULL)
return;
- msg->data[0] = (ctrl->cmd << 1) | 2 | EA; /* command */
- memcpy(msg->data + 1, ctrl->data, ctrl->len);
+ msg->data[0] = (ctrl->cmd << 1) | 2 | EA; /* command */
+ msg->data[1] = ((ctrl->len) << 1) | EA;
+ memcpy(msg->data + 2, ctrl->data, ctrl->len);
gsm_data_queue(gsm->dlci[0], msg);
}
--
2.7.0
[PATCH 8/8] tty: n_gsm: Enable reception of frames separated with a single SOF marker
Frames may be separated with a single SOF (5.2.6.1 of 27.010 mux spec).
While transmission of a single SOF between frames is implemented in
gsm_data_kick, the reception isn't.
As a side effect, it is now possible to receive and ignore a stream of
consecutive SOFs (5.2.5 of 27.010 mux spec).
Signed-off-by: Andrej Krpic
---
drivers/tty/n_gsm.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index b0d9edd..12b149d 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1895,9 +1895,14 @@ static void gsm0_receive(struct gsm_mux *gsm, unsigned
char c)
}
break;
case GSM_ADDRESS: /* Address EA */
- gsm->fcs = gsm_fcs_add(gsm->fcs, c);
+ /* Ignore (not first) GSM0_SOF as it decodes into
+* reserved DLCI 62 (5.6 of the 27.010 mux spec).
+*/
+ if (c == GSM0_SOF)
+ break;
if (gsm_read_ea(>address, c))
gsm->state = GSM_CONTROL;
+ gsm->fcs = gsm_fcs_add(gsm->fcs, c);
break;
case GSM_CONTROL: /* Control Byte */
gsm->fcs = gsm_fcs_add(gsm->fcs, c);
@@ -1944,13 +1949,10 @@ static void gsm0_receive(struct gsm_mux *gsm, unsigned
char c)
case GSM_FCS: /* FCS follows the packet */
gsm->received_fcs = c;
gsm_queue(gsm);
- gsm->state = GSM_SSOF;
- break;
- case GSM_SSOF:
- if (c == GSM0_SOF) {
- gsm->state = GSM_SEARCH;
- break;
- }
+ /* Frames can be separated with a single GSM0_SOF.
+* Deal with consecutive GSM0_SOFs in GSM_ADDRESS.
+*/
+ gsm->state = GSM_SEARCH;
break;
}
}
--
2.7.0
[PATCH 0/8] tty: n_gsm: Make mux work as a responder station
When using n_gsm you have to explicitly set it to work as a initiator station. This led me to believe that it can also work as a responder. In a use case where I needed responder station mux I came to the conclusion that it actually does not work. Second and third patch fix dealings with frame C/R bit that then enable mux to work as a responder station. Next patches in the series then fix bugs that were found after two instances of n_gsm connected over null-modem cable were used. First patch fixes formatting errors, such as space before comma, and most of the warnings reported by the checkpatch script. Andrej Krpic (8): tty: n_gsm: fix formatting errors tty: n_gsm: fix C/R bit when sending as a responder tty: n_gsm: make mux work as a responder station tty: n_gsm: send DM response when accessing an invalid channel tty: n_gsm: replace dead code with a meaningful comment tty: n_gsm: add missing length field in control channel commands tty: n_gsm: properly format Modem Status Command message tty: n_gsm: Enable reception of frames separated with a single SOF marker drivers/tty/n_gsm.c | 191 +--- 1 file changed, 123 insertions(+), 68 deletions(-) -- 2.7.0
[PATCH 0/8] tty: n_gsm: Make mux work as a responder station
When using n_gsm you have to explicitly set it to work as a initiator station. This led me to believe that it can also work as a responder. In a use case where I needed responder station mux I came to the conclusion that it actually does not work. Second and third patch fix dealings with frame C/R bit that then enable mux to work as a responder station. Next patches in the series then fix bugs that were found after two instances of n_gsm connected over null-modem cable were used. First patch fixes formatting errors, such as space before comma, and most of the warnings reported by the checkpatch script. Andrej Krpic (8): tty: n_gsm: fix formatting errors tty: n_gsm: fix C/R bit when sending as a responder tty: n_gsm: make mux work as a responder station tty: n_gsm: send DM response when accessing an invalid channel tty: n_gsm: replace dead code with a meaningful comment tty: n_gsm: add missing length field in control channel commands tty: n_gsm: properly format Modem Status Command message tty: n_gsm: Enable reception of frames separated with a single SOF marker drivers/tty/n_gsm.c | 191 +--- 1 file changed, 123 insertions(+), 68 deletions(-) -- 2.7.0
[PATCH 5/8] tty: n_gsm: replace dead code with a meaningful comment
UI/UIH frame can be received as a command from other station or
as a response to a command we issued earlier.
Add this knowledge to the source code as a comment and remove
useless #if 0/#endif block.
Signed-off-by: Andrej Krpic <a...@tnode.com>
---
drivers/tty/n_gsm.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 8551fa4..3c4c521 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1852,10 +1852,11 @@ static void gsm_queue(struct gsm_mux *gsm)
case UI|PF:
case UIH:
case UIH|PF:
-#if 0
- if (cr)
- goto invalid;
-#endif
+ /* Don't reject frames based on cr value as UI/UIH
+* frame can be received as a command from other
+* station or as a response to a command we issued
+* earlier.
+*/
if (dlci == NULL || dlci->state != DLCI_OPEN) {
gsm_response(gsm, address, DM|PF);
return;
--
2.7.0
[PATCH 5/8] tty: n_gsm: replace dead code with a meaningful comment
UI/UIH frame can be received as a command from other station or
as a response to a command we issued earlier.
Add this knowledge to the source code as a comment and remove
useless #if 0/#endif block.
Signed-off-by: Andrej Krpic
---
drivers/tty/n_gsm.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 8551fa4..3c4c521 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1852,10 +1852,11 @@ static void gsm_queue(struct gsm_mux *gsm)
case UI|PF:
case UIH:
case UIH|PF:
-#if 0
- if (cr)
- goto invalid;
-#endif
+ /* Don't reject frames based on cr value as UI/UIH
+* frame can be received as a command from other
+* station or as a response to a command we issued
+* earlier.
+*/
if (dlci == NULL || dlci->state != DLCI_OPEN) {
gsm_response(gsm, address, DM|PF);
return;
--
2.7.0
[PATCH 4/8] tty: n_gsm: send DM response when accessing an invalid channel
Change C/R bit in a response to a UI/UIH frame sent to
non-existing/closed channel. As DM frame type is only valid as a
response, it should be sent using gsm_response function.
Signed-off-by: Andrej Krpic <a...@tnode.com>
---
drivers/tty/n_gsm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 05b562d..8551fa4 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1857,7 +1857,7 @@ static void gsm_queue(struct gsm_mux *gsm)
goto invalid;
#endif
if (dlci == NULL || dlci->state != DLCI_OPEN) {
- gsm_command(gsm, address, DM|PF);
+ gsm_response(gsm, address, DM|PF);
return;
}
dlci->data(dlci, gsm->buf, gsm->len);
--
2.7.0
[PATCH 4/8] tty: n_gsm: send DM response when accessing an invalid channel
Change C/R bit in a response to a UI/UIH frame sent to
non-existing/closed channel. As DM frame type is only valid as a
response, it should be sent using gsm_response function.
Signed-off-by: Andrej Krpic
---
drivers/tty/n_gsm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 05b562d..8551fa4 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1857,7 +1857,7 @@ static void gsm_queue(struct gsm_mux *gsm)
goto invalid;
#endif
if (dlci == NULL || dlci->state != DLCI_OPEN) {
- gsm_command(gsm, address, DM|PF);
+ gsm_response(gsm, address, DM|PF);
return;
}
dlci->data(dlci, gsm->buf, gsm->len);
--
2.7.0
[PATCH 1/8] tty: n_gsm: fix formatting errors
Minor formatting changes to remove errors and reduce number of
warnings produced by checkpatch.pl script.
Signed-off-by: Andrej Krpic <a...@tnode.com>
---
drivers/tty/n_gsm.c | 138
1 file changed, 95 insertions(+), 43 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c016207..cc3b374 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -431,6 +431,7 @@ static int gsm_read_ea(unsigned int *val, u8 c)
static u8 gsm_encode_modem(const struct gsm_dlci *dlci)
{
u8 modembits = 0;
+
/* FC is true flow control not modem bits */
if (dlci->throttled)
modembits |= MDM_FC;
@@ -489,7 +490,8 @@ static void gsm_print_packet(const char *hdr, int addr, int
cr,
if (!(control & 0x01)) {
pr_cont("I N(S)%d N(R)%d",
(control & 0x0E) >> 1, (control & 0xE0) >> 5);
- } else switch (control & 0x0F) {
+ } else
+ switch (control & 0x0F) {
case RR:
pr_cont("RR(%d)", (control & 0xE0) >> 5);
break;
@@ -511,6 +513,7 @@ static void gsm_print_packet(const char *hdr, int addr, int
cr,
if (dlen) {
int ct = 0;
+
while (dlen--) {
if (ct % 8 == 0) {
pr_cont("\n");
@@ -542,6 +545,7 @@ static void gsm_print_packet(const char *hdr, int addr, int
cr,
static int gsm_stuff_frame(const u8 *input, u8 *output, int len)
{
int olen = 0;
+
while (len--) {
if (*input == GSM1_SOF || *input == GSM1_ESCAPE
|| *input == XON || *input == XOFF) {
@@ -695,7 +699,7 @@ static void gsm_data_kick(struct gsm_mux *gsm)
len += 2;
} else {
gsm->txframe[0] = GSM0_SOF;
- memcpy(gsm->txframe + 1 , msg->data, msg->len);
+ memcpy(gsm->txframe + 1, msg->data, msg->len);
gsm->txframe[msg->len + 1] = GSM0_SOF;
len = msg->len + 2;
}
@@ -711,7 +715,8 @@ static void gsm_data_kick(struct gsm_mux *gsm)
/* 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 */
+* burst
+*/
skip_sof = 1;
list_del(>list);
@@ -750,7 +755,7 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct
gsm_msg *msg)
*--dp = (msg->addr << 2) | 2 | EA;
else
*--dp = (msg->addr << 2) | EA;
- *fcs = gsm_fcs_add_block(INIT_FCS, dp , msg->data - dp);
+ *fcs = gsm_fcs_add_block(INIT_FCS, dp, msg->data - dp);
/* Ugly protocol layering violation */
if (msg->ctrl == UI || msg->ctrl == (UI|PF))
*fcs = gsm_fcs_add_block(*fcs, msg->data, msg->len);
@@ -760,7 +765,8 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct
gsm_msg *msg)
msg->data, msg->len);
/* Move the header back and adjust the length, also allow for the FCS
- now tacked on the end */
+* now tacked on the end
+*/
msg->len += (msg->data - dp) + 1;
msg->data = dp;
@@ -783,6 +789,7 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct
gsm_msg *msg)
static void gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg)
{
unsigned long flags;
+
spin_lock_irqsave(>gsm->tx_lock, flags);
__gsm_data_queue(dlci, msg);
spin_unlock_irqrestore(>gsm->tx_lock, flags);
@@ -821,7 +828,8 @@ static int gsm_dlci_data_output(struct gsm_mux *gsm, struct
gsm_dlci *dlci)
msg = gsm_data_alloc(gsm, dlci->addr, size, gsm->ftype);
/* FIXME: need a timer or something to kick this so it can't
- get stuck with no work outstanding and no buffer free */
+* get stuck with no work outstanding and no buffer free
+*/
if (msg == NULL)
return -ENOMEM;
dp = msg->data;
@@ -829,11 +837,13 @@ static int gsm_dlci_data_output(struct gsm_mux *gsm,
struct gsm_dlci *dlci)
case 1: /* Unstructured */
break;
case 2: /* Unstructed with modem bits.
- Always one byte as we never send inline break data */
+* Always one byte as we never send inline break data
+
[PATCH 1/8] tty: n_gsm: fix formatting errors
Minor formatting changes to remove errors and reduce number of
warnings produced by checkpatch.pl script.
Signed-off-by: Andrej Krpic
---
drivers/tty/n_gsm.c | 138
1 file changed, 95 insertions(+), 43 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c016207..cc3b374 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -431,6 +431,7 @@ static int gsm_read_ea(unsigned int *val, u8 c)
static u8 gsm_encode_modem(const struct gsm_dlci *dlci)
{
u8 modembits = 0;
+
/* FC is true flow control not modem bits */
if (dlci->throttled)
modembits |= MDM_FC;
@@ -489,7 +490,8 @@ static void gsm_print_packet(const char *hdr, int addr, int
cr,
if (!(control & 0x01)) {
pr_cont("I N(S)%d N(R)%d",
(control & 0x0E) >> 1, (control & 0xE0) >> 5);
- } else switch (control & 0x0F) {
+ } else
+ switch (control & 0x0F) {
case RR:
pr_cont("RR(%d)", (control & 0xE0) >> 5);
break;
@@ -511,6 +513,7 @@ static void gsm_print_packet(const char *hdr, int addr, int
cr,
if (dlen) {
int ct = 0;
+
while (dlen--) {
if (ct % 8 == 0) {
pr_cont("\n");
@@ -542,6 +545,7 @@ static void gsm_print_packet(const char *hdr, int addr, int
cr,
static int gsm_stuff_frame(const u8 *input, u8 *output, int len)
{
int olen = 0;
+
while (len--) {
if (*input == GSM1_SOF || *input == GSM1_ESCAPE
|| *input == XON || *input == XOFF) {
@@ -695,7 +699,7 @@ static void gsm_data_kick(struct gsm_mux *gsm)
len += 2;
} else {
gsm->txframe[0] = GSM0_SOF;
- memcpy(gsm->txframe + 1 , msg->data, msg->len);
+ memcpy(gsm->txframe + 1, msg->data, msg->len);
gsm->txframe[msg->len + 1] = GSM0_SOF;
len = msg->len + 2;
}
@@ -711,7 +715,8 @@ static void gsm_data_kick(struct gsm_mux *gsm)
/* 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 */
+* burst
+*/
skip_sof = 1;
list_del(>list);
@@ -750,7 +755,7 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct
gsm_msg *msg)
*--dp = (msg->addr << 2) | 2 | EA;
else
*--dp = (msg->addr << 2) | EA;
- *fcs = gsm_fcs_add_block(INIT_FCS, dp , msg->data - dp);
+ *fcs = gsm_fcs_add_block(INIT_FCS, dp, msg->data - dp);
/* Ugly protocol layering violation */
if (msg->ctrl == UI || msg->ctrl == (UI|PF))
*fcs = gsm_fcs_add_block(*fcs, msg->data, msg->len);
@@ -760,7 +765,8 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct
gsm_msg *msg)
msg->data, msg->len);
/* Move the header back and adjust the length, also allow for the FCS
- now tacked on the end */
+* now tacked on the end
+*/
msg->len += (msg->data - dp) + 1;
msg->data = dp;
@@ -783,6 +789,7 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct
gsm_msg *msg)
static void gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg)
{
unsigned long flags;
+
spin_lock_irqsave(>gsm->tx_lock, flags);
__gsm_data_queue(dlci, msg);
spin_unlock_irqrestore(>gsm->tx_lock, flags);
@@ -821,7 +828,8 @@ static int gsm_dlci_data_output(struct gsm_mux *gsm, struct
gsm_dlci *dlci)
msg = gsm_data_alloc(gsm, dlci->addr, size, gsm->ftype);
/* FIXME: need a timer or something to kick this so it can't
- get stuck with no work outstanding and no buffer free */
+* get stuck with no work outstanding and no buffer free
+*/
if (msg == NULL)
return -ENOMEM;
dp = msg->data;
@@ -829,11 +837,13 @@ static int gsm_dlci_data_output(struct gsm_mux *gsm,
struct gsm_dlci *dlci)
case 1: /* Unstructured */
break;
case 2: /* Unstructed with modem bits.
- Always one byte as we never send inline break data */
+* Always one byte as we never send inline break data
+
[PATCH 0/8] tty: n_gsm: Make mux work as a responder station
When using n_gsm you have to explicitly set it to work as a initiator station. This led me to believe that it can also work as a responder. In a use case where I needed responder station mux I came to the conclusion that it actually does not work. Second and third patch fix dealings with frame C/R bit that then enable mux to work as a responder station. Next patches in the series then fix bugs that were found after two instances of n_gsm connected over null-modem cable were used. First patch fixes formatting errors, such as space before comma, and most of the warnings reported by the checkpatch script. Andrej Krpic (8): tty: n_gsm: fix formatting errors tty: n_gsm: fix C/R bit when sending as a responder tty: n_gsm: make mux work as a responder station tty: n_gsm: send DM response when accessing an invalid channel tty: n_gsm: replace dead code with a meaningful comment tty: n_gsm: add missing length field in control channel commands tty: n_gsm: properly format Modem Status Command message tty: n_gsm: Enable reception of frames separated with a single SOF marker drivers/tty/n_gsm.c | 191 +--- 1 file changed, 123 insertions(+), 68 deletions(-) -- 2.7.0
[PATCH 0/8] tty: n_gsm: Make mux work as a responder station
When using n_gsm you have to explicitly set it to work as a initiator station. This led me to believe that it can also work as a responder. In a use case where I needed responder station mux I came to the conclusion that it actually does not work. Second and third patch fix dealings with frame C/R bit that then enable mux to work as a responder station. Next patches in the series then fix bugs that were found after two instances of n_gsm connected over null-modem cable were used. First patch fixes formatting errors, such as space before comma, and most of the warnings reported by the checkpatch script. Andrej Krpic (8): tty: n_gsm: fix formatting errors tty: n_gsm: fix C/R bit when sending as a responder tty: n_gsm: make mux work as a responder station tty: n_gsm: send DM response when accessing an invalid channel tty: n_gsm: replace dead code with a meaningful comment tty: n_gsm: add missing length field in control channel commands tty: n_gsm: properly format Modem Status Command message tty: n_gsm: Enable reception of frames separated with a single SOF marker drivers/tty/n_gsm.c | 191 +--- 1 file changed, 123 insertions(+), 68 deletions(-) -- 2.7.0
[PATCH 6/8] tty: n_gsm: add missing length field in control channel commands
Observing debug output while running initator and responder using n_gsm
shows all control channel commands sent by initiator are malformed
- they don't include length field (3GPP TS 07.10 ver 7.2.0, 5.4.6.1).
Add length field to transmitted control channel commands in the
gsm_control_transmit) as it is done in gsm_control_reply and expected in
gsm_dlci_command.
Signed-off-by: Andrej Krpic <a...@tnode.com>
---
drivers/tty/n_gsm.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 3c4c521..8aa90e0 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1320,12 +1320,13 @@ static void gsm_control_response(struct gsm_mux *gsm,
unsigned int command,
static void gsm_control_transmit(struct gsm_mux *gsm, struct gsm_control *ctrl)
{
- struct gsm_msg *msg = gsm_data_alloc(gsm, 0, ctrl->len + 1, gsm->ftype);
+ struct gsm_msg *msg = gsm_data_alloc(gsm, 0, ctrl->len + 2, gsm->ftype);
if (msg == NULL)
return;
- msg->data[0] = (ctrl->cmd << 1) | 2 | EA; /* command */
- memcpy(msg->data + 1, ctrl->data, ctrl->len);
+ msg->data[0] = (ctrl->cmd << 1) | 2 | EA; /* command */
+ msg->data[1] = ((ctrl->len) << 1) | EA;
+ memcpy(msg->data + 2, ctrl->data, ctrl->len);
gsm_data_queue(gsm->dlci[0], msg);
}
--
2.7.0
[PATCH 3/8] tty: n_gsm: make mux work as a responder station
Comment suggests that cr == 1 represents a received command and cr == 0
a received response. Received frames are then filtered:
- correctly by rejection of SABM and DISC responses, they are
command only frame types and
- incorrectly by rejection of UA (a response only frame type) responses.
Mux as a initiator successfully establishes DLC by receiving UA response
frame to a previously sent open channel command (SABM). Incorrect
equation (eqA) makes UA "reject cr == 0 commands" case correct, but
filters out all received SABM and DISC command frames.
Change eqA to eqB to match the intent and fix filtering of UA frames.
This enables reception of SABM and DISC frames and consequently
makes mux work as a responder station.
receivedreceiving as eqA eqB 3GPP TS 27.010
CR bit initiator (ir)cr=CR^(1-ir) cr=CR^ir5.2.1.2
0 0 10 0 (response)
1 0 _\ 01 1 (command)
0 1 / 01 1 (command)
1 1 10 0 (response)
Signed-off-by: Andrej Krpic <a...@tnode.com>
---
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 a0fb92c..05b562d 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1798,7 +1798,7 @@ static void gsm_queue(struct gsm_mux *gsm)
gsm_print_packet("<--", address, cr, gsm->control, gsm->buf, gsm->len);
- cr ^= 1 - gsm->initiator; /* Flip so 1 always means command */
+ cr ^= gsm->initiator; /* Flip so 1 always means command */
dlci = gsm->dlci[address];
switch (gsm->control) {
@@ -1829,7 +1829,7 @@ static void gsm_queue(struct gsm_mux *gsm)
break;
case UA:
case UA|PF:
- if (cr == 0 || dlci == NULL)
+ if (cr || dlci == NULL)
break;
switch (dlci->state) {
case DLCI_CLOSING:
--
2.7.0
[PATCH 6/8] tty: n_gsm: add missing length field in control channel commands
Observing debug output while running initator and responder using n_gsm
shows all control channel commands sent by initiator are malformed
- they don't include length field (3GPP TS 07.10 ver 7.2.0, 5.4.6.1).
Add length field to transmitted control channel commands in the
gsm_control_transmit) as it is done in gsm_control_reply and expected in
gsm_dlci_command.
Signed-off-by: Andrej Krpic
---
drivers/tty/n_gsm.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 3c4c521..8aa90e0 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1320,12 +1320,13 @@ static void gsm_control_response(struct gsm_mux *gsm,
unsigned int command,
static void gsm_control_transmit(struct gsm_mux *gsm, struct gsm_control *ctrl)
{
- struct gsm_msg *msg = gsm_data_alloc(gsm, 0, ctrl->len + 1, gsm->ftype);
+ struct gsm_msg *msg = gsm_data_alloc(gsm, 0, ctrl->len + 2, gsm->ftype);
if (msg == NULL)
return;
- msg->data[0] = (ctrl->cmd << 1) | 2 | EA; /* command */
- memcpy(msg->data + 1, ctrl->data, ctrl->len);
+ msg->data[0] = (ctrl->cmd << 1) | 2 | EA; /* command */
+ msg->data[1] = ((ctrl->len) << 1) | EA;
+ memcpy(msg->data + 2, ctrl->data, ctrl->len);
gsm_data_queue(gsm->dlci[0], msg);
}
--
2.7.0
[PATCH 3/8] tty: n_gsm: make mux work as a responder station
Comment suggests that cr == 1 represents a received command and cr == 0
a received response. Received frames are then filtered:
- correctly by rejection of SABM and DISC responses, they are
command only frame types and
- incorrectly by rejection of UA (a response only frame type) responses.
Mux as a initiator successfully establishes DLC by receiving UA response
frame to a previously sent open channel command (SABM). Incorrect
equation (eqA) makes UA "reject cr == 0 commands" case correct, but
filters out all received SABM and DISC command frames.
Change eqA to eqB to match the intent and fix filtering of UA frames.
This enables reception of SABM and DISC frames and consequently
makes mux work as a responder station.
receivedreceiving as eqA eqB 3GPP TS 27.010
CR bit initiator (ir)cr=CR^(1-ir) cr=CR^ir5.2.1.2
0 0 10 0 (response)
1 0 _\ 01 1 (command)
0 1 / 01 1 (command)
1 1 10 0 (response)
Signed-off-by: Andrej Krpic
---
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 a0fb92c..05b562d 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1798,7 +1798,7 @@ static void gsm_queue(struct gsm_mux *gsm)
gsm_print_packet("<--", address, cr, gsm->control, gsm->buf, gsm->len);
- cr ^= 1 - gsm->initiator; /* Flip so 1 always means command */
+ cr ^= gsm->initiator; /* Flip so 1 always means command */
dlci = gsm->dlci[address];
switch (gsm->control) {
@@ -1829,7 +1829,7 @@ static void gsm_queue(struct gsm_mux *gsm)
break;
case UA:
case UA|PF:
- if (cr == 0 || dlci == NULL)
+ if (cr || dlci == NULL)
break;
switch (dlci->state) {
case DLCI_CLOSING:
--
2.7.0
[PATCH 2/8] tty: n_gsm: fix C/R bit when sending as a responder
According to the specification (3GPP TS 27.010 v12.0.0 R1, 5.2.1.2),
C/R bit must be the same for corresponding command and response.
If mux is an initiator (station that initiates DLC 0), valid sent
commands and received responses must have C/R bit set to 1.
For a station that is a responder, valid sent commands and received
responses must have C/R bit set to 0.
Change the value of C/R bit in command and response frames to
depend on whether the station is initator or not.
Signed-off-by: Andrej Krpic <a...@tnode.com>
---
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 cc3b374..a0fb92c 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -622,7 +622,7 @@ static void gsm_send(struct gsm_mux *gsm, int addr, int cr,
int control)
static inline void gsm_response(struct gsm_mux *gsm, int addr, int control)
{
- gsm_send(gsm, addr, 0, control);
+ gsm_send(gsm, addr, gsm->initiator ? 0 : 1, control);
}
/**
@@ -636,7 +636,7 @@ static inline void gsm_response(struct gsm_mux *gsm, int
addr, int control)
static inline void gsm_command(struct gsm_mux *gsm, int addr, int control)
{
- gsm_send(gsm, addr, 1, control);
+ gsm_send(gsm, addr, gsm->initiator ? 1 : 0, control);
}
/* Data transmission */
--
2.7.0
[PATCH 7/8] tty: n_gsm: properly format Modem Status Command message
Change format of Modem Status Command (MSC) message that is sent to the one expected in the receive function gsm_control_modem and specified in 3GPP TS 27.010 version 12.0.0 Release 12, 5.4.6.3.7. Wrongly formatted MSC causes DLC to be marked as constipated. A bug appears after format of transmitted control messages is fixed and control messages start to be recognized. Signed-off-by: Andrej Krpic <a...@tnode.com> --- drivers/tty/n_gsm.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 8aa90e0..b0d9edd 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -2874,12 +2874,11 @@ static int gsmtty_modem_update(struct gsm_dlci *dlci, u8 brk) if (brk) len++; - modembits[0] = len << 1 | EA; /* Data bytes */ - modembits[1] = dlci->addr << 2 | 3; /* DLCI, EA, 1 */ - modembits[2] = gsm_encode_modem(dlci) << 1 | EA; + modembits[0] = dlci->addr << 2 | 3; /* DLCI, EA, 1 */ + modembits[1] = gsm_encode_modem(dlci) << 1 | EA; if (brk) - modembits[3] = brk << 4 | 2 | EA; /* Valid, EA */ - ctrl = gsm_control_send(dlci->gsm, CMD_MSC, modembits, len + 1); + modembits[2] = brk << 4 | 2 | EA; /* Valid, EA */ + ctrl = gsm_control_send(dlci->gsm, CMD_MSC, modembits, len); if (ctrl == NULL) return -ENOMEM; return gsm_control_wait(dlci->gsm, ctrl); -- 2.7.0
[PATCH 4/8] tty: n_gsm: send DM response when accessing an invalid channel
Change C/R bit in a response to a UI/UIH frame sent to
non-existing/closed channel. As DM frame type is only valid as a
response, it should be sent using gsm_response function.
Signed-off-by: Andrej Krpic <a...@tnode.com>
---
drivers/tty/n_gsm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 05b562d..8551fa4 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1857,7 +1857,7 @@ static void gsm_queue(struct gsm_mux *gsm)
goto invalid;
#endif
if (dlci == NULL || dlci->state != DLCI_OPEN) {
- gsm_command(gsm, address, DM|PF);
+ gsm_response(gsm, address, DM|PF);
return;
}
dlci->data(dlci, gsm->buf, gsm->len);
--
2.7.0
[PATCH 2/8] tty: n_gsm: fix C/R bit when sending as a responder
According to the specification (3GPP TS 27.010 v12.0.0 R1, 5.2.1.2),
C/R bit must be the same for corresponding command and response.
If mux is an initiator (station that initiates DLC 0), valid sent
commands and received responses must have C/R bit set to 1.
For a station that is a responder, valid sent commands and received
responses must have C/R bit set to 0.
Change the value of C/R bit in command and response frames to
depend on whether the station is initator or not.
Signed-off-by: Andrej Krpic
---
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 cc3b374..a0fb92c 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -622,7 +622,7 @@ static void gsm_send(struct gsm_mux *gsm, int addr, int cr,
int control)
static inline void gsm_response(struct gsm_mux *gsm, int addr, int control)
{
- gsm_send(gsm, addr, 0, control);
+ gsm_send(gsm, addr, gsm->initiator ? 0 : 1, control);
}
/**
@@ -636,7 +636,7 @@ static inline void gsm_response(struct gsm_mux *gsm, int
addr, int control)
static inline void gsm_command(struct gsm_mux *gsm, int addr, int control)
{
- gsm_send(gsm, addr, 1, control);
+ gsm_send(gsm, addr, gsm->initiator ? 1 : 0, control);
}
/* Data transmission */
--
2.7.0
[PATCH 7/8] tty: n_gsm: properly format Modem Status Command message
Change format of Modem Status Command (MSC) message that is sent to the one expected in the receive function gsm_control_modem and specified in 3GPP TS 27.010 version 12.0.0 Release 12, 5.4.6.3.7. Wrongly formatted MSC causes DLC to be marked as constipated. A bug appears after format of transmitted control messages is fixed and control messages start to be recognized. Signed-off-by: Andrej Krpic --- drivers/tty/n_gsm.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 8aa90e0..b0d9edd 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -2874,12 +2874,11 @@ static int gsmtty_modem_update(struct gsm_dlci *dlci, u8 brk) if (brk) len++; - modembits[0] = len << 1 | EA; /* Data bytes */ - modembits[1] = dlci->addr << 2 | 3; /* DLCI, EA, 1 */ - modembits[2] = gsm_encode_modem(dlci) << 1 | EA; + modembits[0] = dlci->addr << 2 | 3; /* DLCI, EA, 1 */ + modembits[1] = gsm_encode_modem(dlci) << 1 | EA; if (brk) - modembits[3] = brk << 4 | 2 | EA; /* Valid, EA */ - ctrl = gsm_control_send(dlci->gsm, CMD_MSC, modembits, len + 1); + modembits[2] = brk << 4 | 2 | EA; /* Valid, EA */ + ctrl = gsm_control_send(dlci->gsm, CMD_MSC, modembits, len); if (ctrl == NULL) return -ENOMEM; return gsm_control_wait(dlci->gsm, ctrl); -- 2.7.0
[PATCH 4/8] tty: n_gsm: send DM response when accessing an invalid channel
Change C/R bit in a response to a UI/UIH frame sent to
non-existing/closed channel. As DM frame type is only valid as a
response, it should be sent using gsm_response function.
Signed-off-by: Andrej Krpic
---
drivers/tty/n_gsm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 05b562d..8551fa4 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1857,7 +1857,7 @@ static void gsm_queue(struct gsm_mux *gsm)
goto invalid;
#endif
if (dlci == NULL || dlci->state != DLCI_OPEN) {
- gsm_command(gsm, address, DM|PF);
+ gsm_response(gsm, address, DM|PF);
return;
}
dlci->data(dlci, gsm->buf, gsm->len);
--
2.7.0
[PATCH 8/8] tty: n_gsm: Enable reception of frames separated with a single SOF marker
Frames may be separated with a single SOF (5.2.6.1 of 27.010 mux spec).
While transmission of a single SOF between frames is implemented in
gsm_data_kick, the reception isn't.
As a side effect, it is now possible to receive and ignore a stream of
consecutive SOFs (5.2.5 of 27.010 mux spec).
Signed-off-by: Andrej Krpic <a...@tnode.com>
---
drivers/tty/n_gsm.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index b0d9edd..12b149d 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1895,9 +1895,14 @@ static void gsm0_receive(struct gsm_mux *gsm, unsigned
char c)
}
break;
case GSM_ADDRESS: /* Address EA */
- gsm->fcs = gsm_fcs_add(gsm->fcs, c);
+ /* Ignore (not first) GSM0_SOF as it decodes into
+* reserved DLCI 62 (5.6 of the 27.010 mux spec).
+*/
+ if (c == GSM0_SOF)
+ break;
if (gsm_read_ea(>address, c))
gsm->state = GSM_CONTROL;
+ gsm->fcs = gsm_fcs_add(gsm->fcs, c);
break;
case GSM_CONTROL: /* Control Byte */
gsm->fcs = gsm_fcs_add(gsm->fcs, c);
@@ -1944,13 +1949,10 @@ static void gsm0_receive(struct gsm_mux *gsm, unsigned
char c)
case GSM_FCS: /* FCS follows the packet */
gsm->received_fcs = c;
gsm_queue(gsm);
- gsm->state = GSM_SSOF;
- break;
- case GSM_SSOF:
- if (c == GSM0_SOF) {
- gsm->state = GSM_SEARCH;
- break;
- }
+ /* Frames can be separated with a single GSM0_SOF.
+* Deal with consecutive GSM0_SOFs in GSM_ADDRESS.
+*/
+ gsm->state = GSM_SEARCH;
break;
}
}
--
2.7.0
[PATCH 5/8] tty: n_gsm: replace dead code with a meaningful comment
UI/UIH frame can be received as a command from other station or
as a response to a command we issued earlier.
Add this knowledge to the source code as a comment and remove
useless #if 0/#endif block.
Signed-off-by: Andrej Krpic <a...@tnode.com>
---
drivers/tty/n_gsm.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 8551fa4..3c4c521 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1852,10 +1852,11 @@ static void gsm_queue(struct gsm_mux *gsm)
case UI|PF:
case UIH:
case UIH|PF:
-#if 0
- if (cr)
- goto invalid;
-#endif
+ /* Don't reject frames based on cr value as UI/UIH
+* frame can be received as a command from other
+* station or as a response to a command we issued
+* earlier.
+*/
if (dlci == NULL || dlci->state != DLCI_OPEN) {
gsm_response(gsm, address, DM|PF);
return;
--
2.7.0
[PATCH 8/8] tty: n_gsm: Enable reception of frames separated with a single SOF marker
Frames may be separated with a single SOF (5.2.6.1 of 27.010 mux spec).
While transmission of a single SOF between frames is implemented in
gsm_data_kick, the reception isn't.
As a side effect, it is now possible to receive and ignore a stream of
consecutive SOFs (5.2.5 of 27.010 mux spec).
Signed-off-by: Andrej Krpic
---
drivers/tty/n_gsm.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index b0d9edd..12b149d 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1895,9 +1895,14 @@ static void gsm0_receive(struct gsm_mux *gsm, unsigned
char c)
}
break;
case GSM_ADDRESS: /* Address EA */
- gsm->fcs = gsm_fcs_add(gsm->fcs, c);
+ /* Ignore (not first) GSM0_SOF as it decodes into
+* reserved DLCI 62 (5.6 of the 27.010 mux spec).
+*/
+ if (c == GSM0_SOF)
+ break;
if (gsm_read_ea(>address, c))
gsm->state = GSM_CONTROL;
+ gsm->fcs = gsm_fcs_add(gsm->fcs, c);
break;
case GSM_CONTROL: /* Control Byte */
gsm->fcs = gsm_fcs_add(gsm->fcs, c);
@@ -1944,13 +1949,10 @@ static void gsm0_receive(struct gsm_mux *gsm, unsigned
char c)
case GSM_FCS: /* FCS follows the packet */
gsm->received_fcs = c;
gsm_queue(gsm);
- gsm->state = GSM_SSOF;
- break;
- case GSM_SSOF:
- if (c == GSM0_SOF) {
- gsm->state = GSM_SEARCH;
- break;
- }
+ /* Frames can be separated with a single GSM0_SOF.
+* Deal with consecutive GSM0_SOFs in GSM_ADDRESS.
+*/
+ gsm->state = GSM_SEARCH;
break;
}
}
--
2.7.0
[PATCH 5/8] tty: n_gsm: replace dead code with a meaningful comment
UI/UIH frame can be received as a command from other station or
as a response to a command we issued earlier.
Add this knowledge to the source code as a comment and remove
useless #if 0/#endif block.
Signed-off-by: Andrej Krpic
---
drivers/tty/n_gsm.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 8551fa4..3c4c521 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -1852,10 +1852,11 @@ static void gsm_queue(struct gsm_mux *gsm)
case UI|PF:
case UIH:
case UIH|PF:
-#if 0
- if (cr)
- goto invalid;
-#endif
+ /* Don't reject frames based on cr value as UI/UIH
+* frame can be received as a command from other
+* station or as a response to a command we issued
+* earlier.
+*/
if (dlci == NULL || dlci->state != DLCI_OPEN) {
gsm_response(gsm, address, DM|PF);
return;
--
2.7.0
