2010-06-03 11:55:15

by Daniel Mack

[permalink] [raw]
Subject: [PATCH] usb-serial/ftdi_sio: fix DTR/RTS line modes

Call set_mctrl() and clear_mctrl() according to the flow control mode
selected. This makes serial communication for FT232 connected devices
work when CRTSCTS is not set.

This fixes a regression introduced by 4175f3e31 ("tty_port: If we are
opened non blocking we still need to raise the carrier"). This patch
calls the low-level driver's dtr_rts() function which consequently sets
TIOCM_DTR | TIOCM_RTS. A later call to set_termios() without CRTSCTS in
cflags, however, does not reset these bits, and so data is not actually
sent out on the serial wire.

Signed-off-by: Daniel Mack <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: Alan Cox <[email protected]>
Cc: [email protected]
---
drivers/usb/serial/ftdi_sio.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 050211a..79dd1ae 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -2005,6 +2005,8 @@ static void ftdi_set_termios(struct tty_struct *tty,
"urb failed to set to rts/cts flow control\n");
}

+ /* raise DTR/RTS */
+ set_mctrl(port, TIOCM_DTR | TIOCM_RTS);
} else {
/*
* Xon/Xoff code
@@ -2052,6 +2054,8 @@ static void ftdi_set_termios(struct tty_struct *tty,
}
}

+ /* lower DTR/RTS */
+ clear_mctrl(port, TIOCM_DTR | TIOCM_RTS);
}
return;
}
--
1.7.1


2010-06-03 11:57:34

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH] usb-serial/ftdi_sio: fix DTR/RTS line modes

On Thu, Jun 03, 2010 at 01:55:02PM +0200, Daniel Mack wrote:
> Call set_mctrl() and clear_mctrl() according to the flow control mode
> selected. This makes serial communication for FT232 connected devices
> work when CRTSCTS is not set.
>
> This fixes a regression introduced by 4175f3e31 ("tty_port: If we are
> opened non blocking we still need to raise the carrier"). This patch
> calls the low-level driver's dtr_rts() function which consequently sets
> TIOCM_DTR | TIOCM_RTS. A later call to set_termios() without CRTSCTS in
> cflags, however, does not reset these bits, and so data is not actually
> sent out on the serial wire.
>
> Signed-off-by: Daniel Mack <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Johan Hovold <[email protected]>
> Cc: Alan Cox <[email protected]>
> Cc: [email protected]

Oops. I forgot to Cc: [email protected].
This is in fact broken since 2.6.31-something.




> ---
> drivers/usb/serial/ftdi_sio.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index 050211a..79dd1ae 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -2005,6 +2005,8 @@ static void ftdi_set_termios(struct tty_struct *tty,
> "urb failed to set to rts/cts flow control\n");
> }
>
> + /* raise DTR/RTS */
> + set_mctrl(port, TIOCM_DTR | TIOCM_RTS);
> } else {
> /*
> * Xon/Xoff code
> @@ -2052,6 +2054,8 @@ static void ftdi_set_termios(struct tty_struct *tty,
> }
> }
>
> + /* lower DTR/RTS */
> + clear_mctrl(port, TIOCM_DTR | TIOCM_RTS);
> }
> return;
> }
> --
> 1.7.1
>

2010-06-03 12:19:13

by Alan

[permalink] [raw]
Subject: Re: [PATCH] usb-serial/ftdi_sio: fix DTR/RTS line modes

On Thu, 3 Jun 2010 13:55:02 +0200
Daniel Mack <[email protected]> wrote:

> Call set_mctrl() and clear_mctrl() according to the flow control mode
> selected. This makes serial communication for FT232 connected devices
> work when CRTSCTS is not set.
>
> This fixes a regression introduced by 4175f3e31 ("tty_port: If we are
> opened non blocking we still need to raise the carrier"). This patch
> calls the low-level driver's dtr_rts() function which consequently
> sets TIOCM_DTR | TIOCM_RTS. A later call to set_termios() without
> CRTSCTS in cflags, however, does not reset these bits, and so data is
> not actually sent out on the serial wire.

If you've got hardware the other end expecting RTS/CTS signals you
really ought to set CRTSCTS. However you patch makes complete sense in
terms of which way it is best to end up.

2010-06-03 12:23:15

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH] usb-serial/ftdi_sio: fix DTR/RTS line modes

On Thu, Jun 03, 2010 at 12:32:25PM +0100, Alan Cox wrote:
> On Thu, 3 Jun 2010 13:55:02 +0200
> Daniel Mack <[email protected]> wrote:
>
> > Call set_mctrl() and clear_mctrl() according to the flow control mode
> > selected. This makes serial communication for FT232 connected devices
> > work when CRTSCTS is not set.
> >
> > This fixes a regression introduced by 4175f3e31 ("tty_port: If we are
> > opened non blocking we still need to raise the carrier"). This patch
> > calls the low-level driver's dtr_rts() function which consequently
> > sets TIOCM_DTR | TIOCM_RTS. A later call to set_termios() without
> > CRTSCTS in cflags, however, does not reset these bits, and so data is
> > not actually sent out on the serial wire.
>
> If you've got hardware the other end expecting RTS/CTS signals you
> really ought to set CRTSCTS.

I know. It is, however, a regression in my case, and there might be
other cases where RTS/CTS aren't connected at all. The core of the
problem is that the driver sets hardware flow control, even though the
userspace asked for not doing that.

> However you patch makes complete sense in
> terms of which way it is best to end up.

And it shouldn't break anything I'd say.

Thanks,
Daniel

2010-06-03 14:29:42

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] usb-serial/ftdi_sio: fix DTR/RTS line modes

