* [PATCH v5 0/4] serial: sh-sci: Fix fallback to PIO on DMA failure
@ 2019-01-07 16:23 Geert Uytterhoeven
2019-01-07 16:23 ` [PATCH v5 1/4] serial: sh-sci: Extract sci_dma_rx_chan_invalidate() Geert Uytterhoeven
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2019-01-07 16:23 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Wolfram Sang, Ulrich Hecht, Yoshihiro Shimoda, Simon Horman,
Yoshinori Sato, linux-serial, linux-renesas-soc, linux-sh,
Geert Uytterhoeven
Hi Greg, Jiri,
When submitting a DMA request fails, the sh-sci driver is supposed to
fall back to PIO. However, this never really worked due to various
reasons (sh-sci driver issues and dmaengine framework limitations).
There are three places where DMA submission can fail, and the driver
should fall back to PIO:
1. sci_dma_rx_complete(),
2. sci_submit_rx(),
3. work_fn_tx().
This patch series fixes fallback to PIO in case 1, which is the only one
that does not work yet in v5.0-rc1.
Changes compared to v4:
- Drop patches to fix case 2, whch have been applied in v5.0-rc1,
- Add patches to extract common helpers and make naming more
consistent,
- Call new helpers sci_dma_rx_chan_invalidate() and
sci_dma_rx_reenable_irq() instead of open-coding.
Changes compared to v3:
- Let sci_submit_rx() return -EAGAIN instead of -1 on failure,
- Check for negative error in sci_submit_rx() caller.
Changes compared to v2:
- Add missing definition of "u16 scr" to sci_dma_rx_complete(),
- Move label handle_pio inside #ifdef to kill defined but not used
compiler warning when CONFIG_SERIAL_SH_SCI_DMA=n,
- Move call to dmaengine_terminate_async() in sci_dma_rx_complete()
inside the spinlock, for symmetry with sci_submit_rx(),
- Move the call sci_submit_rx() in sci_rx_interrupt() up, as it may
fail, rendering the modification of scr unused,
- Split in multiple patches,
- Drop RFC status.
Changes compared to v1:
- Fix fallback in sci_dma_rx_complete(),
- Fallback in the transmit path already works fine,
- Widen audience, but keep RFC.
This has been tested on r8a7791/koelsch, using SCIF1 on debug serial 1,
and SCIFA3 on EXIO-B, by introducing random failures in DMA submission
code. For testing, this series is also available in the
topic/scif-pio-fallback-v5 branch of my renesas-drivers git repository
at
git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git.
Thanks!
Geert Uytterhoeven (4):
serial: sh-sci: Extract sci_dma_rx_chan_invalidate()
serial: sh-sci: Extract sci_dma_rx_reenable_irq()
serial: sh-sci: Fix fallback to PIO in sci_dma_rx_complete()
serial: sh-sci: Make RX/TX DMA function names consistent
drivers/tty/serial/sh-sci.c | 71 ++++++++++++++++++++++---------------
1 file changed, 43 insertions(+), 28 deletions(-)
--
2.17.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 1/4] serial: sh-sci: Extract sci_dma_rx_chan_invalidate()
2019-01-07 16:23 [PATCH v5 0/4] serial: sh-sci: Fix fallback to PIO on DMA failure Geert Uytterhoeven
@ 2019-01-07 16:23 ` Geert Uytterhoeven
2019-01-08 14:42 ` Simon Horman
2019-01-09 12:47 ` Ulrich Hecht
2019-01-07 16:23 ` [PATCH v5 2/4] serial: sh-sci: Extract sci_dma_rx_reenable_irq() Geert Uytterhoeven
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2019-01-07 16:23 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Wolfram Sang, Ulrich Hecht, Yoshihiro Shimoda, Simon Horman,
Yoshinori Sato, linux-serial, linux-renesas-soc, linux-sh,
Geert Uytterhoeven
The cookies and channel pointer for the DMA receive channel are
invalidated in two places, and one more is planned.
Extract this functionality in a common helper.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v5:
- New.
---
drivers/tty/serial/sh-sci.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 8df0fd8245203f8b..4c75468680cb547d 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1243,12 +1243,22 @@ static int sci_dma_rx_find_active(struct sci_port *s)
return -1;
}
+static void sci_dma_rx_chan_invalidate(struct sci_port *s)
+{
+ unsigned int i;
+
+ s->chan_rx = NULL;
+ for (i = 0; i < ARRAY_SIZE(s->cookie_rx); i++)
+ s->cookie_rx[i] = -EINVAL;
+ s->active_rx = 0;
+}
+
static void sci_rx_dma_release(struct sci_port *s)
{
struct dma_chan *chan = s->chan_rx_saved;
- s->chan_rx_saved = s->chan_rx = NULL;
- s->cookie_rx[0] = s->cookie_rx[1] = -EINVAL;
+ s->chan_rx_saved = NULL;
+ sci_dma_rx_chan_invalidate(s);
dmaengine_terminate_sync(chan);
dma_free_coherent(chan->device->dev, s->buf_len_rx * 2, s->rx_buf[0],
sg_dma_address(&s->sg_rx[0]));
@@ -1367,10 +1377,7 @@ static int sci_submit_rx(struct sci_port *s, bool port_lock_held)
spin_lock_irqsave(&port->lock, flags);
if (i)
dmaengine_terminate_async(chan);
- for (i = 0; i < 2; i++)
- s->cookie_rx[i] = -EINVAL;
- s->active_rx = 0;
- s->chan_rx = NULL;
+ sci_dma_rx_chan_invalidate(s);
sci_start_rx(port);
if (!port_lock_held)
spin_unlock_irqrestore(&port->lock, flags);
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v5 2/4] serial: sh-sci: Extract sci_dma_rx_reenable_irq()
2019-01-07 16:23 [PATCH v5 0/4] serial: sh-sci: Fix fallback to PIO on DMA failure Geert Uytterhoeven
2019-01-07 16:23 ` [PATCH v5 1/4] serial: sh-sci: Extract sci_dma_rx_chan_invalidate() Geert Uytterhoeven
@ 2019-01-07 16:23 ` Geert Uytterhoeven
2019-01-08 14:46 ` Simon Horman
2019-01-09 12:48 ` Ulrich Hecht
2019-01-07 16:23 ` [PATCH v5 3/4] serial: sh-sci: Fix fallback to PIO in sci_dma_rx_complete() Geert Uytterhoeven
2019-01-07 16:23 ` [PATCH v5 4/4] serial: sh-sci: Make RX/TX DMA function names consistent Geert Uytterhoeven
3 siblings, 2 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2019-01-07 16:23 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Wolfram Sang, Ulrich Hecht, Yoshihiro Shimoda, Simon Horman,
Yoshinori Sato, linux-serial, linux-renesas-soc, linux-sh,
Geert Uytterhoeven
Extract the functionality to direct new serial port interrupts back to
the CPU into its own helper, to prepare for using it from a second
callsite.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v5:
- New.
---
drivers/tty/serial/sh-sci.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 4c75468680cb547d..4d814c30c4189b56 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1274,6 +1274,20 @@ static void start_hrtimer_us(struct hrtimer *hrt, unsigned long usec)
hrtimer_start(hrt, t, HRTIMER_MODE_REL);
}
+static void sci_dma_rx_reenable_irq(struct sci_port *s)
+{
+ struct uart_port *port = &s->port;
+ u16 scr;
+
+ /* Direct new serial port interrupts back to CPU */
+ scr = serial_port_in(port, SCSCR);
+ if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
+ scr &= ~SCSCR_RDRQE;
+ enable_irq(s->irqs[SCIx_RXI_IRQ]);
+ }
+ serial_port_out(port, SCSCR, scr | SCSCR_RIE);
+}
+
static void sci_dma_rx_complete(void *arg)
{
struct sci_port *s = arg;
@@ -1453,7 +1467,6 @@ static enum hrtimer_restart rx_timer_fn(struct hrtimer *t)
unsigned long flags;
unsigned int read;
int active, count;
- u16 scr;
dev_dbg(port->dev, "DMA Rx timed out\n");
@@ -1503,13 +1516,7 @@ static enum hrtimer_restart rx_timer_fn(struct hrtimer *t)
if (port->type == PORT_SCIFA || port->type == PORT_SCIFB)
sci_submit_rx(s, true);
- /* Direct new serial port interrupts back to CPU */
- scr = serial_port_in(port, SCSCR);
- if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
- scr &= ~SCSCR_RDRQE;
- enable_irq(s->irqs[SCIx_RXI_IRQ]);
- }
- serial_port_out(port, SCSCR, scr | SCSCR_RIE);
+ sci_dma_rx_reenable_irq(s);
spin_unlock_irqrestore(&port->lock, flags);
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v5 3/4] serial: sh-sci: Fix fallback to PIO in sci_dma_rx_complete()
2019-01-07 16:23 [PATCH v5 0/4] serial: sh-sci: Fix fallback to PIO on DMA failure Geert Uytterhoeven
2019-01-07 16:23 ` [PATCH v5 1/4] serial: sh-sci: Extract sci_dma_rx_chan_invalidate() Geert Uytterhoeven
2019-01-07 16:23 ` [PATCH v5 2/4] serial: sh-sci: Extract sci_dma_rx_reenable_irq() Geert Uytterhoeven
@ 2019-01-07 16:23 ` Geert Uytterhoeven
2019-01-08 14:48 ` Simon Horman
2019-01-09 12:51 ` Ulrich Hecht
2019-01-07 16:23 ` [PATCH v5 4/4] serial: sh-sci: Make RX/TX DMA function names consistent Geert Uytterhoeven
3 siblings, 2 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2019-01-07 16:23 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Wolfram Sang, Ulrich Hecht, Yoshihiro Shimoda, Simon Horman,
Yoshinori Sato, linux-serial, linux-renesas-soc, linux-sh,
Geert Uytterhoeven
When submitting a DMA request fails in sci_dma_rx_complete(), the driver
tries to fall back to PIO, but that does not work: no more data will be
received, or the kernel will even crash.
Fix this similar as in (but not identical to) sci_submit_rx():
- On SCIF, PIO cannot take over if any DMA transactions are pending,
hence they must be terminated first.
- All active cookies must be invalidated, else rx_timer_fn() may
trigger a NULL pointer dereference.
- Restarting the port is not needed, as it is already running, but
serial port interrupts must be directed back from the DMA engine to
the CPU.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v5:
- Call new helpers sci_dma_rx_chan_invalidate() and
sci_dma_rx_reenable_irq() instead of open-coding,
v4:
- No changes,
v3:
- Add missing definition of "u16 scr",
- Move call to dmaengine_terminate_async() inside the spinlock, for
symmetry with sci_submit_rx(),
- Split in multiple patches.
---
drivers/tty/serial/sh-sci.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 4d814c30c4189b56..891833315698b953 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1337,8 +1337,9 @@ static void sci_dma_rx_complete(void *arg)
dev_warn(port->dev, "Failed submitting Rx DMA descriptor\n");
/* Switch to PIO */
spin_lock_irqsave(&port->lock, flags);
- s->chan_rx = NULL;
- sci_start_rx(port);
+ dmaengine_terminate_async(chan);
+ sci_dma_rx_chan_invalidate(s);
+ sci_dma_rx_reenable_irq(s);
spin_unlock_irqrestore(&port->lock, flags);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v5 4/4] serial: sh-sci: Make RX/TX DMA function names consistent
2019-01-07 16:23 [PATCH v5 0/4] serial: sh-sci: Fix fallback to PIO on DMA failure Geert Uytterhoeven
` (2 preceding siblings ...)
2019-01-07 16:23 ` [PATCH v5 3/4] serial: sh-sci: Fix fallback to PIO in sci_dma_rx_complete() Geert Uytterhoeven
@ 2019-01-07 16:23 ` Geert Uytterhoeven
2019-01-08 14:48 ` Simon Horman
2019-01-09 14:42 ` Ulrich Hecht
3 siblings, 2 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2019-01-07 16:23 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Wolfram Sang, Ulrich Hecht, Yoshihiro Shimoda, Simon Horman,
Yoshinori Sato, linux-serial, linux-renesas-soc, linux-sh,
Geert Uytterhoeven
Most RX/TX-specific DMA functions are prefixed with "sci_dma_[rt]x_".
Rename the exceptions to increase consistency.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v5:
- New.
---
drivers/tty/serial/sh-sci.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 891833315698b953..cb3d5d37674f56f2 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1253,7 +1253,7 @@ static void sci_dma_rx_chan_invalidate(struct sci_port *s)
s->active_rx = 0;
}
-static void sci_rx_dma_release(struct sci_port *s)
+static void sci_dma_rx_release(struct sci_port *s)
{
struct dma_chan *chan = s->chan_rx_saved;
@@ -1343,7 +1343,7 @@ static void sci_dma_rx_complete(void *arg)
spin_unlock_irqrestore(&port->lock, flags);
}
-static void sci_tx_dma_release(struct sci_port *s)
+static void sci_dma_tx_release(struct sci_port *s)
{
struct dma_chan *chan = s->chan_tx_saved;
@@ -1356,7 +1356,7 @@ static void sci_tx_dma_release(struct sci_port *s)
dma_release_channel(chan);
}
-static int sci_submit_rx(struct sci_port *s, bool port_lock_held)
+static int sci_dma_rx_submit(struct sci_port *s, bool port_lock_held)
{
struct dma_chan *chan = s->chan_rx;
struct uart_port *port = &s->port;
@@ -1399,7 +1399,7 @@ static int sci_submit_rx(struct sci_port *s, bool port_lock_held)
return -EAGAIN;
}
-static void work_fn_tx(struct work_struct *work)
+static void sci_dma_tx_work_fn(struct work_struct *work)
{
struct sci_port *s = container_of(work, struct sci_port, work_tx);
struct dma_async_tx_descriptor *desc;
@@ -1458,7 +1458,7 @@ static void work_fn_tx(struct work_struct *work)
return;
}
-static enum hrtimer_restart rx_timer_fn(struct hrtimer *t)
+static enum hrtimer_restart sci_dma_rx_timer_fn(struct hrtimer *t)
{
struct sci_port *s = container_of(t, struct sci_port, rx_timer);
struct dma_chan *chan = s->chan_rx;
@@ -1515,7 +1515,7 @@ static enum hrtimer_restart rx_timer_fn(struct hrtimer *t)
}
if (port->type == PORT_SCIFA || port->type == PORT_SCIFB)
- sci_submit_rx(s, true);
+ sci_dma_rx_submit(s, true);
sci_dma_rx_reenable_irq(s);
@@ -1595,7 +1595,7 @@ static void sci_request_dma(struct uart_port *port)
__func__, UART_XMIT_SIZE,
port->state->xmit.buf, &s->tx_dma_addr);
- INIT_WORK(&s->work_tx, work_fn_tx);
+ INIT_WORK(&s->work_tx, sci_dma_tx_work_fn);
s->chan_tx_saved = s->chan_tx = chan;
}
}
@@ -1630,12 +1630,12 @@ static void sci_request_dma(struct uart_port *port)
}
hrtimer_init(&s->rx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
- s->rx_timer.function = rx_timer_fn;
+ s->rx_timer.function = sci_dma_rx_timer_fn;
s->chan_rx_saved = s->chan_rx = chan;
if (port->type == PORT_SCIFA || port->type == PORT_SCIFB)
- sci_submit_rx(s, false);
+ sci_dma_rx_submit(s, false);
}
}
@@ -1644,9 +1644,9 @@ static void sci_free_dma(struct uart_port *port)
struct sci_port *s = to_sci_port(port);
if (s->chan_tx_saved)
- sci_tx_dma_release(s);
+ sci_dma_tx_release(s);
if (s->chan_rx_saved)
- sci_rx_dma_release(s);
+ sci_dma_rx_release(s);
}
static void sci_flush_buffer(struct uart_port *port)
@@ -1684,7 +1684,7 @@ static irqreturn_t sci_rx_interrupt(int irq, void *ptr)
disable_irq_nosync(irq);
scr |= SCSCR_RDRQE;
} else {
- if (sci_submit_rx(s, false) < 0)
+ if (sci_dma_rx_submit(s, false) < 0)
goto handle_pio;
scr &= ~SCSCR_RIE;
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/4] serial: sh-sci: Extract sci_dma_rx_chan_invalidate()
2019-01-07 16:23 ` [PATCH v5 1/4] serial: sh-sci: Extract sci_dma_rx_chan_invalidate() Geert Uytterhoeven
@ 2019-01-08 14:42 ` Simon Horman
2019-01-09 12:47 ` Ulrich Hecht
1 sibling, 0 replies; 13+ messages in thread
From: Simon Horman @ 2019-01-08 14:42 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Greg Kroah-Hartman, Jiri Slaby, Wolfram Sang, Ulrich Hecht,
Yoshihiro Shimoda, Yoshinori Sato, linux-serial,
linux-renesas-soc, linux-sh
On Mon, Jan 07, 2019 at 05:23:17PM +0100, Geert Uytterhoeven wrote:
> The cookies and channel pointer for the DMA receive channel are
> invalidated in two places, and one more is planned.
> Extract this functionality in a common helper.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/4] serial: sh-sci: Extract sci_dma_rx_reenable_irq()
2019-01-07 16:23 ` [PATCH v5 2/4] serial: sh-sci: Extract sci_dma_rx_reenable_irq() Geert Uytterhoeven
@ 2019-01-08 14:46 ` Simon Horman
2019-01-09 12:48 ` Ulrich Hecht
1 sibling, 0 replies; 13+ messages in thread
From: Simon Horman @ 2019-01-08 14:46 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Greg Kroah-Hartman, Jiri Slaby, Wolfram Sang, Ulrich Hecht,
Yoshihiro Shimoda, Yoshinori Sato, linux-serial,
linux-renesas-soc, linux-sh
On Mon, Jan 07, 2019 at 05:23:18PM +0100, Geert Uytterhoeven wrote:
> Extract the functionality to direct new serial port interrupts back to
> the CPU into its own helper, to prepare for using it from a second
> callsite.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 3/4] serial: sh-sci: Fix fallback to PIO in sci_dma_rx_complete()
2019-01-07 16:23 ` [PATCH v5 3/4] serial: sh-sci: Fix fallback to PIO in sci_dma_rx_complete() Geert Uytterhoeven
@ 2019-01-08 14:48 ` Simon Horman
2019-01-09 12:51 ` Ulrich Hecht
1 sibling, 0 replies; 13+ messages in thread
From: Simon Horman @ 2019-01-08 14:48 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Greg Kroah-Hartman, Jiri Slaby, Wolfram Sang, Ulrich Hecht,
Yoshihiro Shimoda, Yoshinori Sato, linux-serial,
linux-renesas-soc, linux-sh
On Mon, Jan 07, 2019 at 05:23:19PM +0100, Geert Uytterhoeven wrote:
> When submitting a DMA request fails in sci_dma_rx_complete(), the driver
> tries to fall back to PIO, but that does not work: no more data will be
> received, or the kernel will even crash.
>
> Fix this similar as in (but not identical to) sci_submit_rx():
> - On SCIF, PIO cannot take over if any DMA transactions are pending,
> hence they must be terminated first.
> - All active cookies must be invalidated, else rx_timer_fn() may
> trigger a NULL pointer dereference.
> - Restarting the port is not needed, as it is already running, but
> serial port interrupts must be directed back from the DMA engine to
> the CPU.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 4/4] serial: sh-sci: Make RX/TX DMA function names consistent
2019-01-07 16:23 ` [PATCH v5 4/4] serial: sh-sci: Make RX/TX DMA function names consistent Geert Uytterhoeven
@ 2019-01-08 14:48 ` Simon Horman
2019-01-09 14:42 ` Ulrich Hecht
1 sibling, 0 replies; 13+ messages in thread
From: Simon Horman @ 2019-01-08 14:48 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Greg Kroah-Hartman, Jiri Slaby, Wolfram Sang, Ulrich Hecht,
Yoshihiro Shimoda, Yoshinori Sato, linux-serial,
linux-renesas-soc, linux-sh
On Mon, Jan 07, 2019 at 05:23:20PM +0100, Geert Uytterhoeven wrote:
> Most RX/TX-specific DMA functions are prefixed with "sci_dma_[rt]x_".
> Rename the exceptions to increase consistency.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/4] serial: sh-sci: Extract sci_dma_rx_chan_invalidate()
2019-01-07 16:23 ` [PATCH v5 1/4] serial: sh-sci: Extract sci_dma_rx_chan_invalidate() Geert Uytterhoeven
2019-01-08 14:42 ` Simon Horman
@ 2019-01-09 12:47 ` Ulrich Hecht
1 sibling, 0 replies; 13+ messages in thread
From: Ulrich Hecht @ 2019-01-09 12:47 UTC (permalink / raw)
To: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby
Cc: Wolfram Sang, Ulrich Hecht, Yoshihiro Shimoda, Simon Horman,
Yoshinori Sato, linux-serial, linux-renesas-soc, linux-sh
> On January 7, 2019 at 5:23 PM Geert Uytterhoeven <geert+renesas@glider.be> wrote:
>
>
> The cookies and channel pointer for the DMA receive channel are
> invalidated in two places, and one more is planned.
> Extract this functionality in a common helper.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v5:
> - New.
> ---
> drivers/tty/serial/sh-sci.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 8df0fd8245203f8b..4c75468680cb547d 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1243,12 +1243,22 @@ static int sci_dma_rx_find_active(struct sci_port *s)
> return -1;
> }
>
> +static void sci_dma_rx_chan_invalidate(struct sci_port *s)
> +{
> + unsigned int i;
> +
> + s->chan_rx = NULL;
> + for (i = 0; i < ARRAY_SIZE(s->cookie_rx); i++)
> + s->cookie_rx[i] = -EINVAL;
> + s->active_rx = 0;
> +}
> +
> static void sci_rx_dma_release(struct sci_port *s)
> {
> struct dma_chan *chan = s->chan_rx_saved;
>
> - s->chan_rx_saved = s->chan_rx = NULL;
> - s->cookie_rx[0] = s->cookie_rx[1] = -EINVAL;
> + s->chan_rx_saved = NULL;
> + sci_dma_rx_chan_invalidate(s);
> dmaengine_terminate_sync(chan);
> dma_free_coherent(chan->device->dev, s->buf_len_rx * 2, s->rx_buf[0],
> sg_dma_address(&s->sg_rx[0]));
> @@ -1367,10 +1377,7 @@ static int sci_submit_rx(struct sci_port *s, bool port_lock_held)
> spin_lock_irqsave(&port->lock, flags);
> if (i)
> dmaengine_terminate_async(chan);
> - for (i = 0; i < 2; i++)
> - s->cookie_rx[i] = -EINVAL;
> - s->active_rx = 0;
> - s->chan_rx = NULL;
> + sci_dma_rx_chan_invalidate(s);
> sci_start_rx(port);
> if (!port_lock_held)
> spin_unlock_irqrestore(&port->lock, flags);
> --
> 2.17.1
>
Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
CU
Uli
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/4] serial: sh-sci: Extract sci_dma_rx_reenable_irq()
2019-01-07 16:23 ` [PATCH v5 2/4] serial: sh-sci: Extract sci_dma_rx_reenable_irq() Geert Uytterhoeven
2019-01-08 14:46 ` Simon Horman
@ 2019-01-09 12:48 ` Ulrich Hecht
1 sibling, 0 replies; 13+ messages in thread
From: Ulrich Hecht @ 2019-01-09 12:48 UTC (permalink / raw)
To: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby
Cc: Wolfram Sang, Ulrich Hecht, Yoshihiro Shimoda, Simon Horman,
Yoshinori Sato, linux-serial, linux-renesas-soc, linux-sh
> On January 7, 2019 at 5:23 PM Geert Uytterhoeven <geert+renesas@glider.be> wrote:
>
>
> Extract the functionality to direct new serial port interrupts back to
> the CPU into its own helper, to prepare for using it from a second
> callsite.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v5:
> - New.
> ---
> drivers/tty/serial/sh-sci.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 4c75468680cb547d..4d814c30c4189b56 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1274,6 +1274,20 @@ static void start_hrtimer_us(struct hrtimer *hrt, unsigned long usec)
> hrtimer_start(hrt, t, HRTIMER_MODE_REL);
> }
>
> +static void sci_dma_rx_reenable_irq(struct sci_port *s)
> +{
> + struct uart_port *port = &s->port;
> + u16 scr;
> +
> + /* Direct new serial port interrupts back to CPU */
> + scr = serial_port_in(port, SCSCR);
> + if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
> + scr &= ~SCSCR_RDRQE;
> + enable_irq(s->irqs[SCIx_RXI_IRQ]);
> + }
> + serial_port_out(port, SCSCR, scr | SCSCR_RIE);
> +}
> +
> static void sci_dma_rx_complete(void *arg)
> {
> struct sci_port *s = arg;
> @@ -1453,7 +1467,6 @@ static enum hrtimer_restart rx_timer_fn(struct hrtimer *t)
> unsigned long flags;
> unsigned int read;
> int active, count;
> - u16 scr;
>
> dev_dbg(port->dev, "DMA Rx timed out\n");
>
> @@ -1503,13 +1516,7 @@ static enum hrtimer_restart rx_timer_fn(struct hrtimer *t)
> if (port->type == PORT_SCIFA || port->type == PORT_SCIFB)
> sci_submit_rx(s, true);
>
> - /* Direct new serial port interrupts back to CPU */
> - scr = serial_port_in(port, SCSCR);
> - if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
> - scr &= ~SCSCR_RDRQE;
> - enable_irq(s->irqs[SCIx_RXI_IRQ]);
> - }
> - serial_port_out(port, SCSCR, scr | SCSCR_RIE);
> + sci_dma_rx_reenable_irq(s);
>
> spin_unlock_irqrestore(&port->lock, flags);
>
> --
> 2.17.1
>
Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
CU
Uli
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 3/4] serial: sh-sci: Fix fallback to PIO in sci_dma_rx_complete()
2019-01-07 16:23 ` [PATCH v5 3/4] serial: sh-sci: Fix fallback to PIO in sci_dma_rx_complete() Geert Uytterhoeven
2019-01-08 14:48 ` Simon Horman
@ 2019-01-09 12:51 ` Ulrich Hecht
1 sibling, 0 replies; 13+ messages in thread
From: Ulrich Hecht @ 2019-01-09 12:51 UTC (permalink / raw)
To: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby
Cc: Wolfram Sang, Ulrich Hecht, Yoshihiro Shimoda, Simon Horman,
Yoshinori Sato, linux-serial, linux-renesas-soc, linux-sh
> On January 7, 2019 at 5:23 PM Geert Uytterhoeven <geert+renesas@glider.be> wrote:
>
>
> When submitting a DMA request fails in sci_dma_rx_complete(), the driver
> tries to fall back to PIO, but that does not work: no more data will be
> received, or the kernel will even crash.
>
> Fix this similar as in (but not identical to) sci_submit_rx():
> - On SCIF, PIO cannot take over if any DMA transactions are pending,
> hence they must be terminated first.
> - All active cookies must be invalidated, else rx_timer_fn() may
> trigger a NULL pointer dereference.
> - Restarting the port is not needed, as it is already running, but
> serial port interrupts must be directed back from the DMA engine to
> the CPU.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v5:
> - Call new helpers sci_dma_rx_chan_invalidate() and
> sci_dma_rx_reenable_irq() instead of open-coding,
>
> v4:
> - No changes,
>
> v3:
> - Add missing definition of "u16 scr",
> - Move call to dmaengine_terminate_async() inside the spinlock, for
> symmetry with sci_submit_rx(),
> - Split in multiple patches.
> ---
> drivers/tty/serial/sh-sci.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 4d814c30c4189b56..891833315698b953 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1337,8 +1337,9 @@ static void sci_dma_rx_complete(void *arg)
> dev_warn(port->dev, "Failed submitting Rx DMA descriptor\n");
> /* Switch to PIO */
> spin_lock_irqsave(&port->lock, flags);
> - s->chan_rx = NULL;
> - sci_start_rx(port);
> + dmaengine_terminate_async(chan);
> + sci_dma_rx_chan_invalidate(s);
> + sci_dma_rx_reenable_irq(s);
> spin_unlock_irqrestore(&port->lock, flags);
> }
>
> --
> 2.17.1
>
Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
CU
Uli
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 4/4] serial: sh-sci: Make RX/TX DMA function names consistent
2019-01-07 16:23 ` [PATCH v5 4/4] serial: sh-sci: Make RX/TX DMA function names consistent Geert Uytterhoeven
2019-01-08 14:48 ` Simon Horman
@ 2019-01-09 14:42 ` Ulrich Hecht
1 sibling, 0 replies; 13+ messages in thread
From: Ulrich Hecht @ 2019-01-09 14:42 UTC (permalink / raw)
To: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby
Cc: Wolfram Sang, Ulrich Hecht, Yoshihiro Shimoda, Simon Horman,
Yoshinori Sato, linux-serial, linux-renesas-soc, linux-sh
> On January 7, 2019 at 5:23 PM Geert Uytterhoeven <geert+renesas@glider.be> wrote:
>
>
> Most RX/TX-specific DMA functions are prefixed with "sci_dma_[rt]x_".
> Rename the exceptions to increase consistency.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v5:
> - New.
> ---
> drivers/tty/serial/sh-sci.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 891833315698b953..cb3d5d37674f56f2 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1253,7 +1253,7 @@ static void sci_dma_rx_chan_invalidate(struct sci_port *s)
> s->active_rx = 0;
> }
>
> -static void sci_rx_dma_release(struct sci_port *s)
> +static void sci_dma_rx_release(struct sci_port *s)
> {
> struct dma_chan *chan = s->chan_rx_saved;
>
> @@ -1343,7 +1343,7 @@ static void sci_dma_rx_complete(void *arg)
> spin_unlock_irqrestore(&port->lock, flags);
> }
>
> -static void sci_tx_dma_release(struct sci_port *s)
> +static void sci_dma_tx_release(struct sci_port *s)
> {
> struct dma_chan *chan = s->chan_tx_saved;
>
> @@ -1356,7 +1356,7 @@ static void sci_tx_dma_release(struct sci_port *s)
> dma_release_channel(chan);
> }
>
> -static int sci_submit_rx(struct sci_port *s, bool port_lock_held)
> +static int sci_dma_rx_submit(struct sci_port *s, bool port_lock_held)
> {
> struct dma_chan *chan = s->chan_rx;
> struct uart_port *port = &s->port;
> @@ -1399,7 +1399,7 @@ static int sci_submit_rx(struct sci_port *s, bool port_lock_held)
> return -EAGAIN;
> }
>
> -static void work_fn_tx(struct work_struct *work)
> +static void sci_dma_tx_work_fn(struct work_struct *work)
> {
> struct sci_port *s = container_of(work, struct sci_port, work_tx);
> struct dma_async_tx_descriptor *desc;
> @@ -1458,7 +1458,7 @@ static void work_fn_tx(struct work_struct *work)
> return;
> }
>
> -static enum hrtimer_restart rx_timer_fn(struct hrtimer *t)
> +static enum hrtimer_restart sci_dma_rx_timer_fn(struct hrtimer *t)
> {
> struct sci_port *s = container_of(t, struct sci_port, rx_timer);
> struct dma_chan *chan = s->chan_rx;
> @@ -1515,7 +1515,7 @@ static enum hrtimer_restart rx_timer_fn(struct hrtimer *t)
> }
>
> if (port->type == PORT_SCIFA || port->type == PORT_SCIFB)
> - sci_submit_rx(s, true);
> + sci_dma_rx_submit(s, true);
>
> sci_dma_rx_reenable_irq(s);
>
> @@ -1595,7 +1595,7 @@ static void sci_request_dma(struct uart_port *port)
> __func__, UART_XMIT_SIZE,
> port->state->xmit.buf, &s->tx_dma_addr);
>
> - INIT_WORK(&s->work_tx, work_fn_tx);
> + INIT_WORK(&s->work_tx, sci_dma_tx_work_fn);
> s->chan_tx_saved = s->chan_tx = chan;
> }
> }
> @@ -1630,12 +1630,12 @@ static void sci_request_dma(struct uart_port *port)
> }
>
> hrtimer_init(&s->rx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> - s->rx_timer.function = rx_timer_fn;
> + s->rx_timer.function = sci_dma_rx_timer_fn;
>
> s->chan_rx_saved = s->chan_rx = chan;
>
> if (port->type == PORT_SCIFA || port->type == PORT_SCIFB)
> - sci_submit_rx(s, false);
> + sci_dma_rx_submit(s, false);
> }
> }
>
> @@ -1644,9 +1644,9 @@ static void sci_free_dma(struct uart_port *port)
> struct sci_port *s = to_sci_port(port);
>
> if (s->chan_tx_saved)
> - sci_tx_dma_release(s);
> + sci_dma_tx_release(s);
> if (s->chan_rx_saved)
> - sci_rx_dma_release(s);
> + sci_dma_rx_release(s);
> }
>
> static void sci_flush_buffer(struct uart_port *port)
> @@ -1684,7 +1684,7 @@ static irqreturn_t sci_rx_interrupt(int irq, void *ptr)
> disable_irq_nosync(irq);
> scr |= SCSCR_RDRQE;
> } else {
> - if (sci_submit_rx(s, false) < 0)
> + if (sci_dma_rx_submit(s, false) < 0)
> goto handle_pio;
>
> scr &= ~SCSCR_RIE;
> --
> 2.17.1
>
Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
CU
Uli
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-01-09 14:42 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-07 16:23 [PATCH v5 0/4] serial: sh-sci: Fix fallback to PIO on DMA failure Geert Uytterhoeven
2019-01-07 16:23 ` [PATCH v5 1/4] serial: sh-sci: Extract sci_dma_rx_chan_invalidate() Geert Uytterhoeven
2019-01-08 14:42 ` Simon Horman
2019-01-09 12:47 ` Ulrich Hecht
2019-01-07 16:23 ` [PATCH v5 2/4] serial: sh-sci: Extract sci_dma_rx_reenable_irq() Geert Uytterhoeven
2019-01-08 14:46 ` Simon Horman
2019-01-09 12:48 ` Ulrich Hecht
2019-01-07 16:23 ` [PATCH v5 3/4] serial: sh-sci: Fix fallback to PIO in sci_dma_rx_complete() Geert Uytterhoeven
2019-01-08 14:48 ` Simon Horman
2019-01-09 12:51 ` Ulrich Hecht
2019-01-07 16:23 ` [PATCH v5 4/4] serial: sh-sci: Make RX/TX DMA function names consistent Geert Uytterhoeven
2019-01-08 14:48 ` Simon Horman
2019-01-09 14:42 ` Ulrich Hecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).