Use DMA in SPL
Subsystems (2)
cover Use DMA in SPL
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(-)
Signature · 1 line
1.7.4.1
PATCH 1/2 nand_spl_simple: Add omap3 DMA usage to SPL
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
--- drivers/mtd/nand/nand_spl_simple.c | 185 ++++++++++++++++++++++++++++++++++-- 1 files changed, 176 insertions(+), 9 deletions(-)
drivers/mtd/nand/nand_spl_simple.c +176 -9
@@ -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 <[email protected]> +* * 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)
Signature · 1 line
1.7.4.1
PATCH 1/2 nand_spl_simple: Add omap3 DMA usage to SPL
Dear Simon Schwarz,
Quoted 12 lines · In message <[email protected]> you…
This adds DMA copy for the nand spl implementation. If
CONFIG_SPL_DMA_SUPPORTis defined the DMA is used.Based on DMA driver patch:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/109744/focus=109747--- drivers/mtd/nand/nand_spl_simple.c | 185 ++++++++++++++++++++++++++++++++++-- 1 files changed, 176 insertions(+), 9 deletions(-)
...
drivers/mtd/nand/nand_spl_simple.c int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst) · line 279
Quoted 3 lines
drivers/mtd/nand/nand_spl_simple.c +3
-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 );
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
Signature · 5 lines
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
PATCH 1/2 nand_spl_simple: Add omap3 DMA usage to SPL
In reply to Simon SchwarzQuoted 5 lines · Simon Schwarz
This adds DMA copy for the nand spl implementation. If
CONFIG_SPL_DMA_SUPPORTis 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).
drivers/mtd/nand/nand_spl_simple.c static int nand_command(int block, int page, uint32_t offs, · line 67
Quoted 12 lines
drivers/mtd/nand/nand_spl_simple.c +2
-2@@ -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
PATCH 1/2 nand_spl_simple: Add omap3 DMA usage to SPL
Dear Scott,
Quoted 4 lines · Scott Wood
Quoted 3 lines · Simon Schwarz
This adds DMA copy for the nand spl implementation. If
CONFIG_SPL_DMA_SUPPORTis defined the DMA is used.
Quoted 8 lines
Quoted 2 lines
Based on DMA driver patch:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/109744/focus=109747As 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 becomedma_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.
Quoted 17 lines
drivers/mtd/nand/nand_spl_simple.c static int nand_command(int block, int page, uint32_t offs, · line 67
Quoted 12 lines
drivers/mtd/nand/nand_spl_simple.c +2
-2@@ -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.
Quoted 2 lines
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
PATCH 1/2 nand_spl_simple: Add omap3 DMA usage to SPL
Quoted 23 lines · Simon Schwarz
Dear Scott,
Quoted 4 lines · Scott Wood
Quoted 3 lines · Simon Schwarz
This adds DMA copy for the nand spl implementation. If
CONFIG_SPL_DMA_SUPPORTis defined the DMA is used.Quoted 8 lines
Quoted 2 lines
Based on DMA driver patch:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/109744/focus=109747As 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_simpleto remove omap parts. Soomap3_dma_wait_for_transfer
will becomedma_wait_for_transfer
etc.
Quoted 2 lines
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.
Quoted 2 lines
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.
Quoted 17 lines
Quoted 17 lines
drivers/mtd/nand/nand_spl_simple.c static int nand_command(int block, int page, uint32_t offs, · line 67
Quoted 12 lines
drivers/mtd/nand/nand_spl_simple.c +2
-2@@ -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.
Quoted 5 lines
Quoted 2 lines
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
PATCH 1/2 nand_spl_simple: Add omap3 DMA usage to SPL
Hi Scott
Quoted 4 lines · Scott Wood
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?
Quoted 5 lines
Quoted 2 lines
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
PATCH 1/2 nand_spl_simple: Add omap3 DMA usage to SPL
Quoted 9 lines · Simon Schwarz
Hi Scott
Quoted 4 lines · Scott Wood
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.
Quoted 7 lines
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.
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
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
PATCH 2/2 devkit8000: Activate DMA support in SPL
In reply to Simon SchwarzThis 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
--- include/configs/devkit8000.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
include/configs/devkit8000.h +2 -0
@@ -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 */
Signature · 1 line
1.7.4.1