All of lore.kernel.org
 help / color / mirror / Atom feed
From: prylowski@metasoft.pl (Rafal Prylowski)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 1/5] dmaengine: add ep93xx DMA support
Date: Tue, 15 Nov 2011 16:02:05 +0100	[thread overview]
Message-ID: <4EC27EED.4030108@metasoft.pl> (raw)
In-Reply-To: <3ea8d56034f25cbe3fd8a4c31d7d0d1540b6ac7e.1306662317.git.mika.westerberg@iki.fi>

Hello.

During adaptation of my experimental ep93xx ide driver to the new dmaengine api,
I discovered two issues with ep93xx dma implementation:
1) Control register is incorrectly programmed for IDE (m2m_hw_setup),
probably copy-paste bug ;)
2) Kernel oops when trying to stop running transfers by calling
dmaengine_terminate_all(...) - caused by dereferencing empty list
in ep93xx_dma_get_active

Following is a patch, which solves these problems for me.

Regards,
Rafal Prylowski

Index: linux-2.6/drivers/dma/ep93xx_dma.c
===================================================================
--- linux-2.6.orig/drivers/dma/ep93xx_dma.c
+++ linux-2.6/drivers/dma/ep93xx_dma.c
@@ -246,6 +246,7 @@ static void ep93xx_dma_set_active(struct
 static struct ep93xx_dma_desc *
 ep93xx_dma_get_active(struct ep93xx_dma_chan *edmac)
 {
+	BUG_ON(list_empty(&edmac->active));
 	return list_first_entry(&edmac->active, struct ep93xx_dma_desc, node);
 }
 
@@ -459,9 +460,6 @@ static int m2m_hw_setup(struct ep93xx_dm
 		 * This IDE part is totally untested. Values below are taken
 		 * from the EP93xx Users's Guide and might not be correct.
 		 */
-		control |= M2M_CONTROL_NO_HDSK;
-		control |= M2M_CONTROL_RSS_IDE;
-		control |= M2M_CONTROL_PW_16;
 
 		if (data->direction == DMA_TO_DEVICE) {
 			/* Worst case from the UG */
@@ -473,6 +471,9 @@ static int m2m_hw_setup(struct ep93xx_dm
 			control |= M2M_CONTROL_SAH;
 			control |= M2M_CONTROL_TM_RX;
 		}
+		control |= M2M_CONTROL_NO_HDSK;
+		control |= M2M_CONTROL_RSS_IDE;
+		control |= M2M_CONTROL_PW_16;
 		break;
 
 	default:
@@ -668,24 +669,28 @@ static void ep93xx_dma_unmap_buffers(str
 static void ep93xx_dma_tasklet(unsigned long data)
 {
 	struct ep93xx_dma_chan *edmac = (struct ep93xx_dma_chan *)data;
-	struct ep93xx_dma_desc *desc, *d;
-	dma_async_tx_callback callback;
-	void *callback_param;
+	struct ep93xx_dma_desc *desc = NULL, *d;
+	dma_async_tx_callback callback = NULL;
+	void *callback_param = NULL;
 	LIST_HEAD(list);
 
 	spin_lock_irq(&edmac->lock);
-	desc = ep93xx_dma_get_active(edmac);
-	if (desc->complete) {
-		edmac->last_completed = desc->txd.cookie;
-		list_splice_init(&edmac->active, &list);
+	if (!list_empty(&edmac->active)) {
+		desc = ep93xx_dma_get_active(edmac);
+		if (desc->complete) {
+			edmac->last_completed = desc->txd.cookie;
+			list_splice_init(&edmac->active, &list);
+		}
 	}
 	spin_unlock_irq(&edmac->lock);
 
 	/* Pick up the next descriptor from the queue */
 	ep93xx_dma_advance_work(edmac);
 
-	callback = desc->txd.callback;
-	callback_param = desc->txd.callback_param;
+	if (desc) {
+		callback = desc->txd.callback;
+		callback_param = desc->txd.callback_param;
+	}
 
 	/* Now we can release all the chained descriptors */
 	list_for_each_entry_safe(desc, d, &list, node) {

WARNING: multiple messages have this Message-ID (diff)
From: Rafal Prylowski <prylowski@metasoft.pl>
To: Mika Westerberg <mika.westerberg@iki.fi>
Cc: linux-arm-kernel@lists.infradead.org, rmallon@gmail.com,
	vinod.koul@intel.com, broonie@opensource.wolfsonmicro.com,
	linux-kernel@vger.kernel.org, grant.likely@secretlab.ca,
	hsweeten@visionengravers.com, dan.j.williams@intel.com,
	lrg@ti.com
Subject: Re: [PATCH v2 1/5] dmaengine: add ep93xx DMA support
Date: Tue, 15 Nov 2011 16:02:05 +0100	[thread overview]
Message-ID: <4EC27EED.4030108@metasoft.pl> (raw)
In-Reply-To: <3ea8d56034f25cbe3fd8a4c31d7d0d1540b6ac7e.1306662317.git.mika.westerberg@iki.fi>

Hello.

During adaptation of my experimental ep93xx ide driver to the new dmaengine api,
I discovered two issues with ep93xx dma implementation:
1) Control register is incorrectly programmed for IDE (m2m_hw_setup),
probably copy-paste bug ;)
2) Kernel oops when trying to stop running transfers by calling
dmaengine_terminate_all(...) - caused by dereferencing empty list
in ep93xx_dma_get_active

Following is a patch, which solves these problems for me.

Regards,
Rafal Prylowski

Index: linux-2.6/drivers/dma/ep93xx_dma.c
===================================================================
--- linux-2.6.orig/drivers/dma/ep93xx_dma.c
+++ linux-2.6/drivers/dma/ep93xx_dma.c
@@ -246,6 +246,7 @@ static void ep93xx_dma_set_active(struct
 static struct ep93xx_dma_desc *
 ep93xx_dma_get_active(struct ep93xx_dma_chan *edmac)
 {
+	BUG_ON(list_empty(&edmac->active));
 	return list_first_entry(&edmac->active, struct ep93xx_dma_desc, node);
 }
 
@@ -459,9 +460,6 @@ static int m2m_hw_setup(struct ep93xx_dm
 		 * This IDE part is totally untested. Values below are taken
 		 * from the EP93xx Users's Guide and might not be correct.
 		 */
-		control |= M2M_CONTROL_NO_HDSK;
-		control |= M2M_CONTROL_RSS_IDE;
-		control |= M2M_CONTROL_PW_16;
 
 		if (data->direction == DMA_TO_DEVICE) {
 			/* Worst case from the UG */
@@ -473,6 +471,9 @@ static int m2m_hw_setup(struct ep93xx_dm
 			control |= M2M_CONTROL_SAH;
 			control |= M2M_CONTROL_TM_RX;
 		}
+		control |= M2M_CONTROL_NO_HDSK;
+		control |= M2M_CONTROL_RSS_IDE;
+		control |= M2M_CONTROL_PW_16;
 		break;
 
 	default:
@@ -668,24 +669,28 @@ static void ep93xx_dma_unmap_buffers(str
 static void ep93xx_dma_tasklet(unsigned long data)
 {
 	struct ep93xx_dma_chan *edmac = (struct ep93xx_dma_chan *)data;
-	struct ep93xx_dma_desc *desc, *d;
-	dma_async_tx_callback callback;
-	void *callback_param;
+	struct ep93xx_dma_desc *desc = NULL, *d;
+	dma_async_tx_callback callback = NULL;
+	void *callback_param = NULL;
 	LIST_HEAD(list);
 
 	spin_lock_irq(&edmac->lock);
-	desc = ep93xx_dma_get_active(edmac);
-	if (desc->complete) {
-		edmac->last_completed = desc->txd.cookie;
-		list_splice_init(&edmac->active, &list);
+	if (!list_empty(&edmac->active)) {
+		desc = ep93xx_dma_get_active(edmac);
+		if (desc->complete) {
+			edmac->last_completed = desc->txd.cookie;
+			list_splice_init(&edmac->active, &list);
+		}
 	}
 	spin_unlock_irq(&edmac->lock);
 
 	/* Pick up the next descriptor from the queue */
 	ep93xx_dma_advance_work(edmac);
 
-	callback = desc->txd.callback;
-	callback_param = desc->txd.callback_param;
+	if (desc) {
+		callback = desc->txd.callback;
+		callback_param = desc->txd.callback_param;
+	}
 
 	/* Now we can release all the chained descriptors */
 	list_for_each_entry_safe(desc, d, &list, node) {

  reply	other threads:[~2011-11-15 15:02 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-29 10:10 [PATCH v2 0/5] ep93xx DMA patches Mika Westerberg
2011-05-29 10:10 ` Mika Westerberg
2011-05-29 10:10 ` [PATCH v2 1/5] dmaengine: add ep93xx DMA support Mika Westerberg
2011-05-29 10:10   ` Mika Westerberg
2011-11-15 15:02   ` Rafal Prylowski [this message]
2011-11-15 15:02     ` Rafal Prylowski
2011-11-15 17:59     ` H Hartley Sweeten
2011-11-15 17:59       ` H Hartley Sweeten
2011-11-16  6:48       ` Mika Westerberg
2011-11-16  6:48         ` Mika Westerberg
2011-11-16  9:00       ` Rafal Prylowski
2011-11-16  9:00         ` Rafal Prylowski
2011-11-16  6:55     ` Mika Westerberg
2011-11-16  6:55       ` Mika Westerberg
2011-11-16  8:31       ` Rafal Prylowski
2011-11-16  8:31         ` Rafal Prylowski
2011-11-19 12:38         ` Mika Westerberg
2011-11-19 12:38           ` Mika Westerberg
2011-11-21  7:44           ` Rafal Prylowski
2011-11-21  7:44             ` Rafal Prylowski
2011-11-21  8:01             ` Mika Westerberg
2011-11-21  8:01               ` Mika Westerberg
2011-11-21  8:32               ` Vinod Koul
2011-11-21  8:32                 ` Vinod Koul
2011-11-22  5:59                 ` Mika Westerberg
2011-11-22  5:59                   ` Mika Westerberg
2011-11-23 10:47                   ` Vinod Koul
2011-11-23 10:47                     ` Vinod Koul
2011-11-21  8:54               ` Rafal Prylowski
2011-11-21  8:54                 ` Rafal Prylowski
2011-11-22  6:47                 ` Mika Westerberg
2011-11-22  6:47                   ` Mika Westerberg
2011-11-22  8:45                   ` Rafal Prylowski
2011-11-22  8:45                     ` Rafal Prylowski
2011-05-29 10:10 ` [PATCH v2 2/5] ep93xx: add dmaengine platform code Mika Westerberg
2011-05-29 10:10   ` Mika Westerberg
2011-05-29 10:10 ` [PATCH v2 3/5] ASoC: ep93xx: convert to use the DMA engine API Mika Westerberg
2011-05-29 10:10   ` Mika Westerberg
2011-05-29 10:10 ` [PATCH v2 4/5] ep93xx: remove the old M2P DMA code Mika Westerberg
2011-05-29 10:10   ` Mika Westerberg
2011-05-29 10:10 ` [PATCH v2 5/5] spi/ep93xx: add DMA support Mika Westerberg
2011-05-29 10:10   ` Mika Westerberg
2011-06-03 20:44   ` Grant Likely
2011-06-03 20:44     ` Grant Likely
2011-06-07 17:14     ` Mika Westerberg
2011-06-07 17:14       ` Mika Westerberg
2011-06-07 18:45       ` Grant Likely
2011-06-07 18:45         ` Grant Likely
2011-06-07 19:06         ` Mika Westerberg
2011-06-07 19:06           ` Mika Westerberg
2011-06-07 19:18           ` Grant Likely
2011-06-07 19:18             ` Grant Likely
2011-06-07 19:29             ` Mika Westerberg
2011-06-07 19:29               ` Mika Westerberg
2011-06-07 19:40               ` Grant Likely
2011-06-07 19:40                 ` Grant Likely
2011-06-08  3:58                 ` Koul, Vinod
2011-06-08  3:58                   ` Koul, Vinod
2011-06-08 21:53                   ` Grant Likely
2011-06-08 21:53                     ` Grant Likely
2011-06-09 16:42                     ` Koul, Vinod
2011-06-09 16:42                       ` Koul, Vinod
2011-06-09 18:46                       ` Grant Likely
2011-06-09 18:46                         ` Grant Likely
2011-06-09 19:15                         ` Mika Westerberg
2011-06-09 19:15                           ` Mika Westerberg
2011-06-05  8:19 ` [PATCH v2 0/5] ep93xx DMA patches Mika Westerberg
2011-06-05  8:19   ` Mika Westerberg
2011-06-06  6:39   ` Koul, Vinod
2011-06-06  6:39     ` Koul, Vinod
2011-06-06 16:42     ` Mika Westerberg
2011-06-06 16:42       ` Mika Westerberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4EC27EED.4030108@metasoft.pl \
    --to=prylowski@metasoft.pl \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.