Re: [PATCH v3 2/3] omap3 nand: cleanup virtual address usages

2010-05-19 Thread Vimal Singh
On Tue, May 18, 2010 at 4:46 PM, Sukumar Ghorai s-gho...@ti.com wrote:
 This patch removes direct reference of gpmc address from generic nand 
 platform code.
 Nand platform code now uses wrapper functions which are implemented in gpmc 
 module.

 Signed-off-by: Sukumar Ghorai s-gho...@ti.com
[...]


 @@ -287,16 +246,15 @@ static void omap_read_buf_pref(struct mtd_info *mtd, 
 u_char *buf, int len)
  {
        struct omap_nand_info *info = container_of(mtd,
                                                struct omap_nand_info, mtd);
 -       uint32_t pfpw_status = 0, r_count = 0;
 +       u32 r_count = 0;
        int ret = 0;
 -       u32 *p = (u32 *)buf;
 +       u32 *p;

        /* take care of subpage reads */
        for (; len % 4 != 0; ) {
                *buf++ = __raw_readb(info-nand.IO_ADDR_R);
                len--;
        }
 -       p = (u32 *) buf;

Above code had an issue, which was fixed by this commit:
http://git.infradead.org/mtd-2.6.git/commitdiff/c3341d0ceb4de1680572024f50233403c6a8b10d

I would suggest you to prepare your patch on MTD tree.


        /* configure and start prefetch transfer */
        ret = gpmc_prefetch_enable(info-gpmc_cs, 0x0, len, 0x0);
 @@ -307,17 +265,18 @@ static void omap_read_buf_pref(struct mtd_info *mtd, 
 u_char *buf, int len)
                else
                        omap_read_buf8(mtd, buf, len);
        } else {
 +               p = (u32 *) buf;
                do {
 -                       pfpw_status = gpmc_prefetch_status();
 -                       r_count = ((pfpw_status  24)  0x7F)  2;
 -                       ioread32_rep(info-nand_pref_fifo_add, p, r_count);
 +                       gpmc_hwcontrol(info-gpmc_cs,
 +                               GPMC_PREFETCH_FIFO_CNT, 0, 0, r_count);
 +                       r_count = r_count  2;
 +                       ioread32_rep(info-nand.IO_ADDR_R, p, r_count);
                        p += r_count;
 -                       len -= r_count  2;
 +                       len -= (r_count  2);

Braces are not required here.

                } while (len);
 -

After call to 'gpmc_prefetch_enable', next line are:
   if (ret) {
   /* PFPW engine is busy, use cpu copy method */
   if (info-nand.options  NAND_BUSWIDTH_16)
   ...
   ...
 -               /* disable and stop the PFPW engine */
 -               gpmc_prefetch_reset(info-gpmc_cs);
        }

So, if above 'if' fails, driver will not get prefetch engine (it was
already busy). Then it doesn't makes sense to call for __reset__.

 +       /* disable and stop the PFPW engine */
 +       gpmc_prefetch_reset(info-gpmc_cs);

(Also see my comments on your other patch.)

  }

  /**
 @@ -331,13 +290,13 @@ static void omap_write_buf_pref(struct mtd_info *mtd,
  {
        struct omap_nand_info *info = container_of(mtd,
                                                struct omap_nand_info, mtd);
 -       uint32_t pfpw_status = 0, w_count = 0;
 +       uint32_t pref_count = 0, w_count = 0;
        int i = 0, ret = 0;
 -       u16 *p = (u16 *) buf;
 +       u16 *p;

        /* take care of subpage writes */
        if (len % 2 != 0) {
 -               writeb(*buf, info-nand.IO_ADDR_R);
 +               writeb(*buf, info-nand.IO_ADDR_W);
                p = (u16 *)(buf + 1);
                len--;
        }
 @@ -351,17 +310,22 @@ static void omap_write_buf_pref(struct mtd_info *mtd,
                else
                        omap_write_buf8(mtd, buf, len);
        } else {
 -               pfpw_status = gpmc_prefetch_status();
 -               while (pfpw_status  0x3FFF) {
 -                       w_count = ((pfpw_status  24)  0x7F)  1;
 +               p = (u16 *) buf;
 +               while (len) {
 +                       gpmc_hwcontrol(info-gpmc_cs,
 +                                       GPMC_PREFETCH_FIFO_CNT, 0, 0, 
 w_count);
 +                       w_count = w_count  1;
                        for (i = 0; (i  w_count)  len; i++, len -= 2)
 -                               iowrite16(*p++, info-nand_pref_fifo_add);
 -                       pfpw_status = gpmc_prefetch_status();
 +                               iowrite16(*p++, info-nand.IO_ADDR_W);
                }
 -
 -               /* disable and stop the PFPW engine */
 -               gpmc_prefetch_reset(info-gpmc_cs);
 +               /* wait for data to flushed-out before reset the prefetch */
 +               do {
 +                       gpmc_hwcontrol(info-gpmc_cs,
 +                               GPMC_PREFETCH_COUNT, 0, 0, pref_count);
 +               } while (pref_count);
        }
 +       /* disable and stop the PFPW engine */
 +       gpmc_prefetch_reset(info-gpmc_cs);

Same as above.


  }

  #ifdef CONFIG_MTD_NAND_OMAP_PREFETCH_DMA
 @@ -448,8 +412,10 @@ static inline int omap_nand_dma_transfer(struct mtd_info 
 *mtd, void *addr,
        /* setup and start DMA using dma_addr */
        

Re: [PATCH v3 2/3] omap3 nand: cleanup virtual address usages

2010-05-19 Thread Vimal Singh
On Wed, May 19, 2010 at 10:54 PM, Ghorai, Sukumar s-gho...@ti.com wrote:
         /* take care of subpage reads */
         for (; len % 4 != 0; ) {
                 *buf++ = __raw_readb(info-nand.IO_ADDR_R);
                 len--;
         }
  -       p = (u32 *) buf;

 Above code had an issue, which was fixed by this commit:
 http://git.infradead.org/mtd-
 2.6.git/commitdiff/c3341d0ceb4de1680572024f50233403c6a8b10d

 I would suggest you to prepare your patch on MTD tree.
 [Ghorai] Patches started posting on lo. And lets continue the same.

Not sure about this. Its your/Tony's call.


 
         /* configure and start prefetch transfer */
         ret = gpmc_prefetch_enable(info-gpmc_cs, 0x0, len, 0x0);
  @@ -307,17 +265,18 @@ static void omap_read_buf_pref(struct mtd_info
 *mtd, u_char *buf, int len)
                 else
                         omap_read_buf8(mtd, buf, len);
         } else {
  +               p = (u32 *) buf;
                 do {
  -                       pfpw_status = gpmc_prefetch_status();
  -                       r_count = ((pfpw_status  24)  0x7F)  2;
  -                       ioread32_rep(info-nand_pref_fifo_add, p,
 r_count);
  +                       gpmc_hwcontrol(info-gpmc_cs,
  +                               GPMC_PREFETCH_FIFO_CNT, 0, 0, r_count);
  +                       r_count = r_count  2;
  +                       ioread32_rep(info-nand.IO_ADDR_R, p, r_count);
                         p += r_count;
  -                       len -= r_count  2;
  +                       len -= (r_count  2);

 Braces are not required here.
 [Ghorai] thanks

                 } while (len);
  -

 After call to 'gpmc_prefetch_enable', next line are:
            if (ret) {
                        /* PFPW engine is busy, use cpu copy method */
                        if (info-nand.options  NAND_BUSWIDTH_16)
                        ...
                        ...
  -               /* disable and stop the PFPW engine */
  -               gpmc_prefetch_reset(info-gpmc_cs);
         }

 So, if above 'if' fails, driver will not get prefetch engine (it was
 already busy). Then it doesn't makes sense to call for __reset__.
 [Ghorai] I will take this clean up as 4th patch. As its not matching with 
 patch description.

  +       /* disable and stop the PFPW engine */
  +       gpmc_prefetch_reset(info-gpmc_cs);

 (Also see my comments on your other patch.)
 [Ghorai] Agree and I will take this kind of cleanup as 4th patch

   }
 
   /**
  @@ -331,13 +290,13 @@ static void omap_write_buf_pref(struct mtd_info
 *mtd,
   {
         struct omap_nand_info *info = container_of(mtd,
                                                 struct omap_nand_info,
 mtd);
  -       uint32_t pfpw_status = 0, w_count = 0;
  +       uint32_t pref_count = 0, w_count = 0;
         int i = 0, ret = 0;
  -       u16 *p = (u16 *) buf;
  +       u16 *p;
 
         /* take care of subpage writes */
         if (len % 2 != 0) {
  -               writeb(*buf, info-nand.IO_ADDR_R);
  +               writeb(*buf, info-nand.IO_ADDR_W);
                 p = (u16 *)(buf + 1);
                 len--;
         }
  @@ -351,17 +310,22 @@ static void omap_write_buf_pref(struct mtd_info
 *mtd,
                 else
                         omap_write_buf8(mtd, buf, len);
         } else {
  -               pfpw_status = gpmc_prefetch_status();
  -               while (pfpw_status  0x3FFF) {
  -                       w_count = ((pfpw_status  24)  0x7F)  1;
  +               p = (u16 *) buf;
  +               while (len) {
  +                       gpmc_hwcontrol(info-gpmc_cs,
  +                                       GPMC_PREFETCH_FIFO_CNT, 0, 0,
 w_count);
  +                       w_count = w_count  1;
                         for (i = 0; (i  w_count)  len; i++, len -= 2)
  -                               iowrite16(*p++, info-
 nand_pref_fifo_add);
  -                       pfpw_status = gpmc_prefetch_status();
  +                               iowrite16(*p++, info-nand.IO_ADDR_W);
                 }
  -
  -               /* disable and stop the PFPW engine */
  -               gpmc_prefetch_reset(info-gpmc_cs);
  +               /* wait for data to flushed-out before reset the
 prefetch */
  +               do {
  +                       gpmc_hwcontrol(info-gpmc_cs,
  +                               GPMC_PREFETCH_COUNT, 0, 0, pref_count);
  +               } while (pref_count);
         }
  +       /* disable and stop the PFPW engine */
  +       gpmc_prefetch_reset(info-gpmc_cs);

 Same as above.
 [Ghorai] Agree and I will take this kind of cleanup as 4th patch, as its not 
 matching with patch description.


   }
 
   #ifdef CONFIG_MTD_NAND_OMAP_PREFETCH_DMA
  @@ -448,8 +412,10 @@ static inline int omap_nand_dma_transfer(struct
 mtd_info *mtd, void *addr,
         /* setup and start DMA using dma_addr */
         wait_for_completion(info-comp);
 
  -       while (0x3fff  (prefetch_status = gpmc_prefetch_status()))
  -               ;
 

Re: [PATCH v3 2/3] omap3 nand: cleanup virtual address usages

2010-05-19 Thread Tony Lindgren
* Vimal Singh vimal.neww...@gmail.com [100519 11:02]:
 On Wed, May 19, 2010 at 10:54 PM, Ghorai, Sukumar s-gho...@ti.com wrote:
          /* take care of subpage reads */
          for (; len % 4 != 0; ) {
                  *buf++ = __raw_readb(info-nand.IO_ADDR_R);
                  len--;
          }
   -       p = (u32 *) buf;
 
  Above code had an issue, which was fixed by this commit:
  http://git.infradead.org/mtd-
  2.6.git/commitdiff/c3341d0ceb4de1680572024f50233403c6a8b10d
 
  I would suggest you to prepare your patch on MTD tree.
  [Ghorai] Patches started posting on lo. And lets continue the same.
 
 Not sure about this. Its your/Tony's call.

I'd assume that fix is on it's way to the mainline kernel,
looks like a trivial rebase after the merge window is over.

Anyways, this will not make it this merge window, we're
out of time.

Regards,

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


[PATCH v3 2/3] omap3 nand: cleanup virtual address usages

2010-05-18 Thread Sukumar Ghorai
This patch removes direct reference of gpmc address from generic nand platform 
code.
Nand platform code now uses wrapper functions which are implemented in gpmc 
module. 

Signed-off-by: Sukumar Ghorai s-gho...@ti.com
---
 arch/arm/mach-omap2/gpmc-nand.c|   39 ++
 arch/arm/mach-omap2/gpmc.c |9 --
 arch/arm/plat-omap/include/plat/gpmc.h |7 +-
 arch/arm/plat-omap/include/plat/nand.h |6 +-
 drivers/mtd/nand/omap2.c   |  242 ++--
 5 files changed, 88 insertions(+), 215 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
index e57fb29..80f5d94
--- a/arch/arm/mach-omap2/gpmc-nand.c
+++ b/arch/arm/mach-omap2/gpmc-nand.c
@@ -19,8 +19,6 @@
 #include plat/board.h
 #include plat/gpmc.h
 
-#define WR_RD_PIN_MONITORING   0x0060
-
 static struct omap_nand_platform_data *gpmc_nand_data;
 
 static struct resource gpmc_nand_resource = {
@@ -71,10 +69,10 @@ static int omap2_nand_gpmc_retime(void)
t.wr_cycle  = gpmc_round_ns_to_ticks(gpmc_nand_data-gpmc_t-wr_cycle);
 
/* Configure GPMC */
-   gpmc_cs_write_reg(gpmc_nand_data-cs, GPMC_CS_CONFIG1,
-   GPMC_CONFIG1_DEVICESIZE(gpmc_nand_data-devsize) |
-   GPMC_CONFIG1_DEVICETYPE_NAND);
-
+   gpmc_hwcontrol(gpmc_nand_data-cs,
+   GPMC_CONFIG_DEV_SIZE, 1, gpmc_nand_data-devsize, NULL);
+   gpmc_hwcontrol(gpmc_nand_data-cs,
+   GPMC_CONFIG_DEV_TYPE, 1, GPMC_DEVICETYPE_NAND, NULL);
err = gpmc_cs_set_timings(gpmc_nand_data-cs, t);
if (err)
return err;
@@ -82,27 +80,13 @@ static int omap2_nand_gpmc_retime(void)
return 0;
 }
 
-static int gpmc_nand_setup(void)
-{
-   struct device *dev = gpmc_nand_device.dev;
-
-   /* Set timings in GPMC */
-   if (omap2_nand_gpmc_retime()  0) {
-   dev_err(dev, Unable to set gpmc timings\n);
-   return -EINVAL;
-   }
-
-   return 0;
-}
-
 int __init gpmc_nand_init(struct omap_nand_platform_data *_nand_data)
 {
-   unsigned int val;
int err = 0;
struct device *dev = gpmc_nand_device.dev;
 
gpmc_nand_data = _nand_data;
-   gpmc_nand_data-nand_setup = gpmc_nand_setup;
+   gpmc_nand_data-nand_setup = omap2_nand_gpmc_retime;
gpmc_nand_device.dev.platform_data = gpmc_nand_data;
 
err = gpmc_cs_request(gpmc_nand_data-cs, NAND_IO_SIZE,
@@ -112,19 +96,17 @@ int __init gpmc_nand_init(struct omap_nand_platform_data 
*_nand_data)
return err;
}
 
-   err = gpmc_nand_setup();
+/* Set timings in GPMC */
+   err = omap2_nand_gpmc_retime();
if (err  0) {
-   dev_err(dev, NAND platform setup failed: %d\n, err);
+   dev_err(dev, Unable to set gpmc timings: %d\n, err);
return err;
}
 
/* Enable RD PIN Monitoring Reg */
if (gpmc_nand_data-dev_ready) {
-   val  = gpmc_cs_read_reg(gpmc_nand_data-cs,
-GPMC_CS_CONFIG1);
-   val |= WR_RD_PIN_MONITORING;
-   gpmc_cs_write_reg(gpmc_nand_data-cs,
-   GPMC_CS_CONFIG1, val);
+   gpmc_hwcontrol(gpmc_nand_data-cs,
+   GPMC_CONFIG_RDY_BSY, 1, 1, NULL);
}
 
err = platform_device_register(gpmc_nand_device);
@@ -140,3 +122,4 @@ out_free_cs:
 
return err;
 }
+
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index f414aeb..cbe0efb
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -589,15 +589,6 @@ int gpmc_prefetch_reset(int cs)
 }
 EXPORT_SYMBOL(gpmc_prefetch_reset);
 
