* [PATCH] serial: imx: fix TX stop function not setting state to OFF in RS232-mode
@ 2021-04-02 11:05 Robert Hodaszi
2021-04-10 8:33 ` Greg Kroah-Hartman
0 siblings, 1 reply; 3+ messages in thread
From: Robert Hodaszi @ 2021-04-02 11:05 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, linux-serial, Uwe Kleine-König, Hodaszi,
Robert
From: Robert Hodaszi <robert.hodaszi@digi.com>
If the mode had been changed to RS-485 after at least one character had
been sent in RS-232 mode, the RS-485 transmission was not working.
Commit cb1a609236096c278ecbfb7be678a693a70283f1 ("serial: imx: implement
rts delaying for rs485") added a TX state variable to keep track, when
it needs to enable / disable the RS-485 transmitter.
In RS-232 mode, the start TX function just sets the state to SEND, and
the stop function supposed to set it to OFF.
In RS-485 mode, the start TX function expects the state to be either
OFF, WAIT_AFTER_SEND, or WAIT_AFTER RTS. It cannot do anything if the
state is set to SEND, expects a stop first.
But stop TX function in RS-232 mode usually didn't set the state to OFF,
as it first checked if the shifter is empty, and if not, it just
returned, waiting for a TransmitComplete interrupt, which is only
enabled in RS-485 mode. So the stop function was never called again.
That check, and the subsequent code part is not needed for RS-232, it
just have to set the TX state to OFF.
Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com>
---
drivers/tty/serial/imx.c | 46 +++++++++++++++++++++-------------------
1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 8257597d034d..511badce3edd 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -443,6 +443,12 @@ static void imx_uart_stop_tx(struct uart_port *port)
ucr1 = imx_uart_readl(sport, UCR1);
imx_uart_writel(sport, ucr1 & ~UCR1_TRDYEN, UCR1);
+ if (!(port->rs485.flags & SER_RS485_ENABLED)) {
+ /* Set the TX state in non-RS485 mode, nothing else to do */
+ sport->tx_state = OFF;
+ return;
+ }
+
usr2 = imx_uart_readl(sport, USR2);
if (!(usr2 & USR2_TXDC)) {
/* The shifter is still busy, so retry once TC triggers */
@@ -453,33 +459,29 @@ static void imx_uart_stop_tx(struct uart_port *port)
ucr4 &= ~UCR4_TCEN;
imx_uart_writel(sport, ucr4, UCR4);
- /* in rs485 mode disable transmitter */
- if (port->rs485.flags & SER_RS485_ENABLED) {
- if (sport->tx_state == SEND) {
- sport->tx_state = WAIT_AFTER_SEND;
- start_hrtimer_ms(&sport->trigger_stop_tx,
- port->rs485.delay_rts_after_send);
- return;
- }
+ if (sport->tx_state == SEND) {
+ sport->tx_state = WAIT_AFTER_SEND;
+ start_hrtimer_ms(&sport->trigger_stop_tx,
+ port->rs485.delay_rts_after_send);
+ return;
+ }
- if (sport->tx_state == WAIT_AFTER_RTS ||
- sport->tx_state == WAIT_AFTER_SEND) {
- u32 ucr2;
+ if (sport->tx_state == WAIT_AFTER_RTS ||
+ sport->tx_state == WAIT_AFTER_SEND) {
+ /* in rs485 mode disable transmitter */
+ u32 ucr2;
- hrtimer_try_to_cancel(&sport->trigger_start_tx);
+ hrtimer_try_to_cancel(&sport->trigger_start_tx);
- ucr2 = imx_uart_readl(sport, UCR2);
- if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
- imx_uart_rts_active(sport, &ucr2);
- else
- imx_uart_rts_inactive(sport, &ucr2);
- imx_uart_writel(sport, ucr2, UCR2);
+ ucr2 = imx_uart_readl(sport, UCR2);
+ if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
+ imx_uart_rts_active(sport, &ucr2);
+ else
+ imx_uart_rts_inactive(sport, &ucr2);
+ imx_uart_writel(sport, ucr2, UCR2);
- imx_uart_start_rx(port);
+ imx_uart_start_rx(port);
- sport->tx_state = OFF;
- }
- } else {
sport->tx_state = OFF;
}
}
--
2.27.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] serial: imx: fix TX stop function not setting state to OFF in RS232-mode
2021-04-02 11:05 [PATCH] serial: imx: fix TX stop function not setting state to OFF in RS232-mode Robert Hodaszi
@ 2021-04-10 8:33 ` Greg Kroah-Hartman
2021-04-20 19:07 ` Robert Hodaszi
0 siblings, 1 reply; 3+ messages in thread
From: Greg Kroah-Hartman @ 2021-04-10 8:33 UTC (permalink / raw)
To: Robert Hodaszi
Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, linux-serial, Uwe Kleine-König
On Fri, Apr 02, 2021 at 01:05:48PM +0200, Robert Hodaszi wrote:
> From: Robert Hodaszi <robert.hodaszi@digi.com>
>
> If the mode had been changed to RS-485 after at least one character had
> been sent in RS-232 mode, the RS-485 transmission was not working.
>
> Commit cb1a609236096c278ecbfb7be678a693a70283f1 ("serial: imx: implement
> rts delaying for rs485") added a TX state variable to keep track, when
> it needs to enable / disable the RS-485 transmitter.
>
> In RS-232 mode, the start TX function just sets the state to SEND, and
> the stop function supposed to set it to OFF.
>
> In RS-485 mode, the start TX function expects the state to be either
> OFF, WAIT_AFTER_SEND, or WAIT_AFTER RTS. It cannot do anything if the
> state is set to SEND, expects a stop first.
>
> But stop TX function in RS-232 mode usually didn't set the state to OFF,
> as it first checked if the shifter is empty, and if not, it just
> returned, waiting for a TransmitComplete interrupt, which is only
> enabled in RS-485 mode. So the stop function was never called again.
>
> That check, and the subsequent code part is not needed for RS-232, it
> just have to set the TX state to OFF.
>
> Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com>
> ---
> drivers/tty/serial/imx.c | 46 +++++++++++++++++++++-------------------
> 1 file changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 8257597d034d..511badce3edd 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -443,6 +443,12 @@ static void imx_uart_stop_tx(struct uart_port *port)
> ucr1 = imx_uart_readl(sport, UCR1);
> imx_uart_writel(sport, ucr1 & ~UCR1_TRDYEN, UCR1);
>
> + if (!(port->rs485.flags & SER_RS485_ENABLED)) {
> + /* Set the TX state in non-RS485 mode, nothing else to do */
> + sport->tx_state = OFF;
> + return;
> + }
> +
> usr2 = imx_uart_readl(sport, USR2);
> if (!(usr2 & USR2_TXDC)) {
> /* The shifter is still busy, so retry once TC triggers */
> @@ -453,33 +459,29 @@ static void imx_uart_stop_tx(struct uart_port *port)
> ucr4 &= ~UCR4_TCEN;
> imx_uart_writel(sport, ucr4, UCR4);
>
> - /* in rs485 mode disable transmitter */
> - if (port->rs485.flags & SER_RS485_ENABLED) {
> - if (sport->tx_state == SEND) {
> - sport->tx_state = WAIT_AFTER_SEND;
> - start_hrtimer_ms(&sport->trigger_stop_tx,
> - port->rs485.delay_rts_after_send);
> - return;
> - }
> + if (sport->tx_state == SEND) {
> + sport->tx_state = WAIT_AFTER_SEND;
> + start_hrtimer_ms(&sport->trigger_stop_tx,
> + port->rs485.delay_rts_after_send);
> + return;
> + }
>
> - if (sport->tx_state == WAIT_AFTER_RTS ||
> - sport->tx_state == WAIT_AFTER_SEND) {
> - u32 ucr2;
> + if (sport->tx_state == WAIT_AFTER_RTS ||
> + sport->tx_state == WAIT_AFTER_SEND) {
> + /* in rs485 mode disable transmitter */
> + u32 ucr2;
>
> - hrtimer_try_to_cancel(&sport->trigger_start_tx);
> + hrtimer_try_to_cancel(&sport->trigger_start_tx);
>
> - ucr2 = imx_uart_readl(sport, UCR2);
> - if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> - imx_uart_rts_active(sport, &ucr2);
> - else
> - imx_uart_rts_inactive(sport, &ucr2);
> - imx_uart_writel(sport, ucr2, UCR2);
> + ucr2 = imx_uart_readl(sport, UCR2);
> + if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> + imx_uart_rts_active(sport, &ucr2);
> + else
> + imx_uart_rts_inactive(sport, &ucr2);
> + imx_uart_writel(sport, ucr2, UCR2);
>
> - imx_uart_start_rx(port);
> + imx_uart_start_rx(port);
>
> - sport->tx_state = OFF;
> - }
> - } else {
> sport->tx_state = OFF;
> }
> }
> --
> 2.27.0
Your patch is corrupted, all the tabs are turned to spaces and it can
not be applied :(
Please fix up and resend.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] serial: imx: fix TX stop function not setting state to OFF in RS232-mode
2021-04-10 8:33 ` Greg Kroah-Hartman
@ 2021-04-20 19:07 ` Robert Hodaszi
0 siblings, 0 replies; 3+ messages in thread
From: Robert Hodaszi @ 2021-04-20 19:07 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, linux-serial, Uwe Kleine-König
On 2021. 04. 10. 10:33, Greg Kroah-Hartman wrote:
> [EXTERNAL EMAIL]
>
> On Fri, Apr 02, 2021 at 01:05:48PM +0200, Robert Hodaszi wrote:
>> From: Robert Hodaszi <robert.hodaszi@digi.com>
>>
>> If the mode had been changed to RS-485 after at least one character had
>> been sent in RS-232 mode, the RS-485 transmission was not working.
>>
>> Commit cb1a609236096c278ecbfb7be678a693a70283f1 ("serial: imx: implement
>> rts delaying for rs485") added a TX state variable to keep track, when
>> it needs to enable / disable the RS-485 transmitter.
>>
>> In RS-232 mode, the start TX function just sets the state to SEND, and
>> the stop function supposed to set it to OFF.
>>
>> In RS-485 mode, the start TX function expects the state to be either
>> OFF, WAIT_AFTER_SEND, or WAIT_AFTER RTS. It cannot do anything if the
>> state is set to SEND, expects a stop first.
>>
>> But stop TX function in RS-232 mode usually didn't set the state to OFF,
>> as it first checked if the shifter is empty, and if not, it just
>> returned, waiting for a TransmitComplete interrupt, which is only
>> enabled in RS-485 mode. So the stop function was never called again.
>>
>> That check, and the subsequent code part is not needed for RS-232, it
>> just have to set the TX state to OFF.
>>
>> Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com>
>> ---
>> drivers/tty/serial/imx.c | 46 +++++++++++++++++++++-------------------
>> 1 file changed, 24 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> index 8257597d034d..511badce3edd 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -443,6 +443,12 @@ static void imx_uart_stop_tx(struct uart_port *port)
>> ucr1 = imx_uart_readl(sport, UCR1);
>> imx_uart_writel(sport, ucr1 & ~UCR1_TRDYEN, UCR1);
>>
>> + if (!(port->rs485.flags & SER_RS485_ENABLED)) {
>> + /* Set the TX state in non-RS485 mode, nothing else to do */
>> + sport->tx_state = OFF;
>> + return;
>> + }
>> +
>> usr2 = imx_uart_readl(sport, USR2);
>> if (!(usr2 & USR2_TXDC)) {
>> /* The shifter is still busy, so retry once TC triggers */
>> @@ -453,33 +459,29 @@ static void imx_uart_stop_tx(struct uart_port *port)
>> ucr4 &= ~UCR4_TCEN;
>> imx_uart_writel(sport, ucr4, UCR4);
>>
>> - /* in rs485 mode disable transmitter */
>> - if (port->rs485.flags & SER_RS485_ENABLED) {
>> - if (sport->tx_state == SEND) {
>> - sport->tx_state = WAIT_AFTER_SEND;
>> - start_hrtimer_ms(&sport->trigger_stop_tx,
>> - port->rs485.delay_rts_after_send);
>> - return;
>> - }
>> + if (sport->tx_state == SEND) {
>> + sport->tx_state = WAIT_AFTER_SEND;
>> + start_hrtimer_ms(&sport->trigger_stop_tx,
>> + port->rs485.delay_rts_after_send);
>> + return;
>> + }
>>
>> - if (sport->tx_state == WAIT_AFTER_RTS ||
>> - sport->tx_state == WAIT_AFTER_SEND) {
>> - u32 ucr2;
>> + if (sport->tx_state == WAIT_AFTER_RTS ||
>> + sport->tx_state == WAIT_AFTER_SEND) {
>> + /* in rs485 mode disable transmitter */
>> + u32 ucr2;
>>
>> - hrtimer_try_to_cancel(&sport->trigger_start_tx);
>> + hrtimer_try_to_cancel(&sport->trigger_start_tx);
>>
>> - ucr2 = imx_uart_readl(sport, UCR2);
>> - if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
>> - imx_uart_rts_active(sport, &ucr2);
>> - else
>> - imx_uart_rts_inactive(sport, &ucr2);
>> - imx_uart_writel(sport, ucr2, UCR2);
>> + ucr2 = imx_uart_readl(sport, UCR2);
>> + if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
>> + imx_uart_rts_active(sport, &ucr2);
>> + else
>> + imx_uart_rts_inactive(sport, &ucr2);
>> + imx_uart_writel(sport, ucr2, UCR2);
>>
>> - imx_uart_start_rx(port);
>> + imx_uart_start_rx(port);
>>
>> - sport->tx_state = OFF;
>> - }
>> - } else {
>> sport->tx_state = OFF;
>> }
>> }
>> --
>> 2.27.0
> Your patch is corrupted, all the tabs are turned to spaces and it can
> not be applied :(
>
> Please fix up and resend.
>
> thanks,
>
> greg k-h
Sorry, for that (and for the slow reply)! My latest Thunderbird setup
seems not playing very. Went back to my older setup, and sent out a v2.
Hopefully, that will be OK.
Thanks,
Robert Hodaszi
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-04-20 19:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-02 11:05 [PATCH] serial: imx: fix TX stop function not setting state to OFF in RS232-mode Robert Hodaszi
2021-04-10 8:33 ` Greg Kroah-Hartman
2021-04-20 19:07 ` Robert Hodaszi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.