Re: [PATCH] [MTD] [NAND] Add prefetch and dma support for omap2/3 NAND driver

2009-06-09 Thread Tony Lindgren
* Singh, Vimal 
 [090609 
08:24]:
> 
> 
> On Tue, Jun 9, 2009 at 3:49 PM, Tony Lindgren wrote:
> > * Singh, Vimal 
> >  [090608 
> > 09:01]:
> >>
> >> On Mon, Jun 8, 2009 at 5:08 PM, Tony Lindgren wrote:
> >> > * Singh, Vimal 
> >> >  
> >> > [090605 05:00]:
> >> >>
> >> >>
> >> >> On Thu, Jun 4, 2009 at 8:58 PM, Tony Lindgren wrote:
> >> >> > * vimal singh  [090604 02:34]:
> >> >> >> On Wed, Jun 3, 2009 at 10:03 PM, Tony Lindgren  
> >> >> >> wrote:
> >> >> >> > * Singh, Vimal
> >> >> >>  
> >> >> >> [090602
> >> >> >> 23:46]:
> >> >> >> >>
> >> >> >> >> On Wed, Jun 3, 2009 at 2:06 AM, Tony Lindgren  
> >> >> >> >> wrote:
> >> >> >> >> > * vimal singh  [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 
> >> >> >> >> >> ---
> >> >> >> >> >> 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

RE: [PATCH] [MTD] [NAND] Add prefetch and dma support for omap2/3 NAND driver

2009-06-09 Thread Singh, Vimal


On Tue, Jun 9, 2009 at 3:49 PM, Tony Lindgren wrote:
> * Singh, Vimal 
>  [090608 
> 09:01]:
>>
>> On Mon, Jun 8, 2009 at 5:08 PM, Tony Lindgren wrote:
>> > * Singh, Vimal 
>> >  [090605 
>> > 05:00]:
>> >>
>> >>
>> >> On Thu, Jun 4, 2009 at 8:58 PM, Tony Lindgren wrote:
>> >> > * vimal singh  [090604 02:34]:
>> >> >> On Wed, Jun 3, 2009 at 10:03 PM, Tony Lindgren  
>> >> >> wrote:
>> >> >> > * Singh, Vimal
>> >> >>  
>> >> >> [090602
>> >> >> 23:46]:
>> >> >> >>
>> >> >> >> On Wed, Jun 3, 2009 at 2:06 AM, Tony Lindgren  
>> >> >> >> wrote:
>> >> >> >> > * vimal singh  [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 
>> >> >> >> >> ---
>> >> >> >> >> 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 

Re: [PATCH] [MTD] [NAND] Add prefetch and dma support for omap2/3 NAND driver

2009-06-09 Thread Tony Lindgren
* Singh, Vimal 
 [090608 
09:01]:
> 
> On Mon, Jun 8, 2009 at 5:08 PM, Tony Lindgren wrote:
> > * Singh, Vimal 
> >  [090605 
> > 05:00]:
> >>
> >>
> >> On Thu, Jun 4, 2009 at 8:58 PM, Tony Lindgren wrote:
> >> > * vimal singh  [090604 02:34]:
> >> >> On Wed, Jun 3, 2009 at 10:03 PM, Tony Lindgren  wrote:
> >> >> > * Singh, Vimal
> >> >>  
> >> >> [090602
> >> >> 23:46]:
> >> >> >>
> >> >> >> On Wed, Jun 3, 2009 at 2:06 AM, Tony Lindgren  
> >> >> >> wrote:
> >> >> >> > * vimal singh  [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 
> >> >> >> >> ---
> >> >> >> >> 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 lik

RE: [PATCH] [MTD] [NAND] Add prefetch and dma support for omap2/3 NAND driver

2009-06-08 Thread Singh, Vimal

On Mon, Jun 8, 2009 at 5:08 PM, Tony Lindgren wrote:
> * Singh, Vimal 
>  [090605 
> 05:00]:
>>
>>
>> On Thu, Jun 4, 2009 at 8:58 PM, Tony Lindgren wrote:
>> > * vimal singh  [090604 02:34]:
>> >> On Wed, Jun 3, 2009 at 10:03 PM, Tony Lindgren  wrote:
>> >> > * Singh, Vimal
>> >>  [090602
>> >> 23:46]:
>> >> >>
>> >> >> On Wed, Jun 3, 2009 at 2:06 AM, Tony Lindgren  wrote:
>> >> >> > * vimal singh  [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 
>> >> >> >> ---
>> >> >> >> 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 

Re: [PATCH] [MTD] [NAND] Add prefetch and dma support for omap2/3 NAND driver

2009-06-08 Thread Tony Lindgren
* Singh, Vimal 
 [090605 
05:00]:
> 
> 
> On Thu, Jun 4, 2009 at 8:58 PM, Tony Lindgren wrote:
> > * vimal singh  [090604 02:34]:
> >> On Wed, Jun 3, 2009 at 10:03 PM, Tony Lindgren  wrote:
> >> > * Singh, Vimal
> >>  [090602
> >> 23:46]:
> >> >>
> >> >> On Wed, Jun 3, 2009 at 2:06 AM, Tony Lindgren  wrote:
> >> >> > * vimal singh  [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 
> >> >> >> ---
> >> >> >> 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.
 
> >
> >> 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 f

RE: [PATCH] [MTD] [NAND] Add prefetch and dma support for omap2/3 NAND driver

2009-06-05 Thread Singh, Vimal


On Thu, Jun 4, 2009 at 8:58 PM, Tony Lindgren wrote:
> * vimal singh  [090604 02:34]:
>> On Wed, Jun 3, 2009 at 10:03 PM, Tony Lindgren  wrote:
>> > * Singh, Vimal
>>  [090602
>> 23:46]:
>> >>
>> >> On Wed, Jun 3, 2009 at 2:06 AM, Tony Lindgren  wrote:
>> >> > * vimal singh  [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 
>> >> >> ---
>> >> >> 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

2009-06-04 Thread Tony Lindgren
* vimal singh  [090604 02:34]:
> On Wed, Jun 3, 2009 at 10:03 PM, Tony Lindgren  wrote:
> > * Singh, Vimal
>  [090602
> 23:46]:
> >>
> >> On Wed, Jun 3, 2009 at 2:06 AM, Tony Lindgren  wrote:
> >> > * vimal singh  [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 
> >> >> ---
> >> >> 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.
 
> 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.

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

2009-06-04 Thread vimal singh
On Wed, Jun 3, 2009 at 10:03 PM, Tony Lindgren  wrote:
> * Singh, Vimal
 [090602
23:46]:
>>
>> On Wed, Jun 3, 2009 at 2:06 AM, Tony Lindgren  wrote:
>> > * vimal singh  [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 
>> >> ---
>> >> 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.

And then there are prefetch start, stop and status functions calls have been
provided to be used by NAND driver.

-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

2009-06-03 Thread Tony Lindgren
* Singh, Vimal 
 [090602 
23:46]:
> 
> On Wed, Jun 3, 2009 at 2:06 AM, Tony Lindgren  wrote:
> > * vimal singh  [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 
> >> ---
> >> 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. 
 
> 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.

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

2009-06-02 Thread Singh, Vimal

On Wed, Jun 3, 2009 at 2:06 AM, Tony Lindgren  wrote:
> * vimal singh  [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 
>> ---
>> 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] [MTD] [NAND] Add prefetch and dma support for omap2/3 NAND driver

2009-06-02 Thread Tony Lindgren
* vimal singh  [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 
> ---
> 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.

Regards,

Tony


> +/**
> + * gpmc_prefetch_start - configures and starts prefetch transfer
> + * @cs: nand cs (chip select) number
> + * @dma_mode: dma mode enable (1) or disable (0)
> + * @u32_count: number of bytes to be transferred
> + * @is_write: prefetch read(0) or write post(1) mode
> + */
> +void gpmc_prefetch_start(int cs, int dma_mode,
> + unsigned int u32_count, int is_write)
> +{
> + uint32_t prefetch_config1;
> + if (is_write) {
> + /* Set the amount of bytes to be prefetched */
> + gpmc_write_reg(GPMC_PREFETCH_CONFIG2, u32_count);
> +
> + /* Set dma/mpu mode, the post write and enable the engine
> +  * Set which cs is using the post write
> +  */
> + prefetch_config1 = gpmc_read_reg(GPMC_PREFETCH_CONFIG1);
> + prefetch_config1 |= ((cs << CS_NUM_SHIFT) |
> + (dma_mode << DMA_MPU_MODE) |
> + (1 << ENABLE_PREFETCH) | 0x1);
> + gpmc_write_reg(GPMC_PREFETCH_CONFIG1, prefetch_config1);
> + } else {
> + /* Set the amount of bytes to be prefetched */
> + gpmc_write_reg(GPMC_PREFETCH_CONFIG2, u32_count);
> +
> + /* Set dma/mpu mode, the prefech read and enable the engine
> +  * Set which cs is using the prefetch
> +  */
> + prefetch_config1 = gpmc_read_reg(GPMC_PREFETCH_CONFIG1);
> + prefetch_config1 |= (((cs << CS_NUM_SHIFT) |
> + (dma_mode << DMA_MPU_MODE) |
> + (1 << ENABLE_PREFETCH)) & ~0x1);
> + gpmc_write_reg(GPMC_PREFETCH_CONFIG1, prefetch_config1);
> + }
> + /*  Start the prefetch engine */
> + gpmc_write_reg(GPMC_PREFETCH_CONTROL, 0x1);
> +}
> +EXPORT_SYMBOL(gpmc_prefetch_start);
> +
> +/**
> + * gpmc_prefetch_stop - disables and stops the prefetch engine
> + */
> +void gpmc_prefetch_stop(void)
> +{
> + uint32_t prefetch_config1;
> + /* stop the PFPW engine */
> + gpmc_write_reg(GPMC_PREFETCH_CONTROL, 0x0);
> +
> + /* Disable the PFPW engine */
> + prefetch_config1 = gpmc_read_reg(GPMC_PREFETCH_CONFIG1);
> + prefetch_config1 &= ~((0x07 << CS_NUM_SHIFT) |
> + (1 << ENABLE_PREFETCH) |
> + (1 << DMA_MPU_MODE) | 0x1);
> + gpmc_write_reg(GPMC_PREFETCH_CONFIG1, prefetch_config1);
> +}
> +EXPORT_SYMBOL(gpmc_prefetch_stop);
> +
> +/**
> + * gpmc_prefetch_status - reads prefetch status of engine
> + */
> +int  gpmc_prefetch_status(void)
> +{
> + return gpmc_read_reg(GPMC_PREFETCH_STATUS);
> +}
> +EXP