Hi Tomasz, On 17-09-2013 11:18, Tomasz Figa wrote:
Hi José, Please see my comments below. On Wednesday 11 of September 2013 11:08:27 José Miguel Gonçalves wrote:The Samsung serial driver currently does not support setting the RTS pin with an ioctl(TIOCMSET) call. This patch adds this support. Signed-off-by: José Miguel Gonçalves <jose.goncalves@xxxxxxx> --- drivers/tty/serial/samsung.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c index f3dfa19..e5dd808 100644 --- a/drivers/tty/serial/samsung.c +++ b/drivers/tty/serial/samsung.c @@ -407,7 +407,14 @@ static unsigned int s3c24xx_serial_get_mctrl(struct uart_port *port) static void s3c24xx_serial_set_mctrl(struct uart_port *port, unsigned int mctrl) { - /* todo - possibly remove AFC and do manual CTS */ + unsigned int umcon = rd_regl(port, S3C2410_UMCON); + + if (mctrl & TIOCM_RTS) + umcon |= S3C2410_UMCOM_RTS_LOW; + else + umcon &= ~S3C2410_UMCOM_RTS_LOW; + + wr_regl(port, S3C2410_UMCON, umcon);I wonder if port capability shouldn't be considered here. Depending on SoC, only selected ports provide modem control capability. For example on S3C64xx only ports 0 and 1 support modem control, while ports 2 and 3 don't.
Same for S3C2416. I also wondered that, but while I have information for all S3C24xx chips and for those a simple test ( port->line < 2) would validate this, I don't know about other SoCs this driver supports. Bearing this in mind and also that the current implementation of s3c24xx_serial_get_mctrl() does not check also for which port it applies, I opted for this solution.
} static void s3c24xx_serial_break_ctl(struct uart_port *port, int break_state) @@ -774,8 +781,6 @@ static void s3c24xx_serial_set_termios(struct uart_port *port, if (termios->c_cflag & CSTOPB) ulcon |= S3C2410_LCON_STOPB; - umcon = (termios->c_cflag & CRTSCTS) ? S3C2410_UMCOM_AFC : 0; - if (termios->c_cflag & PARENB) { if (termios->c_cflag & PARODD) ulcon |= S3C2410_LCON_PODD; @@ -792,6 +797,12 @@ static void s3c24xx_serial_set_termios(struct uart_port *port, wr_regl(port, S3C2410_ULCON, ulcon); wr_regl(port, S3C2410_UBRDIV, quot); + + if (termios->c_cflag & CRTSCTS) + umcon = S3C2410_UMCOM_AFC;Is it correct to override the last manual RTS value set to this register when activating manual flow control? Shouldn't the code be more like the following: umcon = rd_regb(port, S3C2410_UMCON); if (termios->c_cflag & CRTSCTS) umcon |= S3C2410_UMCOM_AFC; else umcon &= ~S3C2410_UMCOM_AFC; wr_regl(port, S3C2410_UMCON, umcon); Probably port capability should be considered here as well.
Looking at the S3C24xx user manuals I've seen that if you set the automatic flow control (AFC) with the S3C2410_UMCOM_AFC mask, the UART controller ignores the manual RTS setting value with the S3C2410_UMCOM_RTS_LOW bitmask, so it is not necessary to do that. Also, the upper bits of UMCON control the FIFO level to trigger the AFC and you should initialize these bits when using AFC (I've set these to 0 to use full FIFO, as it was previously).
Regarding port capability, if it's decided to validate it in s3c24xx_serial_get_mctrl() and s3c24xx_serial_set_mctrl() it should also be validated here. The question is how to validate for the full spectrum of SoCs that this driver supports?
Best regards, José Gonçalves -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
