headers
Ira W. Snyder | 1 Jan 2010 07:10
Picon
Favicon

fsldma: cleanup driver and fix async_tx compatibility

This patch series cleans up the Freescale DMAEngine driver, including
verifying the locking and making sure that all code paths are correct.
There were a few places that seemed suspicious, and they have been fixed.

I have written a quick memory->memory DMAEngine test driver, and the
performance is identical before and after my changes (<0.1% change). I
measured both setting up the DMA operation (via device_prep_dma_interrupt()
and device_prep_dma_memcpy()) and the actual DMA transfer itself.

As an added bonus, the interrupt load is measurably reduced. My test driver
transfers 32MB as 32x 1MB chunks + 1 interrupt descriptor, using the
functions noted above. Previous to this patch series, 31 interrupts were
generated. After this patch series, only a single interrupt is generated
for the whole transaction.

Some testing on 85xx/86xx hardware would be appreciated. Also, some testing
by the users attempting to use async_tx and talitos to handle RAID offload
would be great as well.

 Documentation/powerpc/dts-bindings/fsl/dma.txt |   17 +-
 drivers/dma/fsldma.c                           | 1036 ++++++++++++------------
 drivers/dma/fsldma.h                           |   35 +-
 3 files changed, 556 insertions(+), 532 deletions(-)

Thanks,
Ira
Permalink | Reply | 0 comments |
headers
Ira W. Snyder | 1 Jan 2010 07:10
Picon
Favicon

[PATCH 2/8] fsldma: remove unused structure members

Remove some unused members from the fsldma data structures. A few trivial
uses of struct resource were converted to use the stack rather than keeping
the memory allocated for the lifetime of the driver.

