* [PATCH 5/7] serial: imx: umap sg buffers when DMA channel is released
From: Romain Perier @ 2017-06-30 12:04 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-serial, linux-arm-kernel, linux-kernel, Nandor Han,
Romain Perier
In-Reply-To: <20170630120446.13994-1-romain.perier@collabora.com>
From: Nandor Han <nandor.han@ge.com>
This commits unmaps sg buffers when the DMA channel is released
Signed-off-by: Nandor Han <nandor.han@ge.com>
Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
drivers/tty/serial/imx.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index e8cf7cf..58d6b1c 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1215,6 +1215,12 @@ static void imx_stop_tx_dma(struct imx_port *sport)
temp = readl(sport->port.membase + UCR1);
temp &= ~UCR1_TDMAEN;
writel(temp, sport->port.membase + UCR1);
+
+ if (sport->dma_is_txing) {
+ dma_unmap_sg(sport->port.dev, &sport->tx_sgl[0],
+ sport->dma_tx_nents, DMA_TO_DEVICE);
+ sport->dma_is_txing = 0;
+ }
}
static void imx_stop_rx_dma(struct imx_port *sport)
@@ -1224,6 +1230,12 @@ static void imx_stop_rx_dma(struct imx_port *sport)
temp = readl(sport->port.membase + UCR1);
temp &= ~(UCR1_RDMAEN | UCR1_ATDMAEN);
writel(temp, sport->port.membase + UCR1);
+
+ if (sport->dma_is_rxing) {
+ dma_unmap_sg(sport->port.dev, &sport->rx_sgl, 1,
+ DMA_FROM_DEVICE);
+ sport->dma_is_rxing = 0;
+ }
}
static void imx_enable_dma(struct imx_port *sport)
--
1.8.3.1
^ permalink raw reply related
* [PATCH 6/7] serial: imx: update the stop rx,tx procedures
From: Romain Perier @ 2017-06-30 12:04 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-serial, linux-arm-kernel, linux-kernel, Nandor Han,
Romain Perier
In-Reply-To: <20170630120446.13994-1-romain.perier@collabora.com>
From: Nandor Han <nandor.han@ge.com>
According to "Documentation/serial/driver" both procedures should stop
receiving or sending data. Based on this the procedures should stop the
activity regardless if DMA is enabled or not.
This commit updates both imx_stop_{rx|tx} procedures to stop the
activity and disable the interrupts related to that. In case DMA is used
the sg buffers are also un-maped.
Signed-off-by: Nandor Han <nandor.han@ge.com>
Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
drivers/tty/serial/imx.c | 39 ++++++++++++++++++++-------------------
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 58d6b1c..d5b6e09 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -360,6 +360,9 @@ static void imx_port_rts_auto(struct imx_port *sport, unsigned long *ucr2)
*ucr2 |= UCR2_CTSC;
}
+static void imx_stop_rx_dma(struct imx_port *sport);
+static void imx_stop_tx_dma(struct imx_port *sport);
+
/*
* interrupts disabled on entry
*/
@@ -368,15 +371,14 @@ static void imx_stop_tx(struct uart_port *port)
struct imx_port *sport = (struct imx_port *)port;
unsigned long temp;
- /*
- * We are maybe in the SMP context, so if the DMA TX thread is running
- * on other cpu, we have to wait for it to finish.
- */
- if (sport->dma_is_enabled && sport->dma_is_txing)
- return;
+ sport->tx_bytes = 0;
+
+ if (sport->dma_is_enabled)
+ imx_stop_tx_dma(sport);
temp = readl(port->membase + UCR1);
- writel(temp & ~UCR1_TXMPTYEN, port->membase + UCR1);
+ temp &= ~(UCR1_TXMPTYEN | UCR1_TRDYEN | UCR1_RTSDEN);
+ writel(temp, port->membase + UCR1);
/* in rs485 mode disable transmitter if shifter is empty */
if (port->rs485.flags & SER_RS485_ENABLED &&
@@ -403,21 +405,20 @@ static void imx_stop_rx(struct uart_port *port)
struct imx_port *sport = (struct imx_port *)port;
unsigned long temp;
- if (sport->dma_is_enabled && sport->dma_is_rxing) {
- if (sport->port.suspended) {
- dmaengine_terminate_all(sport->dma_chan_rx);
- sport->dma_is_rxing = 0;
- } else {
- return;
- }
- }
+ if (sport->dma_is_enabled)
+ imx_stop_rx_dma(sport);
+
+ temp = readl(sport->port.membase + UCR1);
+ temp &= ~UCR1_RRDYEN;
+ writel(temp, sport->port.membase + UCR1);
temp = readl(sport->port.membase + UCR2);
- writel(temp & ~UCR2_RXEN, sport->port.membase + UCR2);
+ temp &= ~UCR2_ATEN;
+ writel(temp, sport->port.membase + UCR2);
- /* disable the `Receiver Ready Interrrupt` */
- temp = readl(sport->port.membase + UCR1);
- writel(temp & ~UCR1_RRDYEN, sport->port.membase + UCR1);
+ temp = readl(sport->port.membase + UCR3);
+ temp &= ~UCR3_AWAKEN;
+ writel(temp, sport->port.membase + UCR3);
}
/*
--
1.8.3.1
^ permalink raw reply related
* [PATCH 7/7] serial: imx: Fix imx_shutdown procedure
From: Romain Perier @ 2017-06-30 12:04 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-serial, linux-arm-kernel, linux-kernel, Nandor Han,
Romain Perier
In-Reply-To: <20170630120446.13994-1-romain.perier@collabora.com>
From: Nandor Han <nandor.han@ge.com>
In some cases, It looks that interrupts can happen after the dma was
disabled and port was not yet shutdown. This will result in interrupts
handled by imx_rxint.
This commits updates the shutdown function to ensure that underlying
components are disabled in the right order. This disables RX and TX
blocks, then it disabled interrupts. In case DMA is enabled, it disables
DMA and free corresponding resources. It disables UART port and stop
clocks.
Signed-off-by: Nandor Han <nandor.han@ge.com>
Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
drivers/tty/serial/imx.c | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index d5b6e09..7dc6f0c 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1404,44 +1404,44 @@ static void imx_shutdown(struct uart_port *port)
unsigned long temp;
unsigned long flags;
- if (sport->dma_is_enabled) {
- sport->dma_is_rxing = 0;
- sport->dma_is_txing = 0;
- dmaengine_terminate_sync(sport->dma_chan_tx);
- dmaengine_terminate_sync(sport->dma_chan_rx);
-
+ if (!sport->port.suspended) {
spin_lock_irqsave(&sport->port.lock, flags);
imx_stop_tx(port);
imx_stop_rx(port);
- imx_disable_dma(sport);
spin_unlock_irqrestore(&sport->port.lock, flags);
- imx_uart_dma_exit(sport);
}
- mctrl_gpio_disable_ms(sport->gpios);
+ if (sport->dma_is_inited) {
+ if (sport->dma_is_enabled) {
+ spin_lock_irqsave(&sport->port.lock, flags);
+ imx_disable_dma(sport);
+ spin_unlock_irqrestore(&sport->port.lock, flags);
+ }
+ imx_uart_dma_exit(sport);
+ }
spin_lock_irqsave(&sport->port.lock, flags);
temp = readl(sport->port.membase + UCR2);
- temp &= ~(UCR2_TXEN);
+ temp &= ~(UCR2_TXEN | UCR2_RXEN);
writel(temp, sport->port.membase + UCR2);
+ temp = readl(sport->port.membase + UCR4);
+ temp &= ~UCR4_OREN;
+ writel(temp, sport->port.membase + UCR4);
spin_unlock_irqrestore(&sport->port.lock, flags);
- /*
- * Stop our timer.
- */
- del_timer_sync(&sport->timer);
+ mctrl_gpio_disable_ms(sport->gpios);
- /*
- * Disable all interrupts, port and break condition.
- */
+ /* Stop our timer. */
+ del_timer_sync(&sport->timer);
+ /* Disable port. */
spin_lock_irqsave(&sport->port.lock, flags);
temp = readl(sport->port.membase + UCR1);
- temp &= ~(UCR1_TXMPTYEN | UCR1_RRDYEN | UCR1_RTSDEN | UCR1_UARTEN);
-
+ temp &= ~UCR1_UARTEN;
writel(temp, sport->port.membase + UCR1);
spin_unlock_irqrestore(&sport->port.lock, flags);
+ /* Disable clocks. */
clk_disable_unprepare(sport->clk_per);
clk_disable_unprepare(sport->clk_ipg);
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 3/7] serial: imx: init dma_is_{rx|tx}ing variables
From: Lothar Waßmann @ 2017-06-30 12:13 UTC (permalink / raw)
To: Romain Perier
Cc: Greg Kroah-Hartman, linux-kernel, linux-serial, linux-arm-kernel,
Nandor Han
In-Reply-To: <20170630120446.13994-4-romain.perier@collabora.com>
Hi,
On Fri, 30 Jun 2017 14:04:42 +0200 Romain Perier wrote:
> From: Nandor Han <nandor.han@ge.com>
>
> Initialize both dma_is_{rx|tx}ing variables when DMA is enabled to avoid
> checking uninitialized variables if port shutdown is requested before
> DMA channels get a chance to start.
>
> Signed-off-by: Nandor Han <nandor.han@ge.com>
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> ---
> drivers/tty/serial/imx.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 188063d..81fb413 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -1225,6 +1225,9 @@ static void imx_enable_dma(struct imx_port *sport)
>
> imx_setup_ufcr(sport, TXTL_DMA, RXTL_DMA);
>
> + sport->dma_is_rxing = 0;
> + sport->dma_is_txing = 0;
> +
> sport->dma_is_enabled = 1;
> }
>
sport is devm_kzalloc()ed, so the variables are initialized to 0 anyway.
Lothar Waßmann
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP
From: Fabio Estevam @ 2017-06-30 12:15 UTC (permalink / raw)
To: Clemens Gruber
Cc: linux-serial@vger.kernel.org, Greg Kroah-Hartman, Fabio Estevam,
Uwe Kleine-König, linux-kernel, Fugang Duan, G.Schenk
In-Reply-To: <CAOMZO5DXxaJ6PveJb9912rog8xvfqgsg3O3u3a-ffWUj9ZnpCg@mail.gmail.com>
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
^ permalink raw reply
* Re: [PATCH 4/6] dt-bindings: serial: Document RDA Micro UART
From: Rob Herring @ 2017-06-30 14:27 UTC (permalink / raw)
To: Andreas Färber
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, service, zhao_steven,
Greg Kroah-Hartman, Mark Rutland, linux-serial@vger.kernel.org,
devicetree@vger.kernel.org
In-Reply-To: <e8b8eb0c-0362-0cfb-4d18-3e1ab2b77412@suse.de>
On Fri, Jun 30, 2017 at 6:12 AM, Andreas Färber <afaerber@suse.de> wrote:
> Am 29.06.2017 um 22:10 schrieb Rob Herring:
>> On Tue, Jun 27, 2017 at 02:55:18AM +0200, Andreas Färber wrote:
>>> Add an initial binding for the RDA8810PL UART.
>>>
>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>> ---
>>> Documentation/devicetree/bindings/serial/rda-uart.txt | 13 +++++++++++++
>>> 1 file changed, 13 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/serial/rda-uart.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/serial/rda-uart.txt b/Documentation/devicetree/bindings/serial/rda-uart.txt
>>> new file mode 100644
>>> index 000000000000..6840a8aee035
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/serial/rda-uart.txt
>>> @@ -0,0 +1,13 @@
>>> +RDA Micro UART
>>> +
>>> +Required properties:
>>> +- compatible : "rda,8810pl-uart" for RDA8810PL
>>> +- reg : Offset and length of the register set for the device.
>>
>> No clocks or interrupts?
>
> Not yet. I've only pieced together an earlycon driver so far, no full
> serial driver. The .dtsi doesn't even have an interrupt-controller node
> yet - wasn't clear to me whether this SoC even has a GIC and, if so,
> where, from the downstream pre-DT code.
How far can you boot with no interrupts?
DT bindings shouldn't unnecessarily evolve. Really, anything added
should be optional to maintain compatibility. Sometimes that's
unavoidable, but this isn't one of those cases. So submit this when
you have something more complete.
Rob
^ permalink raw reply
* [PATCH] serial: 8250: fix error handling in of_platform_serial_probe()
From: Alexey Khoroshilov @ 2017-06-30 22:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Arnd Bergmann
Cc: Alexey Khoroshilov, linux-serial, linux-kernel, ldv-project
clk_disable_unprepare(info->clk) is missed in of_platform_serial_probe(),
while irq_dispose_mapping(port->irq) is missed in of_platform_serial_setup().
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/tty/serial/8250/8250_of.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
index 1cbadafc6889..f7a0e7831635 100644
--- a/drivers/tty/serial/8250/8250_of.c
+++ b/drivers/tty/serial/8250/8250_of.c
@@ -86,7 +86,7 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
ret = of_address_to_resource(np, 0, &resource);
if (ret) {
dev_warn(&ofdev->dev, "invalid address\n");
- goto out;
+ goto err_address;
}
spin_lock_init(&port->lock);
@@ -128,7 +128,7 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
dev_warn(&ofdev->dev, "unsupported reg-io-width (%d)\n",
prop);
ret = -EINVAL;
- goto out;
+ goto err_regio;
}
}
@@ -158,7 +158,10 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
port->handle_irq = fsl8250_handle_irq;
return 0;
-out:
+
+err_regio:
+ irq_dispose_mapping(port->irq);
+err_address:
if (info->clk)
clk_disable_unprepare(info->clk);
return ret;
@@ -192,7 +195,7 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
memset(&port8250, 0, sizeof(port8250));
ret = of_platform_serial_setup(ofdev, port_type, &port8250.port, info);
if (ret)
- goto out;
+ goto err_setup;
if (port8250.port.fifosize)
port8250.capabilities = UART_CAP_FIFO;
@@ -208,15 +211,18 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
ret = serial8250_register_8250_port(&port8250);
if (ret < 0)
- goto out;
+ goto err_register;
info->type = port_type;
info->line = ret;
platform_set_drvdata(ofdev, info);
return 0;
-out:
- kfree(info);
+err_register:
irq_dispose_mapping(port8250.port.irq);
+ if (info->clk)
+ clk_disable_unprepare(info->clk);
+err_setup:
+ kfree(info);
return ret;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 4/6] dt-bindings: serial: Document RDA Micro UART
From: Andreas Färber @ 2017-07-02 1:52 UTC (permalink / raw)
To: Rob Herring
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
service-KgY735zpLH1Wk0Htik3J/w, zhao_steven-Y9sIeH5OGRo,
Greg Kroah-Hartman, Mark Rutland,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_JsqLatRUwdA6GeWCHXFTHMUYY27pdmUpPEoHwCCFMg2F21Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Am 30.06.2017 um 16:27 schrieb Rob Herring:
> On Fri, Jun 30, 2017 at 6:12 AM, Andreas Färber <afaerber-l3A5Bk7waGM@public.gmane.org> wrote:
>> Am 29.06.2017 um 22:10 schrieb Rob Herring:
>>> On Tue, Jun 27, 2017 at 02:55:18AM +0200, Andreas Färber wrote:
>>>> Add an initial binding for the RDA8810PL UART.
>>>>
>>>> Signed-off-by: Andreas Färber <afaerber-l3A5Bk7waGM@public.gmane.org>
>>>> ---
>>>> Documentation/devicetree/bindings/serial/rda-uart.txt | 13 +++++++++++++
>>>> 1 file changed, 13 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/serial/rda-uart.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/serial/rda-uart.txt b/Documentation/devicetree/bindings/serial/rda-uart.txt
>>>> new file mode 100644
>>>> index 000000000000..6840a8aee035
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/serial/rda-uart.txt
>>>> @@ -0,0 +1,13 @@
>>>> +RDA Micro UART
>>>> +
>>>> +Required properties:
>>>> +- compatible : "rda,8810pl-uart" for RDA8810PL
>>>> +- reg : Offset and length of the register set for the device.
>>>
>>> No clocks or interrupts?
>>
>> Not yet. I've only pieced together an earlycon driver so far, no full
>> serial driver. The .dtsi doesn't even have an interrupt-controller node
>> yet - wasn't clear to me whether this SoC even has a GIC and, if so,
>> where, from the downstream pre-DT code.
>
> How far can you boot with no interrupts?
That was described in the cover letter, as usual.
> DT bindings shouldn't unnecessarily evolve. Really, anything added
> should be optional to maintain compatibility. Sometimes that's
> unavoidable, but this isn't one of those cases.
Makes sense. Maybe this series should've been marked RFC. It's too late
for 4.13 anyway, so there's lots of time left to extend the bindings.
BTW I saw that Kevin was adding a disclaimer to the Amlogic bindings
that they are not stable. Is that required here, too?
> So submit this when
> you have something more complete.
Negative. I worked weeks and months towards perfect STM32 drivers, and
despite showing it to ST and at a LinuxCon Japan, someone else sent less
sophisticated patches (e.g., fixed-clocks) and took all the credit. Same
with Turris Omnia. Therefore I am now submitting my new patches early,
not just when everything is complete.
It took weeks just to get any earlycon output on the 2G-IoT at all - not
my serial driver or my DT was broken, but the vendor's U-Boot just
wasn't passing the .dtb to the kernel properly, as it turned out. Now
that we know how to boot a DT-based kernel and get serial output, it
becomes much easier for me and potential contributors to add drivers.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP
From: Clemens Gruber @ 2017-07-02 20:17 UTC (permalink / raw)
To: Fabio Estevam, Romain Perier
Cc: linux-serial, Nandor Han, Greg Kroah-Hartman, linux-arm-kernel,
linux-kernel, u.kleine-koenig, Fugang Duan, G.Schenk
In-Reply-To: <CAOMZO5A-fF5V4obGD-XEnSbt8TB2wsCTJRjvKXfxM_SjKXdQXQ@mail.gmail.com>
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
^ permalink raw reply
* Re: [PATCH v4 16/28] tty: serial: owl: Implement console driver
From: Andreas Färber @ 2017-07-02 20:27 UTC (permalink / raw)
To: Alan Cox
Cc: support, 张天益, Greg Kroah-Hartman, 96boards,
linux-kernel, Thomas Liau, mp-cs, linux-serial, 刘炜,
Jiri Slaby, linux-arm-kernel, 张东风,
梅利, support, lee
In-Reply-To: <20170606143407.56d15c3c@lxorguk.ukuu.org.uk>
Am 06.06.2017 um 15:34 schrieb Alan Cox:
>> +static void owl_uart_set_termios(struct uart_port *port,
>> + struct ktermios *termios,
>> + struct ktermios *old)
>> +{
>> + struct owl_uart_port *owl_port = to_owl_uart_port(port);
>> + unsigned int baud;
>> + u32 ctl;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&port->lock, flags);
>> +
>> + /* We don't support modem control lines. */
>> + termios->c_cflag &= ~(HUPCL | CMSPAR);
>> + termios->c_cflag |= CLOCAL;
>
> CLOCAL and HUPCL are software not hardware properties so are probably
> best not forced (it'll confuse some apps if you do)
>> +
>> + /* We don't support BREAK character recognition. */
>> + termios->c_iflag &= ~(IGNBRK | BRKINT);
>
> Ditto these
Fixed. These were obviously forward-ported from the vendor tree.
https://github.com/LeMaker/linux-actions/blob/linux-3.10.y/arch/arm/mach-owl/serial-owl.c
> You do clear CMSPAR which is right if not supported but then later on we
> have:
>
>> + if (termios->c_cflag & PARENB) {
>> + if (termios->c_cflag & CMSPAR) {
>> + if (termios->c_cflag & PARODD)
>> + ctl |= OWL_UART_CTL_PRS_MARK;
>> + else
>> + ctl |= OWL_UART_CTL_PRS_SPACE;
>> + } else if (termios->c_cflag & PARODD)
>> + ctl |= OWL_UART_CTL_PRS_ODD;
>> + else
>> + ctl |= OWL_UART_CTL_PRS_EVEN;
>> + } else
>> + ctl |= OWL_UART_CTL_PRS_NONE;
>
>
> Which seems to indicate you do support CMSPAR so shouldn't be clearing it.
Again that's what the original code was like.
Since these register values do exist, I would rather not rip the code
out, unless it's wrong. In my testing, not clearing CMSPAR works so far.
>> + baud = uart_get_baud_rate(port, termios, old, 9600, 3200000);
>> + owl_uart_change_baudrate(owl_port, baud);
>
> You should re-encode the resulting baud rate into the termios
>
> /* Don't rewrite B0 */
> if (tty_termios_baud_rate(termios))
> tty_termios_encode_baud_rate(termios, baud, baud);
Added, thanks.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply
* Re: serial: imx: disable DMA for RS-485 on i.MX6 SMP
From: Clemens Gruber @ 2017-07-02 20:47 UTC (permalink / raw)
To: Fabio Estevam
Cc: linux-serial, Greg Kroah-Hartman, Fabio Estevam, u.kleine-koenig,
linux-kernel, Fugang Duan
In-Reply-To: <CAOMZO5DA++c95DPrmzOHfFtD+zFcW6G9-0_cRBtg0E8TA8dwXg@mail.gmail.com>
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
^ permalink raw reply
* Re: [PATCH v4 16/28] tty: serial: owl: Implement console driver
From: Andreas Färber @ 2017-07-02 22:36 UTC (permalink / raw)
To: Andy Shevchenko
Cc: support, 张天益, Greg Kroah-Hartman, 96boards,
linux-kernel@vger.kernel.org, Thomas Liau, mp-cs,
linux-serial@vger.kernel.org, 刘炜, Jiri Slaby,
linux-arm Mailing List, 张东风,
梅利, support, lee
In-Reply-To: <CAHp75VcYtdxknAw5b1zp+qLi1MT3ktwm99rOJEivcBrc+9B-cg@mail.gmail.com>
Am 07.06.2017 um 16:37 schrieb Andy Shevchenko:
> On Tue, Jun 6, 2017 at 3:54 AM, Andreas Färber <afaerber@suse.de> wrote:
>> Implement serial console driver to complement earlycon.
>>
>> Based on LeMaker linux-actions tree.
>
>> +#define OWL_UART_CTL_DWLS_MASK (0x3 << 0)
>
> GENMASK() ?
>
>> +#define OWL_UART_CTL_PRS_MASK (0x7 << 4)
>
> Ditto.
>
>> #define OWL_UART_STAT_TRFL_MASK (0x1f << 11)
>
> Ditto.
Changed.
>> +static struct owl_uart_port *owl_uart_ports[OWL_UART_PORT_NUM];
>
> And this is needed for...?
That's what both the downstream driver and several in-tree drivers are
doing. See `git grep '_ports\[' -- drivers/tty/serial/`.
If you feel this is wrong, --verbose explanation please.
>> +static void owl_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
>> +{
>> +}
>
> Do we need an empty stub?
The only flag I have found in the CTL register is AFE for automatic
RTS/CTS flow-control - RTS and CTS can only be read in STAT, not set.
And yes, if I drop this empty callback function, I get no serial output.
>> +static void owl_uart_send_chars(struct uart_port *port)
>> +{
>
>> + xmit->tail = (xmit->tail + 1) & (SERIAL_XMIT_SIZE - 1);
>
> % SERIAL_XMIT_SIZE shorter (I hope it's a power of 2), but it's up to you.
crisv10 and meson_uart have it this way, modulo normalized whitespace.
No serial driver uses % for it.
>> +}
>
>> +static irqreturn_t owl_uart_irq(int irq, void *dev_id)
>> +{
>> + struct uart_port *port = (struct uart_port *)dev_id;
>
> Useless casting.
Fixed.
>> + spin_lock(&port->lock);
>
> spin_lock_irqsave() ?
Fixed, with matching _irqrestore().
> Consider the kernel command option that switches all IRQ handlers to
> be threaded.
>
>> +}
>
>> +static void owl_uart_change_baudrate(struct owl_uart_port *owl_port,
>> + unsigned long baud)
>> +{
>> + clk_set_rate(owl_port->clk, baud * 8);
>> +}
>
>> +static void owl_uart_release_port(struct uart_port *port)
>> +{
>> + struct platform_device *pdev = to_platform_device(port->dev);
>> + struct resource *res;
>> +
>
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (!res)
>> + return;
>> +
>> + if (port->flags & UPF_IOREMAP) {
>> + devm_release_mem_region(port->dev, port->mapbase,
>> + resource_size(res));
>> + devm_iounmap(port->dev, port->membase);
>> + port->membase = NULL;
>> + }
>
> Above is entirely redundant.
Can you explain what this flag is used for and why some other drivers
implement it? That word alone is not helping.
>> +}
>> +
>> +static int owl_uart_request_port(struct uart_port *port)
>> +{
>> + struct platform_device *pdev = to_platform_device(port->dev);
>> + struct resource *res;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (!res)
>> + return -ENXIO;
>> +
>> + if (!devm_request_mem_region(port->dev, port->mapbase,
>> + resource_size(res), dev_name(port->dev)))
>> + return -EBUSY;
>> +
>> + if (port->flags & UPF_IOREMAP) {
>> + port->membase = devm_ioremap_nocache(port->dev, port->mapbase,
>> + resource_size(res));
>> + if (!port->membase)
>> + return -EBUSY;
>> + }
>> +
>> + return 0;
>> +}
>
>> +static void owl_uart_config_port(struct uart_port *port, int flags)
>> +{
>
>> + if (flags & UART_CONFIG_TYPE) {
>
> if (!(...))
> return;
>
> ?
Not a single serial driver does it that way.
I guess it prepares for handling other flags.
>> + port->type = PORT_OWL;
>> + owl_uart_request_port(port);
>> + }
>> +}
>
>
>> +static int owl_uart_probe(struct platform_device *pdev)
>> +{
>> + struct resource *res_mem, *res_irq;
>> + struct owl_uart_port *owl_port;
>> + int ret;
>> +
>> + if (pdev->dev.of_node)
>> + pdev->id = of_alias_get_id(pdev->dev.of_node, "serial");
>> +
>> + if (pdev->id < 0 || pdev->id >= OWL_UART_PORT_NUM) {
>> + dev_err(&pdev->dev, "id %d out of range\n", pdev->id);
>> + return -EINVAL;
>> + }
>> +
>
>
>> + res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (!res_mem) {
>> + dev_err(&pdev->dev, "could not get mem\n");
>> + return -ENODEV;
>> + }
>
> You can use
>
> struct resource *mem;
>
> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> x = devm_ioremap_resource();
> if (IS_ERR(x))
> return PTR_ERR(x);
>
> and remote IOREMAP flag below.
>> + res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>> + if (!res_irq) {
>> + dev_err(&pdev->dev, "could not get irq\n");
>> + return -ENODEV;
>> + }
>
> platform_get_irq()
Adopted.
>> + if (owl_uart_ports[pdev->id]) {
>> + dev_err(&pdev->dev, "port %d already allocated\n", pdev->id);
>> + return -EBUSY;
>> + }
>
> I guess it's redundant. It should be conflicting by resource (for example, IRQ).
No, currently not. Memory resources are allocated on request_port, IRQ
is requested on startup.
>> + owl_port->clk = clk_get(&pdev->dev, NULL);
>
> devm_ ?
Changed.
>> + owl_port->port.iotype = UPIO_MEM;
>> + owl_port->port.mapbase = res_mem->start;
>> + owl_port->port.irq = res_irq->start;
>
>
>> + owl_port->port.uartclk = clk_get_rate(owl_port->clk);
>
> You need to check if it's 0 and use device property instead or bail out.
Fixed. Since this is a new driver, I'd rather not fall back to
clock-frequency property here. The binding has clocks property as required.
>> + owl_port->port.flags = UPF_BOOT_AUTOCONF | UPF_IOREMAP | UPF_LOW_LATENCY;
>> + owl_port->port.x_char = 0;
>> + owl_port->port.fifosize = 16;
>> + owl_port->port.ops = &owl_uart_ops;
>> +
>> + owl_uart_ports[pdev->id] = owl_port;
>
>> + platform_set_drvdata(pdev, owl_port);
>
> It should be just before return 0, right?..
Does it matter?
>> +
>> + ret = uart_add_one_port(&owl_uart_driver, &owl_port->port);
>> + if (ret)
>> + owl_uart_ports[pdev->id] = NULL;
>
> ...and thus, taking into consideration redundancy of that global var:
>
> ret = uart_add_one_port(&owl_uart_driver, &owl_port->port);
> if (ret)
> retrun ret;
>
> platform_set_drvdata(pdev, owl_port);
> return 0;
>
>> + return ret;
>> +}
>
>> +
>> +static int owl_uart_remove(struct platform_device *pdev)
>> +{
>
>> + struct owl_uart_port *owl_port;
>> +
>> + owl_port = platform_get_drvdata(pdev);
>
> Do above in one line.
Done.
>> + uart_remove_one_port(&owl_uart_driver, &owl_port->port);
>
>> + owl_uart_ports[pdev->id] = NULL;
>
> Redundant.
>
>> +
>> + return 0;
>> +}
>
>> +/* Actions Semi Owl UART */
>> +#define PORT_OWL 117
>
> We can use holes for now IIUC.
>
> See 36 or alike
Okay. 36 works, too.
Please point to a good example of a serial driver that is not
"redundant" in your view. That will help also with other pending serial
drivers of mine. Thanks.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply
* Re: serial: imx: disable DMA for RS-485 on i.MX6 SMP
From: Fabio Estevam @ 2017-07-02 23:09 UTC (permalink / raw)
To: Clemens Gruber
Cc: linux-serial@vger.kernel.org, Greg Kroah-Hartman, Fabio Estevam,
Uwe Kleine-König, linux-kernel, Fugang Duan
In-Reply-To: <20170702204731.GA7174@archie.localdomain>
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
^ permalink raw reply
* Re: [PATCH 1/7] serial: imx: only set DMA rx-ing when DMA starts
From: Uwe Kleine-König @ 2017-07-03 6:48 UTC (permalink / raw)
To: Romain Perier
Cc: Greg Kroah-Hartman, Nandor Han, linux-arm-kernel, linux-serial,
linux-kernel
In-Reply-To: <20170630120446.13994-2-romain.perier@collabora.com>
Hello,
On Fri, Jun 30, 2017 at 02:04:40PM +0200, Romain Perier wrote:
> From: Nandor Han <nandor.han@ge.com>
>
> Avoid the situation when `dma_is_rxing` could incorrectly signal that
> DMA RX channel is receiving data in case DMA preparation or sg mapping
> fails.
Just from reading the commit log and the patch I didn't understand the
problem that is supposed to be fixed here. After looking at it for some
I understood. I don't suggest a new commit log here as it has to look
different if you pick up my comment below.
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 5437b34..1d35293 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -725,11 +725,11 @@ static irqreturn_t imx_rxint(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> -static void imx_disable_rx_int(struct imx_port *sport)
> +static void imx_disable_rx_int(struct imx_port *sport, bool is_rxing)
> {
> unsigned long temp;
>
> - sport->dma_is_rxing = 1;
> + sport->dma_is_rxing = is_rxing;
>
> /* disable the receiver ready and aging timer interrupts */
> temp = readl(sport->port.membase + UCR1);
> @@ -762,7 +762,7 @@ static void imx_dma_rxint(struct imx_port *sport)
> temp = readl(sport->port.membase + USR2);
> if ((temp & USR2_RDR) && !sport->dma_is_rxing) {
>
> - imx_disable_rx_int(sport);
> + imx_disable_rx_int(sport, false);
>
> /* tell the DMA to receive the data. */
> start_rx_dma(sport);
> @@ -1083,6 +1083,7 @@ static int start_rx_dma(struct imx_port *sport)
> desc->callback_param = sport;
>
> dev_dbg(dev, "RX: prepare for the DMA.\n");
> + sport->dma_is_rxing = 1;
> sport->rx_cookie = dmaengine_submit(desc);
> dma_async_issue_pending(chan);
> return 0;
> @@ -1362,7 +1363,7 @@ static int imx_startup(struct uart_port *port)
> spin_unlock(&tty->files_lock);
>
> if (readcnt > 0) {
> - imx_disable_rx_int(sport);
> + imx_disable_rx_int(sport, true);
I wonder if it would be saner to not assign to dma_is_rxing in
imx_disable_rx_int() at all but do this only in start_rx_dma() and
imx_dma_rxint().
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH 2/7] serial: imx: move log from error to debug type
From: Uwe Kleine-König @ 2017-07-03 6:50 UTC (permalink / raw)
To: Romain Perier
Cc: Greg Kroah-Hartman, Nandor Han, linux-arm-kernel, linux-serial,
linux-kernel
In-Reply-To: <20170630120446.13994-3-romain.perier@collabora.com>
On Fri, Jun 30, 2017 at 02:04:41PM +0200, Romain Perier wrote:
> From: Nandor Han <nandor.han@ge.com>
>
> During DMA startup we have a data race condition since UART port can
> receive data that can generate different type of errors.
>
> This is not necessarily an error since DMA didn't yet started. The
> situation is minimized but still present even if we try to clear up the
> error before starting the DMA.
>
> Therefore changing the log to debug type we avoid having "false" error
> messages.
This doesn't look right. You say the message "DMA transaction error." is
wrong sometimes and so hide it a bit by using dev_dbg instead of
dev_err.
I don't like that.
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH 3/7] serial: imx: init dma_is_{rx|tx}ing variables
From: Uwe Kleine-König @ 2017-07-03 6:52 UTC (permalink / raw)
To: Romain Perier
Cc: Lothar Waßmann, Greg Kroah-Hartman, Nandor Han,
linux-arm-kernel, linux-serial, linux-kernel
In-Reply-To: <20170630141329.6144cddd@karo-electronics.de>
On Fri, Jun 30, 2017 at 02:13:29PM +0200, Lothar Waßmann wrote:
> Hi,
>
> On Fri, 30 Jun 2017 14:04:42 +0200 Romain Perier wrote:
> > From: Nandor Han <nandor.han@ge.com>
> >
> > Initialize both dma_is_{rx|tx}ing variables when DMA is enabled to avoid
> > checking uninitialized variables if port shutdown is requested before
> > DMA channels get a chance to start.
> >
> > Signed-off-by: Nandor Han <nandor.han@ge.com>
> > Signed-off-by: Romain Perier <romain.perier@collabora.com>
> > ---
> > drivers/tty/serial/imx.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> > index 188063d..81fb413 100644
> > --- a/drivers/tty/serial/imx.c
> > +++ b/drivers/tty/serial/imx.c
> > @@ -1225,6 +1225,9 @@ static void imx_enable_dma(struct imx_port *sport)
> >
> > imx_setup_ufcr(sport, TXTL_DMA, RXTL_DMA);
> >
> > + sport->dma_is_rxing = 0;
> > + sport->dma_is_txing = 0;
> > +
> > sport->dma_is_enabled = 1;
> > }
> >
> sport is devm_kzalloc()ed, so the variables are initialized to 0 anyway.
I'd agree to Lothar's statement. Did you find this issue by inspection,
or does it fix a compiler warning? Do you think there is an actual
problem?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH 4/7] serial: imx: Simplify DMA disablement
From: Uwe Kleine-König @ 2017-07-03 6:58 UTC (permalink / raw)
To: Romain Perier
Cc: Greg Kroah-Hartman, linux-serial, linux-arm-kernel, linux-kernel,
Nandor Han, kernel
In-Reply-To: <20170630120446.13994-5-romain.perier@collabora.com>
On Fri, Jun 30, 2017 at 02:04:43PM +0200, Romain Perier wrote:
> From: Nandor Han <nandor.han@ge.com>
>
> This commits simplify the function imx_disable_dma() by moving
> the code for disabling RX and TX DMAs to dedicated functions.
I'd point out there that this prepares the next commit because in the
current state I'd prefer just improved comments.
> Also move away CTSC and CTS as this is not related to DMA.
Hmm, maybe this is related to the rs485 breakage we're seeing and
deserves a separate commit?
This also has the advantage that the refactoring change is semantically
a no-op.
> Signed-off-by: Nandor Han <nandor.han@ge.com>
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> ---
> drivers/tty/serial/imx.c | 31 ++++++++++++++++++++-----------
> 1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 81fb413..e8cf7cf 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -1208,6 +1208,24 @@ static int imx_uart_dma_init(struct imx_port *sport)
> return ret;
> }
>
> +static void imx_stop_tx_dma(struct imx_port *sport)
> +{
> + unsigned long temp;
> +
> + temp = readl(sport->port.membase + UCR1);
> + temp &= ~UCR1_TDMAEN;
> + writel(temp, sport->port.membase + UCR1);
> +}
> +
> +static void imx_stop_rx_dma(struct imx_port *sport)
> +{
> + unsigned long temp;
> +
> + temp = readl(sport->port.membase + UCR1);
> + temp &= ~(UCR1_RDMAEN | UCR1_ATDMAEN);
> + writel(temp, sport->port.membase + UCR1);
> +}
> +
> static void imx_enable_dma(struct imx_port *sport)
> {
> unsigned long temp;
> @@ -1233,17 +1251,8 @@ static void imx_enable_dma(struct imx_port *sport)
>
> static void imx_disable_dma(struct imx_port *sport)
> {
> - unsigned long temp;
> -
> - /* clear UCR1 */
> - temp = readl(sport->port.membase + UCR1);
> - temp &= ~(UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN);
> - writel(temp, sport->port.membase + UCR1);
> -
> - /* clear UCR2 */
> - temp = readl(sport->port.membase + UCR2);
> - temp &= ~(UCR2_CTSC | UCR2_CTS | UCR2_ATEN);
You also dropped clearing ATEN without mentioning that in the commit
log. Is this done on purpose?
> - writel(temp, sport->port.membase + UCR2);
> + imx_stop_rx_dma(sport);
> + imx_stop_tx_dma(sport);
>
> imx_setup_ufcr(sport, TXTL_DEFAULT, RXTL_DEFAULT);
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH 5/7] serial: imx: umap sg buffers when DMA channel is released
From: Uwe Kleine-König @ 2017-07-03 7:01 UTC (permalink / raw)
To: Romain Perier
Cc: Greg Kroah-Hartman, linux-serial, linux-arm-kernel, linux-kernel,
Nandor Han
In-Reply-To: <20170630120446.13994-6-romain.perier@collabora.com>
$Subject ~= s/umap/unmap/
On Fri, Jun 30, 2017 at 02:04:44PM +0200, Romain Perier wrote:
> From: Nandor Han <nandor.han@ge.com>
>
> This commits unmaps sg buffers when the DMA channel is released
>
> Signed-off-by: Nandor Han <nandor.han@ge.com>
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> ---
> drivers/tty/serial/imx.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index e8cf7cf..58d6b1c 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -1215,6 +1215,12 @@ static void imx_stop_tx_dma(struct imx_port *sport)
> temp = readl(sport->port.membase + UCR1);
> temp &= ~UCR1_TDMAEN;
> writel(temp, sport->port.membase + UCR1);
> +
> + if (sport->dma_is_txing) {
> + dma_unmap_sg(sport->port.dev, &sport->tx_sgl[0],
> + sport->dma_tx_nents, DMA_TO_DEVICE);
> + sport->dma_is_txing = 0;
You don't motivate setting dma_is_txing to zero in the commit log.
Does this mean the driver leaks memory in the current state?
> + }
> }
>
> static void imx_stop_rx_dma(struct imx_port *sport)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH 6/7] serial: imx: update the stop rx,tx procedures
From: Uwe Kleine-König @ 2017-07-03 7:03 UTC (permalink / raw)
To: Romain Perier
Cc: Greg Kroah-Hartman, Nandor Han, linux-arm-kernel, linux-serial,
linux-kernel, kernel
In-Reply-To: <20170630120446.13994-7-romain.perier@collabora.com>
On Fri, Jun 30, 2017 at 02:04:45PM +0200, Romain Perier wrote:
> From: Nandor Han <nandor.han@ge.com>
>
> According to "Documentation/serial/driver" both procedures should stop
> receiving or sending data. Based on this the procedures should stop the
> activity regardless if DMA is enabled or not.
>
> This commit updates both imx_stop_{rx|tx} procedures to stop the
> activity and disable the interrupts related to that. In case DMA is used
> the sg buffers are also un-maped.
This unmapping is implicit, becuae imx_stop_rx_dma unmaps since the
previous commit, right?
>
> Signed-off-by: Nandor Han <nandor.han@ge.com>
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> ---
> drivers/tty/serial/imx.c | 39 ++++++++++++++++++++-------------------
> 1 file changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 58d6b1c..d5b6e09 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -360,6 +360,9 @@ static void imx_port_rts_auto(struct imx_port *sport, unsigned long *ucr2)
> *ucr2 |= UCR2_CTSC;
> }
>
> +static void imx_stop_rx_dma(struct imx_port *sport);
> +static void imx_stop_tx_dma(struct imx_port *sport);
Is it possible to reshuffle the order of functions to make this forward
declaration redundant?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH 7/7] serial: imx: Fix imx_shutdown procedure
From: Uwe Kleine-König @ 2017-07-03 7:08 UTC (permalink / raw)
To: Romain Perier
Cc: Greg Kroah-Hartman, linux-serial, linux-arm-kernel, linux-kernel,
Nandor Han, kernel
In-Reply-To: <20170630120446.13994-8-romain.perier@collabora.com>
On Fri, Jun 30, 2017 at 02:04:46PM +0200, Romain Perier wrote:
> From: Nandor Han <nandor.han@ge.com>
>
> In some cases, It looks that interrupts can happen after the dma was
s/It/it/
> disabled and port was not yet shutdown. This will result in interrupts
> handled by imx_rxint.
>
> This commits updates the shutdown function to ensure that underlying
> components are disabled in the right order. This disables RX and TX
> blocks, then it disabled interrupts. In case DMA is enabled, it disables
> DMA and free corresponding resources. It disables UART port and stop
> clocks.
>
> Signed-off-by: Nandor Han <nandor.han@ge.com>
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> ---
> drivers/tty/serial/imx.c | 38 +++++++++++++++++++-------------------
> 1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index d5b6e09..7dc6f0c 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -1404,44 +1404,44 @@ static void imx_shutdown(struct uart_port *port)
> unsigned long temp;
> unsigned long flags;
>
> - if (sport->dma_is_enabled) {
> - sport->dma_is_rxing = 0;
> - sport->dma_is_txing = 0;
> - dmaengine_terminate_sync(sport->dma_chan_tx);
> - dmaengine_terminate_sync(sport->dma_chan_rx);
> -
> + if (!sport->port.suspended) {
> spin_lock_irqsave(&sport->port.lock, flags);
> imx_stop_tx(port);
> imx_stop_rx(port);
> - imx_disable_dma(sport);
> spin_unlock_irqrestore(&sport->port.lock, flags);
> - imx_uart_dma_exit(sport);
> }
>
> - mctrl_gpio_disable_ms(sport->gpios);
> + if (sport->dma_is_inited) {
> + if (sport->dma_is_enabled) {
> + spin_lock_irqsave(&sport->port.lock, flags);
> + imx_disable_dma(sport);
> + spin_unlock_irqrestore(&sport->port.lock, flags);
> + }
> + imx_uart_dma_exit(sport);
> + }
>
> spin_lock_irqsave(&sport->port.lock, flags);
> temp = readl(sport->port.membase + UCR2);
> - temp &= ~(UCR2_TXEN);
> + temp &= ~(UCR2_TXEN | UCR2_RXEN);
> writel(temp, sport->port.membase + UCR2);
> + temp = readl(sport->port.membase + UCR4);
> + temp &= ~UCR4_OREN;
> + writel(temp, sport->port.membase + UCR4);
> spin_unlock_irqrestore(&sport->port.lock, flags);
The function already had two critical blocks protected by
spin_lock_irqsave on sport->port.lock. With your patch there are three.
Is it possible to grab the lock only once?
>
> - /*
> - * Stop our timer.
> - */
> - del_timer_sync(&sport->timer);
> + mctrl_gpio_disable_ms(sport->gpios);
>
> - /*
> - * Disable all interrupts, port and break condition.
> - */
> + /* Stop our timer. */
Updating the comment style in such a commit makes the diff unnecessarily
hard to read.
> + del_timer_sync(&sport->timer);
>
> + /* Disable port. */
> spin_lock_irqsave(&sport->port.lock, flags);
> temp = readl(sport->port.membase + UCR1);
> - temp &= ~(UCR1_TXMPTYEN | UCR1_RRDYEN | UCR1_RTSDEN | UCR1_UARTEN);
> -
> + temp &= ~UCR1_UARTEN;
> writel(temp, sport->port.membase + UCR1);
> spin_unlock_irqrestore(&sport->port.lock, flags);
>
> + /* Disable clocks. */
> clk_disable_unprepare(sport->clk_per);
> clk_disable_unprepare(sport->clk_ipg);
> }
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-serial" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PULL] serial/gpio: exar: Fixes and support for IOT2000
From: Jan Kiszka @ 2017-07-03 7:21 UTC (permalink / raw)
To: Linus Walleij
Cc: Greg Kroah-Hartman, Alexandre Courbot, Linux Kernel Mailing List,
linux-serial@vger.kernel.org, linux-gpio@vger.kernel.org,
Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <CACRpkdYEvc2De88sy-FWOOkU8-O4KwX00rLZ7dBk6H2H0Q0+LQ@mail.gmail.com>
The following changes since commit 85bb4646f8908eb786dfa19a6bb2ff1423dc8aa4:
gpio: rcar: Add R8A7743 (RZ/G1M) support (2017-06-29 14:22:38 +0200)
are available in the git repository at:
git://git.kiszka.org/linux.git queues/gpio-iot2000
for you to fetch changes up to 413058df4331ce29f9934a5870d582c7e71fe15f:
serial: exar: Add support for IOT2040 device (2017-07-03 08:33:20 +0200)
This corresponds to v6 of the series with additional review and ack
tags.
----------------------------------------------------------------
Jan Kiszka (8):
serial: uapi: Add support for bus termination
gpio-exar/8250-exar: Do not even instantiate a GPIO device for Commtech cards
gpio: exar: Fix iomap request
gpio-exar/8250-exar: Rearrange gpiochip parenthood
serial: exar: Factor out platform hooks
platform: Accept const properties
gpio-exar/8250-exar: Make set of exported GPIOs configurable
serial: exar: Add support for IOT2040 device
drivers/base/platform.c | 2 +-
drivers/gpio/gpio-exar.c | 71 +++++++--------
drivers/tty/serial/8250/8250_exar.c | 170 +++++++++++++++++++++++++++++++++++-
include/linux/platform_device.h | 2 +-
include/uapi/linux/serial.h | 3 +
5 files changed, 208 insertions(+), 40 deletions(-)
--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply
* Re: [PULL] serial/gpio: exar: Fixes and support for IOT2000
From: Linus Walleij @ 2017-07-03 8:10 UTC (permalink / raw)
To: Jan Kiszka
Cc: Greg Kroah-Hartman, Alexandre Courbot, Linux Kernel Mailing List,
linux-serial@vger.kernel.org, linux-gpio@vger.kernel.org,
Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <5de2f417-02e2-b983-67bd-7d82232956b0@siemens.com>
On Mon, Jul 3, 2017 at 9:21 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> The following changes since commit 85bb4646f8908eb786dfa19a6bb2ff1423dc8aa4:
>
> gpio: rcar: Add R8A7743 (RZ/G1M) support (2017-06-29 14:22:38 +0200)
>
> are available in the git repository at:
>
> git://git.kiszka.org/linux.git queues/gpio-iot2000
>
> for you to fetch changes up to 413058df4331ce29f9934a5870d582c7e71fe15f:
>
> serial: exar: Add support for IOT2040 device (2017-07-03 08:33:20 +0200)
>
> This corresponds to v6 of the series with additional review and ack
> tags.
Thanks Jan, pulled in for v4.13.
Also thanks for your tireless and gritty work in hashing this out
and get the right fix out there!
Yours,
Linus Walleij
^ permalink raw reply
* Re: Moxa UPort 1150 and RS-422/485… what's the "proper" way to switch modes
From: Johan Hovold @ 2017-07-03 8:51 UTC (permalink / raw)
To: Alan Cox
Cc: Johan Hovold, Oliver Neukum, Stuart Longland,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170629205032.743ab235@alans-desktop>
On Thu, Jun 29, 2017 at 08:50:32PM +0100, Alan Cox wrote:
> So IMHO there are two cases
>
> 1. The configuration is fixed in the hardware - in which case we should
> automatically set it in kernel, or from device tree.
Right.
> 2. The configuration is soft - in which case you need an ioctl you can
> use on the port to change it - irrespective of whether sysfs also handles
> it. For simple permissions reasons and also to avoid races you need that
> ioctl.
Ok, but if we are going to extend and use the RS485 ioctl for this, I
think we should just stick to that and not add a sysfs-interface as
well.
I'll take a closer look at this.
> We intentionally have lots of spare space left to extend the RS485 ioctl,
> and we have termiox which contains even more room and historically in
> other Unixen dealt with all sorts of weird and wonderful parameters used
> on stuff like synchronous ports.
Yes, your foresight when adding (generalising) that ioctl might come in
handy now. :)
Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [GIT PULL] TTY/Serial patches for 4.13-rc1
From: Greg KH @ 2017-07-03 14:59 UTC (permalink / raw)
To: Linus Torvalds, Jiri Slaby
Cc: Stephen Rothwell, Andrew Morton, linux-kernel, linux-serial
The following changes since commit 3c2993b8c6143d8a5793746a54eba8f86f95240f:
Linux 4.12-rc4 (2017-06-04 16:47:43 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/ tags/tty-4.13-rc1
for you to fetch changes up to a6d7514b14a6ea2a3868bdcb2fca7b2410738595:
tty: serial: lpuart: add a more accurate baud rate calculation method (2017-06-29 17:12:34 +0200)
----------------------------------------------------------------
TTY/Serial patches for 4.13-rc1
Here is the large tty/serial patchset for 4.13-rc1.
A lot of tty and serial driver updates are in here, along with some
fixups for some __get/put_user usages that were reported. Nothing huge,
just lots of development by a number of different developers, full
details in the shortlog.
All of these have been in linux-next for a while. There will be a merge
issue with the arm-soc tree in the include/linux/platform_data/atmel.h
file. Stephen has sent out a fixup for it, so it shouldn't be that
difficult to merge.
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
----------------------------------------------------------------
Adam Borowski (6):
vt: use copy_from/to_user instead of __get/put_user for scrnmap ioctls
vt: fix unchecked __put_user() in tioclinux ioctls
vt: use copy_to_user instead of __put_user in GIO_UNIMAP ioctl
vt: use memdup_user in PIO_UNIMAP ioctl
vt: drop access_ok() calls in unimap ioctls
vt: fix \e[2m using the wrong placeholder color on graphical consoles
Alan Cox (1):
tty: handle the case where we cannot restore a line discipline
Aleksa Sarai (2):
tty: add compat_ioctl callbacks
tty: add TIOCGPTPEER ioctl
Alexandre Belloni (2):
tty/serial: atmel: remove atmel_default_console_device handling
tty/serial: atmel: make the driver DT only
Andreas Färber (2):
dt-bindings: serial: Document Actions Semi Owl UARTs
tty: serial: Add Actions Semi Owl UART earlycon
Andy Shevchenko (3):
serial: 8250_exar: Remove duplicate assignment
tty/vt/keyboard: Remove AVR32 bits from the driver
tty/serial: atmel: Remove AVR32 bits from the driver
Arnd Bergmann (1):
serial: meson: hide an unused function
Arvind Yadav (1):
serial: sirf: make of_device_ids const
Christoph Hellwig (1):
serial/mpsc: switch to dma_alloc_attrs
Colin Ian King (1):
drivers/tty/hvc: fix spelling mistake: "missmanaged" -> "mismanaged"
Dong Aisheng (7):
tty: serial: lpuart: introduce lpuart_soc_data to represent SoC property
tty: serial: lpuart: refactor lpuart32_{read|write} prototype
tty: serial: lpuart: add little endian 32 bit register support
dt-bindings: serial: fsl-lpuart: add i.MX7ULP support
tty: serial: lpuart: add imx7ulp support
tty: serial: lpuart: add earlycon support for imx7ulp
tty: serial: lpuart: add a more accurate baud rate calculation method
Fabio Estevam (1):
serial: imx: Remove unused members from imx_port struct
Geert Uytterhoeven (2):
serial: sh-sci: Fix race condition causing garbage during shutdown
serial: sh-sci: Update warning message in sci_request_dma_chan()
Geliang Tang (2):
tty/serial: atmel: use offset_in_page() macro
serial: pch_uart: use offset_in_page() macro
Greg Kroah-Hartman (4):
Merge 4.12-rc3 into tty-next
tty: reserve N_SPEAKUP number
Merge 4.12-rc4 into tty-next
Merge branch 'vt_copy_cleanup' into tty-next
Heiner Kallweit (6):
serial: meson: fix setting number of stop bits
serial: meson: remove unneeded variable assignment in meson_serial_port_write
serial: meson: remove dead code in meson_uart_change_speed
serial: meson: make use of uart_port member mapsize
serial: meson: remove use of flag UPF_IOREMAP
serial: meson: change interrupt description to tty name
Helmut Klein (2):
dt-bindings: serial: Add bindings for the Amlogic Meson UARTs
tty/serial: meson_uart: update to stable bindings
Jan Kiszka (3):
serial: exar: Preconfigure xr17v35x MPIOs as output
serial: uapi: Add support for bus termination
serial: exar: Leave MPIOs as output for Commtech adapters
Jeremy Kerr (1):
drivers/serial: Add driver for Aspeed virtual UART
Joel Stanley (2):
serial: 8250: Add flag so drivers can avoid THRE probe
serial: 8250_of: Add reset support
Johan Hovold (9):
serial: rate limit custom-speed deprecation notice
USB: serial: ftdi_sio: simplify TIOCSSERIAL flag logic
USB: serial: ftdi_sio: remove broken alt-speed handling
tty: simserial: drop unused alt_speed handling
tty: amiserial: drop broken alt-speed support
tty: cyclades: drop broken alt-speed support
tty: rocket: drop broken alt-speed support
tty: ircomm: remove dead and broken ioctl code
tty: drop unused alt_speed from tty_struct
Matthias Brugger (1):
serial: Delete dead code for CIR serial ports
Nandor Han (2):
serial: imx: Enable RTSD only when needed
serial: imx-serial - move DMA buffer configuration to DT
Nava kishore Manne (1):
serial: uartps: Fix kernel doc warnings
Peter Senna Tschudin (1):
imx-serial: RX DMA startup latency
Phil Elwell (2):
serial: 8250: Add CAP_MINI, set for bcm2835aux
serial: 8250: Fix THRE flag usage for CAP_MINI
Sascha Hauer (1):
tty: n_gsm: do not send/receive in ldisc close path
Shubhrajyoti Datta (1):
serial: xilinx_uartps: Fix the error path
Sjoerd Simons (2):
serial: pl010: Move uart_register_driver call to device probe
serial: sh-sci: Move uart_register_driver call to device probe
Stefan Wahren (1):
tty: serdev-ttyport: return actual baudrate from ttyport_set_baudrate
Thomas Bogendoerfer (1):
Fix serial console on SNI RM400 machines
Vignesh R (2):
serial: 8250: omap: Disable DMA for console UART
serial: 8250: 8250_omap: Fix race b/w dma completion and RX timeout
Documentation/ABI/stable/sysfs-driver-aspeed-vuart | 15 +
Documentation/admin-guide/kernel-parameters.txt | 6 +
Documentation/devicetree/bindings/serial/8250.txt | 3 +
.../bindings/serial/actions,owl-uart.txt | 16 +
.../bindings/serial/amlogic,meson-uart.txt | 38 +++
.../devicetree/bindings/serial/fsl-imx-uart.txt | 2 +
.../devicetree/bindings/serial/fsl-lpuart.txt | 2 +
Documentation/serial/n_gsm.txt | 7 +
MAINTAINERS | 2 +-
arch/alpha/include/uapi/asm/ioctls.h | 1 +
arch/ia64/hp/sim/simserial.c | 13 -
arch/mips/include/uapi/asm/ioctls.h | 1 +
arch/parisc/include/uapi/asm/ioctls.h | 1 +
arch/powerpc/include/uapi/asm/ioctls.h | 1 +
arch/sh/include/uapi/asm/ioctls.h | 1 +
arch/sparc/include/uapi/asm/ioctls.h | 3 +-
arch/xtensa/include/uapi/asm/ioctls.h | 1 +
drivers/tty/Makefile | 3 +-
drivers/tty/amiserial.c | 23 +-
drivers/tty/cyclades.c | 21 +-
drivers/tty/hvc/hvcs.c | 3 +-
drivers/tty/n_gsm.c | 54 ++--
drivers/tty/n_null.c | 80 +++++
drivers/tty/pty.c | 93 +++++-
drivers/tty/rocket.c | 27 +-
drivers/tty/serdev/serdev-ttyport.c | 2 +-
drivers/tty/serial/8250/8250.h | 3 +
drivers/tty/serial/8250/8250_aspeed_vuart.c | 323 +++++++++++++++++++++
drivers/tty/serial/8250/8250_bcm2835aux.c | 2 +-
drivers/tty/serial/8250/8250_core.c | 23 +-
drivers/tty/serial/8250/8250_exar.c | 16 +-
drivers/tty/serial/8250/8250_of.c | 10 +
drivers/tty/serial/8250/8250_omap.c | 27 +-
drivers/tty/serial/8250/8250_port.c | 12 +-
drivers/tty/serial/8250/Kconfig | 10 +
drivers/tty/serial/8250/Makefile | 1 +
drivers/tty/serial/Kconfig | 35 ++-
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/amba-pl010.c | 31 +-
drivers/tty/serial/atmel_serial.c | 164 +++--------
drivers/tty/serial/fsl_lpuart.c | 286 ++++++++++++------
drivers/tty/serial/imx.c | 58 ++--
drivers/tty/serial/meson_uart.c | 153 +++++++---
drivers/tty/serial/mpsc.c | 10 +-
drivers/tty/serial/owl-uart.c | 135 +++++++++
drivers/tty/serial/pch_uart.c | 3 +-
drivers/tty/serial/sccnxp.c | 15 +-
drivers/tty/serial/serial_core.c | 5 +-
drivers/tty/serial/sh-sci.c | 45 ++-
drivers/tty/serial/sirfsoc_uart.c | 2 +-
drivers/tty/serial/xilinx_uartps.c | 10 +-
drivers/tty/tty_ldisc.c | 44 ++-
drivers/tty/vt/consolemap.c | 56 +---
drivers/tty/vt/keyboard.c | 3 +-
drivers/tty/vt/vt.c | 8 +-
drivers/tty/vt/vt_ioctl.c | 8 -
drivers/usb/serial/ftdi_sio.c | 67 +----
fs/compat_ioctl.c | 6 -
include/linux/platform_data/atmel.h | 10 -
include/linux/serial_core.h | 1 +
include/linux/tty.h | 1 -
include/uapi/asm-generic/ioctls.h | 1 +
include/uapi/linux/serial.h | 3 +
include/uapi/linux/serial_core.h | 2 +-
include/uapi/linux/tty.h | 2 +
net/irda/ircomm/ircomm_tty_ioctl.c | 107 +------
66 files changed, 1406 insertions(+), 712 deletions(-)
create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-vuart
create mode 100644 Documentation/devicetree/bindings/serial/actions,owl-uart.txt
create mode 100644 Documentation/devicetree/bindings/serial/amlogic,meson-uart.txt
create mode 100644 drivers/tty/n_null.c
create mode 100644 drivers/tty/serial/8250/8250_aspeed_vuart.c
create mode 100644 drivers/tty/serial/owl-uart.c
^ permalink raw reply
* Re: [PATCH] serial: imx-serial - move DMA buffer configuration to DT
From: Uwe Kleine-König @ 2017-07-03 18:17 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Romain Perier, Jiri Slaby, linux-serial-u79uwXL29TY76Z2rM5mHXA,
Nandor Han, devicetree-u79uwXL29TY76Z2rM5mHXA,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <20170629182618.jpahpmuq364ldcv2-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Hello Greg,
I'm a bit disappointed that you didn't drop this patch. At the time I
sent my review it was still in your tty-testing branch and now it's part
of your pull request for 4.13-rc1 (as commit
a3015affdf76ef279fbbb3710a220bab7e9ea04b). :-|
I'm still convinced that this patch is wrong in its current form.
Best regards
Uwe
On Thu, Jun 29, 2017 at 08:26:18PM +0200, Uwe Kleine-König wrote:
> Cc: += devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>
> On Wed, Jun 28, 2017 at 12:15:14PM +0200, Romain Perier wrote:
> > From: Nandor Han <nandor.han-JJi787mZWgc@public.gmane.org>
> >
> > The size of the DMA buffer can affect the delta time between data being
> > produced and data being consumed. Basically the DMA system will move
> > data to tty buffer when a) DMA buffer is full b) serial line is idle.
> > The situation is visible when producer generates data continuously and
> > there is no possibility for idle line. At this point the DMA buffer is
> > directly affecting the delta time.
>
> This doesn't look like a hw property but a configuration item. Also I
> don't understand the problematic case. The i.MX is sending continuously
> and then doesn't receive bytes until the DMA buffer is full? What is the
> DMA buffer? You don't mean the FIFO here, do you?
>
> That doesn't sound like a good fix but more like a work around. Which
> other options did you test to fix your problem?
>
> > The patch will add the possibility to configure the DMA buffers in DT,
> > which case by case can be configured separately for every driver
> > instance. The DT configuration is optional and in case missing the
> > driver will use the 4096 buffer with 4 periods (as before), therefore no
> > clients are impacted by this change.
> >
> > Signed-off-by: Nandor Han <nandor.han-JJi787mZWgc@public.gmane.org>
> > Signed-off-by: Romain Perier <romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
> > ---
> > .../devicetree/bindings/serial/fsl-imx-uart.txt | 2 ++
> > drivers/tty/serial/imx.c | 25 +++++++++++++++-------
> > 2 files changed, 19 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/serial/fsl-imx-uart.txt
> > index 574c3a2..e6b5724 100644
> > --- a/Documentation/devicetree/bindings/serial/fsl-imx-uart.txt
> > +++ b/Documentation/devicetree/bindings/serial/fsl-imx-uart.txt
> > @@ -9,6 +9,7 @@ Optional properties:
> > - fsl,irda-mode : Indicate the uart supports irda mode
> > - fsl,dte-mode : Indicate the uart works in DTE mode. The uart works
> > in DCE mode by default.
> > +- fsl,dma-size : Indicate the size of the DMA buffer and its periods
>
> This is a sparse description, just from reading that I don't understand
> what it does.
>
> > Please check Documentation/devicetree/bindings/serial/serial.txt
> > for the complete list of generic properties.
> > @@ -28,4 +29,5 @@ uart1: serial@73fbc000 {
> > interrupts = <31>;
> > uart-has-rtscts;
> > fsl,dte-mode;
> > + fsl,dma-size = <1024 4>;
> > };
> > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> > index bbefddd..7327477 100644
> > --- a/drivers/tty/serial/imx.c
> > +++ b/drivers/tty/serial/imx.c
> > @@ -186,6 +186,11 @@
> >
> > #define UART_NR 8
> >
> > +/* RX DMA buffer periods */
> > +#define RX_DMA_PERIODS 4
> > +#define RX_BUF_SIZE (PAGE_SIZE)
>
> These names should include "DEFAULT" in their name now to fit their new
> semantic.
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox