Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1670899 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP Clemens Gruber <clemens.gruber@pqgruber.com> - 2017-06-20 17:50 +0200
    Re: [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP Fabio Estevam <festevam@gmail.com> - 2017-06-20 18:20 +0200
      Re: [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP Fabio Estevam <festevam@gmail.com> - 2017-06-21 01:50 +0200
        Re: serial: imx: disable DMA for RS-485 on i.MX6 SMP Clemens Gruber <clemens.gruber@pqgruber.com> - 2017-07-02 22:50 +0200
          Re: serial: imx: disable DMA for RS-485 on i.MX6 SMP Fabio Estevam <festevam@gmail.com> - 2017-07-03 01:10 +0200
      Re: [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP Clemens Gruber <clemens.gruber@pqgruber.com> - 2017-06-21 16:10 +0200
        Re: [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP Fabio Estevam <festevam@gmail.com> - 2017-06-21 16:20 +0200
          Re: [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP Fabio Estevam <festevam@gmail.com> - 2017-06-30 14:20 +0200
            Re: [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP Clemens Gruber <clemens.gruber@pqgruber.com> - 2017-07-02 22:20 +0200
    AW: [Customers.Eckelmann] [PATCH] serial: imx: disable DMA for  RS-485 on i.MX6 SMP "Schenk, Gavin" <G.Schenk@eckelmann.de> - 2017-06-23 11:40 +0200

#1670899 — [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: <stable@vger.kernel.org>
Signed-off-by: Clemens Gruber <clemens.gruber@pqgruber.com>
---
 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

[toc] | [next] | [standalone]

#1670921

Hi Clemens,

On Tue, Jun 20, 2017 at 12:37 PM, Clemens Gruber
<clemens.gruber@pqgruber.com> 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: <stable@vger.kernel.org>
> Signed-off-by: Clemens Gruber <clemens.gruber@pqgruber.com>
> ---
>  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)

[toc] | [prev] | [next] | [standalone]

#1671242

[Multipart message — attachments visible in raw view] — view raw

Hi Clemens,

On Tue, Jun 20, 2017 at 1:13 PM, Fabio Estevam <festevam@gmail.com> 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.

[toc] | [prev] | [next] | [standalone]

#1679559 — 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 <festevam@gmail.com> 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 <clemens.gruber@pqgruber.com>

Regards,
Clemens

[toc] | [prev] | [next] | [standalone]

#1679579 — 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
<clemens.gruber@pqgruber.com> wrote:

> The two patches work for me!
>
> You can add my Tested-by tag if/when you submit them.
>
> Tested-by: Clemens Gruber <clemens.gruber@pqgruber.com>

Thanks for testing.

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

Regards,

Fabio Estevam

[toc] | [prev] | [next] | [standalone]

#1671692

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

[toc] | [prev] | [next] | [standalone]

#1671700

Hi Clemens,

On Wed, Jun 21, 2017 at 11:05 AM, Clemens Gruber
<clemens.gruber@pqgruber.com> 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.

[toc] | [prev] | [next] | [standalone]

#1678752

Hi Clemens,

On Wed, Jun 21, 2017 at 11:12 AM, Fabio Estevam <festevam@gmail.com> 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

[toc] | [prev] | [next] | [standalone]

#1679556

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 <festevam@gmail.com> 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

[toc] | [prev] | [next] | [standalone]

#1673437 — 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 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web