[U-Boot] [PATCH 0/2] Use DMA in SPL

[U-Boot] [PATCH 0/2] Use DMA in SPL

[Simon Schwarz]

This implements the DMA copy in the SPL.

The DMA usage is activated for devkit8000.

Based on DMA driver patch:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/109744/focus=109747

Simon Schwarz (2):
  nand_spl_simple: Add omap3 DMA usage to SPL
  devkit8000: Activate DMA support in SPL

 drivers/mtd/nand/nand_spl_simple.c |  185 ++++++++++++++++++++++++++++++++++--
 include/configs/devkit8000.h       |    2 +
 2 files changed, 178 insertions(+), 9 deletions(-)

-- 
1.7.4.1

[U-Boot] [PATCH 1/2] nand_spl_simple: Add omap3 DMA usage to SPL

[Simon Schwarz]

This adds DMA copy for the nand spl implementation. If CONFIG_SPL_DMA_SUPPORT
is defined the DMA is used.

Based on DMA driver patch:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/109744/focus=109747

Signed-off-by: Simon Schwarz <simonschwarzcor@gmail.com>
Cc: scottwood at freescale.com
Cc: s-paulraj at ti.com
---
 drivers/mtd/nand/nand_spl_simple.c |  185 ++++++++++++++++++++++++++++++++++--
 1 files changed, 176 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c
index 71491d4..b45322b 100644
--- a/drivers/mtd/nand/nand_spl_simple.c
+++ b/drivers/mtd/nand/nand_spl_simple.c
@@ -2,6 +2,9 @@
  * (C) Copyright 2006-2008
  * Stefan Roese, DENX Software Engineering, sr at denx.de.
  *
+ * Copyright (C) 2011
+ * Corscience GmbH & Co. KG - Simon Schwarz <schwarz@corscience.de>
+ *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
  * published by the Free Software Foundation; either version 2 of
@@ -21,10 +24,21 @@
 #include <common.h>
 #include <nand.h>
 #include <asm/io.h>
+#include <asm/arch/dma.h>
+#include <asm/arch/cpu.h>
 
 static int nand_ecc_pos[] = CONFIG_SYS_NAND_ECCPOS;
 static nand_info_t mtd;
 static struct nand_chip nand_chip;
