[PATCH] serial: sh-sci: submit rx dma the way the DMA driver can handle it

Subsystems: the rest, tty layer and serial drivers

STALE4397d

4 messages, 2 authors, 2014-10-06 · open the first message on its own page

[PATCH] serial: sh-sci: submit rx dma the way the DMA driver can handle it

From: Wolfram Sang <hidden>
Date: 2014-10-02 16:14:08

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The shdmac driver has a problem when one wants to submit two RX
descriptors and then use issue_pending on both (like this driver does):

Submit first desc
-> channel state after: SHDMA_PM_ESTABLISHED

Submit second desc
-> channel state after: SHDMA_PM_PENDING

PENDING here means: "Oh, we will wait until the first desc is finished
and then the second desc will be grabbed from the list anyhow"

However, because issue_pending does nothing for states !SHDMA_PM_ESTABLISHED, nothing ever gets active :(

Since the DMA driver is probably replaced soon anyhow, I decided for the
least intrusive workaround which is to use issue_pending on every RX
desc. (Yes, setting s->active_rx twice is ugly).

Probably not for upstream! However, this patch restores DMA capability
for SCIFA on my Lager board. Sadly, not for SCIF due to interrupt
problems still to be investigated.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Some more words about the not working SCIF:

Once I submit the RX descriptor, I don't get any irq anymore (which the driver
needs to detect timeouts). Before that, I got them.

SCIFA has a bit (SCSCR_RDRQE) to select if the interrupt should go to the CPU
or DMAC and the driver handles this correctly, so DMA works again.

Plain SCIF does not have such a bit. The documentation has a chapter "SCIF
Interrupt Sources and the DMAC" (chapter 51.4 here in v0.9) which is a little
vague to me. But from what I understand, I should get an irq along with the
DMAC transferring data. The implementation of this driver shows that earlier SH
hardware worked this way. However, I don't get interrupts, so the timeout
mechanism fails. DMA works partly, you can receive in 32 bytes chunks only. If
you are happy with that, you can also use SCIF. Good practice for your typing
skills ;)

So, my SCIF->SCIFA patch becomes more useful, too.

 drivers/tty/serial/sh-sci.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index b11819bc95c4..f23b4d2ab51f 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1365,16 +1365,18 @@ static void sci_submit_rx(struct sci_port *s)
 			}
 			dev_warn(s->port.dev,
 				 "failed to re-start DMA, using PIO\n");
+
+			chan->device->device_control(chan, DMA_TERMINATE_ALL, 0);
 			sci_rx_dma_release(s, true);
 			return;
 		}
+
+		s->active_rx = s->cookie_rx[0];
+
+		dma_async_issue_pending(chan);
 		dev_dbg(s->port.dev, "%s(): cookie %d to #%d\n",
 			__func__, s->cookie_rx[i], i);
 	}
-
-	s->active_rx = s->cookie_rx[0];
-
-	dma_async_issue_pending(chan);
 }
 
 static void work_fn_rx(struct work_struct *work)
-- 
2.0.0

Re: [PATCH] serial: sh-sci: submit rx dma the way the DMA driver can handle it

From: Simon Horman <horms@verge.net.au>
Date: 2014-10-06 01:55:40

On Thu, Oct 02, 2014 at 06:14:08PM +0200, Wolfram Sang wrote:
From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The shdmac driver has a problem when one wants to submit two RX
descriptors and then use issue_pending on both (like this driver does):

Submit first desc
-> channel state after: SHDMA_PM_ESTABLISHED

Submit second desc
-> channel state after: SHDMA_PM_PENDING

PENDING here means: "Oh, we will wait until the first desc is finished
and then the second desc will be grabbed from the list anyhow"

However, because issue_pending does nothing for states !> SHDMA_PM_ESTABLISHED, nothing ever gets active :(

Since the DMA driver is probably replaced soon anyhow, I decided for the
least intrusive workaround which is to use issue_pending on every RX
desc. (Yes, setting s->active_rx twice is ugly).

Probably not for upstream! However, this patch restores DMA capability
for SCIFA on my Lager board. Sadly, not for SCIF due to interrupt
problems still to be investigated.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Acked-by: Simon Horman <redacted>

sh-sci patches usually go via linux-serial@vger.kernel.org and
Greg Kroah-Hartman [off-list ref]. Could you repost this patch
with those parties included?
quoted hunk
---

Some more words about the not working SCIF:

Once I submit the RX descriptor, I don't get any irq anymore (which the driver
needs to detect timeouts). Before that, I got them.

SCIFA has a bit (SCSCR_RDRQE) to select if the interrupt should go to the CPU
or DMAC and the driver handles this correctly, so DMA works again.

