Re: [Resend][PATCH - Omapzoom][NAND] Add prefetch and DMA support

2009-01-08 Thread Tony Lindgren
* vimal singh  [080916 14:53]:
> Following patch taken over the omapzoom.org tree adds
> prefetch (MPU and DMA) support to the OMAP2/3 nand driver.
> The prefetch supprot also adds the support for 8-bit
> devices.
> David Brownell's patch (omap2_nand updates) changes
> have been included in this patch manually.

Just few random comments.. Maybe try to get the
drivers/mtd/omap2.c first into the mainline kernel
on the mtd list, then start figuring out what needs
to be done for getting the prefetch support done.

Tony
 

> Signed-off-by: Vimal Singh 
> 
> ---
> 8-bit devices are supported only in prefetch mode.
> 
> vimal.
> 
> 
> ---
>  arch/arm/mach-omap2/gpmc.c |   95 +
>  arch/arm/plat-omap/include/mach/gpmc.h |4
>  drivers/mtd/nand/Kconfig   |   17 ++
>  drivers/mtd/nand/omap2.c   |  240
> + 4 files changed, 332 insertions(+), 24
> deletions(-)
> 
> Index: omapkernel/arch/arm/mach-omap2/gpmc.c
> === ---
> omapkernel.orig/arch/arm/mach-omap2/gpmc.c2008-09-15 17:59:47.0 
> +0530
> +++ omapkernel/arch/arm/mach-omap2/gpmc.c 2008-09-15 18:00:14.0 
> +0530
> @@ -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
> +
>  #ifdef CONFIG_OMAP3_PM
>  /*
>   * Structure to save/restore gpmc context
> @@ -407,6 +413,92 @@
>  }
>  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);
> +}
> +
> +/*
> + * 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);
> +}
> +EXPORT_SYMBOL(gpmc_prefetch_status);
> +#else
> +int  gpmc_prefetch_status(void) {return 0; }
> +void gpmc_prefetch_stop(void) {}
> +void gpmc_prefetch_start(int cs, int dma_mode, unsigned int u32_cou

RE: [Resend][PATCH - Omapzoom][NAND] Add prefetch and DMA support

2008-09-10 Thread Singh, Vimal
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: [Resend][PATCH - Omapzoom][NAND] Add prefetch and DMA support

2008-09-08 Thread David Brownell
On Monday 08 September 2008, Russell King - ARM Linux wrote:
> > - while (len--)
> > - *p++ = cpu_to_le16(readw(info->nand.IO_ADDR_R));
> 
> This driver needs work (see endianness explaination below.)

Already done, but this patch doesn't build on that patch...


--
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

2008-09-08 Thread Russell King - ARM Linux
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.

> @@ -127,6 +148,9 @@
>   unsigned long   phys_base;
>   void __iomem*gpmc_cs_baseaddr;
>   void __iomem*gpmc_baseaddr;
> + void __iomem*nand_pref_fifo_add;

Ok, nand_pref_fifo_add is MMIO.

> + struct completion   comp;
> + int dma_ch;
>  };
>  
>  /*
> @@ -190,6 +214,85 @@
>   __raw_writeb(cmd, info->nand.IO_ADDR_W);
>  }
>  
> +#ifdef CONFIG_MTD_NAND_OMAP_PREFETCH_DMA
> +/*
> + * omap_nand_dma_cb: callback on the completion of dma transfer
> + * @lch: logical channel
> + * @ch_satuts: channel status
> + * @data: pointer to completion data structure
> + */
> +static void omap_nand_dma_cb(int lch, u16 ch_status, void *data)
> +{
> + complete((struct completion *) data);
> +}
> +
> +/*
> + * omap_nand_dma_transfer: configer and start dma transfer
> + * @mtd: MTD device structure
> + * @addr: virtual address in RAM of source/destination
> + * @count: number of data bytes to be transferred
> + * @is_write: flag for read/write operation
> + */
> +static inline int omap_nand_dma_transfer(struct mtd_info *mtd, void *addr,
> + unsigned int count, int is_write)
> +{
> + struct omap_nand_info *info = container_of(mtd,
> + struct omap_nand_info, mtd);
> +
> + uint32_t prefetch_status = 0;
> +
> + /* The fifo depth is 64 bytes. We have a synch at each frame and frame
> +  * length is 64 bytes.
> +  */
> + int buf_len = count/64;
> +
> + if (is_write) {
> + omap_set_dma_dest_params(info->dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
> + info->phys_base, 0, 0);
> + omap_set_dma_dest_burst_mode(info->dma_ch, OMAP_DMA_DATA_BURST_16);
> + omap_set_dma_src_params(info->dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
> + virt_to_phys(addr), 0, 0);
> + omap_set_dma_src_burst_mode(info->dma_ch, OMAP_DMA_DATA_BURST_16);
> + omap_set_dma_transfer_params(info->dma_ch, OMAP_DMA_DATA_TYPE_S32,
> + 0x10, buf_len, OMAP_DMA_SYNC_FRAME,
> + OMAP24XX_DMA_GPMC, OMAP_DMA_DST_SYNC);
> + } else {
> + omap_set_dma_src_params(info->dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
> + info->phys_base, 0, 0);
> + omap_set_dma_src_burst_mode(info->dma_ch, OMAP_DMA_DATA_BURST_16);
> + omap_set_dma_dest_params(info->dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
> + virt_to_phys(addr), 0, 0);
> + omap_set_dma_dest_burst_mode(info->dma_ch, OMAP_DMA_DATA_BURST_16);
> + omap_set_dma_transfer_params(info->dma_ch, OMAP_DMA_DATA_TYPE_S32,
> + 0x10, buf_len, OMAP_DMA_SYNC_FRAME,
> + OMAP24XX_DMA_GPMC, OMAP_DMA_SRC_SYNC);
> + }
> +
> + /*  configure and start prefetch transfer */
> + gpmc_prefetch_start(info->gpmc_cs, 0x1, count, is_write);
> + 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);