+static struct nand_chip *this;
+
+struct ecc_wait_entry {
+	int valid;
+	uint8_t *p;
+	u_char *ecc_code;
+	u_char *ecc_calc;
+};
+static struct ecc_wait_entry ecc_wait;
 
 #if (CONFIG_SYS_NAND_PAGE_SIZE <= 512)
 /*
@@ -33,7 +47,6 @@ static struct nand_chip nand_chip;
 static int nand_command(int block, int page, uint32_t offs,
 	u8 cmd)
 {
-	struct nand_chip *this = mtd.priv;
 	int page_addr = page + block * CONFIG_SYS_NAND_PAGE_COUNT;
 
 	while (!this->dev_ready(&mtd))
@@ -46,11 +59,11 @@ static int nand_command(int block, int page, uint32_t offs,
 	this->cmd_ctrl(&mtd, offs, NAND_CTRL_ALE | NAND_CTRL_CHANGE);
 	this->cmd_ctrl(&mtd, page_addr & 0xff, NAND_CTRL_ALE); /* A[16:9] */
 	this->cmd_ctrl(&mtd, (page_addr >> 8) & 0xff,
-		       NAND_CTRL_ALE); /* A[24:17] */
+			NAND_CTRL_ALE); /* A[24:17] */
 #ifdef CONFIG_SYS_NAND_4_ADDR_CYCLE
 	/* One more address cycle for devices > 32MiB */
 	this->cmd_ctrl(&mtd, (page_addr >> 16) & 0x0f,
-		       NAND_CTRL_ALE); /* A[28:25] */
+			NAND_CTRL_ALE); /* A[28:25] */
 #endif
 	/* Latch in address */
 	this->cmd_ctrl(&mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
@@ -93,20 +106,20 @@ static int nand_command(int block, int page, uint32_t offs,
 	/* Set ALE and clear CLE to start address cycle */
 	/* Column address */
 	hwctrl(&mtd, offs & 0xff,
-		       NAND_CTRL_ALE | NAND_CTRL_CHANGE); /* A[7:0] */
+			NAND_CTRL_ALE | NAND_CTRL_CHANGE); /* A[7:0] */
 	hwctrl(&mtd, (offs >> 8) & 0xff, NAND_CTRL_ALE); /* A[11:9] */
 	/* Row address */
 	hwctrl(&mtd, (page_addr & 0xff), NAND_CTRL_ALE); /* A[19:12] */
 	hwctrl(&mtd, ((page_addr >> 8) & 0xff),
-		       NAND_CTRL_ALE); /* A[27:20] */
+			NAND_CTRL_ALE); /* A[27:20] */
 #ifdef CONFIG_SYS_NAND_5_ADDR_CYCLE
 	/* One more address cycle for devices > 128MiB */
 	hwctrl(&mtd, (page_addr >> 16) & 0x0f,
-		       NAND_CTRL_ALE); /* A[31:28] */
+			NAND_CTRL_ALE); /* A[31:28] */
 #endif
 	/* Latch in address */
 	hwctrl(&mtd, NAND_CMD_READSTART,
-		       NAND_CTRL_CLE | NAND_CTRL_CHANGE);
+			NAND_CTRL_CLE | NAND_CTRL_CHANGE);
 	hwctrl(&mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
 
 	/*
@@ -121,7 +134,6 @@ static int nand_command(int block, int page, uint32_t offs,
 
 static int nand_is_bad_block(int block)
 {
-	struct nand_chip *this = mtd.priv;
 
 	nand_command(block, 0, CONFIG_SYS_NAND_BAD_BLOCK_POS,
 		NAND_CMD_READOOB);
@@ -187,7 +199,7 @@ static int nand_read_page(int block, int page, void *dst)
 	return 0;
 }
 
-int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
+int nand_spl_load_image_nondma(uint32_t offs, unsigned int size, void *dst)
 {
 	unsigned int block, lastblock;
 	unsigned int page;
@@ -221,16 +233,171 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
 	return 0;
 }
 
+#ifdef CONFIG_SPL_DMA_SUPPORT
+void correct_ecc_dma(void)
+{
+	int eccsize = CONFIG_SYS_NAND_ECCSIZE;
+	int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
+	int eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
+	int i;
+	uint8_t *p = ecc_wait.p;
+
+	for (i = 0 ; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
+		/* No chance to do something with the possible error message
+		 * from correct_data(). We just hope that all possible errors
+		 * are corrected by this routine.
+		 */
+		this->ecc.correct(&mtd, ecc_wait.p, &ecc_wait.ecc_code[i],
+			&ecc_wait.ecc_calc[i]);
+	}
+	ecc_wait.valid = 0;
+}
+
+static int nand_read_page_dma(int block, int page, void *dst)
+{
+	u_char *ecc_calc;
+	u_char *ecc_code;
+	u_char *oob_data;
+	int i;
+	int res;
+	int eccsize = CONFIG_SYS_NAND_ECCSIZE;
+	int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
+	int eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
+	uint8_t *p = dst;
+
+	nand_command(block, page, 0, NAND_CMD_READ0);
+
+	/* No malloc available for now, just use some temporary locations
+	 * in SDRAM
+	 */
+	ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000);
+	ecc_code = ecc_calc + 0x100;
+	oob_data = ecc_calc + 0x200;
+	res = 0;
+	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
+		res += omap3_dma_conf_transfer(0, nand_chip.IO_ADDR_R,
+			(uint32_t *)p, CONFIG_SYS_NAND_ECCSIZE/4);
+		this->ecc.hwctl(&mtd, NAND_ECC_READ);
+		res += omap3_dma_start_transfer(0);
+		/* correct ecc from former transfer */
+		if (ecc_wait.valid != 0)
+			correct_ecc_dma();
+		res += omap3_dma_wait_for_transfer(0);
+		this->ecc.calculate(&mtd, p, &ecc_calc[i]);
+		if (res) {
+			printf("spl: DMA error while data read\n");
+			return -1;
+		}
+	}
+
+	/*this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);*/
+	res = omap3_dma_conf_transfer(0, nand_chip.IO_ADDR_R,
+		(uint32_t *)oob_data, CONFIG_SYS_NAND_OOBSIZE/4);
+	res += omap3_dma_start_transfer(0);
+	res += omap3_dma_wait_for_transfer(0);
+	if (res) {
+		printf("spl: DMA error while oob read\n");
+		return -1;
+	}
+
+	/* Pick the ECC bytes out of the oob data */
+	for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
+		ecc_code[i] = oob_data[nand_ecc_pos[i]];
+
+	/* add to ecc_wait - just ok until the nex ecc.calc!*/
+	ecc_wait.valid = 1;
+	ecc_wait.p = dst;
+	ecc_wait.ecc_code = ecc_code;
+	ecc_wait.ecc_calc = ecc_calc;
+	return 0;
+}
+
+int nand_spl_load_image_dma(uint32_t offs, unsigned int size, void *dst)
+{
+	unsigned int block, lastblock;
+	unsigned int page;
+
+	/*
+	 * offs has to be aligned to a page address!
+	 */
+	block = offs / CONFIG_SYS_NAND_BLOCK_SIZE;
+	lastblock = (offs + size - 1) / CONFIG_SYS_NAND_BLOCK_SIZE;
+	page = (offs % CONFIG_SYS_NAND_BLOCK_SIZE) / CONFIG_SYS_NAND_PAGE_SIZE;
+	ecc_wait.valid = 0;
+	while (block <= lastblock) {
+		if (!nand_is_bad_block(block)) {
+			/*
+			 * Skip bad blocks
+			 */
+			while (page < CONFIG_SYS_NAND_PAGE_COUNT) {
+				nand_read_page_dma(block, page, dst);
+				dst += CONFIG_SYS_NAND_PAGE_SIZE;
+				page++;
+			}
+			if (ecc_wait.valid != 0)
+				correct_ecc_dma();
+
+			page = 0;
+		} else {
+			lastblock++;
+		}
+
+		block++;
+	}
+	return 0;
+}
+
+/* Read from NAND with DMA if appropriate
+ * offs: Offset in NAND to read from
+ * size: site to read
+ * dst: destination pointer (multiple of page sizes ->
+ *	CONFIG_SYS_NAND_PAGE_SIZE*roundup(size/CONFIG_SYS_NAND_PAGE_SIZE))
+ *
+ * RETURN non-null means error
+ */
+int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
+{
+	struct dma4_chan cfg;
+	debug("using DMA load...\n");
+	omap3_dma_init();
+
+	/* config the channel */
+	omap3_dma_get_conf_chan(0, &cfg);
+	cfg.csdp = CSDP_DATA_TYPE_32BIT | CSDP_SRC_BURST_EN_64BYTES |
+		CSDP_DST_BURST_EN_64BYTES | CSDP_DST_ENDIAN_LOCK_ADAPT |
+		CSDP_DST_ENDIAN_LITTLE | CSDP_SRC_ENDIAN_LOCK_ADAPT |
+		CSDP_SRC_ENDIAN_LITTLE;
+	cfg.cfn = 1;
+	cfg.ccr = CCR_READ_PRIORITY_HIGH | CCR_SRC_AMODE_CONSTANT |
+		CCR_DST_AMODE_POST_INC;
+	cfg.cicr = (1 << 5) | (1 << 11) | (1 << 10) | (1 << 8);
+	omap3_dma_conf_chan(0, &cfg);
+
+
+	nand_spl_load_image_dma(offs, size, dst);
+	return 0;
+}
+
+#else /* use cpu copy */
+int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
+	__attribute__ ((alias("nand_spl_load_image_nondma")));
+
+#endif /* CONFIG_SPL_DMA_SUPPORT */
+
+
+
 /* nand_init() - initialize data to make nand usable by SPL */
 void nand_init(void)
 {
 	/*
 	 * Init board specific nand support
 	 */
+	debug(">>nand_init()\n");
 	mtd.priv = &nand_chip;
 	nand_chip.IO_ADDR_R = nand_chip.IO_ADDR_W =
 		(void  __iomem *)CONFIG_SYS_NAND_BASE;
 	nand_chip.options = 0;
+	this = mtd.priv;
 	board_nand_init(&nand_chip);
 
 	if (nand_chip.select_chip)
-- 
1.7.4.1

[U-Boot] [PATCH 2/2] devkit8000: Activate DMA support in SPL

[Simon Schwarz]

This activates the DMA support in the SPL.

Based on DMA driver patch:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/109744/focus=109747

Signed-off-by: Simon Schwarz <simonschwarzcor@gmail.com>
Cc: scottwood at freescale.com
Cc: s-paulraj at ti.com
---
 include/configs/devkit8000.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h
index 5d1014b..4799c6d 100644
--- a/include/configs/devkit8000.h
+++ b/include/configs/devkit8000.h
@@ -366,4 +366,6 @@
 
 #define CONFIG_SYS_NAND_SPL_KERNEL_OFFS	0x280000
 #define CONFIG_SYS_SPL_ARGS_ADDR	(PHYS_SDRAM_1 + 0x100)
+#define CONFIG_OMAP3_DMA
+#define CONFIG_SPL_DMA_SUPPORT
 #endif /* __CONFIG_H */
-- 
1.7.4.1

[U-Boot] [PATCH 1/2] nand_spl_simple: Add omap3 DMA usage to SPL

[Wolfgang Denk]

Dear Simon Schwarz,

In message <1318759804-18688-2-git-send-email-simonschwarzcor@gmail.com> you wrote:
> This adds DMA copy for the nand spl implementation. If CONFIG_SPL_DMA_SUPPORT
> is defined the DMA is used.
> 
> Based on DMA driver patch:
> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/109744/focus=109747
> 
> Signed-off-by: Simon Schwarz <simonschwarzcor@gmail.com>
> Cc: scottwood at freescale.com
> Cc: s-paulraj at ti.com
> ---
>  drivers/mtd/nand/nand_spl_simple.c |  185 ++++++++++++++++++++++++++++++++++--
>  1 files changed, 176 insertions(+), 9 deletions(-)
...
> +	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
> +		res += omap3_dma_conf_transfer(0, nand_chip.IO_ADDR_R,
> +			(uint32_t *)p, CONFIG_SYS_NAND_ECCSIZE/4);

IIUC, drivers/mtd/nand/nand_spl_simple.c is a global, architecture
independent file.  However, you are adding OMAP3 specific code here.
If we did the same for all other potentially supported architectures
and SoCs, we'd soon have a serious mess.

Please move  architecture / SoC specific code out of such global
files.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There are always alternatives.
	-- Spock, "The Galileo Seven", stardate 2822.3

[U-Boot] [PATCH 1/2] nand_spl_simple: Add omap3 DMA usage to SPL

[Scott Wood]

On 10/16/2011 05:10 AM, Simon Schwarz wrote:
> This adds DMA copy for the nand spl implementation. If CONFIG_SPL_DMA_SUPPORT
> is defined the DMA is used.
> 
> Based on DMA driver patch:
> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/109744/focus=109747

As Wolfgang pointed out, this doesn't belong here.  Create your own
alternate SPL driver if your hardware doesn't work with the simple one
(similar to the not-yet-migrated nand_spl/nand_boot_fsl_elbc.c,
nand_spl/nand_boot_fsl_ifc.c, etc).

> @@ -46,11 +59,11 @@ static int nand_command(int block, int page, uint32_t offs,
>  	this->cmd_ctrl(&mtd, offs, NAND_CTRL_ALE | NAND_CTRL_CHANGE);
>  	this->cmd_ctrl(&mtd, page_addr & 0xff, NAND_CTRL_ALE); /* A[16:9] */
>  	this->cmd_ctrl(&mtd, (page_addr >> 8) & 0xff,
> -		       NAND_CTRL_ALE); /* A[24:17] */
> +			NAND_CTRL_ALE); /* A[24:17] */
>  #ifdef CONFIG_SYS_NAND_4_ADDR_CYCLE
>  	/* One more address cycle for devices > 32MiB */
>  	this->cmd_ctrl(&mtd, (page_addr >> 16) & 0x0f,
> -		       NAND_CTRL_ALE); /* A[28:25] */
> +			NAND_CTRL_ALE); /* A[28:25] */
>  #endif

