RE: [PATCH v3 1/3] omap3 gpmc: functionality enhancement

2010-05-25 Thread Ghorai, Sukumar
Vimal,

 -Original Message-
 From: Vimal Singh [mailto:vimal.neww...@gmail.com]
 Sent: Thursday, May 20, 2010 12:01 AM
 To: Ghorai, Sukumar
 Cc: linux-omap@vger.kernel.org; linux-...@lists.infradead.org;
 t...@atomide.com; sako...@gmail.com; m...@compulab.co.il;
 artem.bityuts...@nokia.com; peter.bar...@gmail.com
 Subject: Re: [PATCH v3 1/3] omap3 gpmc: functionality enhancement
 
 On Wed, May 19, 2010 at 11:34 PM, Ghorai, Sukumar s-gho...@ti.com wrote:
+
+       case GPMC_CONFIG_RDY_BSY:
+               regval  = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
+               regval |= WR_RD_PIN_MONITORING;
+               gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, regval);
+               break;
  
   IIRC, at least in OMAP2/3, ready/busy pin is not in use (not
 connected).
 
  On the Logic OMAP3530 LV SOM and Torpedo modules, the R/B# pin of the
  NAND in the Micron mt29c2g4maklajg-6it POP part is connected to the
  WAIT0 pin on the OMAP3530 and I'm looking to use it to speed up NAND
  accesses
  [Ghorai] So better keep this feature,
 
 Yes, looks like there are some boards which can still take advantage of
 this.
 
 
 [...]
@@ -456,13 +572,20 @@ EXPORT_SYMBOL(gpmc_prefetch_enable);
 /**
 * gpmc_prefetch_reset - disables and stops the prefetch engine
 */