> + prefetch_status = gpmc_prefetch_status();
> + while (prefetch_status & 0x3FFF)
> + prefetch_status = gpmc_prefetch_status();
> +
> + /* disable and stop the PFPW engine */
> + gpmc_prefetch_stop();
> + return 0;
> +}
> +#else
> +static void omap_nand_dma_cb(int lch, u16 ch_status, void *data) {}
> +static inline int omap_nand_dma_transfer(struct mtd_info *mtd, void *addr,
> + unsigned int count, int is_write)
> +{return 0; }
> +#endif
> +
>  /*
>   * omap_read_bu

[Resend][PATCH - Omapzoom][NAND] Add prefetch and DMA support

2008-09-08 Thread nskamat
From: Vimal Singh <[EMAIL PROTECTED]>

Following patch taken over the omapzoom.org tree adds
prefetch and DMA support to the OMAP2/3 nand driver.

Signed-off-by: Vimal Singh <[EMAIL PROTECTED]>
Signed-off-by: Nishant Kamat <[EMAIL PROTECTED]>
---
 arch/arm/mach-omap2/gpmc.c |   95 +++
 arch/arm/plat-omap/include/mach/gpmc.h |4 
 drivers/mtd/nand/Kconfig   |   30 
 drivers/mtd/nand/omap2.c   |  199 ++---
 4 files changed, 310 insertions(+), 18 deletions(-)

Index: omapkernel/arch/arm/mach-omap2/gpmc.c
===
--- omapkernel.orig/arch/arm/mach-omap2/gpmc.c  2008-09-08 18:23:29.0 
+0530
+++ omapkernel/arch/arm/mach-omap2/gpmc.c   2008-09-08 18:34:22.0 
+0530
@@ -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_PREFETCH7
+#define DMA_MPU_MODE   2
+#endif
+
 #ifdef CONFIG_OMAP3_PM
 /*
  * Structure to save/restore gpmc context
@@ -407,6 +413,92 @@
 }
 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);
+}
+
+/*
+ * 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);
+}
+EXPORT_SYMBOL(gpmc_prefetch_status);
+#else
+int  gpmc_prefetch_status(void) {return 0; }
+void gpmc_prefetch_stop(void) {}
+void gpmc_prefetch_start(int cs, int dma_mode, unsigned int u32_count,
+   int is_write) {}
+#endif
+
 static void __init gpmc_mem_init(void)
 {
int cs;
@@ -462,6 +554,9 @@
gpmc_freq_cfg.freq_cfg = NULL;
gpmc_freq_cfg.total_no_of_freq = 0;
 #endif
+#ifdef CONFIG_MTD_NAND_OMAP_PREFETCH
+   gpmc_prefetch_init();
+#endif
gpmc_mem_init();
 }
 
Index: omapkernel/arch/arm/plat-omap/include/mach/gpmc.h
===
--- omapkernel.orig/arch/arm/pla