-/**
- * 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);
-
 static void __init gpmc_mem_init(void)
 {
int cs;
diff --git a/arch/arm/plat-omap/include/plat/gpmc.h 
b/arch/arm/plat-omap/include/plat/gpmc.h
index 67a3442..2386ff6
--- a/arch/arm/plat-omap/include/plat/gpmc.h
+++ b/arch/arm/plat-omap/include/plat/gpmc.h
@@ -25,9 +25,6 @@
 #define GPMC_CS_NAND_ADDRESS   0x20
 #define GPMC_CS_NAND_DATA  0x24
 
-#define GPMC_CONFIG0x50
-#define GPMC_STATUS0x54
-
 /* Control Commands */
 #define GPMC_CONFIG_WP 0x0001
 #define GPMC_CONFIG_RDY_BSY0x0002
@@ -63,7 +60,6 @@
 #define GPMC_CONFIG1_DEVICESIZE_16  GPMC_CONFIG1_DEVICESIZE(1)
 #define GPMC_CONFIG1_DEVICETYPE(val)((val  3)  10)
 #define GPMC_CONFIG1_DEVICETYPE_NOR GPMC_CONFIG1_DEVICETYPE(0)
-#define GPMC_CONFIG1_DEVICETYPE_NANDGPMC_CONFIG1_DEVICETYPE(2)
 #define GPMC_CONFIG1_MUXADDDATA (1  9)
 #define GPMC_CONFIG1_TIME_PARA_GRAN (1  4)
 #define GPMC_CONFIG1_FCLK_DIV(val)  (val  3)
@@ -77,7 +73,7