lkml.org 
[lkml]   [2013]   [Sep]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] serial: samsung: add support for manual RTS setting
Date
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@inov.pt>
> ---
> 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.

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

Best regards,
Tomasz

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2013-09-17 12:41    [W:0.095 / U:0.548 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site