On Thu, Jun 03, 2010 at 01:57:27PM +0200, Daniel Mack wrote:
> On Thu, Jun 03, 2010 at 01:55:02PM +0200, Daniel Mack wrote:
> > Call set_mctrl() and clear_mctrl() according to the flow control mode
> > selected. This makes serial communication for FT232 connected devices
> > work when CRTSCTS is not set.
> >
> > This fixes a regression introduced by 4175f3e31 ("tty_port: If we are
> > opened non blocking we still need to raise the carrier"). This patch
> > calls the low-level driver's dtr_rts() function which consequently sets
> > TIOCM_DTR | TIOCM_RTS. A later call to set_termios() without CRTSCTS in
> > cflags, however, does not reset these bits, and so data is not actually
> > sent out on the serial wire.
> >
> > Signed-off-by: Daniel Mack <[email protected]>
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: Johan Hovold <[email protected]>
> > Cc: Alan Cox <[email protected]>
> > Cc: [email protected]
>
> Oops. I forgot to Cc: [email protected].
> This is in fact broken since 2.6.31-something.

Thanks for letting me know, I'll add that marking to the patch when I
queue it up.

thanks,

greg k-h

2010-06-03 16:22:07

by Gene Heskett

[permalink] [raw]
Subject: Re: [PATCH] usb-serial/ftdi_sio: fix DTR/RTS line modes

On Thursday 03 June 2010, Daniel Mack wrote:
>On Thu, Jun 03, 2010 at 01:55:02PM +0200, Daniel Mack wrote:
>> Call set_mctrl() and clear_mctrl() according to the flow control mode
>> selected. This makes serial communication for FT232 connected devices
>> work when CRTSCTS is not set.
>>
>> This fixes a regression introduced by 4175f3e31 ("tty_port: If we are
>> opened non blocking we still need to raise the carrier"). This patch
>> calls the low-level driver's dtr_rts() function which consequently sets
>> TIOCM_DTR | TIOCM_RTS. A later call to set_termios() without CRTSCTS in
>> cflags, however, does not reset these bits, and so data is not actually
>> sent out on the serial wire.
>>
>> Signed-off-by: Daniel Mack <[email protected]>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> Cc: Johan Hovold <[email protected]>
>> Cc: Alan Cox <[email protected]>
>> Cc: [email protected]
>
>Oops. I forgot to Cc: [email protected].
>This is in fact broken since 2.6.31-something.
>
>> ---
>> drivers/usb/serial/ftdi_sio.c | 4 ++++
>> 1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/usb/serial/ftdi_sio.c
>> b/drivers/usb/serial/ftdi_sio.c index 050211a..79dd1ae 100644
>> --- a/drivers/usb/serial/ftdi_sio.c
>> +++ b/drivers/usb/serial/ftdi_sio.c
>> @@ -2005,6 +2005,8 @@ static void ftdi_set_termios(struct tty_struct
>> *tty, "urb failed to set to rts/cts flow control\n");
>> }
>>
>> + /* raise DTR/RTS */
>> + set_mctrl(port, TIOCM_DTR | TIOCM_RTS);
>> } else {
>> /*
>> * Xon/Xoff code
>> @@ -2052,6 +2054,8 @@ static void ftdi_set_termios(struct tty_struct
>> *tty, }
>> }
>>
>> + /* lower DTR/RTS */
>> + clear_mctrl(port, TIOCM_DTR | TIOCM_RTS);
>> }
>> return;
>> }
>

How soon can this get in? I ask because I have been crippled in all
attempts to use an ftdi adaptor to talk to an old legacy machine I have for
about that long now. xon/xoff seems completely broken, and 7wire seems to
scramble things to the point where no null modem cable I have made (and I
have a book on them) will allow the 7wire flow controls to function.

--
Cheers, Gene
"There are four boxes to be used in defense of liberty:
soap, ballot, jury, and ammo. Please use in that order."
-Ed Howdershelt (Author)
"Being against torture ought to be sort of a multipartisan thing."
-- Karl Lehenbauer, as amended by Jeff Daiell, a Libertarian

2010-06-03 16:32:42

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] usb-serial/ftdi_sio: fix DTR/RTS line modes

On Thu, Jun 03, 2010 at 12:21:56PM -0400, Gene Heskett wrote:
> How soon can this get in? I ask because I have been crippled in all
> attempts to use an ftdi adaptor to talk to an old legacy machine I have for
> about that long now. xon/xoff seems completely broken, and 7wire seems to
> scramble things to the point where no null modem cable I have made (and I
> have a book on them) will allow the 7wire flow controls to function.