Please refrain from making random unrelated whitespace changes in a
patch that also makes functional changes, particularly when they are
extensive enough to make it hard to spot the functional changes.

In this particular case, I think the whitespace was fine the way it was;
the continuation lines were nicely aligned.

-Scott

[U-Boot] [PATCH 1/2] nand_spl_simple: Add omap3 DMA usage to SPL

[Simon Schwarz]

Dear Scott,

On 10/25/2011 08:24 PM, Scott Wood wrote:
> On 10/16/2011 05:10 AM, Simon Schwarz wrote:
>> This adds DMA copy for the nand spl implementation. If CONFIG_SPL_DMA_SUPPORT
>> is defined the DMA is used.
>>
>> Based on DMA driver patch:
>> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/109744/focus=109747
>
> As Wolfgang pointed out, this doesn't belong here.  Create your own
> alternate SPL driver if your hardware doesn't work with the simple one
> (similar to the not-yet-migrated nand_spl/nand_boot_fsl_elbc.c,
> nand_spl/nand_boot_fsl_ifc.c, etc).
>

Hm. The naming of the functions was a fault. Will rename the calls in 
nand_spl_simple to remove omap parts. So
omap3_dma_wait_for_transfer
will become
dma_wait_for_transfer
etc.

So a board which intents to use DMA in SPL can implement these 
functions. Would this be ok?

