Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> writes: > On Fri, May 31, 2019 at 07:52:51AM +0300, Sergey Organov wrote: >> Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> writes: >> >> > On Thu, May 30, 2019 at 06:29:43PM +0300, Sergey Organov wrote: >> >> imx_set_mctrl() had TIOCM_DTR meaning inverted >> >> >> >> Signed-off-by: Sergey Organov <sorganov@xxxxxxxxx> >> >> --- >> >> drivers/tty/serial/imx.c | 2 +- >> >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> >> >> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c >> >> index dff75dc..e9e812a 100644 >> >> --- a/drivers/tty/serial/imx.c >> >> +++ b/drivers/tty/serial/imx.c >> >> @@ -974,7 +974,7 @@ static void imx_uart_set_mctrl(struct uart_port *port, unsigned int mctrl) >> >> } >> >> >> >> ucr3 = imx_uart_readl(sport, UCR3) & ~UCR3_DSR; >> >> - if (!(mctrl & TIOCM_DTR)) >> >> + if (mctrl & TIOCM_DTR) >> >> ucr3 |= UCR3_DSR; >> >> imx_uart_writel(sport, ucr3, UCR3); >> > >> > I'm not sure this is right and your commit log is too short to convince >> > me otherwise. >> > >> > In the past I had several customers that used handshaking on an imx UART >> > so I'd be surprised if such a bug would have stayed unnoticed until now. >> > >> > The i.MX25 Reference manual states: >> > >> > This bit [UCR3_DSR] used by software to control the DSR/DTR output pin >> > for the modem interface. In DCE mode it applies to DSR and in DTE mode >> > it applies to DTR. >> > >> > 0 DSR/ DTR pin is logic zero >> > 1 DSR/ DTR pin is logic one >> > >> > >> > Semantically if TIOCM_DTR is set in .set_mctrl, the DTR output should >> > become active (i.e. low). Without testing I'm not sure if "active" >> > corresponds to "logic one" which would make your patch correct. >> >> Yeah, I was not sure myself, and this pin is not wired on my board, so >> in fact I can't check either. >> >> Unfortunately, the language in the manual is inconsistent. For CTS/ RTS >> is clearly says: >> >> 0 The CTS_B pin is high (inactive) >> 1 The CTS_B pin is low (active) > > Ack. > > @NXP: Maybe you can pick up this and improve the documentation here. > >> and the code in the driver is: >> >> if (mctrl & TIOCM_RTS) >> ucr2 |= UCR2_CTS; >> >> that works correct. >> >> My best reasoning was that DSR/ DTR is likely implemented the same as >> CTS/ RTS in the metal, and I found other drivers where both RTS and DSR >> are inverted, so I guessed it could be a remnant of old copy-paste. > > This is not a good enough reason to "fix" that. Yeah, I agree. I rather mostly kept it in the series not to forget about the issue. I should have said that in the comments, sorry. -- Sergey
