On Tuesday 17 of September 2013 20:08:33 José Miguel Gonçalves wrote:

On 17-09-2013 16:21, Tomasz Figa wrote:

quoted

I had the following scenario in mind:
1) enable CRTSCTS,
2) set RTS status using the IOCTL,
3) disable CRTSCTS,
4) do something,
5) enable CRTSCTS again.

I would expect that the value set in point 2 would be still valid after
point 5, while it will be reset.

Maybe I'm missing something but, as I see it, as soon as you enable AFC
in the UART controller the RTS pin is no more manually controllable, so
after point 5 the pin is set automattically by the controller according
with the Rx FIFO contents. Don't you want to say instead that the value
set in 2) will be valid in 4)?

Ahh, right, I inverted CRTSCTS bit polarity in points 1, 3 and 5. It should 
be:
1) disable CRTSCTS,
2) set RTS status using the IOCTL,
3) enable CRTSCTS,
4) do something,
5) disable CRTSCTS again.

quoted

quoted

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?

Hmm, since the driver is already broken in this aspect, ignoring this
in
your patch might be fine for now. A follow up patch fixing this would
be
welcome, though. However I don't have any good idea how to implement
this at the moment.

First thing that comes to my mind is using the variant data structures
to store information about per port capability (different port FIFO
sizes are already handled like this), but this would imply splitting
some of the groups, as S5PC100 supports modem control for different
subset of ports than S3C64xx, while they both use the same variant
data.

Using device tree, this could be passed as an extra property, but some
of the platforms using samsung serial driver can be booted without
device tree, so it wouldn't cover all the cases.

So, as I understand, for now is enough to resubmit the patch with the
UMCON register setting in s3c24xx_serial_set_termios() preserving the
previous manual setting of the RTS pin, correct? I was thinking in
something like this:

     umcon = rd_regb(port, S3C2410_UMCON);
     if (termios->c_cflag & CRTSCTS) {
         umcon |= S3C2410_UMCOM_AFC;
         umcon &= ~S3C2412_UMCON_AFC_8; // Disable RTS when RX FIFO
contains 63 bytes } else {
         umcon &= ~S3C2410_UMCOM_AFC;
     }
     wr_regl(port, S3C2410_UMCON, umcon);

Yes, this looks fine.

Best regards,
Tomasz

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