A whole new driver is IMHO not the right thing as there is too much 
duplicated code then.

>> @@ -46,11 +59,11 @@ static int nand_command(int block, int page, uint32_t offs,
>>   	this->cmd_ctrl(&mtd, offs, NAND_CTRL_ALE | NAND_CTRL_CHANGE);
>>   	this->cmd_ctrl(&mtd, page_addr&  0xff, NAND_CTRL_ALE); /* A[16:9] */
>>   	this->cmd_ctrl(&mtd, (page_addr>>  8)&  0xff,
>> -		       NAND_CTRL_ALE); /* A[24:17] */
>> +			NAND_CTRL_ALE); /* A[24:17] */
>>   #ifdef CONFIG_SYS_NAND_4_ADDR_CYCLE
>>   	/* One more address cycle for devices>  32MiB */
>>   	this->cmd_ctrl(&mtd, (page_addr>>  16)&  0x0f,
>> -		       NAND_CTRL_ALE); /* A[28:25] */
>> +			NAND_CTRL_ALE); /* A[28:25] */
>>   #endif
>
> Please refrain from making random unrelated whitespace changes in a
> patch that also makes functional changes, particularly when they are
> extensive enough to make it hard to spot the functional changes.
>
> In this particular case, I think the whitespace was fine the way it was;
> the continuation lines were nicely aligned.


