RE: [PATCH] [MTD] [NAND] Add prefetch and dma support for omap2/3 NAND driver
On Tue, Jun 9, 2009 at 3:49 PM, Tony Lindgrent...@atomide.com wrote: * Singh, Vimal imceaex-_o=ti_ou=bd_cn=recipients_cn=x0094...@dlee86.itg.ti.com [090608 09:01]: On Mon, Jun 8, 2009 at 5:08 PM, Tony Lindgrent...@atomide.com wrote: * Singh, Vimal imceaex-_o=ti_ou=bd_cn=recipients_cn=x0094...@dlee86.itg.ti.com [090605 05:00]: On Thu, Jun 4, 2009 at 8:58 PM, Tony Lindgrent...@atomide.com wrote: * vimal singh vimalsi...@ti.com [090604 02:34]: On Wed, Jun 3, 2009 at 10:03 PM, Tony Lindgren t...@atomide.com wrote: * Singh, Vimal imceaex-_o=ti_ou=bd_cn=recipients_cn=x0094...@dlee86.itg.ti.com [090602 23:46]: On Wed, Jun 3, 2009 at 2:06 AM, Tony Lindgren t...@atomide.com wrote: * vimal singh vimalsi...@ti.com [090602 05:40]: This patch adds prefetch support to access nand flash in both mpu and dma mode. This patch also adds 8-bit nand support (omap_read/write_buf8). Prefetch can be used for both 8- and 16-bit devices. This should be reviewed on the linux-omap@vger.kernel.org list for sure. One other comment below. Signed-off-by: Vimal Singh vimalsi...@ti.com --- I prepared this patch on top of OMAP2 / OMAP3 NAND driver patch: http://lists.infradead.org/pipermail/linux-mtd/2009-May/025562.html --- arch/arm/mach-omap2/gpmc.c | 102 ++ arch/arm/plat-omap/include/mach/gpmc.h |4 drivers/mtd/nand/Kconfig | 17 + drivers/mtd/nand/omap2.c | 308 - 4 files changed, 422 insertions(+), 9 deletions(-) Index: mtd-2.6/arch/arm/mach-omap2/gpmc.c === --- mtd-2.6.orig/arch/arm/mach-omap2/gpmc.c +++ mtd-2.6/arch/arm/mach-omap2/gpmc.c @@ -54,6 +54,12 @@ #define GPMC_CHUNK_SHIFT 24 /* 16 MB */ #define GPMC_SECTION_SHIFT 28 /* 128 MB */ +#ifdef CONFIG_MTD_NAND_OMAP_PREFETCH +#define CS_NUM_SHIFT 24 +#define ENABLE_PREFETCH 7 +#define DMA_MPU_MODE 2 +#endif + static struct resource gpmc_mem_root; static struct resource gpmc_cs_mem[GPMC_CS_NUM]; static DEFINE_SPINLOCK(gpmc_mem_lock); @@ -383,6 +389,99 @@ void gpmc_cs_free(int cs) } EXPORT_SYMBOL(gpmc_cs_free); +#ifdef CONFIG_MTD_NAND_OMAP_PREFETCH +/** + * gpmc_prefetch_init - configures default configuration for prefetch engine + */ +static void gpmc_prefetch_init(void) +{ + /* Setting the default threshold to 64 */ + gpmc_write_reg(GPMC_PREFETCH_CONTROL, 0x0); + gpmc_write_reg(GPMC_PREFETCH_CONFIG1, 0x40 8); + gpmc_write_reg(GPMC_PREFETCH_CONFIG2, 0x0); +} Why would you want to have NAND specific init code int gpmc.c? The purpose if gpmc.c is to provide access to configuring the General Purpose Memory Controller (GPMC). You should just provide functions in gpmc.c for the platform init code to use, and then the drivers can stay platform independent. In my understanding, this 'prefetch' engine is part of GPMC itself, it is a kind of feature provided by GPMC which can be utilized by NAND driver. So, to me, it makes sens to get initialized prefetch by GPMC itself so that NAND driver can use it. But it should not have a dependency to NAND. This engine, in GPMC, is dedicated for NAND devices only. Another reason was that all read / write to GPMC register are done by functions 'gpmc_read_reg' / 'gpmc_write_reg', which have been made 'static' in nature. That's why you need to provide a generic function in gpmc.c to enable prefetch that the platform code for any driver can use. Exactly, and whenever a platform code uses gpmc init call, gpmc initializes this engine too. Since prefetch engine is the part of GPMC, IMHOP, it should get initialized as part of GPMC initialization. No, the driver needing it should initialize it. There should not be any ifdefs needed in the gpmc.c for that. Are you suggesting to move gpmc prefetch specific code to 'drivers/mtd/nand/omap2.c'? This can be done but then, we will be accessing gpmc registers outside gpmc.c. Is that ok? Well I'm thinking that you just basically need something like this in gpmc.c: int gpmc_prefetch_request(int cs); int gpmc_prefetch_enable(int cs); int gpmc_prefetch_reset(int cs); ... And gpmc_prefetch_request would then return an error if already taken. No need then to tinker with the GPMC prefetch registers in the NAND driver. This seems exactly what in this patch has been done, other than renaming the functions and suggestion that gpmc prefetch init call
RE: [PATCH] [MTD] [NAND] Add prefetch and dma support for omap2/3 NAND driver
On Mon, Jun 8, 2009 at 5:08 PM, Tony Lindgrent...@atomide.com wrote: * Singh, Vimal imceaex-_o=ti_ou=bd_cn=recipients_cn=x0094...@dlee86.itg.ti.com [090605 05:00]: On Thu, Jun 4, 2009 at 8:58 PM, Tony Lindgrent...@atomide.com wrote: * vimal singh vimalsi...@ti.com [090604 02:34]: On Wed, Jun 3, 2009 at 10:03 PM, Tony Lindgren t...@atomide.com wrote: * Singh, Vimal imceaex-_o=ti_ou=bd_cn=recipients_cn=x0094...@dlee86.itg.ti.com [090602 23:46]: On Wed, Jun 3, 2009 at 2:06 AM, Tony Lindgren t...@atomide.com wrote: * vimal singh vimalsi...@ti.com [090602 05:40]: This patch adds prefetch support to access nand flash in both mpu and dma mode. This patch also adds 8-bit nand support (omap_read/write_buf8). Prefetch can be used for both 8- and 16-bit devices. This should be reviewed on the linux-omap@vger.kernel.org list for sure. One other comment below. Signed-off-by: Vimal Singh vimalsi...@ti.com --- I prepared this patch on top of OMAP2 / OMAP3 NAND driver patch: http://lists.infradead.org/pipermail/linux-mtd/2009-May/025562.html --- arch/arm/mach-omap2/gpmc.c | 102 ++ arch/arm/plat-omap/include/mach/gpmc.h |4 drivers/mtd/nand/Kconfig | 17 + drivers/mtd/nand/omap2.c | 308 - 4 files changed, 422 insertions(+), 9 deletions(-) Index: mtd-2.6/arch/arm/mach-omap2/gpmc.c === --- mtd-2.6.orig/arch/arm/mach-omap2/gpmc.c +++ mtd-2.6/arch/arm/mach-omap2/gpmc.c @@ -54,6 +54,12 @@ #define GPMC_CHUNK_SHIFT 24 /* 16 MB */ #define GPMC_SECTION_SHIFT 28 /* 128 MB */ +#ifdef CONFIG_MTD_NAND_OMAP_PREFETCH +#define CS_NUM_SHIFT 24 +#define ENABLE_PREFETCH 7 +#define DMA_MPU_MODE 2 +#endif + static struct resource gpmc_mem_root; static struct resource gpmc_cs_mem[GPMC_CS_NUM]; static DEFINE_SPINLOCK(gpmc_mem_lock); @@ -383,6 +389,99 @@ void gpmc_cs_free(int cs) } EXPORT_SYMBOL(gpmc_cs_free); +#ifdef CONFIG_MTD_NAND_OMAP_PREFETCH +/** + * gpmc_prefetch_init - configures default configuration for prefetch engine + */ +static void gpmc_prefetch_init(void) +{ + /* Setting the default threshold to 64 */ + gpmc_write_reg(GPMC_PREFETCH_CONTROL, 0x0); + gpmc_write_reg(GPMC_PREFETCH_CONFIG1, 0x40 8); + gpmc_write_reg(GPMC_PREFETCH_CONFIG2, 0x0); +} Why would you want to have NAND specific init code int gpmc.c? The purpose if gpmc.c is to provide access to configuring the General Purpose Memory Controller (GPMC). You should just provide functions in gpmc.c for the platform init code to use, and then the drivers can stay platform independent. In my understanding, this 'prefetch' engine is part of GPMC itself, it is a kind of feature provided by GPMC which can be utilized by NAND driver. So, to me, it makes sens to get initialized prefetch by GPMC itself so that NAND driver can use it. But it should not have a dependency to NAND. This engine, in GPMC, is dedicated for NAND devices only. Another reason was that all read / write to GPMC register are done by functions 'gpmc_read_reg' / 'gpmc_write_reg', which have been made 'static' in nature. That's why you need to provide a generic function in gpmc.c to enable prefetch that the platform code for any driver can use. Exactly, and whenever a platform code uses gpmc init call, gpmc initializes this engine too. Since prefetch engine is the part of GPMC, IMHOP, it should get initialized as part of GPMC initialization. No, the driver needing it should initialize it. There should not be any ifdefs needed in the gpmc.c for that. Are you suggesting to move gpmc prefetch specific code to 'drivers/mtd/nand/omap2.c'? This can be done but then, we will be accessing gpmc registers outside gpmc.c. Is that ok? Well I'm thinking that you just basically need something like this in gpmc.c: int gpmc_prefetch_request(int cs); int gpmc_prefetch_enable(int cs); int gpmc_prefetch_reset(int cs); ... And gpmc_prefetch_request would then return an error if already taken. No need then to tinker with the GPMC prefetch registers in the NAND driver. This seems exactly what in this patch has been done, other than renaming the functions and suggestion that gpmc prefetch init call should happen from NAND driver and should return success or failure based on whether it has already been initialized or not. Am I correct? But since, the prefetch and write-posting engine is a single-context engine that can be allocated to only one chip-select at a time for a read prefetch or a write-posting process, I
RE: [PATCH] [MTD] [NAND] Add prefetch and dma support for omap2/3 NAND driver
On Thu, Jun 4, 2009 at 8:58 PM, Tony Lindgrent...@atomide.com wrote: * vimal singh vimalsi...@ti.com [090604 02:34]: On Wed, Jun 3, 2009 at 10:03 PM, Tony Lindgren t...@atomide.com wrote: * Singh, Vimal imceaex-_o=ti_ou=bd_cn=recipients_cn=x0094...@dlee86.itg.ti.com [090602 23:46]: On Wed, Jun 3, 2009 at 2:06 AM, Tony Lindgren t...@atomide.com wrote: * vimal singh vimalsi...@ti.com [090602 05:40]: This patch adds prefetch support to access nand flash in both mpu and dma mode. This patch also adds 8-bit nand support (omap_read/write_buf8). Prefetch can be used for both 8- and 16-bit devices. This should be reviewed on the linux-omap@vger.kernel.org list for sure. One other comment below. Signed-off-by: Vimal Singh vimalsi...@ti.com --- I prepared this patch on top of OMAP2 / OMAP3 NAND driver patch: http://lists.infradead.org/pipermail/linux-mtd/2009-May/025562.html --- arch/arm/mach-omap2/gpmc.c | 102 ++ arch/arm/plat-omap/include/mach/gpmc.h |4 drivers/mtd/nand/Kconfig | 17 + drivers/mtd/nand/omap2.c | 308 - 4 files changed, 422 insertions(+), 9 deletions(-) Index: mtd-2.6/arch/arm/mach-omap2/gpmc.c === --- mtd-2.6.orig/arch/arm/mach-omap2/gpmc.c +++ mtd-2.6/arch/arm/mach-omap2/gpmc.c @@ -54,6 +54,12 @@ #define GPMC_CHUNK_SHIFT 24 /* 16 MB */ #define GPMC_SECTION_SHIFT 28 /* 128 MB */ +#ifdef CONFIG_MTD_NAND_OMAP_PREFETCH +#define CS_NUM_SHIFT 24 +#define ENABLE_PREFETCH 7 +#define DMA_MPU_MODE 2 +#endif + static struct resource gpmc_mem_root; static struct resource gpmc_cs_mem[GPMC_CS_NUM]; static DEFINE_SPINLOCK(gpmc_mem_lock); @@ -383,6 +389,99 @@ void gpmc_cs_free(int cs) } EXPORT_SYMBOL(gpmc_cs_free); +#ifdef CONFIG_MTD_NAND_OMAP_PREFETCH +/** + * gpmc_prefetch_init - configures default configuration for prefetch engine + */ +static void gpmc_prefetch_init(void) +{ + /* Setting the default threshold to 64 */ + gpmc_write_reg(GPMC_PREFETCH_CONTROL, 0x0); + gpmc_write_reg(GPMC_PREFETCH_CONFIG1, 0x40 8); + gpmc_write_reg(GPMC_PREFETCH_CONFIG2, 0x0); +} Why would you want to have NAND specific init code int gpmc.c? The purpose if gpmc.c is to provide access to configuring the General Purpose Memory Controller (GPMC). You should just provide functions in gpmc.c for the platform init code to use, and then the drivers can stay platform independent. In my understanding, this 'prefetch' engine is part of GPMC itself, it is a kind of feature provided by GPMC which can be utilized by NAND driver. So, to me, it makes sens to get initialized prefetch by GPMC itself so that NAND driver can use it. But it should not have a dependency to NAND. This engine, in GPMC, is dedicated for NAND devices only. Another reason was that all read / write to GPMC register are done by functions 'gpmc_read_reg' / 'gpmc_write_reg', which have been made 'static' in nature. That's why you need to provide a generic function in gpmc.c to enable prefetch that the platform code for any driver can use. Exactly, and whenever a platform code uses gpmc init call, gpmc initializes this engine too. Since prefetch engine is the part of GPMC, IMHOP, it should get initialized as part of GPMC initialization. No, the driver needing it should initialize it. There should not be any ifdefs needed in the gpmc.c for that. Are you suggesting to move gpmc prefetch specific code to 'drivers/mtd/nand/omap2.c'? This can be done but then, we will be accessing gpmc registers outside gpmc.c. Is that ok? And then there are prefetch start, stop and status functions calls have been provided to be used by NAND driver. These should really be done in the platform init code for the NAND driver. The NAND driver really should be generic, not omap specific. Are you talking about different omap versions or omap itself, I am bit confused. This NAND driver is omap specific only. -Vimal Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] [MTD] [NAND] Add prefetch and dma support for omap2/3 NAND driver
On Wed, Jun 3, 2009 at 2:06 AM, Tony Lindgren t...@atomide.com wrote: * vimal singh vimalsi...@ti.com [090602 05:40]: This patch adds prefetch support to access nand flash in both mpu and dma mode. This patch also adds 8-bit nand support (omap_read/write_buf8). Prefetch can be used for both 8- and 16-bit devices. This should be reviewed on the linux-omap@vger.kernel.org list for sure. One other comment below. Signed-off-by: Vimal Singh vimalsi...@ti.com --- I prepared this patch on top of OMAP2 / OMAP3 NAND driver patch: http://lists.infradead.org/pipermail/linux-mtd/2009-May/025562.html --- arch/arm/mach-omap2/gpmc.c | 102 ++ arch/arm/plat-omap/include/mach/gpmc.h |4 drivers/mtd/nand/Kconfig | 17 + drivers/mtd/nand/omap2.c | 308 - 4 files changed, 422 insertions(+), 9 deletions(-) Index: mtd-2.6/arch/arm/mach-omap2/gpmc.c === --- mtd-2.6.orig/arch/arm/mach-omap2/gpmc.c +++ mtd-2.6/arch/arm/mach-omap2/gpmc.c @@ -54,6 +54,12 @@ #define GPMC_CHUNK_SHIFT 24 /* 16 MB */ #define GPMC_SECTION_SHIFT 28 /* 128 MB */ +#ifdef CONFIG_MTD_NAND_OMAP_PREFETCH +#define CS_NUM_SHIFT 24 +#define ENABLE_PREFETCH 7 +#define DMA_MPU_MODE 2 +#endif + static struct resource gpmc_mem_root; static struct resource gpmc_cs_mem[GPMC_CS_NUM]; static DEFINE_SPINLOCK(gpmc_mem_lock); @@ -383,6 +389,99 @@ void gpmc_cs_free(int cs) } EXPORT_SYMBOL(gpmc_cs_free); +#ifdef CONFIG_MTD_NAND_OMAP_PREFETCH +/** + * gpmc_prefetch_init - configures default configuration for prefetch engine + */ +static void gpmc_prefetch_init(void) +{ + /* Setting the default threshold to 64 */ + gpmc_write_reg(GPMC_PREFETCH_CONTROL, 0x0); + gpmc_write_reg(GPMC_PREFETCH_CONFIG1, 0x40 8); + gpmc_write_reg(GPMC_PREFETCH_CONFIG2, 0x0); +} Why would you want to have NAND specific init code int gpmc.c? The purpose if gpmc.c is to provide access to configuring the General Purpose Memory Controller (GPMC). You should just provide functions in gpmc.c for the platform init code to use, and then the drivers can stay platform independent. In my understanding, this 'prefetch' engine is part of GPMC itself, it is a kind of feature provided by GPMC which can be utilized by NAND driver. So, to me, it makes sens to get initialized prefetch by GPMC itself so that NAND driver can use it. Another reason was that all read / write to GPMC register are done by functions 'gpmc_read_reg' / 'gpmc_write_reg', which have been made 'static' in nature. Regards, vimal-- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] OMAP2 NAND: Fix __raw_readsl() length argument
- __raw_readsl(nand-IO_ADDR_R, buf, len / 2); + __raw_readsw(nand-IO_ADDR_R, buf, len / 2); I would prefer len 1 rather len / 2. --- vimal-- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RESENDING][PATCH 1/2]OMAP3 NAND: Add NAND support on OMAP3430
On Modday 06 October 2008, Devid Brownell wrote: On Monday 06 October 2008, Tony Lindgren wrote: --- linux-omap-2.6_27_08_2008.orig/include/linux/mtd/nand.h +++ linux-omap-2.6_27_08_2008/include/linux/mtd/nand.h @@ -45,7 +45,7 @@ */ #define NAND_MAX_OOBSIZE 64 #define NAND_MAX_PAGESIZE2048 - +#define NAND_BLOCK_SIZE SZ_128K /* This is specific to 3430 SDP though ... there are plenty of other NAND block sizes in the world. Suggest moving this definition to board-3430sdp-flash.c where it's relevant. Agree. I will send a patch to move it. ... vimal-- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [Resend][PATCH - Omapzoom][NAND] Add prefetch and DMA support
Hi, On Mon, Sep 08, 2008 at 01:03:35PM -0500, [EMAIL PROTECTED] wrote: +#ifdef CONFIG_MTD_NAND_OMAP_PREFETCH + static int use_prefetch = 1; + + /* modprobe ... use_prefetch=0 etc */ + module_param(use_prefetch, bool, 0); + MODULE_PARM_DESC(use_prefetch, enable/disable use of PREFETCH); Don't indent C code just because there's preprocessor stuff around it. Ok, I will remove it. + init_completion(info-comp); + dma_sync_single_for_cpu(info-pdev-dev, + virt_to_phys(addr), count, DMA_TO_DEVICE); Please read the DMA API and use the proper functions. This is an abuse of the DMA API. + omap_start_dma(info-dma_ch); + wait_for_completion(info-comp); + if (!is_write) + dma_sync_single_for_cpu(info-pdev-dev, + virt_to_phys(addr), count, DMA_FROM_DEVICE); Ditto. This is an abuse of the API. This is how it's supposed to work: enum dma_direction dir = is_write ? DMA_TO_DEVICE : DMA_FROM_DEVICE; dma_addr_t dma_addr; dma_addr = dma_map_single(info-pdev-dev, addr, count, dir); /* setup and start DMA using dma_addr */ wait_for_completion(info-comp); dma_unmap_single(info-pdev-dev, dma_addr, count, dir); I will fix this in my next patch. @@ -201,11 +304,33 @@ struct omap_nand_info *info = container_of(mtd, struct omap_nand_info, mtd); u16 *p = (u16 *) buf; - - len = 1; - - while (len--) - *p++ = cpu_to_le16(readw(info-nand.IO_ADDR_R)); This driver needs work (see endianness explaination below.) This has been fixed by David Brownell, and I will re-base my patch on top of it. + while (len) { + bytes_to_read = ((prefetch_status 24) 0x7F) 1; + for (i = 0; (i bytes_to_read) (len); i++, len -= 2) + *p++ = cpu_to_le16(*(volatile u16 *)(info-nand_pref_fifo_add)); Yet you dereference it. This is illegal according to the Linux APIs. Use __raw_readw() here. Ok, I will do this. Moreover, 'p' is a pointer to CPU endian memory, not le16 memory, so using cpu_to_le16 is wrong here. What endian is there data in flash? Accessing nand flash is done by prefetch engine, which is also part of omap, so has same endianness. Yes I will remove cpu_to_le16 in my next patch. Thanks, vimal-- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] ARM: OMAP: Kconfig support HW ECC for nand driver on OMAP3
Hi, This stuff should be all going to the MTD list now. If we still have some MTD drivers that are not included in the MTD tree, we should prepare them for merging and send them to the MTD list. Thanks. Ok, I will be taking it to mtd after few weeks. Thanks, Tony-- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html