These alterations allow the usage of the non-transparent mode of the gsm
mux, it works well enough to establish a ppp session using pppd. The
increased buffer size allows the usage of the default MRU and MTU sizes
in pppd.
Tested with a Telit GE863-Pro^3.
This is my first attempt at a patch so I think it is rather crude,
suggestions are welcomed :)
There is a problem that happens if the process that is holding the
attached line discipline tries to detach it before a process using a
virtual com manages to close it. Both processes end up dealocked. I
think this has to do with the tty lock. I don't have the backtrace with
me
Signed-off-by: Mário Isidoro <[email protected]>
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index aa2e5d3..c244292 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -77,8 +77,8 @@ module_param(debug, int, 0600);
* Semi-arbitary buffer size limits. 0710 is normally run with 32-64
byte
* limits so this is plenty
*/
-#define MAX_MRU 512
-#define MAX_MTU 512
+#define MAX_MRU 2048
+#define MAX_MTU 2048
/*
* Each block of data we have queued to go out is in the form of
@@ -417,6 +417,30 @@ static u8 gsm_encode_modem(const struct gsm_dlci
*dlci)
}
/**
+ * gsm_print_modem - Print modem control lines
+ * @dlci: DLCI to encode from
+ *
+ * Print the status of the modem control lines
+ */
+
+static void gsm_print_modem(int mlines)
+{
+ pr_debug("Modem lines: %04X\n", mlines);
+ pr_debug(" RTS:\t%s\n",
+ ((mlines & TIOCM_RTS) == TIOCM_RTS) ? ("ON") : ("OFF"));
+ pr_debug(" CTS:\t%s\n",
+ ((mlines & TIOCM_CTS) == TIOCM_CTS) ? ("ON") : ("OFF"));
+ pr_debug(" DSR:\t%s\n",
+ ((mlines & TIOCM_DSR) == TIOCM_DSR) ? ("ON") : ("OFF"));
+ pr_debug(" DTR:\t%s\n",
+ ((mlines & TIOCM_DTR) == TIOCM_DTR) ? ("ON") : ("OFF"));
+ pr_debug(" DCD:\t%s\n",
+ ((mlines & TIOCM_CD) == TIOCM_CD) ? ("ON") : ("OFF"));
+ pr_debug(" RI :\t%s\n",
+ ((mlines & TIOCM_RI) == TIOCM_RI) ? ("ON") : ("OFF"));
+}
+
+/**
* gsm_print_packet - display a frame for debug
* @hdr: header to print before decode
* @addr: address EA from the frame
@@ -588,7 +612,7 @@ static void gsm_send(struct gsm_mux *gsm, int addr,
int cr, int control)
return;
}
gsm->output(gsm, cbuf, len);
- gsm_print_packet("-->", addr, cr, control, NULL, 0);
+ gsm_print_packet("->-", addr, cr, control, NULL, 0);
}
/**
@@ -999,22 +1023,25 @@ static void gsm_control_reply(struct gsm_mux
*gsm, int cmd, u8 *data,
static void gsm_process_modem(struct tty_struct *tty, struct gsm_dlci
*dlci,
u32 modem)
{
- int mlines = 0;
+ int mlines = dlci->modem_tx;
u8 brk = modem >> 6;
+ modem = modem >> 1 ;
+
/* Flow control/ready to communicate */
if (modem & MDM_FC) {
+ pr_debug("Flux throttled\n");
/* Need to throttle our output on this device */
dlci->constipated = 1;
}
if (modem & MDM_RTC) {
- mlines |= TIOCM_DSR | TIOCM_DTR;
+ mlines |= TIOCM_DSR ;
dlci->constipated = 0;
gsm_dlci_data_kick(dlci);
}
/* Map modem bits */
if (modem & MDM_RTR)
- mlines |= TIOCM_RTS | TIOCM_CTS;
+ mlines |= TIOCM_CTS;
if (modem & MDM_IC)
mlines |= TIOCM_RI;
if (modem & MDM_DV)
@@ -1068,18 +1095,25 @@ static void gsm_control_modem(struct gsm_mux
*gsm, u8 *data, int clen)
return;
dlci = gsm->dlci[addr];
- while (gsm_read_ea(&modem, *dp++) == 0) {
- len--;
- if (len == 0)
- return;
+
+ /* Signals frame comes with EA = 0 in GE863-PRO3*/
+ if (len == 1) {
+ modem = *dp ;
+ } else {
+ while (gsm_read_ea(&modem, *dp++) == 0) {
+ len--;
+ if (len == 0)
+ return;
+ }
}
+
tty = tty_port_tty_get(&dlci->port);
gsm_process_modem(tty, dlci, modem);
if (tty) {
tty_wakeup(tty);
tty_kref_put(tty);
}
- gsm_control_reply(gsm, CMD_MSC, data, clen);
+ /*gsm_control_reply(gsm, CMD_MSC, data, clen);*/
}
/**
@@ -1234,6 +1268,10 @@ static void gsm_control_response(struct gsm_mux
*gsm, unsigned int command,
/* Rejected by the other end */
if (command == CMD_NSC)
ctrl->error = -EOPNOTSUPP;
+ if (command == CMD_MSC) {
+ /* Out of band modem line change indicator for a DLCI */
+ gsm_control_modem(gsm, data, clen);
+ }
ctrl->done = 1;
wake_up(&gsm->event);
}
@@ -1251,7 +1289,7 @@ 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|PF);
+ gsm->ftype/*|PF*/);
if (msg == NULL)
return;
msg->data[0] = (ctrl->cmd << 1) | 2 | EA; /* command */
@@ -1548,6 +1586,7 @@ static void gsm_dlci_command(struct gsm_dlci
*dlci, u8 *data, int len)
{
/* See what command is involved */
unsigned int command = 0;
+ pr_debug("%d bytes of command data\n", len);
while (len-- > 0) {
if (gsm_read_ea(&command, *data++) == 1) {
int clen = *data++;
@@ -1555,8 +1594,10 @@ static void gsm_dlci_command(struct gsm_dlci
*dlci, u8 *data, int len)
/* FIXME: this is properly an EA */
clen >>= 1;
/* Malformed command ? */
- if (clen > len)
+ if (clen > len) {
+ pr_debug("Malformed Command\n");
return;
+ }
if (command & 1)
gsm_control_message(dlci->gsm, command,
data, clen);
@@ -1673,7 +1714,7 @@ static void gsm_queue(struct gsm_mux *gsm)
cr = gsm->address & 1; /* C/R bit */
- gsm_print_packet("<--", address, cr, gsm->control, gsm->buf,
gsm->len);
+ gsm_print_packet("-<-", address, cr, gsm->control, gsm->buf,
gsm->len);
cr ^= 1 - gsm->initiator; /* Flip so 1 always means command */
dlci = gsm->dlci[address];
@@ -1706,7 +1747,7 @@ static void gsm_queue(struct gsm_mux *gsm)
break;
case UA:
case UA|PF:
- if (cr == 0 || dlci == NULL)
+ if (dlci == NULL)
break;
switch (dlci->state) {
case DLCI_CLOSING:
@@ -2017,6 +2058,9 @@ int gsm_activate_mux(struct gsm_mux *gsm)
dlci = gsm_dlci_alloc(gsm, 0);
if (dlci == NULL)
return -ENOMEM;
+ /* At least Siemens/Cinterion and Telit modems require that the
control
+ channel be open within 5 seconds of starting the cmux mode */
+ gsm_dlci_begin_open(dlci);
gsm->dead = 0; /* Tty opens are now permissible */
return 0;
}
@@ -2093,7 +2137,7 @@ static int gsmld_output(struct gsm_mux *gsm, u8
*data, int len)
return -ENOSPC;
}
if (debug & 4) {
- pr_debug("-->%d bytes out\n", len);
+ pr_debug("%d bytes out\n", len);
hex_packet(data, len);
}
gsm->tty->ops->write(gsm->tty, data, len);
@@ -2150,7 +2194,7 @@ static void gsmld_receive_buf(struct tty_struct
*tty, const unsigned char *cp,
char flags;
if (debug & 4) {
- pr_debug("Inbytes %dd\n", count);
+ pr_debug("%d bytes in\n", count);
hex_packet(cp, count);
}
@@ -2508,10 +2552,10 @@ static int gsmtty_modem_update(struct gsm_dlci
*dlci, u8 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[1] = (dlci->addr << 2) | 3; /* DLCI, EA, 1 */
+ modembits[2] = gsm_encode_modem(dlci) << 1 /*| EA*/;
if (brk)
- modembits[3] = brk << 4 | 2 | EA; /* Valid, EA */
+ modembits[3] = (brk << 4) | 2 | EA; /* Valid, EA */
ctrl = gsm_control_send(dlci->gsm, CMD_MSC, modembits, len + 1);
if (ctrl == NULL)
return -ENOMEM;
@@ -2537,6 +2581,10 @@ static void gsm_dtr_rts(struct tty_port *port,
int onoff)
modem_tx |= TIOCM_DTR | TIOCM_RTS;
else
modem_tx &= ~(TIOCM_DTR | TIOCM_RTS);
+
+ if (debug & 16)
+ gsm_print_modem(modem_tx);
+
if (modem_tx != dlci->modem_tx) {
dlci->modem_tx = modem_tx;
gsmtty_modem_update(dlci, 0);
@@ -2556,6 +2604,7 @@ static int gsmtty_open(struct tty_struct *tty,
struct file *filp)
struct tty_port *port;
unsigned int line = tty->index;
unsigned int mux = line >> 6;
+ int ret;
line = line & 0x3F;
@@ -2583,8 +2632,18 @@ static int gsmtty_open(struct tty_struct *tty,
struct file *filp)
/* We could in theory open and close before we wait - eg if we get
a DM straight back. This is ok as that will have caused a hangup */
set_bit(ASYNCB_INITIALIZED, &port->flags);
+
+ if (debug & 16)
+ pr_debug("Request to open DLCI %d\n", dlci->addr);
+
/* Start sending off SABM messages */
gsm_dlci_begin_open(dlci);
+
+ /* Wait for the modem to send a acknowledge to the open command */
+ ret = wait_event_interruptible(gsm->event, dlci->state == DLCI_OPEN);
+ if (ret < 0)
+ return -ERESTARTSYS;
+
/* And wait for virtual carrier */
return tty_port_block_til_ready(port, tty, filp);
}
@@ -2612,8 +2671,13 @@ static int gsmtty_write(struct tty_struct *tty,
const unsigned char *buf,
int len)
{
struct gsm_dlci *dlci = tty->driver_data;
+ int sent;
+
+ if (debug & 16)
+ pr_debug("%d bytes for dlci %d\n", len, dlci->addr);
+
/* Stuff the bytes into the fifo queue */
- int sent = kfifo_in_locked(dlci->fifo, buf, len, &dlci->lock);
+ sent = kfifo_in_locked(dlci->fifo, buf, len, &dlci->lock);
/* Need to kick the channel */
gsm_dlci_data_kick(dlci);
return sent;
@@ -2661,9 +2725,12 @@ static int gsmtty_tiocmset(struct tty_struct
*tty, struct file *filp,
struct gsm_dlci *dlci = tty->driver_data;
unsigned int modem_tx = dlci->modem_tx;
- modem_tx &= clear;
+ modem_tx &= ~clear;
modem_tx |= set;
+ if (debug & 16)
+ gsm_print_modem(modem_tx);
+
if (modem_tx != dlci->modem_tx) {
dlci->modem_tx = modem_tx;
return gsmtty_modem_update(dlci, 0);
> These alterations allow the usage of the non-transparent mode of the gsm
> mux, it works well enough to establish a ppp session using pppd. The
> increased buffer size allows the usage of the default MRU and MTU sizes
> in pppd.
In the tty like adaption layers the buffer size and PPP sizes are not
linked, because stuff is diced and packetised. Ok it's probably a
performance win. Really it needs network support for best results. There
is some prototype code for the network type ones but there are some
*horrid* and quite hard to fix races in it that need fixing before that
goes upstream
> There is a problem that happens if the process that is holding the
> attached line discipline tries to detach it before a process using a
> virtual com manages to close it. Both processes end up dealocked. I
> think this has to do with the tty lock. I don't have the backtrace with
> me
That one is a known recent breakage caused by Arnd's big kernel lock
removal. Needs looking at but it looks 'interesting' to fix.
> gsm->output(gsm, cbuf, len);
> - gsm_print_packet("-->", addr, cr, control, NULL, 0);
> + gsm_print_packet("->-", addr, cr, control, NULL, 0);
Why ? It's important not to put in gratuitious changes.
> }
>
> /**
> @@ -999,22 +1023,25 @@ static void gsm_control_reply(struct gsm_mux
> *gsm, int cmd, u8 *data,
> static void gsm_process_modem(struct tty_struct *tty, struct gsm_dlci
> *dlci,
> u32 modem)
> {
> - int mlines = 0;
> + int mlines = dlci->modem_tx;
> u8 brk = modem >> 6;
>
> + modem = modem >> 1 ;
> +
This seems ot be a symptom of an earlier bug (see below) - plus if you
did that you'd need to fix the brk flag ?
> /* Flow control/ready to communicate */
> if (modem & MDM_FC) {
> + pr_debug("Flux throttled\n");
> /* Need to throttle our output on this device */
> dlci->constipated = 1;
> }
> if (modem & MDM_RTC) {
> - mlines |= TIOCM_DSR | TIOCM_DTR;
> + mlines |= TIOCM_DSR ;
> dlci->constipated = 0;
> gsm_dlci_data_kick(dlci);
> }
> /* Map modem bits */
> if (modem & MDM_RTR)
> - mlines |= TIOCM_RTS | TIOCM_CTS;
> + mlines |= TIOCM_CTS;
> if (modem & MDM_IC)
> mlines |= TIOCM_RI;
> if (modem & MDM_DV)
> @@ -1068,18 +1095,25 @@ static void gsm_control_modem(struct gsm_mux
> *gsm, u8 *data, int clen)
> return;
> dlci = gsm->dlci[addr];
>
> - while (gsm_read_ea(&modem, *dp++) == 0) {
> - len--;
> - if (len == 0)
> - return;
> +
> + /* Signals frame comes with EA = 0 in GE863-PRO3*/
> + if (len == 1) {
> + modem = *dp ;
Probably worth checking the standard here. If not your workaround seems
reasonable but buggy - surely you need to shift the bits here not in the
modem processing where you otherwise overdo the shifts on the gsm_read_ea
path. In fact would it be better to simply be more robust and do
if (len == 0)
break; /* No EA, try what we have*/
?
> + while (gsm_read_ea(&modem, *dp++) == 0) {
> + len--;
> + if (len == 0)
> + return;
> tty = tty_port_tty_get(&dlci->port);
> gsm_process_modem(tty, dlci, modem);
> if (tty) {
> tty_wakeup(tty);
> tty_kref_put(tty);
> }
> - gsm_control_reply(gsm, CMD_MSC, data, clen);
> + /*gsm_control_reply(gsm, CMD_MSC, data, clen);*/
We need to reply to control messages in all cases as far as I can see
from the specification ? Is there a reason for this tweak ?
> + if (command == CMD_MSC) {
> + /* Out of band modem line change indicator for a DLCI */
> + gsm_control_modem(gsm, data, clen);
> + }
Ah I see what you are trying to do with that - if we get a modem status
command we need to reply, if we get a response we need not to - but
doesn't the modem line handling have to be different in each case anyway ?
> struct gsm_msg *msg = gsm_data_alloc(gsm, 0, ctrl->len + 1,
> - gsm->ftype|PF);
> + gsm->ftype/*|PF*/);
Correct - and recently committed fix.
> case UA:
> case UA|PF:
> - if (cr == 0 || dlci == NULL)
> + if (dlci == NULL)
> break;
A UA without C/R set is not valid, this change seems wrong ?
> + /* At least Siemens/Cinterion and Telit modems require that the
> control
> + channel be open within 5 seconds of starting the cmux mode */
> + gsm_dlci_begin_open(dlci);
Correct - which is easy to do from user space, however if we are being
run as the modem end (we do both) we cannot start sending SABM(P) messages
at this point as the user has no ability to specify which way they want
it.
The LDISC doesn't really use write so you've got a pass through after
setting the ldisc if need be, but 5 seconds to set an ldisc, and issue
two ioctls isn't hard even in perl...
> @@ -2583,8 +2632,18 @@ static int gsmtty_open(struct tty_struct *tty,
> struct file *filp)
> /* We could in theory open and close before we wait - eg if we get
> a DM straight back. This is ok as that will have caused a hangup */
> set_bit(ASYNCB_INITIALIZED, &port->flags);
> +
> + if (debug & 16)
> + pr_debug("Request to open DLCI %d\n", dlci->addr);
> +
> /* Start sending off SABM messages */
> gsm_dlci_begin_open(dlci);
> +
> + /* Wait for the modem to send a acknowledge to the open command */
> + ret = wait_event_interruptible(gsm->event, dlci->state == DLCI_OPEN);
> + if (ret < 0)
> + return -ERESTARTSYS;
No - we don't want to do this, there are cases where waiting is bad, the
current code uses the modem responses of the other end to do open
completion management. Think about O_NDELAY and a failed modem, or think
about being the modem end.
> @@ -2661,9 +2725,12 @@ static int gsmtty_tiocmset(struct tty_struct
> *tty, struct file *filp,
> struct gsm_dlci *dlci = tty->driver_data;
> unsigned int modem_tx = dlci->modem_tx;
>
> - modem_tx &= clear;
> + modem_tx &= ~clear;
> modem_tx |= set;
Eep
I think what would be a useful starting point would be to submit several
*small* patches that each fix one of the things you've touched
eg
- Allow for the missing EA
- Bracketing on the shifts (stylewise it does look clearer)
- TIOCMSET fix
then drop out the various random --> to ->- type changes and see what is
left that needs discussion, checking with the spec and submitting.
Alan
>> There is a problem that happens if the process that is holding the
>> attached line discipline tries to detach it before a process using a
>> virtual com manages to close it. Both processes end up dealocked. I
>> think this has to do with the tty lock. I don't have the backtrace with
>> me
here is the log I get here :
[ 1051.950000] ------------[ cut here ]------------
[ 1051.950000] WARNING: at drivers/tty/tty_mutex.c:31 tty_lock+0x30/0x50()
[ 1051.960000] Modules linked in:
[ 1051.960000] [<c0029478>] (unwind_backtrace+0x0/0xe4) from [<c0034f30>]
(warn_slowpath_common+0x4c/0x64)
[ 1051.970000] [<c0034f30>] (warn_slowpath_common+0x4c/0x64) from [<c0034f60>]
(warn_slowpath_null+0x18/0x1c)
[ 1051.980000] [<c0034f60>] (warn_slowpath_null+0x18/0x1c) from [<c031550c>]
(tty_lock+0x30/0x50)
[ 1051.990000] [<c031550c>] (tty_lock+0x30/0x50) from [<c01af5b0>]
(__tty_hangup+0x78/0x47c)
[ 1052.000000] [<c01af5b0>] (__tty_hangup+0x78/0x47c) from [<c01ba69c>]
(gsm_cleanup_mux+0x198/0x214)
[ 1052.010000] [<c01ba69c>] (gsm_cleanup_mux+0x198/0x214) from [<c01baaf4>]
(gsmld_close+0x28/0x4c)
[ 1052.020000] [<c01baaf4>] (gsmld_close+0x28/0x4c) from [<c01b5974>]
(tty_ldisc_close+0x58/0x64)
[ 1052.030000] [<c01b5974>] (tty_ldisc_close+0x58/0x64) from [<c01b5b48>]
(tty_ldisc_release+0x38/0x70)
[ 1052.040000] [<c01b5b48>] (tty_ldisc_release+0x38/0x70) from [<c01b0e2c>]
(tty_release+0x41c/0x480)
[ 1052.050000] [<c01b0e2c>] (tty_release+0x41c/0x480) from [<c009667c>]
(fput+0x108/0x200)
[ 1052.060000] [<c009667c>] (fput+0x108/0x200) from [<c00939c8>]
(filp_close+0x60/0x6c)
[ 1052.070000] [<c00939c8>] (filp_close+0x60/0x6c) from [<c0036eb8>]
(put_files_struct+0x80/0xdc)
[ 1052.080000] [<c0036eb8>] (put_files_struct+0x80/0xdc) from [<c0038500>]
(do_exit+0x1b8/0x6bc)
[ 1052.080000] [<c0038500>] (do_exit+0x1b8/0x6bc) from [<c0038ac4>]
(do_group_exit+0xc0/0xf4)
[ 1052.090000] [<c0038ac4>] (do_group_exit+0xc0/0xf4) from [<c0044460>]
(get_signal_to_deliver+0x3ac/0x40c)
[ 1052.100000] [<c0044460>] (get_signal_to_deliver+0x3ac/0x40c) from
[<c0027190>] (do_signal+0x68/0x614)
[ 1052.110000] [<c0027190>] (do_signal+0x68/0x614) from [<c0027754>]
(do_notify_resume+0x18/0x60)
[ 1052.120000] [<c0027754>] (do_notify_resume+0x18/0x60) from [<c0024f94>]
(work_pending+0x24/0x28)
[ 1052.130000] ---[ end trace ac06c34c1914deef ]---
Eric
Hi,
Thank you for your comments. I think the biggest problem with what I did
was forget that the driver should also work as a modem end, I'll recheck
my changes using your suggestions and try to submit the new patches
soon.
On Thu, 2011-03-10 at 11:06 +0000, Alan Cox wrote:
> > These alterations allow the usage of the non-transparent mode of the gsm
> > mux, it works well enough to establish a ppp session using pppd. The
> > increased buffer size allows the usage of the default MRU and MTU sizes
> > in pppd.
>
> In the tty like adaption layers the buffer size and PPP sizes are not
> linked, because stuff is diced and packetised. Ok it's probably a
> performance win. Really it needs network support for best results. There
> is some prototype code for the network type ones but there are some
> *horrid* and quite hard to fix races in it that need fixing before that
> goes upstream
I've retested it with the 512 limits and it sometimes works. I'm not
sure where the problem is as it wasn't me that configured the ppp
server, but I will see if I can track down the real problem. Bigger
limits on the buffer seems to work better but it can be bias on my part.
> > There is a problem that happens if the process that is holding the
> > attached line discipline tries to detach it before a process using a
> > virtual com manages to close it. Both processes end up dealocked. I
> > think this has to do with the tty lock. I don't have the backtrace with
> > me
>
> That one is a known recent breakage caused by Arnd's big kernel lock
> removal. Needs looking at but it looks 'interesting' to fix.
>
> > gsm->output(gsm, cbuf, len);
> > - gsm_print_packet("-->", addr, cr, control, NULL, 0);
> > + gsm_print_packet("->-", addr, cr, control, NULL, 0);
>
> Why ? It's important not to put in gratuitious changes.
This is just because the string '<--' seems to get eaten by a printk
somewhere, so I changed it to '-<-', that change is just to be
consistent but it's not very important.
> Probably worth checking the standard here. If not your workaround seems
> reasonable but buggy - surely you need to shift the bits here not in the
> modem processing where you otherwise overdo the shifts on the gsm_read_ea
> path. In fact would it be better to simply be more robust and do
>
> if (len == 0)
> break; /* No EA, try what we have*/
>
I definitely like your solution better. If I read the standard
correctly EA should be set to 1 if there is no break octet.
However this is what happens:
OUT: F9 03 EF 09 E3 05 07 0D FB F9
IN: F9 03 EF 09 E1 05 07 0C FB F9
I think it's a bug on the modem, but I can be reading the standard
wrong, have to try with a modem from another manufacturer and check what
happens.
>
> We need to reply to control messages in all cases as far as I can see
> from the specification ? Is there a reason for this tweak ?
>
> > + if (command == CMD_MSC) {
> > + /* Out of band modem line change indicator for a DLCI */
> > + gsm_control_modem(gsm, data, clen);
> > + }
>
> Ah I see what you are trying to do with that - if we get a modem status
> command we need to reply, if we get a response we need not to - but
> doesn't the modem line handling have to be different in each case anyway ?
I will think a bit more about how to do this. I should have considered
what happens if the driver is behaving as the modem.
>
>
> > case UA:
> > case UA|PF:
> > - if (cr == 0 || dlci == NULL)
> > + if (dlci == NULL)
> > break;
>
> A UA without C/R set is not valid, this change seems wrong ?
My mistake, now that I think back I can't find the reason for why I did
this. :)
>
> > + /* At least Siemens/Cinterion and Telit modems require that the
> > control
> > + channel be open within 5 seconds of starting the cmux mode */
> > + gsm_dlci_begin_open(dlci);
>
> Correct - which is easy to do from user space, however if we are being
> run as the modem end (we do both) we cannot start sending SABM(P) messages
> at this point as the user has no ability to specify which way they want
> it.
>
> The LDISC doesn't really use write so you've got a pass through after
> setting the ldisc if need be, but 5 seconds to set an ldisc, and issue
> two ioctls isn't hard even in perl...
>
I hadn't noticed that doing a GSMIOC_SETCONF fires a DLCI 0 open
command, oops.
> No - we don't want to do this, there are cases where waiting is bad, the
> current code uses the modem responses of the other end to do open
> completion management. Think about O_NDELAY and a failed modem, or think
> about being the modem end.
>
My idea was to wait for the acknowledge to the SAMB message before
firing the MSC, but if this is wrong I think its better to leave it out.
At least the GE863-Pro^3 seems to reply to the SAMB with the acknowledge
and an MSC independently if we asked for it or not.
Hi Mário,
On 10/03/2011 03:39, Mário Isidoro wrote:
> @@ -2661,9 +2725,12 @@ static int gsmtty_tiocmset(struct tty_struct
> *tty, struct file *filp,
> struct gsm_dlci *dlci = tty->driver_data;
> unsigned int modem_tx = dlci->modem_tx;
>
> - modem_tx &= clear;
> + modem_tx &= ~clear;
> modem_tx |= set;
>
good catch : this fix (and only this one in your patch) makes Telit modems
work realiable here with ppp (which was failing before).
Tested on GC864-QUAD-V2, UC864-E and UC864-WD
Thanks !
Eric
Hi Eric
My GE863-Pro^3 works well without it, however, without it the control
signals from the computer side do not seem to work correctly.
>From my tests, without the patch, the value in modem_tx will never
change when we send TIOCMSET commands.
By the way, can you confirm if your Telit modems respect the EA bit on
the MSC frame ?
Thank you,
Mário Isidoro
On Fri, 2011-03-11 at 14:34 +0100, Eric Bénard wrote:
> Hi Mário,
>
> On 10/03/2011 03:39, Mário Isidoro wrote:
> > @@ -2661,9 +2725,12 @@ static int gsmtty_tiocmset(struct tty_struct
> > *tty, struct file *filp,
> > struct gsm_dlci *dlci = tty->driver_data;
> > unsigned int modem_tx = dlci->modem_tx;
> >
> > - modem_tx &= clear;
> > + modem_tx &= ~clear;
> > modem_tx |= set;
> >
> good catch : this fix (and only this one in your patch) makes Telit modems
> work realiable here with ppp (which was failing before).
> Tested on GC864-QUAD-V2, UC864-E and UC864-WD
>
> Thanks !
>
> Eric
Hi Mário,
On 11/03/2011 16:27, Mário Isidoro wrote:
> Hi Eric
> My GE863-Pro^3 works well without it, however, without it the control
> signals from the computer side do not seem to work correctly.
>
here without this fix, ppp gets blocked during init.
>> From my tests, without the patch, the value in modem_tx will never
> change when we send TIOCMSET commands.
>
> By the way, can you confirm if your Telit modems respect the EA bit on
> the MSC frame ?
>
I get the exact same byte sequence as you do.
Eric