[PATCH v5 0/4] serial: sh-sci: Fix fallback to PIO on DMA failure

[PATCH v5 0/4] serial: sh-sci: Fix fallback to PIO on DMA failure

[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

[PATCH v5 1/4] serial: sh-sci: Extract sci_dma_rx_chan_invalidate()

[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

[PATCH v5 2/4] serial: sh-sci: Extract sci_dma_rx_reenable_irq()

[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

[PATCH v5 3/4] serial: sh-sci: Fix fallback to PIO in sci_dma_rx_complete()

[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

[PATCH v5 4/4] serial: sh-sci: Make RX/TX DMA function names consistent

[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

Re: [PATCH v5 1/4] serial: sh-sci: Extract sci_dma_rx_chan_invalidate()

[Simon Horman]

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>

Re: [PATCH v5 2/4] serial: sh-sci: Extract sci_dma_rx_reenable_irq()

[Simon Horman]

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>

Re: [PATCH v5 3/4] serial: sh-sci: Fix fallback to PIO in sci_dma_rx_complete()

[Simon Horman]

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>

Re: [PATCH v5 4/4] serial: sh-sci: Make RX/TX DMA function names consistent

[Simon Horman]

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>

Re: [PATCH v5 1/4] serial: sh-sci: Extract sci_dma_rx_chan_invalidate()

[Ulrich Hecht]

> 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

Re: [PATCH v5 2/4] serial: sh-sci: Extract sci_dma_rx_reenable_irq()

[Ulrich Hecht]

> 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

Re: [PATCH v5 3/4] serial: sh-sci: Fix fallback to PIO in sci_dma_rx_complete()

[Ulrich Hecht]

> 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

Re: [PATCH v5 4/4] serial: sh-sci: Make RX/TX DMA function names consistent

[Ulrich Hecht]

> 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