Plain SCIF does not have such a bit. The documentation has a chapter "SCIF
Interrupt Sources and the DMAC" (chapter 51.4 here in v0.9) which is a little
vague to me. But from what I understand, I should get an irq along with the
DMAC transferring data. The implementation of this driver shows that earlier SH
hardware worked this way. However, I don't get interrupts, so the timeout
mechanism fails. DMA works partly, you can receive in 32 bytes chunks only. If
you are happy with that, you can also use SCIF. Good practice for your typing
skills ;)

So, my SCIF->SCIFA patch becomes more useful, too.

 drivers/tty/serial/sh-sci.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index b11819bc95c4..f23b4d2ab51f 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1365,16 +1365,18 @@ static void sci_submit_rx(struct sci_port *s)
 			}
 			dev_warn(s->port.dev,
 				 "failed to re-start DMA, using PIO\n");
+
+			chan->device->device_control(chan, DMA_TERMINATE_ALL, 0);
 			sci_rx_dma_release(s, true);
 			return;
 		}
+
+		s->active_rx = s->cookie_rx[0];
+
+		dma_async_issue_pending(chan);
 		dev_dbg(s->port.dev, "%s(): cookie %d to #%d\n",
 			__func__, s->cookie_rx[i], i);
 	}
-
-	s->active_rx = s->cookie_rx[0];
-
-	dma_async_issue_pending(chan);
 }
 
 static void work_fn_rx(struct work_struct *work)
-- 
2.0.0

Re: [PATCH] serial: sh-sci: submit rx dma the way the DMA driver can handle it

From: Wolfram Sang <hidden>
Date: 2014-10-06 05:59:15

On Mon, Oct 06, 2014 at 10:55:40AM +0900, Simon Horman wrote:
On Thu, Oct 02, 2014 at 06:14:08PM +0200, Wolfram Sang wrote:
quoted
From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The shdmac driver has a problem when one wants to submit two RX
descriptors and then use issue_pending on both (like this driver does):

Submit first desc
-> channel state after: SHDMA_PM_ESTABLISHED

Submit second desc
-> channel state after: SHDMA_PM_PENDING

PENDING here means: "Oh, we will wait until the first desc is finished
and then the second desc will be grabbed from the list anyhow"

However, because issue_pending does nothing for states !=
SHDMA_PM_ESTABLISHED, nothing ever gets active :(

Since the DMA driver is probably replaced soon anyhow, I decided for the
least intrusive workaround which is to use issue_pending on every RX
desc. (Yes, setting s->active_rx twice is ugly).

Probably not for upstream! However, this patch restores DMA capability
for SCIFA on my Lager board. Sadly, not for SCIF due to interrupt
problems still to be investigated.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Acked-by: Simon Horman <redacted>

sh-sci patches usually go via linux-serial@vger.kernel.org and
Greg Kroah-Hartman [off-list ref]. Could you repost this patch
with those parties included?
I wanted to send this patch as RFC, but forgot to rename it :) I'd
really like some discussion about it, probably in Düsseldorf even. I am
still not sure this patch should go upstream, but we should talk about
it.

Thanks,

   Wolfram

Re: [PATCH] serial: sh-sci: submit rx dma the way the DMA driver can handle it

From: Simon Horman <horms@verge.net.au>
Date: 2014-10-06 07:39:20

On Mon, Oct 06, 2014 at 07:59:15AM +0200, Wolfram Sang wrote:
On Mon, Oct 06, 2014 at 10:55:40AM +0900, Simon Horman wrote:
quoted
On Thu, Oct 02, 2014 at 06:14:08PM +0200, Wolfram Sang wrote:
quoted
From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The shdmac driver has a problem when one wants to submit two RX
descriptors and then use issue_pending on both (like this driver does):

Submit first desc
-> channel state after: SHDMA_PM_ESTABLISHED

Submit second desc
-> channel state after: SHDMA_PM_PENDING

PENDING here means: "Oh, we will wait until the first desc is finished
and then the second desc will be grabbed from the list anyhow"

However, because issue_pending does nothing for states !> > > SHDMA_PM_ESTABLISHED, nothing ever gets active :(

Since the DMA driver is probably replaced soon anyhow, I decided for the
least intrusive workaround which is to use issue_pending on every RX
desc. (Yes, setting s->active_rx twice is ugly).

Probably not for upstream! However, this patch restores DMA capability
for SCIFA on my Lager board. Sadly, not for SCIF due to interrupt
problems still to be investigated.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Acked-by: Simon Horman <redacted>

sh-sci patches usually go via linux-serial@vger.kernel.org and
Greg Kroah-Hartman [off-list ref]. Could you repost this patch
with those parties included?
I wanted to send this patch as RFC, but forgot to rename it :) I'd
really like some discussion about it, probably in Düsseldorf even. I am
still not sure this patch should go upstream, but we should talk about
it.
Thanks, I understand. Lets put this on the agenda for Düsseldorf.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help