2017-06-20 15:47:10

by Clemens Gruber

[permalink] [raw]
Subject: [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP

DMA support for half-duplex RS-485 never worked correctly on i.MX6Q/D
due to an undiscovered SMP-related bug, instead of the real data being
sent out, the rest of the transmit buffer is sent (xmit->tail jumps over
xmit->head in imx_transmit_buffer and UART_XMIT_SIZE bytes are sent out)
More details: https://lkml.org/lkml/2017/1/4/579

Disable it for that configuration until the bug is found and fixed.

We need to know at probe time if we can enable DMA. (RS-485 could be
enabled after that). Let's therefore only enable DMA if it is not an
i.MX6Q/D UART with uart-has-rtscts in the DT and CONFIG_SMP enabled.

Fixes: 17b8f2a3fdca ("serial: imx: add support for half duplex rs485")
Cc: <[email protected]>
Signed-off-by: Clemens Gruber <[email protected]>
---
drivers/tty/serial/imx.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index bbefddd92bfe..000949f686a4 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1277,8 +1277,14 @@ static int imx_startup(struct uart_port *port)

writel(temp & ~UCR4_DREN, sport->port.membase + UCR4);

- /* Can we enable the DMA support? */
- if (!uart_console(port) && !sport->dma_is_inited)
+ /*
+ * Can we enable the DMA support?
+ * RS-485 DMA is broken on i.MX6D/Q SMP systems. Do not use DMA on
+ * UARTs with RTS/CTS to prevent misbehavior until the bug is fixed.
+ */
+ if (!uart_console(port) && !sport->dma_is_inited &&
+ !(sport->have_rtscts && is_imx6q_uart(sport) &&
+ IS_ENABLED(CONFIG_SMP)))
imx_uart_dma_init(sport);

spin_lock_irqsave(&sport->port.lock, flags);
--
2.13.1


2017-06-20 16:13:21

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP

Hi Clemens,

On Tue, Jun 20, 2017 at 12:37 PM, Clemens Gruber
<[email protected]> wrote:
> DMA support for half-duplex RS-485 never worked correctly on i.MX6Q/D
> due to an undiscovered SMP-related bug, instead of the real data being
> sent out, the rest of the transmit buffer is sent (xmit->tail jumps over
> xmit->head in imx_transmit_buffer and UART_XMIT_SIZE bytes are sent out)
> More details: https://lkml.org/lkml/2017/1/4/579
>
> Disable it for that configuration until the bug is found and fixed.
>
> We need to know at probe time if we can enable DMA. (RS-485 could be
> enabled after that). Let's therefore only enable DMA if it is not an
> i.MX6Q/D UART with uart-has-rtscts in the DT and CONFIG_SMP enabled.
>
> Fixes: 17b8f2a3fdca ("serial: imx: add support for half duplex rs485")
> Cc: <[email protected]>
> Signed-off-by: Clemens Gruber <[email protected]>
> ---
> drivers/tty/serial/imx.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index bbefddd92bfe..000949f686a4 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -1277,8 +1277,14 @@ static int imx_startup(struct uart_port *port)
>
> writel(temp & ~UCR4_DREN, sport->port.membase + UCR4);
>
> - /* Can we enable the DMA support? */
> - if (!uart_console(port) && !sport->dma_is_inited)
> + /*
> + * Can we enable the DMA support?
> + * RS-485 DMA is broken on i.MX6D/Q SMP systems. Do not use DMA on
> + * UARTs with RTS/CTS to prevent misbehavior until the bug is fixed.
> + */
> + if (!uart_console(port) && !sport->dma_is_inited &&
> + !(sport->have_rtscts && is_imx6q_uart(sport) &&
> + IS_ENABLED(CONFIG_SMP)))
> imx_uart_dma_init(sport);

The subject gives the impression that the DMA will only be disabled
for RS485, but the impact of this change is wider.

For example: if I have a mx6q system with a Bluetooth serial
connection I can no longer use DMA with your change applied.

Ideally we should fix the RS485 DMA bug. If that is not possible, then
at least we need to restrict this change to the RS485 case.

Maybe we need to pass "linux,rs485-enabled-at-boot-time" in device
tree and then use this property to deceide if DMA will be enabled or
not:

if (!uart_console(port) && !sport->dma_is_inited && !sport->rs485_enabled)

2017-06-20 23:49:34

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP

Hi Clemens,

On Tue, Jun 20, 2017 at 1:13 PM, Fabio Estevam <[email protected]> wrote:

> The subject gives the impression that the DMA will only be disabled
> for RS485, but the impact of this change is wider.
>
> For example: if I have a mx6q system with a Bluetooth serial
> connection I can no longer use DMA with your change applied.
>
> Ideally we should fix the RS485 DMA bug. If that is not possible, then
> at least we need to restrict this change to the RS485 case.
>
> Maybe we need to pass "linux,rs485-enabled-at-boot-time" in device
> tree and then use this property to deceide if DMA will be enabled or
> not:
>
> if (!uart_console(port) && !sport->dma_is_inited && !sport->rs485_enabled)

Could you please test the two attached patches and see if it solves the issue?

Unfortunately I no longer have access to the RS485 half-duplex board.

Just make sure to pass 'linux,rs485-enabled-at-boot-time' in your
device tree, thanks.


Attachments:
0001-serial-imx-Allow-passing-linux-rs485-enabled-at-boot.patch (1.14 kB)
0002-serial-imx-Disable-DMA-for-RS485.patch (2.19 kB)
Download all attachments

2017-06-21 14:06:02

by Clemens Gruber

[permalink] [raw]
Subject: Re: [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP

Hi Fabio,

On Tue, Jun 20, 2017 at 01:13:18PM -0300, Fabio Estevam wrote:
> The subject gives the impression that the DMA will only be disabled
> for RS485, but the impact of this change is wider.
>
> For example: if I have a mx6q system with a Bluetooth serial
> connection I can no longer use DMA with your change applied.
>
> Ideally we should fix the RS485 DMA bug. If that is not possible, then
> at least we need to restrict this change to the RS485 case.
>
> Maybe we need to pass "linux,rs485-enabled-at-boot-time" in device
> tree and then use this property to deceide if DMA will be enabled or
> not:
>
> if (!uart_console(port) && !sport->dma_is_inited && !sport->rs485_enabled)

I'd also prefer fixing the underlying problem.
If you take a look at the DMA parts in drivers/tty/serial/imx.c, can you
spot anything SMP-unsafe?
By the way, can you get your hands on an i.MX6Q board with RS-485 to
reproduce it?

We could try for a few more weeks to find the bug and if we don't find
it, adding this rs485-enabled-at-boot-time property sounds good!

Thanks,
Clemens

2017-06-21 14:12:26

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP

Hi Clemens,

On Wed, Jun 21, 2017 at 11:05 AM, Clemens Gruber
<[email protected]> wrote:
>
> I'd also prefer fixing the underlying problem.

Yes, that would be much better.

> If you take a look at the DMA parts in drivers/tty/serial/imx.c, can you
> spot anything SMP-unsafe?

Not really.

> By the way, can you get your hands on an i.MX6Q board with RS-485 to
> reproduce it?

Unfortunately I don't have such hardware. Even the mx6solo board that
I used to have access in the past I do not have it anymore.

> We could try for a few more weeks to find the bug and if we don't find
> it, adding this rs485-enabled-at-boot-time property sounds good!

Sounds like a good plan, thanks.

2017-06-23 09:36:55

by Schenk, Gavin

[permalink] [raw]
Subject: AW: [Customers.Eckelmann] [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP

Hi,

>
> DMA support for half-duplex RS-485 never worked correctly on i.MX6Q/D
> due to an undiscovered SMP-related bug, instead of the real data being
> sent out, the rest of the transmit buffer is sent (xmit->tail jumps over
> xmit->head in imx_transmit_buffer and UART_XMIT_SIZE bytes are sent out)
> More details: https://lkml.org/lkml/2017/1/4/579
>

We see exactly this issue on our platform, based on i.MX6dl. We use the RS485 mode to implement a Modbus interface. We have another platform based on i.MX25 running the same Linux 4.9.30 code that is not affected!

> Disable it for that configuration until the bug is found and fixed.
>
> We need to know at probe time if we can enable DMA. (RS-485 could be
> enabled after that). Let's therefore only enable DMA if it is not an
> i.MX6Q/D UART with uart-has-rtscts in the DT and CONFIG_SMP enabled.
>

Uwe Kleine-König had the idea to disable DMA on the i.MX6dl based platform via dts and this is our current workaround.

Regards
Gavin Schenk

Eckelmann AG
Vorstand: Dipl.-Ing. Peter Frankenbach (Sprecher) Dipl.-Wi.-Ing. Philipp Eckelmann
Dr.-Ing. Marco Münchhof Dr.-Ing. Frank Uhlemann
Vorsitzender des Aufsichtsrats: Hubertus G. Krossa
Stv. Vorsitzender des Aufsichtsrats: Dr.-Ing. Gerd Eckelmann
Sitz der Gesellschaft: Berliner Str. 161, 65205 Wiesbaden, Amtsgericht Wiesbaden HRB 12636
http://www.eckelmann.de


2017-06-30 12:15:35

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP

Hi Clemens,

On Wed, Jun 21, 2017 at 11:12 AM, Fabio Estevam <[email protected]> wrote:

>> I'd also prefer fixing the underlying problem.
>
> Yes, that would be much better.

Just saw Romain's patch series that addresses several imx uart DMA issues:
http://lists.infradead.org/pipermail/linux-arm-kernel/2017-June/516845.html

If you have a chance, please give it a try to see if it helps on the
RS485 DMA case.

Regards,

Fabio Estevam

2017-07-02 20:17:21

by Clemens Gruber

[permalink] [raw]
Subject: Re: [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP

Hi Fabio, Hi Romain,

On Fri, Jun 30, 2017 at 09:15:31AM -0300, Fabio Estevam wrote:
> Hi Clemens,
>
> On Wed, Jun 21, 2017 at 11:12 AM, Fabio Estevam <[email protected]> wrote:
>
> >> I'd also prefer fixing the underlying problem.
> >
> > Yes, that would be much better.
>
> Just saw Romain's patch series that addresses several imx uart DMA issues:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-June/516845.html
>
> If you have a chance, please give it a try to see if it helps on the
> RS485 DMA case.

Thanks, I just finished my first tests with Romain's patch series:

It looks like these patches fixed some of the bugs, causing this
behavior, but not all: The behavior changed, the rest of the circular
buffer is no longer sent out as seen in the previous bug report
(https://pqgruber.com/rs485_results.png)

But now, with the patch series applied, if I transmit "Test", the logic
analyzer records the following:
https://pqgruber.com/rs485txtest.png

Most of the time it looked like this (T e T e s s t t LF LF), but in a
few cases I observed another pattern (T e T s e s t t LF LF) when
transmitting "Test" by calling echo Test > /dev/ttymxc4.

If I do a echo A > /dev/ttymxc4 as in my first bug report, now I always
see the pattern A LF A LF on the TX line, but no longer A LF A LF 0 0 ..

Interestingly, the bug does not appear the first time I try echo A after
a reboot.

Romain: What board did you use to test your patch series?
The RS485 bug, I reported, only appears on i.MX6D and i.MX6Q, but not on
single-core / non-SMP systems. Would be great if you could reproduce it!

Regards,
Clemens

2017-07-02 20:47:36

by Clemens Gruber

[permalink] [raw]
Subject: Re: serial: imx: disable DMA for RS-485 on i.MX6 SMP

Hi Fabio,

On Tue, Jun 20, 2017 at 08:49:28PM -0300, Fabio Estevam wrote:
> Hi Clemens,
>
> On Tue, Jun 20, 2017 at 1:13 PM, Fabio Estevam <[email protected]> wrote:
>
> > The subject gives the impression that the DMA will only be disabled
> > for RS485, but the impact of this change is wider.
> >
> > For example: if I have a mx6q system with a Bluetooth serial
> > connection I can no longer use DMA with your change applied.
> >
> > Ideally we should fix the RS485 DMA bug. If that is not possible, then
> > at least we need to restrict this change to the RS485 case.
> >
> > Maybe we need to pass "linux,rs485-enabled-at-boot-time" in device
> > tree and then use this property to deceide if DMA will be enabled or
> > not:
> >
> > if (!uart_console(port) && !sport->dma_is_inited && !sport->rs485_enabled)
>
> Could you please test the two attached patches and see if it solves the issue?
>
> Unfortunately I no longer have access to the RS485 half-duplex board.
>
> Just make sure to pass 'linux,rs485-enabled-at-boot-time' in your
> device tree, thanks.

The two patches work for me!

You can add my Tested-by tag if/when you submit them.

Tested-by: Clemens Gruber <[email protected]>

Regards,
Clemens

2017-07-02 23:09:36

by Fabio Estevam

[permalink] [raw]
Subject: Re: serial: imx: disable DMA for RS-485 on i.MX6 SMP

Hi Clemens,

On Sun, Jul 2, 2017 at 5:47 PM, Clemens Gruber
<[email protected]> wrote:

> The two patches work for me!
>
> You can add my Tested-by tag if/when you submit them.
>
> Tested-by: Clemens Gruber <[email protected]>

Thanks for testing.

I will wait a bit longer to see if we can fix DMA usage with RS485.

Regards,

Fabio Estevam

2017-07-17 11:35:00

by Fabio Estevam

[permalink] [raw]
Subject: Re: [Customers.Eckelmann] [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP

Hi Gavin,

On Fri, Jun 23, 2017 at 6:26 AM, Schenk, Gavin <[email protected]> wrote:

> Uwe Kleine-König had the idea to disable DMA on the i.MX6dl based platform via dts and this is our current workaround.

Please test Ian's patch:
http://marc.info/?l=linux-serial&m=150005865213531&w=2

It should fix this DMA problem with RS485.