If I remember right I changed these because of checkpatch errors.

Regards
Simon

[U-Boot] [PATCH 1/2] nand_spl_simple: Add omap3 DMA usage to SPL

[Scott Wood]

On 10/31/2011 03:56 AM, Simon Schwarz wrote:
> Dear Scott,
> 
> On 10/25/2011 08:24 PM, Scott Wood wrote:
>> On 10/16/2011 05:10 AM, Simon Schwarz wrote:
>>> This adds DMA copy for the nand spl implementation. If CONFIG_SPL_DMA_SUPPORT
>>> is defined the DMA is used.
>>>
>>> Based on DMA driver patch:
>>> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/109744/focus=109747
>>
>> As Wolfgang pointed out, this doesn't belong here.  Create your own
>> alternate SPL driver if your hardware doesn't work with the simple one
>> (similar to the not-yet-migrated nand_spl/nand_boot_fsl_elbc.c,
>> nand_spl/nand_boot_fsl_ifc.c, etc).
>>
> 
> Hm. The naming of the functions was a fault. Will rename the calls in 
> nand_spl_simple to remove omap parts. So
> omap3_dma_wait_for_transfer
> will become
> dma_wait_for_transfer
> etc.
> 
> So a board which intents to use DMA in SPL can implement these 
> functions. Would this be ok?