Give it a week or so to get out to all the stable kernel releases.

thanks,

greg k-h

2010-06-03 16:53:31

by Gene Heskett

[permalink] [raw]
Subject: Re: [PATCH] usb-serial/ftdi_sio: fix DTR/RTS line modes

On Thursday 03 June 2010, Greg KH wrote:
>On Thu, Jun 03, 2010 at 12:21:56PM -0400, Gene Heskett wrote:
>> How soon can this get in? I ask because I have been crippled in all
>> attempts to use an ftdi adaptor to talk to an old legacy machine I have
>> for about that long now. xon/xoff seems completely broken, and 7wire
>> seems to scramble things to the point where no null modem cable I have
>> made (and I have a book on them) will allow the 7wire flow controls to
>> function.
>
>Give it a week or so to get out to all the stable kernel releases.
>
>thanks,
>
>greg k-h
>
Thanks Greg.

--
Cheers, Gene
"There are four boxes to be used in defense of liberty:
soap, ballot, jury, and ammo. Please use in that order."
-Ed Howdershelt (Author)
Well, the handwriting is on the floor.
-- Joe E. Lewis

2010-06-06 09:39:32

by Ruud Linders

[permalink] [raw]
Subject: Re: [PATCH] usb-serial/ftdi_sio: fix DTR/RTS line modes


On 06/03/2010 01:57 PM, Daniel Mack wrote:
> On Thu, Jun 03, 2010 at 01:55:02PM +0200, Daniel Mack wrote:
>> Call set_mctrl() and clear_mctrl() according to the flow control mode
>> selected. This makes serial communication for FT232 connected devices
>> work when CRTSCTS is not set.
>>
>> This fixes a regression introduced by 4175f3e31 ("tty_port: If we are
>> opened non blocking we still need to raise the carrier"). This patch
>> calls the low-level driver's dtr_rts() function which consequently sets
>> TIOCM_DTR | TIOCM_RTS. A later call to set_termios() without CRTSCTS in
>> cflags, however, does not reset these bits, and so data is not actually
>> sent out on the serial wire.
>>
>> Signed-off-by: Daniel Mack <[email protected]>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> Cc: Johan Hovold <[email protected]>
>> Cc: Alan Cox <[email protected]>
>> Cc: [email protected]
>
> Oops. I forgot to Cc: [email protected].
> This is in fact broken since 2.6.31-something.
>
>
>
>
>> ---
>> drivers/usb/serial/ftdi_sio.c | 4 ++++
>> 1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
>> index 050211a..79dd1ae 100644
>> --- a/drivers/usb/serial/ftdi_sio.c
>> +++ b/drivers/usb/serial/ftdi_sio.c
>> @@ -2005,6 +2005,8 @@ static void ftdi_set_termios(struct tty_struct *tty,
>> "urb failed to set to rts/cts flow control\n");
>> }
>>
>> + /* raise DTR/RTS */
>> + set_mctrl(port, TIOCM_DTR | TIOCM_RTS);
>> } else {
>> /*
>> * Xon/Xoff code
>> @@ -2052,6 +2054,8 @@ static void ftdi_set_termios(struct tty_struct *tty,
>> }
>> }
>>
>> + /* lower DTR/RTS */
>> + clear_mctrl(port, TIOCM_DTR | TIOCM_RTS);
>> }
>> return;
>> }
>> --
>> 1.7.1
>>
> --

Hmm, I tried this patch on plain 2.6.34 but still have a problem.
As you indicated since 2.6.31-something something is broken.

I'm using a receive only device (http://rfxcom.com/receivers.htm)
which works fine until the system becomes more busy, usually it runs
almost idle and all is fine.

However, when this happens, it appears the received characters get
buffered somewhere as nothing seems to get through or is delayed for
many minutes.
When I then =write= a character to the serial device
echo > /dev/ttyUSB0
the data is received in sudden burst, seems no data is actually lost
just seriously delayed.

Any ideas ?

2010-06-06 09:44:13

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH] usb-serial/ftdi_sio: fix DTR/RTS line modes

On Sun, Jun 06, 2010 at 11:33:06AM +0200, Ruud Linders wrote:
> Hmm, I tried this patch on plain 2.6.34 but still have a problem.
> As you indicated since 2.6.31-something something is broken.
>
> I'm using a receive only device (http://rfxcom.com/receivers.htm)
> which works fine until the system becomes more busy, usually it runs
> almost idle and all is fine.
>
> However, when this happens, it appears the received characters get
> buffered somewhere as nothing seems to get through or is delayed for
> many minutes.
> When I then =write= a character to the serial device
> echo > /dev/ttyUSB0
> the data is received in sudden burst, seems no data is actually lost
> just seriously delayed.
>
> Any ideas ?

Hmm, no, sorry. Things work fine for me again now.

Did you try reverting the commit I blamed in the commit log? If this
doesn't help, I can only suggest you run a bisect to see which commit
between 2.6.31 and 2.6.34 broke it for you.

Daniel