Skip to contentSkip to conversation

artpec.c / serial_core.c hardware flow control problem

PATCH Mikael Johansson 3 participants 4 messages Jun 28, 2012 – Jun 29, 2012 TTY layer and serial drivers

lore
Share & tools
lore

Mikael Johansson

author
#0 lore raw

Greetings,

We have a problem with drivers/serial/artpec.c (Not yet in main tree) which uses hardware supported flow control (XON/XOFF). We do cfmakeraw() and tcsetattr() from userspace to set a termios struct where (c_iflag & IXON) is not set. The problem is that the call to tcsetattr() is not propagated down to serial_artpec_set_termios(), the reason being that uart_set_termios() doesn't think that IXON is a RELEVANT_IFLAG and returns prematurely instead of calling uart_change_speed --> ops->set_termios().

Could this be fixed by making IXON a RELEVANT_IFLAG?:

drivers/serial/serial_core.c +1 -1
index 7f28307..d45473d 100644
@@ -1211,7 +1211,7 @@ static void uart_set_termios(struct tty_struct *tty,
      * bits in c_cflag; c_[io]speed will always be set
      * appropriately by set_termios() in tty_ioctl.c
      */
-#define RELEVANT_IFLAG(iflag)    ((iflag) & (IGNBRK|BRKINT|IGNPAR|PARMRK|INPCK))
+#define RELEVANT_IFLAG(iflag)    ((iflag) & (IGNBRK|BRKINT|IGNPAR|PARMRK|INPCK|IXON))
     if ((cflag ^ old_termios->c_cflag) == 0 &&
         tty->termios->c_ospeed == old_termios->c_ospeed &&
         tty->termios->c_ispeed == old_termios->c_ispeed &&

Or should it be fixed at some other level?

Note: This was tested on 2.6.35 but appears to have the same code in the newer kernels. Not many drivers seem to look at the IXON flag, only crisv10, omap_serial and jsm_neo.

Best regards,
Mikael Johansson

#1 lore raw
Quoted 11 lines · On Thu, 28 Jun 2012 17:29:27 +0200 Mikael Johansson…

Greetings,

We have a problem with drivers/serial/artpec.c (Not yet in main tree) which uses hardware supported flow control (XON/XOFF). We do cfmakeraw() and tcsetattr() from userspace to set a termios struct where (c_iflag & IXON) is not set. The problem is that the call to tcsetattr() is not propagated down to serial_artpec_set_termios(), the reason being that uart_set_termios() doesn't think that IXON is a RELEVANT_IFLAG and returns prematurely instead of calling uart_change_speed --> ops->set_termios().

Quoted 1 line

Could this be fixed by making IXON a RELEVANT_IFLAG?:

That's actually insufficient for those cases - you can set the characters used for things as well plus IXANY and IXOFF.

It's a stupid "optimisation" and the only reason I didn't get rid of it before was in case it broke something else. Time for it to go.

So yeah - I'd just delete it.

If you are doing hardware XON/XOFF watch c_cc[VSTART] and c_cc[VSTOP]. Those control the symbol used for soft flow control.

Alan

#2 lore raw
Quoted 25 lines

-----Original Message-----
From: [email protected] [mailto:linux-serial- [email protected]] On Behalf Of Alan Cox
Sent: den 28 juni 2012 18:24

Quoted 4 lines · On Thu, 28 Jun 2012 17:29:27 +0200 Mikael Johansson…

Greetings,

We have a problem with drivers/serial/artpec.c (Not yet in main tree) which uses hardware supported flow control (XON/XOFF). We do

cfmakeraw()

Quoted 1 line

and tcsetattr() from userspace to set a termios struct where (c_iflag

&

Quoted 7 lines

IXON) is not set. The problem is that the call to tcsetattr() is not propagated down to serial_artpec_set_termios(), the reason being that uart_set_termios() doesn't think that IXON is a RELEVANT_IFLAG and returns prematurely instead of calling uart_change_speed --> ops->set_termios().

Could this be fixed by making IXON a RELEVANT_IFLAG?:

Quoted 8 lines

That's actually insufficient for those cases - you can set the characters
used for things as well plus IXANY and IXOFF.

It's a stupid "optimisation" and the only reason I didn't get rid of it before was in case it broke something else. Time for it to go.

So yeah - I'd just delete it.

Do you mean the entire if () { return; } statement or just the RELEVANT_IFLAG part?

Quoted 2 lines

If you are doing hardware XON/XOFF watch c_cc[VSTART] and c_cc[VSTOP]. Those control the symbol used for soft flow control.

We use STOP_CHAR(tty), we only have hardware support for stopping transfer, the tty layer kicks in and enables it again.

Quoted 1 line

Alan

/Johan

#3 lore raw
Quoted 1 line

Do you mean the entire if () { return; } statement or just the RELEVANT_IFLAG part?

It's not enough to check the iflag in this case - a termios changing c_cc[VSTOP) - which is what STOP_CHAR(tty) is, also might need the stop handling reprogramming.

Alan