-void gpmc_prefetch_reset(void)
+int gpmc_prefetch_reset(int cs)
 {
+       if (gpmc_pref_used == cs)
+               gpmc_pref_used = -EINVAL;
+       else
+               return -EINVAL;
+
  
   This is also not required. As, this function will be called only if
   prefetch was used.
  [Ghorai] Agree. Can you see this input too?
  http://www.mail-archive.com/linux-omap@vger.kernel.org/msg28520.html
 
 Exactly, this is what I am telling here. Enable prefetch engine call
 is already being check for *busy* or not.
[Ghorai] Agree.
 
 
 [...]
+int gpmc_ecc_init(int cs, int ecc_size)
+{
+       unsigned int val = 0x0;
+
+       /* check if ecc engine already by another cs */
+       if (gpmc_ecc_used == -EINVAL)
+               gpmc_ecc_used = cs;
+       else
+               return -EBUSY;
   Here few things need be to consider:
   1. 'init' is supposed to done once for every instance of driver
 during
  probe
   2. But ECC engine, too, have only one instance at a time, So
   3. As long as all NAND chip are supposed to use same ECC machenism,
 we
   can go for only one time 'init' for all drivers, perhaps in
   gpmc_nand.c.
   4. But in case, different instances of driver (or NAND chip) requires
   different ECC machenism (for ex. Hamming or BCH, or even with
   different capabilities of error correction),
   this will no longer vailid. Then rather we should have something like
   'gpmc_ecc_config' call to configer ECC engine for everytime a driver
   needs it (something like as it is done for prefetch engine).
  [Ghorai]
  a. do you think it will reduce the throughput?
 No. But in current implementation it will be called for each instance
 driver. (see my 3rd point)
 
  b. Moreover I think we will take this as 5th patch as cleanup/
 improvemnt.
  c. And how to know that ECC engine is in used other driver should not
 use it? Any bit to know that ecc engine is busy, as we check for prefetch?
 Do not really remember config registers. Perhaps there is no way.
 But I guess you should check into register GPMC_ECC_CONFIG at bit 1.
 This is the bit we are setting to enable ECC calculation, IIRC.
[Ghorai] there are two functions -
   info-nand.ecc.calculate= omap_calculate_ecc;
   info-nand.ecc.hwctl= omap_enable_hwecc;
And GPMC_ECC_CONFIG register..
3:1 ECCCS Selects the CS where ECC is computed 
0 ECCENABLE Enables the ECC feature
Now we enable omap_enable_hwecc (with GPMC_ECC_CONFIG[ECCENABLE]=1); and its 
get disabled (automatically) when ecc_size data transfer over. But say still it 
did not read the ecc value yet (omap_calculate_ecc).
So how to protect following omap_enable_hwecc() before omap_calculate_ecc()  
without additional flag? Any input is welcome.

 
  d. any further input on http://www.mail-archive.com/linux-
 o...@vger.kernel.org/msg28520.html
 And this what I was suggesting in my point 4. In my example
 'gpmc_ecc_config' is analogy to 'gpmc_ecc_request'.
 I said *config*, since in such scenario you need to configer HW
 ECCconfig register everytime as well, rather just checking
 availability and enabling.
[Ghorai] As above.
 
 
 [...]
+int gpmc_ecc_reset(int cs)
+{
+       if (gpmc_ecc_used == cs)
+               gpmc_ecc_used = -EINVAL;
+       else
+               return -EINVAL;
+
+       return 0;
+}
 
 I guess in this function you should also clear gpmc ecc config
 register explicitly.
 
 
 --
 Regards,
 Vimal Singh
--
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

Re: [PATCH v3 1/3] omap3 gpmc: functionality enhancement

2010-05-25 Thread Vimal Singh
On Tue, May 25, 2010 at 8:07 PM, Ghorai, Sukumar s-gho...@ti.com wrote:
[...]
  c. And how to know that ECC engine is in used other driver should not
 use it? Any bit to know that ecc engine is busy, as we check for prefetch?
 Do not really remember config registers. Perhaps there is no way.
 But I guess you should check into register GPMC_ECC_CONFIG at bit 1.
 This is the bit we are setting to enable ECC calculation, IIRC.
 [Ghorai] there are two functions -
       info-nand.ecc.calculate        = omap_calculate_ecc;
       info-nand.ecc.hwctl            = omap_enable_hwecc;
 And GPMC_ECC_CONFIG register..
        3:1 ECCCS Selects the CS where ECC is computed
        0 ECCENABLE Enables the ECC feature
 Now we enable omap_enable_hwecc (with GPMC_ECC_CONFIG[ECCENABLE]=1); and its 
 get disabled (automatically) when ecc_size data transfer over.
 But say still it did not read the ecc value yet (omap_calculate_ecc).
 So how to protect following omap_enable_hwecc() before omap_calculate_ecc()  
 without additional flag? Any input is welcome.

Oh yes, that's is a problem. Perhaps in that case you have to protect
it in very much same way you already did.

-- 
Regards,
Vimal Singh
--
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 v3 1/3] omap3 gpmc: functionality enhancement

2010-05-20 Thread Vimal Singh
On Thu, May 20, 2010 at 11:08 AM, Ghorai, Sukumar s-gho...@ti.com wrote:
 Vimal,

 -Original Message-
 From: Vimal Singh [mailto:vimal.neww...@gmail.com]
 Sent: 2010-05-20 00:01
 To: Ghorai, Sukumar
 Cc: linux-omap@vger.kernel.org; linux-...@lists.infradead.org;
 t...@atomide.com; sako...@gmail.com; m...@compulab.co.il;
 artem.bityuts...@nokia.com; peter.bar...@gmail.com
 Subject: Re: [PATCH v3 1/3] omap3 gpmc: functionality enhancement

 On Wed, May 19, 2010 at 11:34 PM, Ghorai, Sukumar s-gho...@ti.com wrote:
+
+       case GPMC_CONFIG_RDY_BSY:
+               regval  = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
+               regval |= WR_RD_PIN_MONITORING;
+               gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, regval);
+               break;
  
   IIRC, at least in OMAP2/3, ready/busy pin is not in use (not
 connected).
 
  On the Logic OMAP3530 LV SOM and Torpedo modules, the R/B# pin of the
  NAND in the Micron mt29c2g4maklajg-6it POP part is connected to the
  WAIT0 pin on the OMAP3530 and I'm looking to use it to speed up NAND
  accesses
  [Ghorai] So better keep this feature,

 Yes, looks like there are some boards which can still take advantage of
 this.

 
 [...]
@@ -456,13 +572,20 @@ EXPORT_SYMBOL(gpmc_prefetch_enable);
 /**
 * gpmc_prefetch_reset - disables and stops the prefetch engine
 */
