Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Message-ID: <dbe5959d-2b68-4f16-89ee-32538aab4f34@gmx.de>
Date:   Tue, 21 Nov 2023 21:49:01 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To:     sherry.sun@nxp.com
Cc:     festevam@gmail.com, gregkh@linuxfoundation.org,
        jirislaby@kernel.org, kernel@pengutronix.de,
        linux-arm-kernel@lists.infradead.org, linux-imx@nxp.com,
        linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
        rasmus.villemoes@prevas.dk, s.hauer@pengutronix.de,
        shawnguo@kernel.org, eberhard.stoll@kontron.de
References: <AS8PR04MB8404D066C247F5B2979CBD1F92BBA@AS8PR04MB8404.eurprd04.prod.outlook.com>
Subject: RE: [PATCH] serial: imx: also enable Transmit Complete interrupt in
 rs232 mode
Content-Language: en-US
From:   Eberhard Stoll <eberhard.stoll@gmx.de>
In-Reply-To: <AS8PR04MB8404D066C247F5B2979CBD1F92BBA@AS8PR04MB8404.eurprd04.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable
UI-OutboundReport: notjunk:1;M01:P0:dcBJgA+KZ6s=;vXF1LbhEFjHoTELQPVDO66apRJD
 bQrbbjT4Sxdy/iWw51b6t5HjZj2HGYUpgZt0ywoDdlSizwKspTPRdXZ7ITibbHfPKv6sVmsaY
 Fg/GOhUuO9zJiaVJl+jM7PgexGXCdBJocWXNvQeBrk8+u7L3dyPkPn6TyB5khM7m9pWolspXW
 MaV9TCH7dGYKXj+9RjTYnjtduV9hlp+sbctzUTCovxQ7oDyBrxzCNufgaK8jR5JmECwcF1svM
 FpmTCMo8CYJmVpMxOB46jTTqYK1LKh7DBUL1mqYGaDwcSbsQaQOxN0SnwLT8J2s5t/ycl5Kwt
 Je3d3cDyE998KgvhcfSoStFBe3LzRUoZIgG736pcybRxrZpQZL+WHX5TSpw/7YmfczjjhN07x
 7ZO9jOOAREWel+xnHwBfQVpj2Eh2mPrfsoEytzJRvW8Jh0VuI7QrnIa03zfGiS2QvnbqoKsa8
 9N965MRQVw6PtfdViDCmlVjtO2/3nxHkwB46+mfp2mgyR5XbXTmaZdhZCmX5jy3Y1OHYnWl4+
 zyL79EVJXM9ZunCzKn4mDs3HFziF4h+BY3NBeBHtzHcUrTLTOUx0E+aUH067FnGvyE+K4r1uR
 6rdIpfZcA0AdsycWYKfYi9yEgRFSsVyFRyj6s/VxB1uGMSDz2lSuJ6blTfXVAMY5CDWYztbZS
 UWBjE8CB/hLOmjzxYRN9ISalJcQWjURWMIa702KNNfqmheSqcyn7mTP5UnHSXJ5BAkxgr3wlk
 eLG6orNVjtjP1F7CUIUfEIzRR1ZIIJ4ikbrk43DxK1vSHZ9AVkQcPzMXqbP4QMb0sLwIrR5jr
 PJm4DaictPdsy7nfgn0NvniJIBQOSXAd7J2Dph9XmGRMP7WuOS+M68ps4YHSlso32BZraPno3
 og3MwdV6mpcPl/qV9K4zv8umUSuzvZ+e1fkho8JU3GUctfIH6ELUYR0XqZYXObDYS8rsZZJzl
 F8Z4ASLfMhwXW69rPtsELnlMQXo=
Precedence: bulk

> 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 =3D=
=3D
> OFF)" branch. Hence nothing is actually transmitted.
>
> The problem is that in rs232 mode, ->tx_state never gets set to OFF,
> due to
>
> 	usr2 =3D 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.
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
> 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 =3D OFF;".
>
> If someone has a better suggestion, I'm all ears.


Hello Rasmus,

i can observe a very similar situation, but with a litte different
configuration. This is how i can trigger the situation very quickly:

   1) open the port
   2) send 1 byte out
   3) close the port

Do it in a loop. As faster, the lockup may occur earlier (but not
mandatory, 100ms is sufficient in my setup at 115200 Baud on an
i.mx8mm board).
With this configuration i get the lockup in around 1 minute.

For my setup it's clear what happens:

   - when the tty is closed imx_uart_shutdown() is called. This calls
     imx_uart_stop_tx()
   - for a lockup, the shifter is still busy and imx_uart_stop_tx()
     returns early (as you explained) without modifying ->tx_state.
   - imx_uart_shutdown() proceeds and finally closes the port. Due to
     imx_uart_stop_tx() is not executed completely tx_state is left in
     state ->tx_state =3D=3D SEND.
   - When the port is opened again, tx_state is SEND and nothing can
     be transmitted any more. The tx path has locked up!

Setting ->tx_state =3D SEND in imx_uart_shutdown() helps for my issue
(and should be ok IMHO).

But IMHO there is one next issue with this situation: When the port
operates with WAIT_AFTER_RTS and WAIT_AFTER_SEND then some timers
for callback functions might be active. I did not discover where they
are stopped for the case when the serial port is closed. Maybe stopping
is not required ...

I'd appreciate someone with more experience could review or revise it

Best Regards
Eberhard