What would the semantics of a generic dma_wait_for_transfer() be?

I just don't see how this is generic at all, whatever the name.

> A whole new driver is IMHO not the right thing as there is too much 
> duplicated code then.

So factor the common bits out into a separate file.

>>> @@ -46,11 +59,11 @@ static int nand_command(int block, int page, uint32_t offs,
>>>   	this->cmd_ctrl(&mtd, offs, NAND_CTRL_ALE | NAND_CTRL_CHANGE);
>>>   	this->cmd_ctrl(&mtd, page_addr&  0xff, NAND_CTRL_ALE); /* A[16:9] */
>>>   	this->cmd_ctrl(&mtd, (page_addr>>  8)&  0xff,
>>> -		       NAND_CTRL_ALE); /* A[24:17] */
>>> +			NAND_CTRL_ALE); /* A[24:17] */
>>>   #ifdef CONFIG_SYS_NAND_4_ADDR_CYCLE
>>>   	/* One more address cycle for devices>  32MiB */
>>>   	this->cmd_ctrl(&mtd, (page_addr>>  16)&  0x0f,
>>> -		       NAND_CTRL_ALE); /* A[28:25] */
>>> +			NAND_CTRL_ALE); /* A[28:25] */
>>>   #endif
>>
>> Please refrain from making random unrelated whitespace changes in a
>> patch that also makes functional changes, particularly when they are
>> extensive enough to make it hard to spot the functional changes.
>>
>> In this particular case, I think the whitespace was fine the way it was;
>> the continuation lines were nicely aligned.
> 
> 
> If I remember right I changed these because of checkpatch errors.

I believe checkpatch only complains when you have 8 or more spaces in a
row, which isn't the case here.  I don't think there's any prohibition
on lining things up with single-column granularity.

Further, checkpatch should not be complaining about lines that you don't
touch.

Where reformatting is warranted, it should be a separate patch.

-Scott

[U-Boot] [PATCH 1/2] nand_spl_simple: Add omap3 DMA usage to SPL

[Simon Schwarz]

Hi Scott

On 10/31/2011 10:22 PM, Scott Wood wrote:

> What would the semantics of a generic dma_wait_for_transfer() be?
>
> I just don't see how this is generic at all, whatever the name.
>

Hm. It would be a check if the given DMA channel is active - and if it 
is busy waiting for it.

So, what would then be a generic interface for DMA? I see that this is a 
verrry basic solution - but where do you see the actual problems 
implementing this interface for other DMA controllers? Or do you think 
that the interface is to simple?

>> A whole new driver is IMHO not the right thing as there is too much
>> duplicated code then.
>
> So factor the common bits out into a separate file.
>
I haven't given up on the general solution yet. If I don't see another 
way I will do that.

Regards
Simon

[U-Boot] [PATCH 1/2] nand_spl_simple: Add omap3 DMA usage to SPL

[Scott Wood]

On 11/02/2011 04:57 AM, Simon Schwarz wrote:
> Hi Scott
> 
> On 10/31/2011 10:22 PM, Scott Wood wrote:
> 
>> What would the semantics of a generic dma_wait_for_transfer() be?
>>
>> I just don't see how this is generic at all, whatever the name.
>>
> 
> Hm. It would be a check if the given DMA channel is active - and if it
> is busy waiting for it.
> 
> So, what would then be a generic interface for DMA? I see that this is a
> verrry basic solution - but where do you see the actual problems
> implementing this interface for other DMA controllers? Or do you think
> that the interface is to simple?

I'd stick with something closer to the read_buf() interface -- something
like read_buf_async() and wait_for_async().  Let the controller driver
deal with the details of how DMA is done.  Parameter is the mtd pointer,
not a channel number.

Certainly all the DMA setup stuff in nand_spl_load_image() needs to go
elsewhere.

>>> A whole new driver is IMHO not the right thing as there is too much
>>> duplicated code then.

I think it can be done with less duplication than in this patch --
should only need some ifdefs in the current code.  There's no need to
support both modes of operation in one SPL image, right?

-Scott