-void gpmc_prefetch_reset(void)
+int gpmc_prefetch_reset(int cs)
 {
+       if (gpmc_pref_used == cs)
+               gpmc_pref_used = -EINVAL;
+       else
+               return -EINVAL;
+
  
   This is also not required. As, this function will be called only if
   prefetch was used.
  [Ghorai] Agree. Can you see this input too?
  http://www.mail-archive.com/linux-omap@vger.kernel.org/msg28520.html

 Exactly, this is what I am telling here. Enable prefetch engine call
 is already being check for *busy* or not.

 
 [...]
+int gpmc_ecc_init(int cs, int ecc_size)
+{
+       unsigned int val = 0x0;
+
+       /* check if ecc engine already by another cs */
+       if (gpmc_ecc_used == -EINVAL)
+               gpmc_ecc_used = cs;
+       else
+               return -EBUSY;
   Here few things need be to consider:
   1. 'init' is supposed to done once for every instance of driver
 during
  probe
   2. But ECC engine, too, have only one instance at a time, So
   3. As long as all NAND chip are supposed to use same ECC machenism,
 we
   can go for only one time 'init' for all drivers, perhaps in
   gpmc_nand.c.
   4. But in case, different instances of driver (or NAND chip) requires
   different ECC machenism (for ex. Hamming or BCH, or even with
   different capabilities of error correction),
   this will no longer vailid. Then rather we should have something like
   'gpmc_ecc_config' call to configer ECC engine for everytime a driver
   needs it (something like as it is done for prefetch engine).
  [Ghorai]
  a. do you think it will reduce the throughput?
 No. But in current implementation it will be called for each instance
 driver. (see my 3rd point)

  b. Moreover I think we will take this as 5th patch as cleanup/
 improvemnt.
  c. And how to know that ECC engine is in used other driver should not
 use it? Any bit to know that ecc engine is busy, as we check for prefetch?
 Do not really remember config registers. Perhaps there is no way.
 But I guess you should check into register GPMC_ECC_CONFIG at bit 1.
 This is the bit we are setting to enable ECC calculation, IIRC.

  d. any further input on http://www.mail-archive.com/linux-
 o...@vger.kernel.org/msg28520.html
 And this what I was suggesting in my point 4. In my example
 'gpmc_ecc_config' is analogy to 'gpmc_ecc_request'.
 I said *config*, since in such scenario you need to configer HW
 ECCconfig register everytime as well, rather just checking
 availability and enabling.
 [Ghorai] still I feel we should not mix this patch with cleanup. And yes if 
 possible this will be the 5th one as cleanup.
 4th one - prefetch cleanup
 5th one - ecc cleanup.
 Do you think still missing anything for this patch?

As long as you take care of current comments, I do not have any
further comment for now.


-- 
Regards,
Vimal Singh
--
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 v3 1/3] omap3 gpmc: functionality enhancement

2010-05-19 Thread Vimal Singh
On Tue, May 18, 2010 at 4:46 PM, Sukumar Ghorai s-gho...@ti.com wrote:
 few functions added in gpmc module and to be used by other drivers like NAND.
 E.g.: - ioctl function
      - ecc functions

 Signed-off-by: Sukumar Ghorai s-gho...@ti.com
 ---
  arch/arm/mach-omap2/gpmc.c             |  246 
 +++-
  arch/arm/plat-omap/include/plat/gpmc.h |   35 -
  drivers/mtd/nand/omap2.c               |    4 +-
  3 files changed, 274 insertions(+), 11 deletions(-)

 diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
 index 5bc3ca0..7e6d821
 --- a/arch/arm/mach-omap2/gpmc.c
 +++ b/arch/arm/mach-omap2/gpmc.c
 @@ -46,8 +46,9 @@
  #define GPMC_ECC_CONFIG                0x1f4
  #define GPMC_ECC_CONTROL       0x1f8
  #define GPMC_ECC_SIZE_CONFIG   0x1fc
 +#define GPMC_ECC1_RESULT        0x200

 -#define GPMC_CS0               0x60
 +#define GPMC_CS0_BASE          0x60
  #define GPMC_CS_SIZE           0x30

  #define GPMC_MEM_START         0x
 @@ -92,7 +93,9 @@ struct omap3_gpmc_regs {
  static struct resource gpmc_mem_root;
  static struct resource gpmc_cs_mem[GPMC_CS_NUM];
  static DEFINE_SPINLOCK(gpmc_mem_lock);
 -static unsigned                gpmc_cs_map;
 +static unsigned        int gpmc_cs_map;        /* flag for cs which are 
 initialized */
 +static int gpmc_pref_used = -EINVAL;   /* cs using prefetch engine */
 +static int gpmc_ecc_used = -EINVAL;    /* cs using ecc engine */

  static void __iomem *gpmc_base;

 @@ -108,11 +111,27 @@ static u32 gpmc_read_reg(int idx)
        return __raw_readl(gpmc_base + idx);
  }

 +static void gpmc_cs_write_byte(int cs, int idx, u8 val)
 +{
 +       void __iomem *reg_addr;
 +
 +       reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) + idx;
 +       __raw_writeb(val, reg_addr);
 +}
 +
 +static u8 gpmc_cs_read_byte(int cs, int idx)
 +{
 +       void __iomem *reg_addr;
 +
 +       reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) + idx;
 +       return __raw_readb(reg_addr);
 +}
 +
  void gpmc_cs_write_reg(int cs, int idx, u32 val)
  {
        void __iomem *reg_addr;

 -       reg_addr = gpmc_base + GPMC_CS0 + (cs * GPMC_CS_SIZE) + idx;
 +       reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) + idx;
        __raw_writel(val, reg_addr);
  }

 @@ -120,7 +139,7 @@ u32 gpmc_cs_read_reg(int cs, int idx)
  {
        void __iomem *reg_addr;

 -       reg_addr = gpmc_base + GPMC_CS0 + (cs * GPMC_CS_SIZE) + idx;
 +       reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) + idx;
        return __raw_readl(reg_addr);
  }

 @@ -419,8 +438,100 @@ void gpmc_cs_free(int cs)
  EXPORT_SYMBOL(gpmc_cs_free);

  /**
 + * gpmc_hwcontrol - hardware specific access (read/ write) control
 + * @cs: chip select number
 + * @cmd: command type
 + * @write: 1 for write; 0 for read
 + * @wval: value to write
 + * @rval: read pointer
 + */
 +int gpmc_hwcontrol(int cs, int cmd, int write, int wval, int *rval)
 +{
 +       u32 regval = 0;
 +
 +       if (!write  !rval)
 +               return -EINVAL;
 +
 +       switch (cmd) {
 +       case GPMC_STATUS_BUFFER:
 +               regval = gpmc_read_reg(GPMC_STATUS);
 +               /* 1 : buffer is available to write */
 +               *rval = regval  GPMC_STATUS_BUFF_EMPTY;
 +               break;
 +
 +       case GPMC_GET_SET_IRQ_STATUS:
 +               if (write)
 +                       gpmc_write_reg(GPMC_IRQSTATUS, wval);
 +               else
 +                       *rval = gpmc_read_reg(GPMC_IRQSTATUS);
 +               break;
 +
 +       case GPMC_PREFETCH_FIFO_CNT:
 +               regval = gpmc_read_reg(GPMC_PREFETCH_STATUS);
 +               *rval = GPMC_PREFETCH_STATUS_FIFO_CNT(regval);
 +               break;
 +
 +       case GPMC_PREFETCH_COUNT:
 +               regval = gpmc_read_reg(GPMC_PREFETCH_STATUS);
 +               *rval = GPMC_PREFETCH_STATUS_COUNT(regval);
 +               break;
 +
 +       case GPMC_CONFIG_WP:
 +               regval = gpmc_read_reg(GPMC_CONFIG);
 +               if (wval)
 +                       regval = ~GPMC_CONFIG_WRITEPROTECT; /* WP is ON */
 +               else
 +                       regval |= GPMC_CONFIG_WRITEPROTECT;  /* WP is OFF */
 +               gpmc_write_reg(GPMC_CONFIG, regval);
 +               break;
 +
 +       case GPMC_CONFIG_RDY_BSY:
 +               regval  = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
 +               regval |= WR_RD_PIN_MONITORING;
 +               gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, regval);
 +               break;

IIRC, at least in OMAP2/3, ready/busy pin is not in use (not connected).

 +
 +       case GPMC_CONFIG_DEV_SIZE:
 +               regval  = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
 +               regval |= GPMC_CONFIG1_DEVICESIZE(wval);
 +               gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, regval);
 +               break;
 +
 +       case GPMC_CONFIG_DEV_TYPE:
 +               regval  = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
 +               regval |= 

