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

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-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 15:21    [W:0.076 / U:0.368 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site