Hello,
Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> writes:
> Hello,
>
> On Fri, May 31, 2019 at 09:38:37PM +0300, Sergey Organov wrote:
>> Russell King <rmk@xxxxxxxxxxxxxxx> writes:
>> > If this is not done, data loss will occur: characters will be received
>> > from the FIFO, and the attempt to place them into the kernel buffer
>> > will fail, resulting in the characters being discarded. This would not
>> > be an effective hardware flow control implementation.
>>
>> Why? Doesn't kernel stop its receiving machinery anyway when software
>> receive buffers get filled?
>>
>> If it does, that seems logical, then it will rather stop reading from
>> FIFO, and once FIFO fills above threshold (if at all), hardware will
>> de-assert RTS by itself, no complications required.
>
> Not all hardware has a FIFO
Actually, most (all?) have. TxD + shifter = FIFO of depth 1 ;-)
> and the necessary mechanisms to auto-deassert RTS when it fills.
This is correct.
> So there must be support in software to deassert RTS, too.
So there is. The question is why it still needs to be de-asserted
manually when there is hardware support to de-assert it automatically?
Hardware is capable to de-assert RTS automatically. Why do I need to do it
manually?
I still see this as unnecessary complication of the logic of low-level
driver. (single) Top level could have been smarter to make (multiple)
low-level simpler.
Anyway, to get back to the patches, to fulfill current serial core
requirements, how about this:
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index dff75dc..ea95fe4 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -966,10 +966,17 @@ static void imx_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
if (!(port->rs485.flags & SER_RS485_ENABLED)) {
u32 ucr2;
+ /*
+ * Turn off autoRTS (UCR2_CTSC) if RTS is lowered and restore
+ * autoRTS setting if RTS is raised
+ */
ucr2 = imx_uart_readl(sport, UCR2);
ucr2 &= ~(UCR2_CTS | UCR2_CTSC);
- if (mctrl & TIOCM_RTS)
- ucr2 |= UCR2_CTS | UCR2_CTSC;
+ if (mctrl & TIOCM_RTS) {
+ ucr2 |= UCR2_CTS;
+ if (port->status & UPSTAT_AUTORTS)
+ ucr2 |= UCR2_CTSC;
+ }
imx_uart_writel(sport, ucr2, UCR2);
}
> And even in the presence of a "smart" FIFO, the software then usually
> knows earlier about having to stop the other side and this might be
> the necessary margin that makes them stop before the local buffer is
> over full.
Suppose you have 4k software buffer and 4k hardware FIFO. Do you argue
wasting 4k of hardware FIFO space by manually stopping sender earlier is
the best thing to do?
It's more logical not to read from FIFO and disable Rx IRQ once there is
no space in the software buffer. Nothing complex. Just saying.
Regards,
-- Sergey