Re: [PATCH v3 1/3] omap3 gpmc: functionality enhancement

2010-05-19 Thread Peter Barada
On Wed, 2010-05-19 at 20:16 +0530, Vimal Singh wrote:
 On Tue, May 18, 2010 at 4:46 PM, Sukumar Ghorai s-gho...@ti.com wrote:
  few functions added in gpmc module and to be used by other drivers like 
  NAND.
  E.g.: - ioctl function
   - ecc functions
 
  Signed-off-by: Sukumar Ghorai s-gho...@ti.com
  ---
   arch/arm/mach-omap2/gpmc.c |  246 
  +++-
   arch/arm/plat-omap/include/plat/gpmc.h |   35 -
   drivers/mtd/nand/omap2.c   |4 +-
   3 files changed, 274 insertions(+), 11 deletions(-)
 
  diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
  index 5bc3ca0..7e6d821
  --- a/arch/arm/mach-omap2/gpmc.c
  +++ b/arch/arm/mach-omap2/gpmc.c
  @@ -46,8 +46,9 @@
   #define GPMC_ECC_CONFIG0x1f4
   #define GPMC_ECC_CONTROL   0x1f8
   #define GPMC_ECC_SIZE_CONFIG   0x1fc
  +#define GPMC_ECC1_RESULT0x200
 
  -#define GPMC_CS0   0x60
  +#define GPMC_CS0_BASE  0x60
   #define GPMC_CS_SIZE   0x30
 
   #define GPMC_MEM_START 0x
  @@ -92,7 +93,9 @@ struct omap3_gpmc_regs {
   static struct resource gpmc_mem_root;
   static struct resource gpmc_cs_mem[GPMC_CS_NUM];
   static DEFINE_SPINLOCK(gpmc_mem_lock);
  -static unsignedgpmc_cs_map;
  +static unsignedint gpmc_cs_map;/* flag for cs which are 
  initialized */
  +static int gpmc_pref_used = -EINVAL;   /* cs using prefetch engine */
  +static int gpmc_ecc_used = -EINVAL;/* cs using ecc engine */
 
   static void __iomem *gpmc_base;
 
  @@ -108,11 +111,27 @@ static u32 gpmc_read_reg(int idx)
 return __raw_readl(gpmc_base + idx);
   }
 
  +static void gpmc_cs_write_byte(int cs, int idx, u8 val)
  +{
  +   void __iomem *reg_addr;
  +
  +   reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) + idx;
  +   __raw_writeb(val, reg_addr);
  +}
  +
  +static u8 gpmc_cs_read_byte(int cs, int idx)
  +{
  +   void __iomem *reg_addr;
  +
  +   reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) + idx;
  +   return __raw_readb(reg_addr);
  +}
  +
   void gpmc_cs_write_reg(int cs, int idx, u32 val)
   {
 void __iomem *reg_addr;
 
  -   reg_addr = gpmc_base + GPMC_CS0 + (cs * GPMC_CS_SIZE) + idx;
  +   reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) + idx;
 __raw_writel(val, reg_addr);
   }
 
  @@ -120,7 +139,7 @@ u32 gpmc_cs_read_reg(int cs, int idx)
   {
 void __iomem *reg_addr;
 
  -   reg_addr = gpmc_base + GPMC_CS0 + (cs * GPMC_CS_SIZE) + idx;
  +   reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) + idx;
 return __raw_readl(reg_addr);
   }
 
  @@ -419,8 +438,100 @@ void gpmc_cs_free(int cs)
   EXPORT_SYMBOL(gpmc_cs_free);
 
   /**
  + * gpmc_hwcontrol - hardware specific access (read/ write) control
  + * @cs: chip select number
  + * @cmd: command type
  + * @write: 1 for write; 0 for read
  + * @wval: value to write
  + * @rval: read pointer
  + */
  +int gpmc_hwcontrol(int cs, int cmd, int write, int wval, int *rval)
  +{
  +   u32 regval = 0;
  +
  +   if (!write  !rval)
  +   return -EINVAL;
  +
  +   switch (cmd) {
  +   case GPMC_STATUS_BUFFER:
  +   regval = gpmc_read_reg(GPMC_STATUS);
  +   /* 1 : buffer is available to write */
  +   *rval = regval  GPMC_STATUS_BUFF_EMPTY;
  +   break;
  +
  +   case GPMC_GET_SET_IRQ_STATUS:
  +   if (write)
  +   gpmc_write_reg(GPMC_IRQSTATUS, wval);
  +   else
  +   *rval = gpmc_read_reg(GPMC_IRQSTATUS);
  +   break;
  +
  +   case GPMC_PREFETCH_FIFO_CNT:
  +   regval = gpmc_read_reg(GPMC_PREFETCH_STATUS);
  +   *rval = GPMC_PREFETCH_STATUS_FIFO_CNT(regval);
  +   break;
  +
  +   case GPMC_PREFETCH_COUNT:
  +   regval = gpmc_read_reg(GPMC_PREFETCH_STATUS);
  +   *rval = GPMC_PREFETCH_STATUS_COUNT(regval);
  +   break;
  +
  +   case GPMC_CONFIG_WP:
  +   regval = gpmc_read_reg(GPMC_CONFIG);
  +   if (wval)
  +   regval = ~GPMC_CONFIG_WRITEPROTECT; /* WP is ON */
  +   else
  +   regval |= GPMC_CONFIG_WRITEPROTECT;  /* WP is OFF */
  +   gpmc_write_reg(GPMC_CONFIG, regval);
  +   break;
  +
  +   case GPMC_CONFIG_RDY_BSY:
  +   regval  = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
  +   regval |= WR_RD_PIN_MONITORING;
  +   gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, regval);
  +   break;
 
 IIRC, at least in OMAP2/3, ready/busy pin is not in use (not connected).

On the Logic OMAP3530 LV SOM and Torpedo modules, the R/B# pin of the
NAND in the Micron mt29c2g4maklajg-6it POP part is connected to the
WAIT0 pin on the OMAP3530 and I'm looking to use it to speed up 

RE: [PATCH v3 1/3] omap3 gpmc: functionality enhancement

2010-05-19 Thread Ghorai, Sukumar
Vimal,

 -Original Message-
 From: Peter Barada [mailto:peter.bar...@gmail.com]
 Sent: 2010-05-19 21:18
 To: Vimal Singh
 Cc: Ghorai, Sukumar; linux-omap@vger.kernel.org; linux-
 m...@lists.infradead.org; t...@atomide.com; sako...@gmail.com;
 m...@compulab.co.il; artem.bityuts...@nokia.com
 Subject: Re: [PATCH v3 1/3] omap3 gpmc: functionality enhancement

 On Wed, 2010-05-19 at 20:16 +0530, Vimal Singh wrote:
  On Tue, May 18, 2010 at 4:46 PM, Sukumar Ghorai s-gho...@ti.com wrote:
   few functions added in gpmc module and to be used by other drivers
 like NAND.
   E.g.: - ioctl function
- ecc functions
  
   Signed-off-by: Sukumar Ghorai s-gho...@ti.com
   ---
arch/arm/mach-omap2/gpmc.c |  246
 +++-
arch/arm/plat-omap/include/plat/gpmc.h |   35 -
drivers/mtd/nand/omap2.c   |4 +-
3 files changed, 274 insertions(+), 11 deletions(-)
  
   diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
   index 5bc3ca0..7e6d821
   --- a/arch/arm/mach-omap2/gpmc.c
   +++ b/arch/arm/mach-omap2/gpmc.c
   @@ -46,8 +46,9 @@
#define GPMC_ECC_CONFIG0x1f4
#define GPMC_ECC_CONTROL   0x1f8
#define GPMC_ECC_SIZE_CONFIG   0x1fc
   +#define GPMC_ECC1_RESULT0x200
  
   -#define GPMC_CS0   0x60
   +#define GPMC_CS0_BASE  0x60
#define GPMC_CS_SIZE   0x30
  
#define GPMC_MEM_START 0x
   @@ -92,7 +93,9 @@ struct omap3_gpmc_regs {
static struct resource gpmc_mem_root;
static struct resource gpmc_cs_mem[GPMC_CS_NUM];
static DEFINE_SPINLOCK(gpmc_mem_lock);
   -static unsignedgpmc_cs_map;
   +static unsignedint gpmc_cs_map;/* flag for cs which
 are initialized */
   +static int gpmc_pref_used = -EINVAL;   /* cs using prefetch engine */
   +static int gpmc_ecc_used = -EINVAL;/* cs using ecc engine */
  
static void __iomem *gpmc_base;
  
   @@ -108,11 +111,27 @@ static u32 gpmc_read_reg(int idx)
  return __raw_readl(gpmc_base + idx);
}
  
   +static void gpmc_cs_write_byte(int cs, int idx, u8 val)
   +{
   +   void __iomem *reg_addr;
   +
   +   reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) +
 idx;
   +   __raw_writeb(val, reg_addr);
   +}
   +
   +static u8 gpmc_cs_read_byte(int cs, int idx)
   +{
   +   void __iomem *reg_addr;
   +
   +   reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) +
 idx;
   +   return __raw_readb(reg_addr);
   +}
   +