Signed-off-by: Ira W. Snyder <iws <at> ovro.caltech.edu>
---
 drivers/dma/fsldma.c |   17 ++++++++---------
 drivers/dma/fsldma.h |    4 ----
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 0bad741..0b4e638 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
 <at>  <at>  -1072,6 +1072,7  <at>  <at>  static int __devinit fsl_dma_chan_probe(struct fsl_dma_device *fdev,
 	struct device_node *node, u32 feature, const char *compatible)
 {
 	struct fsl_dma_chan *new_fsl_chan;
+	struct resource res;
 	int err;

 	/* alloc channel */
 <at>  <at>  -1083,7 +1084,7  <at>  <at>  static int __devinit fsl_dma_chan_probe(struct fsl_dma_device *fdev,
 	}

 	/* get dma channel register base */
-	err = of_address_to_resource(node, 0, &new_fsl_chan->reg);
+	err = of_address_to_resource(node, 0, &res);
 	if (err) {
 		dev_err(fdev->dev, "Can't get %s property 'reg'\n",
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ira W. Snyder | 1 Jan 2010 07:10
Picon
Favicon

[PATCH 1/8] fsldma: reduce kernel text size

Some of the functions are written in a way where they use multiple reads
and writes where a single read/write pair could suffice. This shrinks the
kernel text size measurably, while making the functions easier to
understand.

add/remove: 0/0 grow/shrink: 1/4 up/down: 4/-196 (-192)
function                                     old     new   delta
fsl_chan_set_request_count                   120     124      +4
dma_halt                                     300     272     -28
fsl_chan_set_src_loop_size                   208     156     -52
fsl_chan_set_dest_loop_size                  208     156     -52
fsl_chan_xfer_ld_queue                       500     436     -64

Signed-off-by: Ira W. Snyder <iws <at> ovro.caltech.edu>
---
 drivers/dma/fsldma.c |   83 +++++++++++++++++++++++++++-----------------------
 1 files changed, 45 insertions(+), 38 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 296f9e7..0bad741 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
 <at>  <at>  -143,43 +143,45  <at>  <at>  static int dma_is_idle(struct fsl_dma_chan *fsl_chan)

 static void dma_start(struct fsl_dma_chan *fsl_chan)
 {
-	u32 mr_set = 0;
-
-	if (fsl_chan->feature & FSL_DMA_CHAN_PAUSE_EXT) {
-		DMA_OUT(fsl_chan, &fsl_chan->reg_base->bcr, 0, 32);
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ira W. Snyder | 1 Jan 2010 07:10
Picon
Favicon

[PATCH 4/8] fsldma: rename dest to dst for uniformity

Most functions in the standard library use "dst" as a parameter, rather
than "dest". This renames all use of "dest" to "dst" to match the usual
convention.

Signed-off-by: Ira W. Snyder <iws <at> ovro.caltech.edu>
---
 drivers/dma/fsldma.c |   32 ++++++++++++++++----------------
 drivers/dma/fsldma.h |    2 +-
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 6795d96..c2db754 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
 <at>  <at>  -90,14 +90,14  <at>  <at>  static void set_desc_src(struct fsldma_chan *fsl_chan,
 	hw->src_addr = CPU_TO_DMA(fsl_chan, snoop_bits | src, 64);
 }

-static void set_desc_dest(struct fsldma_chan *fsl_chan,
-				struct fsl_dma_ld_hw *hw, dma_addr_t dest)
+static void set_desc_dst(struct fsldma_chan *fsl_chan,
+				struct fsl_dma_ld_hw *hw, dma_addr_t dst)
 {
 	u64 snoop_bits;

 	snoop_bits = ((fsl_chan->feature & FSL_DMA_IP_MASK) == FSL_DMA_IP_85XX)
 		? ((u64)FSL_DMA_DATR_DWRITETYPE_SNOOP_WRITE << 32) : 0;
-	hw->dst_addr = CPU_TO_DMA(fsl_chan, snoop_bits | dest, 64);
+	hw->dst_addr = CPU_TO_DMA(fsl_chan, snoop_bits | dst, 64);
 }
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ira W. Snyder | 1 Jan 2010 07:10
Picon
Favicon

[PATCH 3/8] fsldma: rename struct fsl_dma_chan to struct fsldma_chan

This is the beginning of a cleanup which will change all instances of
"fsl_dma" to "fsldma" to match the name of the driver itself.

Signed-off-by: Ira W. Snyder <iws <at> ovro.caltech.edu>
---
 drivers/dma/fsldma.c |  128 ++++++++++++++++++++++++++-----------------------
 drivers/dma/fsldma.h |   26 +++++-----
 2 files changed, 81 insertions(+), 73 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 0b4e638..6795d96 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
 <at>  <at>  -37,7 +37,7  <at>  <at> 
 #include <asm/fsldma.h>
 #include "fsldma.h"

-static void dma_init(struct fsl_dma_chan *fsl_chan)
+static void dma_init(struct fsldma_chan *fsl_chan)
 {
 	/* Reset the channel */
 	DMA_OUT(fsl_chan, &fsl_chan->reg_base->mr, 0, 32);
 <at>  <at>  -64,23 +64,23  <at>  <at>  static void dma_init(struct fsl_dma_chan *fsl_chan)

 }

-static void set_sr(struct fsl_dma_chan *fsl_chan, u32 val)
+static void set_sr(struct fsldma_chan *fsl_chan, u32 val)
 {
 	DMA_OUT(fsl_chan, &fsl_chan->reg_base->sr, val, 32);
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ira W. Snyder | 1 Jan 2010 07:10
Picon
Favicon

[PATCH 6/8] fsldma: simplify IRQ probing and handling

The IRQ probing is needlessly complex. All off the 83xx device trees in
arch/powerpc/boot/dts/ specify 5 interrupts per DMA controller: one for the
controller, and one for each channel. These interrupts are all attached to
the same IRQ line.

This causes an interesting situation if two channels interrupt at the same
time. The controller's handler will handle the first channel, and the
channel handler will handle the remaining channels. Instead of this, just
let the channel handler handle all channels, and ignore the controller
handler completely.

The same can be accomplished on 83xx by removing the controller's interrupt
property from the device tree. Testing has not shown any problems with this
configuration. All in-tree device trees already have an interrupt property
specified for each channel.

Signed-off-by: Ira W. Snyder <iws <at> ovro.caltech.edu>
---
 Documentation/powerpc/dts-bindings/fsl/dma.txt |   17 +++++---
 drivers/dma/fsldma.c                           |   49 +++++++-----------------
 2 files changed, 25 insertions(+), 41 deletions(-)

diff --git a/Documentation/powerpc/dts-bindings/fsl/dma.txt b/Documentation/powerpc/dts-bindings/fsl/dma.txt
index 0732cdd..d5d2d3d 100644
--- a/Documentation/powerpc/dts-bindings/fsl/dma.txt
+++ b/Documentation/powerpc/dts-bindings/fsl/dma.txt
 <at>  <at>  -12,6 +12,9  <at>  <at>  Required properties:
 - ranges		: Should be defined as specified in 1) to describe the
 		  DMA controller channels.
 - cell-index        : controller index.  0 for controller  <at>  0x8100
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ira W. Snyder | 1 Jan 2010 07:10
Picon
Favicon

[PATCH 5/8] fsldma: clean up the OF subsystem routines

This fixes some errors in the cleanup paths of the OF subsystem, including
missing checks for ioremap failing. Also, some variables were renamed for
brevity.

Signed-off-by: Ira W. Snyder <iws <at> ovro.caltech.edu>
---
 drivers/dma/fsldma.c |  259 +++++++++++++++++++++++++------------------------
 drivers/dma/fsldma.h |    4 +-
 2 files changed, 134 insertions(+), 129 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index c2db754..507b297 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
 <at>  <at>  -40,7 +40,7  <at>  <at> 
 static void dma_init(struct fsldma_chan *fsl_chan)
 {
 	/* Reset the channel */
-	DMA_OUT(fsl_chan, &fsl_chan->reg_base->mr, 0, 32);
+	DMA_OUT(fsl_chan, &fsl_chan->regs->mr, 0, 32);

 	switch (fsl_chan->feature & FSL_DMA_IP_MASK) {
 	case FSL_DMA_IP_85XX:
 <at>  <at>  -49,7 +49,7  <at>  <at>  static void dma_init(struct fsldma_chan *fsl_chan)
 		 * EOSIE - End of segments interrupt enable (basic mode)
 		 * EOLNIE - End of links interrupt enable
 		 */
-		DMA_OUT(fsl_chan, &fsl_chan->reg_base->mr, FSL_DMA_MR_EIE
+		DMA_OUT(fsl_chan, &fsl_chan->regs->mr, FSL_DMA_MR_EIE
 				| FSL_DMA_MR_EOLNIE | FSL_DMA_MR_EOSIE, 32);
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ira W. Snyder | 1 Jan 2010 07:10
Picon
Favicon

[PATCH 8/8] fsldma: major cleanups and fixes

Fix locking. Use two queues in the driver, one for pending transacions, and
one for transactions which are actually running on the hardware. Call
dma_run_dependencies() on descriptor cleanup so that the async_tx API works
correctly.

There are a number of places throughout the code where lists of descriptors
are freed in a loop. Create functions to handle this, and use them instead
of open-coding the loop each time.

Signed-off-by: Ira W. Snyder <iws <at> ovro.caltech.edu>
---
 drivers/dma/fsldma.c |  386 ++++++++++++++++++++++++++-----------------------
 drivers/dma/fsldma.h |    3 +-
 2 files changed, 207 insertions(+), 182 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index f65b28b..d49bdff 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
 <at>  <at>  -61,7 +61,6  <at>  <at>  static void dma_init(struct fsldma_chan *fchan)
 				| FSL_DMA_MR_PRC_RM, 32);
 		break;
 	}
-
 }

 static void set_sr(struct fsldma_chan *fchan, u32 val)
 <at>  <at>  -120,11 +119,6  <at>  <at>  static dma_addr_t get_cdar(struct fsldma_chan *fchan)
 	return DMA_IN(fchan, &fchan->regs->cdar, 64) & ~FSL_DMA_SNEN;
 }
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ira W. Snyder | 1 Jan 2010 07:10
Picon
Favicon

[PATCH 7/8] fsldma: rename fsl_chan to fchan

The name fsl_chan seems too long, so it has been shortened to fchan.

Signed-off-by: Ira W. Snyder <iws <at> ovro.caltech.edu>
---
 drivers/dma/fsldma.c |  446 +++++++++++++++++++++++++-------------------------
 1 files changed, 223 insertions(+), 223 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index d8cc05b..f65b28b 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
 <at>  <at>  -37,19 +37,19  <at>  <at> 
 #include <asm/fsldma.h>
 #include "fsldma.h"

-static void dma_init(struct fsldma_chan *fsl_chan)
+static void dma_init(struct fsldma_chan *fchan)
 {
 	/* Reset the channel */
-	DMA_OUT(fsl_chan, &fsl_chan->regs->mr, 0, 32);
+	DMA_OUT(fchan, &fchan->regs->mr, 0, 32);

-	switch (fsl_chan->feature & FSL_DMA_IP_MASK) {
+	switch (fchan->feature & FSL_DMA_IP_MASK) {
 	case FSL_DMA_IP_85XX:
 		/* Set the channel to below modes:
 		 * EIE - Error interrupt enable
 		 * EOSIE - End of segments interrupt enable (basic mode)
 		 * EOLNIE - End of links interrupt enable
 		 */
(Continue reading)

Permalink | Reply | 0 comments |
headers
Wolfgang Denk | 1 Jan 2010 11:44
Picon
Picon
Favicon

Re: [PATCH v2 3/3] powerpc: Add support for ram filesystems in FIT uImages

Dear Peter,

In message <1262301038.29396.137.camel <at> localhost.localdomain> you wrote:
> 
> > Why chose a different name at all? We could still call it "uImage",
> > meaning "U-Boot image" - U-Boot is clever enought o detect
> > automatically if we pass it an old style or a fit image.
> 
> I agree with your point to an extent, but having 2 types of uImages is
> somewhat confusing to a user, even if U-Boot can differentiate between
> them.  And if the legacy image and FIT image had the same Make target,
> how does a user specify which type they want to build?  The fact that
> both "legacy" and FIT images would reside at arch/powerpc/boot/uImage
> doesn't make things any less confusing to Joe User.

Agreed.

> Currently U-Boot supports booting:
> 1 "legacy" uImages
> 2 "new" Flattened Image Tree (FIT) uImages

The "legacy" uImage format has a number of restrictions not unsimilar
to the restrictions we had in the bootloader / kernel interface when
using the old binary bd_info data structur. For the kernel interface
this has been replaced by using the device tree, and I would like to
see the same happen in U-Boot.

The "new" FIT image type should become the default, and old "legacy"
images should only be generated upon special request (i. e. if some-
one needs these for compatibility with an old, not yet FIT-aware
(Continue reading)

Permalink | Reply | 0 comments |

Gmane