On Wed, 2009-08-19 at 12:40 +0100, Alan Cox wrote:
> That test needs tweaking - it would have worked historically but its no
> longer valid.
>
> if (old_termios && !tty_termios_hw_change(old_termios,&tty->termios))
> return;
That's a lot neater. Done.
> > + /* Parity stuff */
> > + if (tty->termios->c_cflag & PARENB) {
> > + if (tty->termios->c_cflag & PARODD)
> > + LCR_change_to |= QT2_SERIAL_ODD_PARITY;
> > + else
> > + LCR_change_to |= QT2_SERIAL_EVEN_PARITY;
> > + }
> > + if (tty->termios->c_cflag & CSTOPB)
> > + LCR_change_to |= QT2_SERIAL_TWO_STOPB;
> > + else
> > + LCR_change_to |= QT2_SERIAL_ONE_STOPB;
>
> Looks good. As mark space isn't supported it should also do
>
> tty->termios->c_cflag &= ~CMSPAR;
Done.
> > + divisor = QT2_MAX_BAUD_RATE / baud;
> > + remainder = QT2_MAX_BAUD_RATE % baud;
> > + /* Round to nearest divisor */
> > + if (((remainder * 2) >= baud) && (baud != 110))
> > + divisor++;
> >
> > + status = qt2_boxsetuart(serial, UartNumber, (unsigned short) divisor,
> > + LCR_change_to);
> > + if (status < 0) {
> > + dbg("qt2_boxsetuart() failed");
> > + return;
>
> Should on succes then really work out the baud rate selected which I
> assume is QT2_MAX_BAUD_RATE / divisor and call tty_encode_baud_rate(tty,
> rate, rate);
Would seem to make sense, the caller then finds out what baud rate they
have been given, seeing as the remainder doesn't get sent to the
hardware, which restricts the baud rates it can actually implement.
Done.
> > + /* if we are implementing XON/XOFF, set the start and stop character
> > + * in the device */
> > + if (I_IXOFF(tty) || I_IXON(tty)) {
> > + unsigned char stop_char = STOP_CHAR(tty);
> > + unsigned char start_char = START_CHAR(tty);
> > + status = qt2_boxsetsw_flowctl(serial, UartNumber, stop_char,
> > + start_char);
> > + if (status < 0)
> > + dbg("qt2_boxsetsw_flowctl (enabled) failed");
>
> Not sure what should happen here for IXANY but thats a minor detail
I see what you mean. I also note by comparison with Greg's example code
(old LJ article) that we are only handling input XON / XOFF, which in
the interface could be separate to output XON / XOFF. Should the driver
be doing anything to indicate back to the user that the two settings are
tied together inside? Setting both the inbound and outbound flags within
the if (I_IXOFF(tty) || I_IXON(tty)) block seems a good idea, but I
can't see obvious macros for doing the bit banging, which makes me
suspicious.
This patch follows up on the complete set of 3, i.e. is additional
rather than replacing the original patch.
Richard
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
