> -----Original Message-----
> From: Rasmus Villemoes <rasmus.villemoes@xxxxxxxxx>
> Sent: 2023年11月20日 21:23
> To: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Jiri Slaby
> <jirislaby@xxxxxxxxxx>; Shawn Guo <shawnguo@xxxxxxxxxx>; Sascha Hauer
> <s.hauer@xxxxxxxxxxxxxx>; Pengutronix Kernel Team
> <kernel@xxxxxxxxxxxxxx>; Fabio Estevam <festevam@xxxxxxxxx>; dl-linux-
> imx <linux-imx@xxxxxxx>
> Cc: Rasmus Villemoes <rasmus.villemoes@xxxxxxxxx>; linux-
> kernel@xxxxxxxxxxxxxxx; linux-serial@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx
> Subject: [PATCH] serial: imx: also enable Transmit Complete interrupt in
> rs232 mode
>
> Currently, if one switches to rs232 mode, writes something to the device, and
> then switches to rs485 mode, the imx_port's ->tx_state is left as SEND. This
> then prevents a subsequent write in rs485 mode from properly asserting the
> rts pin (i.e. enabling the transceiver), because imx_uart_start_rx() does not
> enter the "if (sport->tx_state == OFF)" branch. Hence nothing is actually
> transmitted.
>
> The problem is that in rs232 mode, ->tx_state never gets set to OFF, due to
>
> usr2 = imx_uart_readl(sport, USR2);
> if (!(usr2 & USR2_TXDC)) {
> /* The shifter is still busy, so retry once TC triggers */
> return;
> }
>
> in imx_uart_stop_tx(), and TC never triggers because the Transmit Complete
> interrupt is not enabled for rs232.
Hi Rasmus,
I am afraid this is not right, USR2_TXDC is just a flag, it is not affected by whether UCR4_TCEN is set or not, UCR4_TCEN only determines if USR2_TXDC flag can generate a interrupt or not.
I tried on imx8mp-evk board, for rs232, sport->tx_state can get set to OFF even we don’t set UCR4_TCEN.
Best Regards
Sherry
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@xxxxxxxxx>
> ---
> I'm not sure this is the best fix.
>
> At first I considered doing something much more targeted, but definitely also
> more hacky: In imx_uart_rs485_config(), if switching on rs485 mode, simply
> add "sport->tx_state = OFF;".
>
> If someone has a better suggestion, I'm all ears.
>
> drivers/tty/serial/imx.c | 24 +++++++++---------------
> 1 file changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index
> 7341d060f85c..ffee157e13cd 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -708,25 +708,19 @@ static void imx_uart_start_tx(struct uart_port
> *port)
> || sport->tx_state == WAIT_AFTER_RTS) {
>
> hrtimer_try_to_cancel(&sport->trigger_stop_tx);
> -
> - /*
> - * Enable transmitter and shifter empty irq only if
> DMA
> - * is off. In the DMA case this is done in the
> - * tx-callback.
> - */
> - if (!sport->dma_is_enabled) {
> - u32 ucr4 = imx_uart_readl(sport, UCR4);
> - ucr4 |= UCR4_TCEN;
> - imx_uart_writel(sport, ucr4, UCR4);
> - }
> -
> - sport->tx_state = SEND;
> }
> - } else {
> - sport->tx_state = SEND;
> }
>
> + sport->tx_state = SEND;
> +
> + /*
> + * Enable transmitter and shifter empty irq only if DMA is
> + * off. In the DMA case this is done in the tx-callback.
> + */
> if (!sport->dma_is_enabled) {
> + u32 ucr4 = imx_uart_readl(sport, UCR4);
> + ucr4 |= UCR4_TCEN;
> + imx_uart_writel(sport, ucr4, UCR4);
> ucr1 = imx_uart_readl(sport, UCR1);
> imx_uart_writel(sport, ucr1 | UCR1_TRDYEN, UCR1);
> }
> --
> 2.40.1.1.g1c60b9335d