void gpmc_cs_write_reg(int cs, int idx, u32 val)
{
  void __iomem *reg_addr;
  
   -   reg_addr = gpmc_base + GPMC_CS0 + (cs * GPMC_CS_SIZE) + idx;
   +   reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) +
 idx;
  __raw_writel(val, reg_addr);
}
  
   @@ -120,7 +139,7 @@ u32 gpmc_cs_read_reg(int cs, int idx)
{
  void __iomem *reg_addr;
  
   -   reg_addr = gpmc_base + GPMC_CS0 + (cs * GPMC_CS_SIZE) + idx;
   +   reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) +
 idx;
  return __raw_readl(reg_addr);
}
  
   @@ -419,8 +438,100 @@ void gpmc_cs_free(int cs)
EXPORT_SYMBOL(gpmc_cs_free);
  
/**
   + * gpmc_hwcontrol - hardware specific access (read/ write) control
   + * @cs: chip select number
   + * @cmd: command type
   + * @write: 1 for write; 0 for read
   + * @wval: value to write
   + * @rval: read pointer
   + */
   +int gpmc_hwcontrol(int cs, int cmd, int write, int wval, int *rval)
   +{
   +   u32 regval = 0;
   +
   +   if (!write  !rval)
   +   return -EINVAL;
   +
   +   switch (cmd) {
   +   case GPMC_STATUS_BUFFER:
   +   regval = gpmc_read_reg(GPMC_STATUS);
   +   /* 1 : buffer is available to write */
   +   *rval = regval  GPMC_STATUS_BUFF_EMPTY;
   +   break;
   +
   +   case GPMC_GET_SET_IRQ_STATUS:
   +   if (write)
   +   gpmc_write_reg(GPMC_IRQSTATUS, wval);
   +   else
   +   *rval = gpmc_read_reg(GPMC_IRQSTATUS);
   +   break;
   +
   +   case GPMC_PREFETCH_FIFO_CNT:
   +   regval = gpmc_read_reg(GPMC_PREFETCH_STATUS);
   +   *rval = GPMC_PREFETCH_STATUS_FIFO_CNT(regval);
   +   break;
   +
   +   case GPMC_PREFETCH_COUNT:
   +   regval = gpmc_read_reg(GPMC_PREFETCH_STATUS);
   +   *rval = GPMC_PREFETCH_STATUS_COUNT(regval);
   +   break;
   +
   +   case GPMC_CONFIG_WP:
   +   regval = gpmc_read_reg(GPMC_CONFIG);
   +   if (wval)
   +   regval = ~GPMC_CONFIG_WRITEPROTECT; /* WP is
 ON */
   +   else
   +   regval |= GPMC_CONFIG_WRITEPROTECT;  /* WP is
 OFF */
   +   gpmc_write_reg(GPMC_CONFIG, regval);
   +   break;
   +
   +   case

RE: [PATCH v3 1/3] omap3 gpmc: functionality enhancement

2010-05-19 Thread Ghorai, Sukumar
Vimal,

 -Original Message-
 From: Vimal Singh [mailto:vimal.neww...@gmail.com]
 Sent: 2010-05-20 00:01
 To: Ghorai, Sukumar
 Cc: linux-omap@vger.kernel.org; linux-...@lists.infradead.org;
 t...@atomide.com; sako...@gmail.com; m...@compulab.co.il;
 artem.bityuts...@nokia.com; peter.bar...@gmail.com
 Subject: Re: [PATCH v3 1/3] omap3 gpmc: functionality enhancement
 
 On Wed, May 19, 2010 at 11:34 PM, Ghorai, Sukumar s-gho...@ti.com wrote:
+
+       case GPMC_CONFIG_RDY_BSY:
+               regval  = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
+               regval |= WR_RD_PIN_MONITORING;
+               gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, regval);
+               break;
  
   IIRC, at least in OMAP2/3, ready/busy pin is not in use (not
 connected).
 
  On the Logic OMAP3530 LV SOM and Torpedo modules, the R/B# pin of the
  NAND in the Micron mt29c2g4maklajg-6it POP part is connected to the
  WAIT0 pin on the OMAP3530 and I'm looking to use it to speed up NAND
  accesses
  [Ghorai] So better keep this feature,
 
 Yes, looks like there are some boards which can still take advantage of
 this.
 
 
 [...]
@@ -456,13 +572,20 @@ EXPORT_SYMBOL(gpmc_prefetch_enable);
 /**
 * gpmc_prefetch_reset - disables and stops the prefetch engine
 */
-void gpmc_prefetch_reset(void)
+int gpmc_prefetch_reset(int cs)
 {
+       if (gpmc_pref_used == cs)
+               gpmc_pref_used = -EINVAL;
+       else
+               return -EINVAL;
+
  
   This is also not required. As, this function will be called only if
   prefetch was used.
  [Ghorai] Agree. Can you see this input too?
  http://www.mail-archive.com/linux-omap@vger.kernel.org/msg28520.html
 
 Exactly, this is what I am telling here. Enable prefetch engine call
 is already being check for *busy* or not.
 
 
 [...]
+int gpmc_ecc_init(int cs, int ecc_size)
+{
+       unsigned int val = 0x0;
+
+       /* check if ecc engine already by another cs */
+       if (gpmc_ecc_used == -EINVAL)
+               gpmc_ecc_used = cs;
+       else
+               return -EBUSY;
   Here few things need be to consider:
   1. 'init' is supposed to done once for every instance of driver
 during
  probe
   2. But ECC engine, too, have only one instance at a time, So
   3. As long as all NAND chip are supposed to use same ECC machenism,
 we
   can go for only one time 'init' for all drivers, perhaps in
   gpmc_nand.c.
   4. But in case, different instances of driver (or NAND chip) requires
   different ECC machenism (for ex. Hamming or BCH, or even with
   different capabilities of error correction),
   this will no longer vailid. Then rather we should have something like
   'gpmc_ecc_config' call to configer ECC engine for everytime a driver
   needs it (something like as it is done for prefetch engine).
  [Ghorai]
  a. do you think it will reduce the throughput?
 No. But in current implementation it will be called for each instance
 driver. (see my 3rd point)
 
  b. Moreover I think we will take this as 5th patch as cleanup/
 improvemnt.
  c. And how to know that ECC engine is in used other driver should not
 use it? Any bit to know that ecc engine is busy, as we check for prefetch?
 Do not really remember config registers. Perhaps there is no way.
 But I guess you should check into register GPMC_ECC_CONFIG at bit 1.
 This is the bit we are setting to enable ECC calculation, IIRC.
 
  d. any further input on http://www.mail-archive.com/linux-
 o...@vger.kernel.org/msg28520.html
 And this what I was suggesting in my point 4. In my example
 'gpmc_ecc_config' is analogy to 'gpmc_ecc_request'.
 I said *config*, since in such scenario you need to configer HW
 ECCconfig register everytime as well, rather just checking
 availability and enabling.
[Ghorai] still I feel we should not mix this patch with cleanup. And yes if 
possible this will be the 5th one as cleanup.
4th one - prefetch cleanup
5th one - ecc cleanup.
Do you think still missing anything for this patch?

 
 
 [...]
+int gpmc_ecc_reset(int cs)
+{
+       if (gpmc_ecc_used == cs)
+               gpmc_ecc_used = -EINVAL;
+       else
+               return -EINVAL;
+
+       return 0;
+}
 
 I guess in this function you should also clear gpmc ecc config
 register explicitly.
 
 
 --
 Regards,
 Vimal Singh
--
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 1/3] omap3 gpmc: functionality enhancement

2010-05-18 Thread Sukumar Ghorai
few functions added in gpmc module and to be used by other drivers like NAND.
E.g.: - ioctl function
  - ecc functions

Signed-off-by: Sukumar Ghorai s-gho...@ti.com
---
 arch/arm/mach-omap2/gpmc.c |  246 +++-
 arch/arm/plat-omap/include/plat/gpmc.h |   35 -
 drivers/mtd/nand/omap2.c   |4 +-
 3 files changed, 274 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 5bc3ca0..7e6d821
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -46,8 +46,9 @@
 #define GPMC_ECC_CONFIG0x1f4
 #define GPMC_ECC_CONTROL   0x1f8
 #define GPMC_ECC_SIZE_CONFIG   0x1fc
+#define GPMC_ECC1_RESULT0x200
 
-#define GPMC_CS0   0x60
+#define GPMC_CS0_BASE  0x60
 #define GPMC_CS_SIZE   0x30
 
 #define GPMC_MEM_START 0x
@@ -92,7 +93,9 @@ struct omap3_gpmc_regs {
 static struct resource gpmc_mem_root;
 static struct resource gpmc_cs_mem[GPMC_CS_NUM];
 static DEFINE_SPINLOCK(gpmc_mem_lock);
-static unsignedgpmc_cs_map;
+static unsignedint gpmc_cs_map;/* flag for cs which are 
initialized */
+static int gpmc_pref_used = -EINVAL;   /* cs using prefetch engine */
+static int gpmc_ecc_used = -EINVAL;/* cs using ecc engine */
 
 static void __iomem *gpmc_base;
 
@@ -108,11 +111,27 @@ static u32 gpmc_read_reg(int idx)
return __raw_readl(gpmc_base + idx);
 }
 
+static void gpmc_cs_write_byte(int cs, int idx, u8 val)
+{
+   void __iomem *reg_addr;
+
+   reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) + idx;
+   __raw_writeb(val, reg_addr);
+}
+
+static u8 gpmc_cs_read_byte(int cs, int idx)
+{
+   void __iomem *reg_addr;
+
+   reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) + idx;
+   return __raw_readb(reg_addr);
+}
+
 void gpmc_cs_write_reg(int cs, int idx, u32 val)
 {
void __iomem *reg_addr;
 
-   reg_addr = gpmc_base + GPMC_CS0 + (cs * GPMC_CS_SIZE) + idx;
+   reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) + idx;
__raw_writel(val, reg_addr);
 }
 
