artpec.c / serial_core.c hardware flow control problem

artpec.c / serial_core.c hardware flow control problem

[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().

Could this be fixed by making IXON a RELEVANT_IFLAG?:

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 7f28307..d45473d 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -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

Re: artpec.c / serial_core.c hardware flow control problem

[Alan Cox]

On Thu, 28 Jun 2012 17:29:27 +0200
Mikael Johansson <mikael.lars.johansson@axis.com> wrote:

> 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?:

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

RE: artpec.c / serial_core.c hardware flow control problem

[Johan Adolfsson]

> -----Original Message-----
> From: linux-serial-owner@vger.kernel.org [mailto:linux-serial-
> owner@vger.kernel.org] On Behalf Of Alan Cox
> Sent: den 28 juni 2012 18:24
> 
> On Thu, 28 Jun 2012 17:29:27 +0200
> Mikael Johansson <mikael.lars.johansson@axis.com> wrote:
> 
> > 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?:
> 
> 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?

> 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.

> Alan

/Johan

Re: artpec.c / serial_core.c hardware flow control problem

[Alan Cox]

> 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