Re: [PATCH] serial: samsung: add support for manual RTS setting

From: José Miguel Gonçalves
Date: Tue Sep 17 2013 - 15:09:17 EST


On 17-09-2013 16:21, Tomasz Figa wrote:
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)?


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);

Best regards,
Tomasz

P.S. Please remember to add all the relevant people to Cc when sending
patches. You can use scripts/get_maintainer.pl to find them.

OK, sorry for that.

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