@@ -120,7 +139,7 @@ u32 gpmc_cs_read_reg(int cs, int idx)
 {
void __iomem *reg_addr;
 
-   reg_addr = gpmc_base + GPMC_CS0 + (cs * GPMC_CS_SIZE) + idx;
+   reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) + idx;
return __raw_readl(reg_addr);
 }
 
@@ -419,8 +438,100 @@ void gpmc_cs_free(int cs)
 EXPORT_SYMBOL(gpmc_cs_free);
 
 /**
+ * gpmc_hwcontrol - hardware specific access (read/ write) control
+ * @cs: chip select number
+ * @cmd: command type
+ * @write: 1 for write; 0 for read
+ * @wval: value to write
+ * @rval: read pointer
+ */
+int gpmc_hwcontrol(int cs, int cmd, int write, int wval, int *rval)
+{
+   u32 regval = 0;
+
+   if (!write  !rval)
+   return -EINVAL;
+
+   switch (cmd) {
+   case GPMC_STATUS_BUFFER:
+   regval = gpmc_read_reg(GPMC_STATUS);
+   /* 1 : buffer is available to write */
+   *rval = regval  GPMC_STATUS_BUFF_EMPTY;
+   break;
+
+   case GPMC_GET_SET_IRQ_STATUS:
+   if (write)
+   gpmc_write_reg(GPMC_IRQSTATUS, wval);
+   else
+   *rval = gpmc_read_reg(GPMC_IRQSTATUS);
+   break;
+
+   case GPMC_PREFETCH_FIFO_CNT:
+   regval = gpmc_read_reg(GPMC_PREFETCH_STATUS);
+   *rval = GPMC_PREFETCH_STATUS_FIFO_CNT(regval);
+   break;
+
+   case GPMC_PREFETCH_COUNT:
+   regval = gpmc_read_reg(GPMC_PREFETCH_STATUS);
+   *rval = GPMC_PREFETCH_STATUS_COUNT(regval);
+   break;
+
+   case GPMC_CONFIG_WP:
+   regval = gpmc_read_reg(GPMC_CONFIG);
+   if (wval)
+   regval = ~GPMC_CONFIG_WRITEPROTECT; /* WP is ON */
+   else
+   regval |= GPMC_CONFIG_WRITEPROTECT;  /* WP is OFF */
+   gpmc_write_reg(GPMC_CONFIG, regval);
+   break;
+
+   case GPMC_CONFIG_RDY_BSY:
+   regval  = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
+   regval |= WR_RD_PIN_MONITORING;
+   gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, regval);
+   break;
+
+   case GPMC_CONFIG_DEV_SIZE:
+   regval  = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
+   regval |= GPMC_CONFIG1_DEVICESIZE(wval);
+   gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, regval);
+   break;
+
+   case GPMC_CONFIG_DEV_TYPE:
+   regval  = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
+   regval |= GPMC_CONFIG1_DEVICETYPE(wval);
+   if (wval == GPMC_DEVICETYPE_NOR)
+   regval |= GPMC_CONFIG1_MUXADDDATA;
+   gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, regval);
+   break;
+
+   case GPMC_NAND_COMMAND:
+