* [PATCH] serial: imx: fix tx statemachine deadlock @ 2023-10-04 9:57 Paul Geurts 2023-10-04 10:20 ` Uwe Kleine-König 2023-10-04 12:13 ` Greg KH 0 siblings, 2 replies; 14+ messages in thread From: Paul Geurts @ 2023-10-04 9:57 UTC (permalink / raw) To: gregkh, jirislaby, shawnguo, s.hauer, kernel, festevam, linux-imx, linux-kernel, linux-serial, linux-arm-kernel Cc: Paul Geurts When using the serial port as RS485 port, the tx statemachine is used to control the RTS pin to drive the RS485 transceiver TX_EN pin. When the TTY port is closed in the middle of a transmission (for instance during userland application crash), imx_uart_shutdown disables the interface and disables the Transmission Complete interrupt. afer that, imx_uart_stop_tx bails on an incomplete transmission, to be retriggered by the TC interrupt. This interrupt is disabled and therefore the tx statemachine never transitions out of SEND. The statemachine is in deadlock now, and the TX_EN remains low, making the interface useless. imx_uart_stop_tx now checks for incomplete transmission AND whether TC interrupts are enabled before bailing to be retriggered. This makes sure the state machine handling is reached, and is properly set to WAIT_AFTER_SEND. Signed-off-by: Paul Geurts <paul_geurts@live.nl> --- drivers/tty/serial/imx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index 13cb78340709..90a4b7841030 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -421,13 +421,13 @@ static void imx_uart_stop_tx(struct uart_port *port) ucr1 = imx_uart_readl(sport, UCR1); imx_uart_writel(sport, ucr1 & ~UCR1_TRDYEN, UCR1); + ucr4 = imx_uart_readl(sport, UCR4); usr2 = imx_uart_readl(sport, USR2); - if (!(usr2 & USR2_TXDC)) { + if ((!(usr2 & USR2_TXDC)) && (ucr4 & UCR4_TCEN)) { /* The shifter is still busy, so retry once TC triggers */ return; } - ucr4 = imx_uart_readl(sport, UCR4); ucr4 &= ~UCR4_TCEN; imx_uart_writel(sport, ucr4, UCR4); -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] serial: imx: fix tx statemachine deadlock 2023-10-04 9:57 [PATCH] serial: imx: fix tx statemachine deadlock Paul Geurts @ 2023-10-04 10:20 ` Uwe Kleine-König 2023-10-05 8:51 ` Paul Geurts 2023-10-04 12:13 ` Greg KH 1 sibling, 1 reply; 14+ messages in thread From: Uwe Kleine-König @ 2023-10-04 10:20 UTC (permalink / raw) To: Paul Geurts Cc: gregkh, jirislaby, shawnguo, s.hauer, kernel, festevam, linux-imx, linux-kernel, linux-serial, linux-arm-kernel [-- Attachment #1.1: Type: text/plain, Size: 2339 bytes --] Hello Paul, On Wed, Oct 04, 2023 at 11:57:22AM +0200, Paul Geurts wrote: > When using the serial port as RS485 port, the tx statemachine is used to > control the RTS pin to drive the RS485 transceiver TX_EN pin. When the > TTY port is closed in the middle of a transmission (for instance during > userland application crash), imx_uart_shutdown disables the interface > and disables the Transmission Complete interrupt. afer that, > imx_uart_stop_tx bails on an incomplete transmission, to be retriggered > by the TC interrupt. This interrupt is disabled and therefore the tx > statemachine never transitions out of SEND. The statemachine is in > deadlock now, and the TX_EN remains low, making the interface useless. > > imx_uart_stop_tx now checks for incomplete transmission AND whether TC > interrupts are enabled before bailing to be retriggered. This makes sure > the state machine handling is reached, and is properly set to > WAIT_AFTER_SEND. Sounds reasonable. A Fixes: line would be nice. > Signed-off-by: Paul Geurts <paul_geurts@live.nl> > --- > drivers/tty/serial/imx.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > index 13cb78340709..90a4b7841030 100644 > --- a/drivers/tty/serial/imx.c > +++ b/drivers/tty/serial/imx.c > @@ -421,13 +421,13 @@ static void imx_uart_stop_tx(struct uart_port *port) > ucr1 = imx_uart_readl(sport, UCR1); > imx_uart_writel(sport, ucr1 & ~UCR1_TRDYEN, UCR1); > > + ucr4 = imx_uart_readl(sport, UCR4); > usr2 = imx_uart_readl(sport, USR2); > - if (!(usr2 & USR2_TXDC)) { > + if ((!(usr2 & USR2_TXDC)) && (ucr4 & UCR4_TCEN)) { > /* The shifter is still busy, so retry once TC triggers */ > return; > } So the new thing is: If the hardware is still busy sending stuff but /dev/ttymxcX isn't open any more (i.e. .shutdown was called), the transmitter gets disabled. I wonder if in this case disabling the transmitter should be delayed until the shifter is empty? Or maybe this should be handled in .shutdown, that is only disable TCEN once the shifter is empty? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] serial: imx: fix tx statemachine deadlock 2023-10-04 10:20 ` Uwe Kleine-König @ 2023-10-05 8:51 ` Paul Geurts 0 siblings, 0 replies; 14+ messages in thread From: Paul Geurts @ 2023-10-05 8:51 UTC (permalink / raw) To: u.kleine-koenig Cc: festevam, gregkh, jirislaby, kernel, linux-arm-kernel, linux-imx, linux-kernel, linux-serial, paul_geurts, s.hauer, shawnguo Hi Uwe, > Sounds reasonable. > > A Fixes: line would be nice. Yes, I will add the Fixes line for sure. > So the new thing is: If the hardware is still busy sending stuff but > /dev/ttymxcX isn't open any more (i.e. .shutdown was called), the > transmitter gets disabled. I wonder if in this case disabling the > transmitter should be delayed until the shifter is empty? Or maybe this > should be handled in .shutdown, that is only disable TCEN once the > shifter is empty? Good point. I am wondering whether this would be necessary. Writing to the TTY is blocking until the shifter is done, so closing it before the shifter is done is an error condition anyway, right? So if it already is an error condition, the data is already unreliable. Making sure the shifter is empty on shutdown would mean waiting for it, or doing it asynchronously, which IMO is both not a great idea. Maybe we can just dump the buffer on shutdown but I don't know whether the IP can do that. Let me know what you think. br, Paul _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] serial: imx: fix tx statemachine deadlock 2023-10-04 9:57 [PATCH] serial: imx: fix tx statemachine deadlock Paul Geurts 2023-10-04 10:20 ` Uwe Kleine-König @ 2023-10-04 12:13 ` Greg KH 2023-11-24 12:38 ` [PATCH v2] " Paul Geurts 2023-11-24 12:53 ` Paul Geurts 1 sibling, 2 replies; 14+ messages in thread From: Greg KH @ 2023-10-04 12:13 UTC (permalink / raw) To: Paul Geurts Cc: jirislaby, shawnguo, s.hauer, kernel, festevam, linux-imx, linux-kernel, linux-serial, linux-arm-kernel On Wed, Oct 04, 2023 at 11:57:22AM +0200, Paul Geurts wrote: > When using the serial port as RS485 port, the tx statemachine is used to > control the RTS pin to drive the RS485 transceiver TX_EN pin. When the > TTY port is closed in the middle of a transmission (for instance during > userland application crash), imx_uart_shutdown disables the interface > and disables the Transmission Complete interrupt. afer that, > imx_uart_stop_tx bails on an incomplete transmission, to be retriggered > by the TC interrupt. This interrupt is disabled and therefore the tx > statemachine never transitions out of SEND. The statemachine is in > deadlock now, and the TX_EN remains low, making the interface useless. > > imx_uart_stop_tx now checks for incomplete transmission AND whether TC > interrupts are enabled before bailing to be retriggered. This makes sure > the state machine handling is reached, and is properly set to > WAIT_AFTER_SEND. > > Signed-off-by: Paul Geurts <paul_geurts@live.nl> > --- > drivers/tty/serial/imx.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) What commit id does this fix? thanks, greg k-h _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] serial: imx: fix tx statemachine deadlock 2023-10-04 12:13 ` Greg KH @ 2023-11-24 12:38 ` Paul Geurts 2023-11-24 12:45 ` Greg KH 2023-11-24 12:53 ` Paul Geurts 1 sibling, 1 reply; 14+ messages in thread From: Paul Geurts @ 2023-11-24 12:38 UTC (permalink / raw) To: gregkh, jirislaby, shawnguo, s.hauer, kernel, festevam, linux-imx, u.kleine-koenig, linux-kernel, linux-serial, linux-arm-kernel Cc: Paul Geurts When using the serial port as RS485 port, the tx statemachine is used to control the RTS pin to drive the RS485 transceiver TX_EN pin. When the TTY port is closed in the middle of a transmission (for instance during userland application crash), imx_uart_shutdown disables the interface and disables the Transmission Complete interrupt. afer that, imx_uart_stop_tx bails on an incomplete transmission, to be retriggered by the TC interrupt. This interrupt is disabled and therefore the tx statemachine never transitions out of SEND. The statemachine is in deadlock now, and the TX_EN remains low, making the interface useless. imx_uart_stop_tx now checks for incomplete transmission AND whether TC interrupts are enabled before bailing to be retriggered. This makes sure the state machine handling is reached, and is properly set to WAIT_AFTER_SEND. Fixes: cb1a60923609 serial: imx: implement rts delaying for rs485 Signed-off-by: Paul Geurts <paul_geurts@live.nl> --- drivers/tty/serial/imx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index 708b9852a575..ad36c49c7898 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -415,13 +415,13 @@ static void imx_uart_stop_tx(struct uart_port *port) ucr1 = imx_uart_readl(sport, UCR1); imx_uart_writel(sport, ucr1 & ~UCR1_TRDYEN, UCR1); + ucr4 = imx_uart_readl(sport, UCR4); usr2 = imx_uart_readl(sport, USR2); - if (!(usr2 & USR2_TXDC)) { + if ((!(usr2 & USR2_TXDC)) && (ucr4 & UCR4_TCEN)) { /* The shifter is still busy, so retry once TC triggers */ return; } - ucr4 = imx_uart_readl(sport, UCR4); ucr4 &= ~UCR4_TCEN; imx_uart_writel(sport, ucr4, UCR4); -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] serial: imx: fix tx statemachine deadlock 2023-11-24 12:38 ` [PATCH v2] " Paul Geurts @ 2023-11-24 12:45 ` Greg KH 0 siblings, 0 replies; 14+ messages in thread From: Greg KH @ 2023-11-24 12:45 UTC (permalink / raw) To: Paul Geurts Cc: jirislaby, shawnguo, s.hauer, kernel, festevam, linux-imx, u.kleine-koenig, linux-kernel, linux-serial, linux-arm-kernel On Fri, Nov 24, 2023 at 01:38:39PM +0100, Paul Geurts wrote: > When using the serial port as RS485 port, the tx statemachine is used to > control the RTS pin to drive the RS485 transceiver TX_EN pin. When the > TTY port is closed in the middle of a transmission (for instance during > userland application crash), imx_uart_shutdown disables the interface > and disables the Transmission Complete interrupt. afer that, > imx_uart_stop_tx bails on an incomplete transmission, to be retriggered > by the TC interrupt. This interrupt is disabled and therefore the tx > statemachine never transitions out of SEND. The statemachine is in > deadlock now, and the TX_EN remains low, making the interface useless. > > imx_uart_stop_tx now checks for incomplete transmission AND whether TC > interrupts are enabled before bailing to be retriggered. This makes sure > the state machine handling is reached, and is properly set to > WAIT_AFTER_SEND. > > Fixes: cb1a60923609 serial: imx: implement rts delaying for rs485 > > Signed-off-by: Paul Geurts <paul_geurts@live.nl> > --- > drivers/tty/serial/imx.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > index 708b9852a575..ad36c49c7898 100644 > --- a/drivers/tty/serial/imx.c > +++ b/drivers/tty/serial/imx.c > @@ -415,13 +415,13 @@ static void imx_uart_stop_tx(struct uart_port *port) > ucr1 = imx_uart_readl(sport, UCR1); > imx_uart_writel(sport, ucr1 & ~UCR1_TRDYEN, UCR1); > > + ucr4 = imx_uart_readl(sport, UCR4); > usr2 = imx_uart_readl(sport, USR2); > - if (!(usr2 & USR2_TXDC)) { > + if ((!(usr2 & USR2_TXDC)) && (ucr4 & UCR4_TCEN)) { > /* The shifter is still busy, so retry once TC triggers */ > return; > } > > - ucr4 = imx_uart_readl(sport, UCR4); > ucr4 &= ~UCR4_TCEN; > imx_uart_writel(sport, ucr4, UCR4); > > -- > 2.20.1 > > Hi, This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree. You are receiving this message because of the following common error(s) as indicated below: - This looks like a new version of a previously submitted patch, but you did not list below the --- line any changes from the previous version. Please read the section entitled "The canonical patch format" in the kernel file, Documentation/process/submitting-patches.rst for what needs to be done here to properly describe this. If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers. thanks, greg k-h's patch email bot _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] serial: imx: fix tx statemachine deadlock 2023-10-04 12:13 ` Greg KH 2023-11-24 12:38 ` [PATCH v2] " Paul Geurts @ 2023-11-24 12:53 ` Paul Geurts 2023-11-24 12:58 ` Fabio Estevam 1 sibling, 1 reply; 14+ messages in thread From: Paul Geurts @ 2023-11-24 12:53 UTC (permalink / raw) To: gregkh, jirislaby, shawnguo, s.hauer, kernel, festevam, linux-imx, u.kleine-koenig, linux-kernel, linux-serial, linux-arm-kernel Cc: Paul Geurts When using the serial port as RS485 port, the tx statemachine is used to control the RTS pin to drive the RS485 transceiver TX_EN pin. When the TTY port is closed in the middle of a transmission (for instance during userland application crash), imx_uart_shutdown disables the interface and disables the Transmission Complete interrupt. afer that, imx_uart_stop_tx bails on an incomplete transmission, to be retriggered by the TC interrupt. This interrupt is disabled and therefore the tx statemachine never transitions out of SEND. The statemachine is in deadlock now, and the TX_EN remains low, making the interface useless. imx_uart_stop_tx now checks for incomplete transmission AND whether TC interrupts are enabled before bailing to be retriggered. This makes sure the state machine handling is reached, and is properly set to WAIT_AFTER_SEND. Fixes: cb1a60923609 serial: imx: implement rts delaying for rs485 Signed-off-by: Paul Geurts <paul_geurts@live.nl> --- V1 -> V2: Added fixes line to the commit message drivers/tty/serial/imx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index 708b9852a575..ad36c49c7898 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -415,13 +415,13 @@ static void imx_uart_stop_tx(struct uart_port *port) ucr1 = imx_uart_readl(sport, UCR1); imx_uart_writel(sport, ucr1 & ~UCR1_TRDYEN, UCR1); + ucr4 = imx_uart_readl(sport, UCR4); usr2 = imx_uart_readl(sport, USR2); - if (!(usr2 & USR2_TXDC)) { + if ((!(usr2 & USR2_TXDC)) && (ucr4 & UCR4_TCEN)) { /* The shifter is still busy, so retry once TC triggers */ return; } - ucr4 = imx_uart_readl(sport, UCR4); ucr4 &= ~UCR4_TCEN; imx_uart_writel(sport, ucr4, UCR4); -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] serial: imx: fix tx statemachine deadlock 2023-11-24 12:53 ` Paul Geurts @ 2023-11-24 12:58 ` Fabio Estevam 2023-11-24 13:03 ` Greg KH 0 siblings, 1 reply; 14+ messages in thread From: Fabio Estevam @ 2023-11-24 12:58 UTC (permalink / raw) To: Paul Geurts Cc: gregkh, jirislaby, shawnguo, s.hauer, kernel, linux-imx, u.kleine-koenig, linux-kernel, linux-serial, linux-arm-kernel Hi Paul, On Fri, Nov 24, 2023 at 9:55 AM Paul Geurts <paul_geurts@live.nl> wrote: > > When using the serial port as RS485 port, the tx statemachine is used to > control the RTS pin to drive the RS485 transceiver TX_EN pin. When the > TTY port is closed in the middle of a transmission (for instance during > userland application crash), imx_uart_shutdown disables the interface > and disables the Transmission Complete interrupt. afer that, > imx_uart_stop_tx bails on an incomplete transmission, to be retriggered > by the TC interrupt. This interrupt is disabled and therefore the tx > statemachine never transitions out of SEND. The statemachine is in > deadlock now, and the TX_EN remains low, making the interface useless. > > imx_uart_stop_tx now checks for incomplete transmission AND whether TC > interrupts are enabled before bailing to be retriggered. This makes sure > the state machine handling is reached, and is properly set to > WAIT_AFTER_SEND. > > Fixes: cb1a60923609 serial: imx: implement rts delaying for rs485 One nit: the correct format for the Fixes tag is: Fixes: cb1a60923609 ("serial: imx: implement rts delaying for rs485") And no blank line is needed between the Fixes and Signed-off-by line. > Signed-off-by: Paul Geurts <paul_geurts@live.nl> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] serial: imx: fix tx statemachine deadlock 2023-11-24 12:58 ` Fabio Estevam @ 2023-11-24 13:03 ` Greg KH 2023-11-24 13:11 ` [PATCH v3] " Paul Geurts 0 siblings, 1 reply; 14+ messages in thread From: Greg KH @ 2023-11-24 13:03 UTC (permalink / raw) To: Fabio Estevam Cc: Paul Geurts, jirislaby, shawnguo, s.hauer, kernel, linux-imx, u.kleine-koenig, linux-kernel, linux-serial, linux-arm-kernel On Fri, Nov 24, 2023 at 09:58:19AM -0300, Fabio Estevam wrote: > Hi Paul, > > On Fri, Nov 24, 2023 at 9:55 AM Paul Geurts <paul_geurts@live.nl> wrote: > > > > When using the serial port as RS485 port, the tx statemachine is used to > > control the RTS pin to drive the RS485 transceiver TX_EN pin. When the > > TTY port is closed in the middle of a transmission (for instance during > > userland application crash), imx_uart_shutdown disables the interface > > and disables the Transmission Complete interrupt. afer that, > > imx_uart_stop_tx bails on an incomplete transmission, to be retriggered > > by the TC interrupt. This interrupt is disabled and therefore the tx > > statemachine never transitions out of SEND. The statemachine is in > > deadlock now, and the TX_EN remains low, making the interface useless. > > > > imx_uart_stop_tx now checks for incomplete transmission AND whether TC > > interrupts are enabled before bailing to be retriggered. This makes sure > > the state machine handling is reached, and is properly set to > > WAIT_AFTER_SEND. > > > > Fixes: cb1a60923609 serial: imx: implement rts delaying for rs485 > > One nit: the correct format for the Fixes tag is: > > Fixes: cb1a60923609 ("serial: imx: implement rts delaying for rs485") > > And no blank line is needed between the Fixes and Signed-off-by line. It's not really a "nit", our tools will complain if this is in the wrong format as so many things depend on this being correct. Paul, can you make a v3 with this change? And really, this should have been v3 anyway, as the difference being you added a changelog from v2 :) thanks, greg k-h _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3] serial: imx: fix tx statemachine deadlock 2023-11-24 13:03 ` Greg KH @ 2023-11-24 13:11 ` Paul Geurts 2023-11-24 13:37 ` Rasmus Villemoes 0 siblings, 1 reply; 14+ messages in thread From: Paul Geurts @ 2023-11-24 13:11 UTC (permalink / raw) To: gregkh, jirislaby, shawnguo, s.hauer, kernel, festevam, linux-imx, u.kleine-koenig, linux-kernel, linux-serial, linux-arm-kernel Cc: Paul Geurts When using the serial port as RS485 port, the tx statemachine is used to control the RTS pin to drive the RS485 transceiver TX_EN pin. When the TTY port is closed in the middle of a transmission (for instance during userland application crash), imx_uart_shutdown disables the interface and disables the Transmission Complete interrupt. afer that, imx_uart_stop_tx bails on an incomplete transmission, to be retriggered by the TC interrupt. This interrupt is disabled and therefore the tx statemachine never transitions out of SEND. The statemachine is in deadlock now, and the TX_EN remains low, making the interface useless. imx_uart_stop_tx now checks for incomplete transmission AND whether TC interrupts are enabled before bailing to be retriggered. This makes sure the state machine handling is reached, and is properly set to WAIT_AFTER_SEND. Fixes: cb1a60923609 ("serial: imx: implement rts delaying for rs485") Signed-off-by: Paul Geurts <paul_geurts@live.nl> --- V1 -> V2: Added fixes line to the commit message V2 -> V3: Fixed up the fixes line by using the correct format drivers/tty/serial/imx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index 708b9852a575..ad36c49c7898 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -415,13 +415,13 @@ static void imx_uart_stop_tx(struct uart_port *port) ucr1 = imx_uart_readl(sport, UCR1); imx_uart_writel(sport, ucr1 & ~UCR1_TRDYEN, UCR1); + ucr4 = imx_uart_readl(sport, UCR4); usr2 = imx_uart_readl(sport, USR2); - if (!(usr2 & USR2_TXDC)) { + if ((!(usr2 & USR2_TXDC)) && (ucr4 & UCR4_TCEN)) { /* The shifter is still busy, so retry once TC triggers */ return; } - ucr4 = imx_uart_readl(sport, UCR4); ucr4 &= ~UCR4_TCEN; imx_uart_writel(sport, ucr4, UCR4); -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3] serial: imx: fix tx statemachine deadlock 2023-11-24 13:11 ` [PATCH v3] " Paul Geurts @ 2023-11-24 13:37 ` Rasmus Villemoes 2023-11-24 13:55 ` Paul Geurts 2023-11-30 9:09 ` Rasmus Villemoes 0 siblings, 2 replies; 14+ messages in thread From: Rasmus Villemoes @ 2023-11-24 13:37 UTC (permalink / raw) To: Paul Geurts, gregkh, jirislaby, shawnguo, s.hauer, kernel, festevam, linux-imx, u.kleine-koenig, linux-kernel, linux-serial, linux-arm-kernel Cc: Eberhard Stoll On 24/11/2023 14.11, Paul Geurts wrote: > When using the serial port as RS485 port, the tx statemachine is used to > control the RTS pin to drive the RS485 transceiver TX_EN pin. When the > TTY port is closed in the middle of a transmission (for instance during > userland application crash), imx_uart_shutdown disables the interface > and disables the Transmission Complete interrupt. afer that, > imx_uart_stop_tx bails on an incomplete transmission, to be retriggered > by the TC interrupt. This interrupt is disabled and therefore the tx > statemachine never transitions out of SEND. The statemachine is in > deadlock now, and the TX_EN remains low, making the interface useless. > > imx_uart_stop_tx now checks for incomplete transmission AND whether TC > interrupts are enabled before bailing to be retriggered. This makes sure > the state machine handling is reached, and is properly set to > WAIT_AFTER_SEND. > > Fixes: cb1a60923609 ("serial: imx: implement rts delaying for rs485") > Signed-off-by: Paul Geurts <paul_geurts@live.nl> Hi Paul Interestingly, both Eberhard (cc'ed) and I have hit similar problems in this driver recently. See the thread https://lore.kernel.org/lkml/20231120132256.136625-1-rasmus.villemoes@prevas.dk/ . It is possible that this also fixes the problems I/we saw, but I can't get around to testing until sometime next week. Rasmus _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] serial: imx: fix tx statemachine deadlock 2023-11-24 13:37 ` Rasmus Villemoes @ 2023-11-24 13:55 ` Paul Geurts 2023-11-30 9:09 ` Rasmus Villemoes 1 sibling, 0 replies; 14+ messages in thread From: Paul Geurts @ 2023-11-24 13:55 UTC (permalink / raw) To: gregkh, jirislaby, shawnguo, s.hauer, kernel, festevam, linux-imx, u.kleine-koenig, linux-kernel, linux-serial, linux-arm-kernel, rasmus.villemoes Cc: eberhard.stoll > > When using the serial port as RS485 port, the tx statemachine is used to > > control the RTS pin to drive the RS485 transceiver TX_EN pin. When the > > TTY port is closed in the middle of a transmission (for instance during > > userland application crash), imx_uart_shutdown disables the interface > > and disables the Transmission Complete interrupt. afer that, > > imx_uart_stop_tx bails on an incomplete transmission, to be retriggered > > by the TC interrupt. This interrupt is disabled and therefore the tx > > statemachine never transitions out of SEND. The statemachine is in > > deadlock now, and the TX_EN remains low, making the interface useless. > > > > imx_uart_stop_tx now checks for incomplete transmission AND whether TC > > interrupts are enabled before bailing to be retriggered. This makes sure > > the state machine handling is reached, and is properly set to > > WAIT_AFTER_SEND. > > > > Fixes: cb1a60923609 ("serial: imx: implement rts delaying for rs485") > > Signed-off-by: Paul Geurts <paul_geurts@live.nl> > > Hi Paul > > Interestingly, both Eberhard (cc'ed) and I have hit similar problems in > this driver recently. See the thread > https://lore.kernel.org/lkml/20231120132256.136625-1-rasmus.villemoes@prevas.dk/ > . > > It is possible that this also fixes the problems I/we saw, but I can't > get around to testing until sometime next week. > > Rasmus This might very well be a similar issue with the tx statemachine. The patch you mention however pulls in the state machine behaviour into normal RS232 mode, while it was (AFAIK) specifically designed for controlling the TXEN in RS485 mode. Therefore, Whent his patch also fixes your problem, I would suggest to use this instead of the beforementioned patch. br, Paul _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] serial: imx: fix tx statemachine deadlock 2023-11-24 13:37 ` Rasmus Villemoes 2023-11-24 13:55 ` Paul Geurts @ 2023-11-30 9:09 ` Rasmus Villemoes 2023-12-01 16:55 ` Eberhard Stoll 1 sibling, 1 reply; 14+ messages in thread From: Rasmus Villemoes @ 2023-11-30 9:09 UTC (permalink / raw) To: Paul Geurts, gregkh, jirislaby, shawnguo, s.hauer, kernel, festevam, linux-imx, u.kleine-koenig, linux-kernel, linux-serial, linux-arm-kernel Cc: Eberhard Stoll On 24/11/2023 14.37, Rasmus Villemoes wrote: > On 24/11/2023 14.11, Paul Geurts wrote: >> When using the serial port as RS485 port, the tx statemachine is used to >> control the RTS pin to drive the RS485 transceiver TX_EN pin. When the >> TTY port is closed in the middle of a transmission (for instance during >> userland application crash), imx_uart_shutdown disables the interface >> and disables the Transmission Complete interrupt. afer that, >> imx_uart_stop_tx bails on an incomplete transmission, to be retriggered >> by the TC interrupt. This interrupt is disabled and therefore the tx >> statemachine never transitions out of SEND. The statemachine is in >> deadlock now, and the TX_EN remains low, making the interface useless. >> >> imx_uart_stop_tx now checks for incomplete transmission AND whether TC >> interrupts are enabled before bailing to be retriggered. This makes sure >> the state machine handling is reached, and is properly set to >> WAIT_AFTER_SEND. >> >> Fixes: cb1a60923609 ("serial: imx: implement rts delaying for rs485") >> Signed-off-by: Paul Geurts <paul_geurts@live.nl> > > Hi Paul > > Interestingly, both Eberhard (cc'ed) and I have hit similar problems in > this driver recently. See the thread > https://lore.kernel.org/lkml/20231120132256.136625-1-rasmus.villemoes@prevas.dk/ > . > > It is possible that this also fixes the problems I/we saw, but I can't > get around to testing until sometime next week. This also seems to fix the problem I had when switching to rs232 and back to rs485, and I agree that it seems to be a cleaner fix than mine. I also tried reproducing what Eberhard reported, and I think I managed to do that, and at least my way of reproducing the tx lockup also seems to be fixed by this patch. Eberhard, can you test this patch in your setup? In any case, Tested-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> Rasmus _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] serial: imx: fix tx statemachine deadlock 2023-11-30 9:09 ` Rasmus Villemoes @ 2023-12-01 16:55 ` Eberhard Stoll 0 siblings, 0 replies; 14+ messages in thread From: Eberhard Stoll @ 2023-12-01 16:55 UTC (permalink / raw) To: Rasmus Villemoes, Paul Geurts, gregkh, jirislaby, shawnguo, s.hauer, kernel, festevam, linux-imx, u.kleine-koenig, linux-kernel, linux-serial, linux-arm-kernel On 30.11.23 10:09, Rasmus Villemoes wrote: > On 24/11/2023 14.37, Rasmus Villemoes wrote: >> On 24/11/2023 14.11, Paul Geurts wrote: >>> When using the serial port as RS485 port, the tx statemachine is used to >>> control the RTS pin to drive the RS485 transceiver TX_EN pin. When the >>> TTY port is closed in the middle of a transmission (for instance during >>> userland application crash), imx_uart_shutdown disables the interface >>> and disables the Transmission Complete interrupt. afer that, >>> imx_uart_stop_tx bails on an incomplete transmission, to be retriggered >>> by the TC interrupt. This interrupt is disabled and therefore the tx >>> statemachine never transitions out of SEND. The statemachine is in >>> deadlock now, and the TX_EN remains low, making the interface useless. >>> >>> imx_uart_stop_tx now checks for incomplete transmission AND whether TC >>> interrupts are enabled before bailing to be retriggered. This makes sure >>> the state machine handling is reached, and is properly set to >>> WAIT_AFTER_SEND. >>> >>> Fixes: cb1a60923609 ("serial: imx: implement rts delaying for rs485") >>> Signed-off-by: Paul Geurts <paul_geurts@live.nl> >> >> Hi Paul >> >> Interestingly, both Eberhard (cc'ed) and I have hit similar problems in >> this driver recently. See the thread >> https://lore.kernel.org/lkml/20231120132256.136625-1-rasmus.villemoes@prevas.dk/ >> . >> >> It is possible that this also fixes the problems I/we saw, but I can't >> get around to testing until sometime next week. > > This also seems to fix the problem I had when switching to rs232 and > back to rs485, and I agree that it seems to be a cleaner fix than mine. > > I also tried reproducing what Eberhard reported, and I think I managed > to do that, and at least my way of reproducing the tx lockup also seems > to be fixed by this patch. Eberhard, can you test this patch in your setup? > > In any case, > > Tested-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> > > Rasmus > Yes, it also works for my test setup! Tested-by: Eberhard Stoll <eberhard.stoll@gmx.de> Best regards Eberhard _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-12-01 16:55 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-04 9:57 [PATCH] serial: imx: fix tx statemachine deadlock Paul Geurts 2023-10-04 10:20 ` Uwe Kleine-König 2023-10-05 8:51 ` Paul Geurts 2023-10-04 12:13 ` Greg KH 2023-11-24 12:38 ` [PATCH v2] " Paul Geurts 2023-11-24 12:45 ` Greg KH 2023-11-24 12:53 ` Paul Geurts 2023-11-24 12:58 ` Fabio Estevam 2023-11-24 13:03 ` Greg KH 2023-11-24 13:11 ` [PATCH v3] " Paul Geurts 2023-11-24 13:37 ` Rasmus Villemoes 2023-11-24 13:55 ` Paul Geurts 2023-11-30 9:09 ` Rasmus Villemoes 2023-12-01 16:55 ` Eberhard Stoll
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).